Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Provide reporting event for browser's slow extension warnings #456

Open
kzar opened this issue Sep 25, 2023 · 17 comments
Open

Proposal: Provide reporting event for browser's slow extension warnings #456

kzar opened this issue Sep 25, 2023 · 17 comments
Labels
discussion Needs further discussion documentation Improvements or additions to documentation proposal Proposal for a change or new feature supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari

Comments

@kzar
Copy link

kzar commented Sep 25, 2023

Background

Browsers sometimes display a warning to the user that one of the browser extensions that they have enabled is slowing down their experience. Browsers seem to trigger such warnings under different circumstances, and there doesn't appear to be much documentation around this, or ways for extension developers to diagnose the performance issues. It would be nice to support extension developers in proactively improving their extension's performance.

Suggestion

  • Provide some way of reporting performance issues to extension developers in an actionable way. For example, perhaps an event could be fired that included at least some details (e.g. a stack trace) of the slow extension code? I realise this would need to be balanced with user privacy however.
  • I believe that so far, Firefox will warn the users if a content script is extremely slow (and the user tries to interact with the page), whereas Chrome will warn the user if a background script (in particular inside a blocking onBeforeRequest event handler) is slow. Perhaps the event could also include a description (e.g. "Slow content script", or "Slow onBeforeRequest handler") to point developers in the right direction?

References

@patrickkettner
Copy link
Contributor

May I suggest you change the issue to be just about the reporting portion? While I agree with everything, I do not believe this is the right venue you for the conversation for topics other than the reporting piece. The WECG does not provide documentation, and what actually triggers such a warning is an implementation detail, rather than something that would normally be covered by standards.

@kzar kzar changed the title Proposal: Provide documentation and reporting for browser's slow extension warnings Proposal: Provide reporting API/event for browser's slow extension warnings Sep 26, 2023
@kzar kzar changed the title Proposal: Provide reporting API/event for browser's slow extension warnings Proposal: Provide reporting event for browser's slow extension warnings Sep 26, 2023
@kzar
Copy link
Author

kzar commented Sep 26, 2023

Thanks for the feedback @patrickkettner 👍 , I've updated the issue.

@xeenon xeenon added documentation Improvements or additions to documentation proposal Proposal for a change or new feature and removed needs-triage labels Sep 26, 2023
@dotproto
Copy link
Member

dotproto commented Oct 2, 2023

While this group has not produced documentation and we don't have immediate plans to do so, I should note that that kind of material is within the scope of community group deliverables:

Community Group deliverables may be anything, including documents, test suites, tutorials, demos, code, discussion, etc.

- https://www.w3.org/community/about/process/#deliverables

For instance, a Community Group might … convene to have discussions about a tutorial for an existing specification.

- https://www.w3.org/community/about/faq/#what-is-a-w3c-community-group

As I see it, the critical question is whether group members are interested in and motivated to work on this kind of deliverable. If so, I think we can make room in the WECG tent for folks to pursue such projects. I'd only caution that our primary focus in the WECG has been building consensus across browsers and specifying a common WebExtensions platform, so there may be limited interest in creating tutorials or other reference documentation.

Or to put it another way, if a member says, "I want to create a guide for X," I'm happy to support their efforts. But if someone says, "I think the WECG should make a guide for X," that's going to be a harder sell.

@dotproto
Copy link
Member

dotproto commented Oct 2, 2023

I'm intrigued by the idea of a performance reporting API. I currently feel that extension performance is notably different from website performance in that a poorly performing site slows down a single origin while a poorly performing extensions can slow down the entire browser. While browser vendors and community members can create reference material that helps extension developers better understand how to leverage existing tools and APIs to monitor and measure extension performance, I think we should consider how the platform itself can help steer developers in the right direction.

At the moment I'm imagining an API that allows platform implementers to notify an extension about a variety of conditions, such as a long loop, slow function calls, slow scripts injected into pages, aggregate time of injected script execution, abnormal impact on the user's battery, frequent wakes without work, etc. Such an API could be structured so that it covers the mechanisms used to notify an extension of abnormal conditions, the shape of the events, the format browsers use to identify the relevant file and line number, and other relevant data without being prescriptive about what performance events browsers support.

I'm curious how browser reps feel about this general direction.

@dotproto dotproto added the discussion Needs further discussion label Oct 2, 2023
@kzar
Copy link
Author

kzar commented Oct 2, 2023

At the moment I'm imagining an API...

Yea, that's definitely an interesting idea. Perhaps if we did add the "bad performance" event I proposed, browser vendors could think of additional situations in which to fire that event. Anything that gives developers something actionable like a stack trace and a brief description sounds good to me.

My perspective is mostly as an extension developer. Over the years, I've debugged these issues a few times:

  • Sometimes, you know roughly what code is slow. For those cases, you can make sure there's decent test coverage, start measuring the performance to get a good baseline, then start experimenting with changes. Sometimes the required changes aren't obvious, e.g. when the JavaScript engine is able to optimise certain ways of doing things, so trial and error is required. The profiling tools and things like the performance API can get you there, so long as you put the work in.
  • Sometimes, you get a bug report from a user. Usually the bug report will be something vague that mentions either that the extension is slow, or that a website is slow, or that they saw a warning about the extension being slow. Debugging these kinds of reports can be really hard. With no way to reproduce the issue, or idea about the cause, it wastes a whole bunch of time trying random things. You know there's a problem somewhere, and maybe which website the problem can trigger the problem... but not much else! I've spent weeks investigating problems like this before. If I could be proactively sent stack-traces and some kind of description by the extension, it would save a whole bunch of my time + user frustration!

In any case, the more I think about it, the more I think this is a good idea. Showing a warning to the user should be a last resort really. Also making that warning available to developers seems like a minimum, and sharing extra warnings with developers as a proactive/preventative measure like @dotproto suggests would be even better.

@xeenon
Copy link
Collaborator

xeenon commented Oct 2, 2023

While a good idea, I have some concern with having an API for this.

  • What is an extension expected to do in response to this event firing? (This likely needs actionable details in the event to be useful.)
  • Can an extension even do anything if it is hung in its process? The event might be undeliverable.)

@kzar
Copy link
Author

kzar commented Oct 3, 2023

What is an extension expected to do in response to this event firing? (This likely needs actionable details in the event to be useful.)

Good question, here's an example of how I see the event being used:

In the DuckDuckGo Privacy Essentials browser extension, we attempt to protect the user by blocking requests to tracking scripts, and things like that. Sometimes those protections cause a website to break, at which point the user can disable protections by clicking on the extension's icon and clicking to disable protections in the popup UI. If they do that, they are asked if they'd like to report the website as broken. If they do, they are asked a few questions and a breakage report is sent in to us, which includes relevant details such as which protections were active on that page.

If we had this "bad performance" event, the first thing I'd like to do would be to listen for it, and then include details of when it fired with any related breakage reports. It would be hugely useful to have something like a stack trace and a note for performance issues included in those reports. If we saw a spike in breakage reports pointing to the performance of some specific piece of code, the issue would be much easier to address.

Another option, which maybe would be more controversial, could be that the API works in a similar way to runtime.setUninstallURL. Perhaps then, the reports could be fired off in a similar way to CSP violation reports.

Can an extension even do anything if it is hung in its process? The event might be undeliverable.)

Yea, that's a good point. I think some of the time the event would be deliverable, depending on the browser and which process had hung or what the warning was related to. That's my guess at least, it would be good to loop in some more browser devs to check that. Perhaps sometimes the event would not be immediately deliverable, but I think the event would still be useful to extension devs even if it fired with a delay.

I suppose if we went with the runtime.setUninstallURL style API instead of an event this would be less of an issue.

@zombie zombie added the supportive: firefox Supportive from Firefox label Oct 12, 2023
@zombie
Copy link
Collaborator

zombie commented Oct 12, 2023

Firefox is supportive of this reporting event, probably including the url of the page that triggered it, along with some other metadata that might be useful. We would be happy to propose/prototype a design if other implementers are interested as well.

@kzar
Copy link
Author

kzar commented Oct 13, 2023

Firefox is supportive of this reporting event, probably including the url of the page that triggered it, along with some other metadata that might be useful. We would be happy to propose/prototype a design if other implementers are interested as well.

Thanks @zombie. Please shout if I can help with that work, best way to reach me is to drop me an email to [email protected] .

Also, it's maybe worth pointing out that even if this event is only implemented for Firefox or a subset of the major browsers, it will still be immediately useful for extension developers. So long as it's possible to check the event exists before adding a listener, we'll be able to put it to use right away.

@xeenon xeenon added the neutral: safari Not opposed or supportive from Safari label Oct 18, 2023
@zombie
Copy link
Collaborator

zombie commented Dec 10, 2023

An update on this, Dave has been working to contribute a proposal for this API in Firefox bug 1861445. If anyone is interested to try it out, and/or has any feedback, please comment (here or in the patch).

@xeenon xeenon added supportive: safari Supportive from Safari and removed neutral: safari Not opposed or supportive from Safari labels Dec 14, 2023
@xeenon
Copy link
Collaborator

xeenon commented Dec 14, 2023

Changing to supportive for Safari after seeing the patch for Firefox.

@rdcronin
Copy link
Contributor

From our discussion last week -- I'm worried about the proposed name of the event being runtime.onWarning. I feel like this is too broad and can be easily confused with other warnings (my immediate thought would be a JavaScript console warning). It sounded like @Rob--W shared some of the same concerns.

We discussed other alternatives, such as onBrowserWarning (that sounded too much like a warning for the browser), onWarningFromBrowser(sounds clunky), etc. We ended on proposing that we useruntime.onPerformanceWarning` and limit this to performance-related concerns. If we wanted to add other classes of warnings in the future, they can be added as separate events (which also helps developers trivially filter them by types they're interested in).

@kzar
Copy link
Author

kzar commented Dec 19, 2023

That's fair, happy to change the event name to runtime.onPerformanceWarning if folks prefer that!

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 4, 2024
… r=zombie

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jan 5, 2024
… r=zombie

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jan 16, 2024
… r=zombie

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708

UltraBlame original commit: 199b3c24b8b43b8053831d7c287bbe83e43037c4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jan 16, 2024
… r=zombie

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708

UltraBlame original commit: 199b3c24b8b43b8053831d7c287bbe83e43037c4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jan 16, 2024
… r=zombie

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708

UltraBlame original commit: 199b3c24b8b43b8053831d7c287bbe83e43037c4
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 29, 2024
… r=zombie,robwu

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jan 30, 2024
… r=zombie,robwu

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 1, 2024
… r=zombie,robwu

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708

UltraBlame original commit: 9d33a3994005a05a8fbc4ad307e4bd6fb4ab2a63
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 1, 2024
… r=zombie,robwu

When an extension's content script is very slow and causes a webpage
to hang noticeably, a warning banner is displayed to the user. It
would be useful to also notify the extension developer when that
happens, so that they can address the issue.

Let's add a new event runtime.onPerformanceWarning that can be
dispatched when the browser needs to warn an extension of runtime
performance issues. For now, let's just dispatch that event when the
slow extension warning banner is displayed to the user.

See also w3c/webextensions#456

Differential Revision: https://phabricator.services.mozilla.com/D194708

UltraBlame original commit: 9d33a3994005a05a8fbc4ad307e4bd6fb4ab2a63
@Rob--W
Copy link
Member

Rob--W commented Mar 8, 2024

@kzar Is there any reason for specifying the recognized performance warnings in enums exposed on the runtime namespace? The current version that landed in Firefox has OnPerformanceWarningSeverity ("high", "medium", "low") and OnPerformanceWarningCategory ("content_scripts").

There is no way to use these values in any meaningful way, because they are emitted as potential event values from the API, and not taken as input. These enums are currently two extra objects on the runtime namespace, and I wonder whether we should just inline the definition in the event.

I can imagine a future use case, of adding a filter to the API to filter for events of interest. That would enable extensions to subscribe to specific warning events without being notified of all warnings, and enable the browser to optimize its implementation when there are differences between the underlying event sources.

@kzar
Copy link
Author

kzar commented Mar 8, 2024

As long as the possible category + severity values are defined in enums, I don't have a strong preference about if they're exposed as Objects under the runtime API or not. That said, my vote would be to keep them exposed as I've found that useful in the past with similar events.

To guess at future use cases, yes I suppose you're right that we might add the ability to filter the event and for that having access to the enums would be useful. Also I suppose an extension might take care to check the enums before adding a listener, e.g. to see if the "content_scripts" category is listed first.

@kzar
Copy link
Author

kzar commented Sep 20, 2024

So as a quick update, the runtime.onPerformanceWarning event is in Firefox now and we're already using it in the DuckDuckGo Privacy Essentials extension successfully.

I read today that Microsoft are adding extension performance warnings to Edge, is there anyone from Microsoft here that could consider adding support for the runtime.onPerformanceWarning event for those new performance warnings? As an extension developer, I'd love to be informed if Edge is warning users that the extension I'm working on is slow!

@xeenon
Copy link
Collaborator

xeenon commented Sep 20, 2024

@mukul-p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Needs further discussion documentation Improvements or additions to documentation proposal Proposal for a change or new feature supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari
Projects
None yet
Development

No branches or pull requests

7 participants