Closed Bug 1356334 Opened 7 years ago Closed 7 years ago

Add UI for flagging long running Web Extension scripts and provide the option to stop them

Categories

(WebExtensions :: Frontend, enhancement, P1)

Product:
WebExtensions
Component:
Frontend
Type:
enhancement

Tracking

(Performance Impact:high, firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla57
Project Flags:
Performance Impact high
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: kmag)

References

Details

(Whiteboard: , triaged)

Attachments

(7 files, 1 obsolete file)

Bug 1356334: Part 1 - Add helpers for safely casting principals without COM overhead.
59 bytes, text/x-review-board-request
bholley
: review+
Details
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts.
59 bytes, text/x-review-board-request
kmag
: review+
Details
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID.
59 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1356334: Part 5 - Add add-on name to slow script messages.
59 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1356334: Part 6 - Allow completely terminating a slow content script sandbox.
59 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1356334: Part 7 - Destroy content script sandboxes after slow script termination.
59 bytes, text/x-review-board-request
mixedpuppy
: review+
Details
e10s slow script notification
30.69 KB, image/png
Details 
See bug 1345633 comment 12.

It would be nice to customize our long running script on web pages UI to cover Web Extensions, something along like:

[Addon Foo] is making Firefox run slowly, what would you like to do?
       [ Wait ] [ Stop ]

We can potentially use a shorter timeout for add-on content script sandboxes to avoid disaster situations like bug 1345633.
We should probably also include the URL or title of the page the content script is running on.

And, in the case of content scripts, unlike page scripts, we can kill the content script entirely rather than just the current stack, so we should do that either as an option or by default.
Most of the code to handle slow scripts is here:

http://searchfox.org/mozilla-central/rev/944f87c575e8a0bcefc1ed8efff10b34cf7a5169/js/xpconnect/src/XPCJSContext.cpp#1313

Ideally, I'd like to separately record the time spent in chrome/content/extension compartments since the last checkpoint, but the current code just checks the context at the top of the stack when the interrupt happens. Since I think we're probably better off having something soon than something perfect, I'll just stick to that logic for now, and hopefully we can do something better down the road.
Assignee: nobody → kmaglione+bmo
Ideally this would have tests, but there don't appear to be any existing tests for the slow script checker, and I don't have any particularly good ideas about how to implement them from scratch.
Comment on attachment 8858664 [details]
Bug 1356334: Part 1 - Add helpers for safely casting principals without COM overhead.

https://reviewboard.mozilla.org/r/130640/#review133302
Attachment #8858664 - Flags: review?(bobbyholley) → review+
In bug 1309946 we removed the "this addon might be making Firefox run slowly" for a few reasons, it was inaccurate and it didn't seem that effective. I'm concerned about any user facing warning from WebExtensions for similar reasons.

At the least, can we get this on a Nightly only until we've got some real world feedback?

Does this path have any telemetry showing how often this is shown?

Need info'ing Kev because I'm sure he'd have some opinion on this one.
Flags: needinfo?(kev)
commenting
Flags: needinfo?(amckay)
Spoke to Kris and he pointed out that this warning already happens, this bug just changes it so it shows the add-on in the script instead of something intelligible to the end user. Based on that not worried about this any more and removing the need info. Sorry about the noise.
Flags: needinfo?(kev)
Flags: needinfo?(amckay)
Comment on attachment 8858670 [details]
Bug 1356334: Part 7 - Destroy content script sandboxes after slow script termination.

https://reviewboard.mozilla.org/r/130652/#review133458
Attachment #8858670 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8858665 [details]
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts.

https://reviewboard.mozilla.org/r/130642/#review134518

I'm pretty sure this will break bug 1329731 (or something like it). As we discussed, we need to use a different concept of "scriptability" for killing scripts due to timeouts than we use for never allowing JS at all.

::: js/xpconnect/src/XPCJSContext.cpp:352
(Diff revision 1)
>  
>      MOZ_CRASH("Chain parser loop does not terminate");
>  }
>  
> +static inline bool
> +IsExtensionPrincipal(nsIPrincipal* principal, nsAString& addonId)

Please call this IsWebExtensionPrincipal. It's a little weird that GetAddonId below only returns values for WebExtensions, so I'd like to make it clear here.

::: js/xpconnect/src/XPCJSContext.cpp:359
(Diff revision 1)
> +    return (NS_SUCCEEDED(principal->GetAddonId(addonId)) &&
> +            !addonId.IsEmpty());
> +}
> +
> +static bool
> +IsExtensionContentScript(BasePrincipal* principal, nsAString& addonId)

This should be called IsWebExtensionContentScript.

::: js/xpconnect/src/XPCJSContext.cpp:390
(Diff revision 1)
> -    // ExpandedPrincipal gets a free pass.
> -    nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(aPrincipal);
> -    if (ep)
> +    auto principal = BasePrincipal::Cast(aPrincipal);
> +
> +    // ExpandedPrincipal gets a free pass unless it's an extension content script.
> +    nsString addonId;
> +    if (principal->Is<ExpandedPrincipal>()) {
> +        return !IsExtensionContentScript(principal, addonId);

I'm really confused. So when JS is disabled, then WebExtension background pages are still allowed to run. WebExtension content scripts are not. And other content scripts (SDK, etc.) are allowed to run. What's the justification?
Attachment #8858665 - Flags: review?(wmccloskey) → review-
Comment on attachment 8858666 [details]
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts.

https://reviewboard.mozilla.org/r/130644/#review134526

::: js/xpconnect/src/XPCJSContext.cpp:1226
(Diff revision 1)
>              if (chromeTime <= 0)
>                  chromeTime = INT32_MAX;
> -            mWatchdog->SetMinScriptRunTimeSeconds(std::min(contentTime, chromeTime));
> +            int32_t extTime = Preferences::GetInt(PREF_MAX_SCRIPT_RUN_TIME_EXT_CONTENT, 5);
> +            if (extTime <= 0)
> +                extTime = INT32_MAX;
> +            mWatchdog->SetMinScriptRunTimeSeconds(std::min(std::min(contentTime, chromeTime), extTime));

I think you can now do:
std::min({a, b, c})

::: js/xpconnect/src/XPCJSContext.cpp:1387
(Diff revision 1)
> +    auto principal = BasePrincipal::Cast(nsContentUtils::SubjectPrincipal(cx));
> +    bool chrome = principal->Is<SystemPrincipal>();
> +    if (chrome) {
> +        prefName = PREF_MAX_SCRIPT_RUN_TIME_CHROME;
> +        limit = Preferences::GetInt(prefName, 20);
> +    } else if (IsExtensionContentScript(principal, addonId)) {

The function you defined in part 2 only worked for WebExtensions. It would be nice if we could instead use the add-on ID from the current compartment. That should work more generally.
Attachment #8858666 - Flags: review?(wmccloskey) → review+
Comment on attachment 8858667 [details]
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID.

https://reviewboard.mozilla.org/r/130646/#review134538

It's unfortunate that this only works for WebExtensions, but I looked at the add-on manager and it seems like it would be horrible to make it work more generally :-(. Now I'm not sure if we should make it work for normal add-ons or not.

::: toolkit/components/extensions/ExtensionManagement.jsm:86
(Diff revision 1)
>    initialized: false,
>  
>    // Map[uuid -> extension].
>    // extension can be an Extension (parent process) or BrowserExtensionContent (child process).
>    uuidMap: new Map(),
> +  idMap: new Map(),

Please make a similar comment to the one above about the domain and range of this map.
Attachment #8858667 - Flags: review?(wmccloskey) → review+
Comment on attachment 8858668 [details]
Bug 1356334: Part 5 - Add add-on name to slow script messages.

https://reviewboard.mozilla.org/r/130648/#review134578

::: dom/base/nsGlobalWindow.cpp:11662
(Diff revision 1)
> +    failed = failed || NS_FAILED(rv) || result.IsEmpty();
> +    return Move(result);
> +  };
> +
> +  bool isAddonScript = !aAddonId.IsEmpty();
> +  bool showDebugButton = debugCallback && !isAddonScript;

Does the debug button not work for add-ons?
Attachment #8858668 - Flags: review?(wmccloskey) → review+
Comment on attachment 8858669 [details]
Bug 1356334: Part 6 - Allow completely terminating a slow content script sandbox.

https://reviewboard.mozilla.org/r/130650/#review134598

::: js/xpconnect/src/XPCJSContext.cpp:1466
(Diff revision 1)
>      if (response == nsGlobalWindow::KillSlowScript) {
>          if (Preferences::GetBool("dom.global_stop_script", true))
>              xpc::Scriptability::Get(global).Block();
>          return false;
>      }
> +    if (response == nsGlobalWindow::KillScriptGlobal) {

I'm unclear on exactly how this differs from KillSlowScript when the dom.global_stop_script pref is set. Can you explain?

::: js/xpconnect/src/XPCJSContext.cpp:1477
(Diff revision 1)
> +        // Notify the extensions framework that the sandbox should be killed.
> +        nsIXPConnect* xpc = nsContentUtils::XPConnect();
> +        JS::RootedObject wrapper(cx, JS_NewPlainObject(cx));
> +        nsCOMPtr<nsISupports> supports;
> +
> +        if (!wrapper ||

Please comment what this is meant to do. I'd feel better if Blake looked at this part. Maybe there's a better way to do it that I'm not aware of.
Attachment #8858669 - Flags: review?(wmccloskey) → review+
Comment on attachment 8858667 [details]
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID.

https://reviewboard.mozilla.org/r/130646/#review134538

Yeah, I was thinking about trying to make it work for normal add-ons as a follow-up. The problem is that the add-on manager doesn't know the name of the add-on until the database is loaded, and we don't normally load it at startup for performance reasons (although telemetry currently forces it to be loaded pretty early...)

I was considering adding a private synchronous API to get the name of an add-on, and synchronously load the DB if necessary, but I'm a bit worried that it might be abused. Anyway, I'll file a follow-up for that. I just didn't want this to block on it.
Comment on attachment 8858666 [details]
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts.

https://reviewboard.mozilla.org/r/130644/#review134526

> I think you can now do:
> std::min({a, b, c})

Oh, nice.

> The function you defined in part 2 only worked for WebExtensions. It would be nice if we could instead use the add-on ID from the current compartment. That should work more generally.

Well, we only want to apply this to content script compartments, which means we'd have to either check if the global is a sandbox, or check the isWebExtensionContentScript compartment flag. Checking the global seems a bit flaky, and checking the compartment flag would still only give us WebExtension sandboxes anyway.
Comment on attachment 8858665 [details]
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts.

https://reviewboard.mozilla.org/r/130642/#review134518

> I'm really confused. So when JS is disabled, then WebExtension background pages are still allowed to run. WebExtension content scripts are not. And other content scripts (SDK, etc.) are allowed to run. What's the justification?

I was mainly just trying to limit the changes to what was necessary for this bug. I think it probably makes sense to remove the excemption for expanded principals and WebExtension principals in general, though, so I'm happy to do that if that's what you'd prefer.
Comment on attachment 8858668 [details]
Bug 1356334: Part 5 - Add add-on name to slow script messages.

https://reviewboard.mozilla.org/r/130648/#review134578

> Does the debug button not work for add-ons?

As far as I can tell, it doesn't. But I was mainly just trying to avoid having to deal with all of the extra message possibilities in the debug+addonId case.
Comment on attachment 8858669 [details]
Bug 1356334: Part 6 - Allow completely terminating a slow content script sandbox.

https://reviewboard.mozilla.org/r/130650/#review134598

> I'm unclear on exactly how this differs from KillSlowScript when the dom.global_stop_script pref is set. Can you explain?

In the case of a content script sandbox (which is the only case this is returned for), we actually completely and cleanly teardown the global, rather than just blocking scripts. But I realized after I pushed this that we probably shouldn't block scriptability in the KillSlowScript case for content scripts.

> Please comment what this is meant to do. I'd feel better if Blake looked at this part. Maybe there's a better way to do it that I'm not aware of.

I couldn't think of or find one. The only other places I could find where we were doing anything like this (e.g., [1]), we were doing it more or less the same way, so I decided it was good enough.

[1]: http://searchfox.org/mozilla-central/rev/d8ac097a1de2544f0e51d186bc850662c5b7430a/dom/console/Console.cpp#1089
Thanks a lot Kris for the patches!

Hi Markus, I asked bwinton for some UX opinion about how to message this in the UI since he worked on WebExtensions UX and he recommended to get your help on this.  Part 5 of this patchset adds a message specific to WebExtensions here when the injected content script is slowing down the page, do you mind taking a look please and provide some feedback?  Thanks!
Flags: needinfo?(mjaritz)
Hi Ehsan, Kris, can you please share the mentioned UI with me as a screenshot, or a screen recording if interactivity is important.
Or as a test-build and info on how to trigger the UI if you think I might need a really close look. Thank you!
Flags: needinfo?(ehsan)
Here's the e10s version:

https://people.mozilla.com/~kmaglione/images/b5ceac730f4e2c47.png

And the non-e10s version:

https://people.mozilla.com/~kmaglione/images/594872b50356a186.png

I can get you a link to a try build to test on, if necessary, but I don't think they'll tell you much more than the screenshots.
Flags: needinfo?(ehsan)
Also note that none of this UI is new. The only changes are the messaging for slow scripts that belong to add-ons, and the option to kill the content script.
(In reply to Kris Maglione [:kmag] from comment #28)
> Here's the e10s version:
> https://people.mozilla.com/~kmaglione/images/b5ceac730f4e2c47.png
>
> And the non-e10s version:
> https://people.mozilla.com/~kmaglione/images/594872b50356a186.png

Looks good to me. But it would be great to have Michelle check the copy on it as that seams to be the crucial aspect.

Looks like a typo in the non-e10s version. responding instead of responsing


> I can get you a link to a try build to test on, if necessary, but I don't
> think they'll tell you much more than the screenshots.
Shouldn't be necessary. Thanks.
Flags: needinfo?(mjaritz) → needinfo?(mheubusch)
Priority: -- → P1
Whiteboard: [qf:p1] → [qf:p1], triaged
(In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #30)
> (In reply to Kris Maglione [:kmag] from comment #28)
> > Here's the e10s version:
> > https://people.mozilla.com/~kmaglione/images/b5ceac730f4e2c47.png
> >
> > And the non-e10s version:
> > https://people.mozilla.com/~kmaglione/images/594872b50356a186.png
> 
> Looks good to me. But it would be great to have Michelle check the copy on
> it as that seams to be the crucial aspect.
> 
> Looks like a typo in the non-e10s version. responding instead of responsing
> 
> 
> > I can get you a link to a try build to test on, if necessary, but I don't
> > think they'll tell you much more than the screenshots.
> Shouldn't be necessary. Thanks.

Are these two versions (e10s and non-e10s) meant to communicate the same thing? If so, can I write one set of strings and button copy for both cases?  Why is one a notification bar and the other a pop-up window?  Also, can someone explain the difference between disabling the add-on and stopping it or is it stopping the script only not the add-on?  Also, what happens if I choose wait and there is still a problem? will I get another opportunity to choose stop?  

I'd like to recommend something like the following but want to make sure I've covered the cases:

A script in <extension name> is causing <Firefox> to slow down.

You can disable this extension, stop the script, or wait. Learn more (links to SUMO)
[Disable Extension][Stop Script] [Wait]
Flags: needinfo?(mjaritz)
Flags: needinfo?(mheubusch)
Flags: needinfo?(kmaglione+bmo)
(In reply to mheubusch from comment #31)
> Are these two versions (e10s and non-e10s) meant to communicate the same
> thing? If so, can I write one set of strings and button copy for both cases?
> Why is one a notification bar and the other a pop-up window?

They're more or less mean to communicate the same thing, but there's a
fundamental difference between the e10s and non-e10s cases: In e10s, the
script only blocks the content process. The main browser UI is still
responsive, but the content pages running in the same process largely aren't.
In the non-e10s case, the entire browser UI is frozen while the script is
executing, so we need to open a modal dialog and deal with it immediately.

> Also, can someone explain the difference between disabling the add-on and
> stopping it or is it stopping the script only not the add-on?

In the first case, we stop the add-on from running on the page at all, until
the next time it loads. So if it has timers, for instance, that repeatedly
cause problems, those won't bet a chance to run anymore.

In the second case, we just interrupt the current operation, but the add-on
will still be able to interact with the page.

> Also, what happens if I choose wait and there is still a problem? will I get
> another opportunity to choose stop?

Yes, after the slow script delay elapses again. Currently that's 5 seconds for
add-on content scripts.
Flags: needinfo?(kmaglione+bmo)
(In reply to mheubusch from comment #31)
> Are these two versions (e10s and non-e10s) meant to communicate the same
> thing? If so, can I write one set of strings and button copy for both cases?

Without e10s the whole browser will become unresponsive as all UI is part of the same process that is slowed down. Therefor separate dialog. And because it is a more severe hang than what people will experience with e10s.
So in the non-e10s case the browser will be completely unresponsive prior to the message, where as with e10s UI will still be clickable while content got slowed down. (Therefor I think a difference in wording makes sense: unresponsive - slow )

> Why is one a notification bar and the other a pop-up window? 
Pop-up because the whole rest of the browser is unresponsive. And notification bar, because it is usual warning UI.

> Also, can someone explain the difference between disabling the add-on and 
> stopping it or is it stopping the script only not the add-on?  

I read "it" as referring to the script. (but I see how that can be misleading after the first button.)

> Also, what happens if I choose wait and there is still a problem? will I get 
> another opportunity to choose stop?  

I think I recall it re-surfacing after a while. (I see this as the expert option for people that know what they are doing...)

> I'd like to recommend something like the following but want to make sure
> I've covered the cases:
> 
> A script in <extension name> is causing <Firefox> to slow down.
> 
> You can disable this extension, stop the script, or wait. Learn more (links
> to SUMO)
> [Disable Extension][Stop Script] [Wait]

As the non-e10s hang will be much more severe, I think warrants some more explanation. (and if we reduce the copy of it, we should consider reducing the copy on scripts from web pages too. (currently the copy is very similar to that one)
Clearing my needinfo.
Flags: needinfo?(mjaritz)
(In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #33)
> (In reply to mheubusch from comment #31)
> > Are these two versions (e10s and non-e10s) meant to communicate the same
> > thing? If so, can I write one set of strings and button copy for both cases?
> 
> Without e10s the whole browser will become unresponsive as all UI is part of
> the same process that is slowed down. Therefor separate dialog. And because
> it is a more severe hang than what people will experience with e10s.
> So in the non-e10s case the browser will be completely unresponsive prior to
> the message, where as with e10s UI will still be clickable while content got
> slowed down. (Therefor I think a difference in wording makes sense:
> unresponsive - slow )
> 
> > Why is one a notification bar and the other a pop-up window? 
> Pop-up because the whole rest of the browser is unresponsive. And
> notification bar, because it is usual warning UI.
> 
> > Also, can someone explain the difference between disabling the add-on and 
> > stopping it or is it stopping the script only not the add-on?  
> 
> I read "it" as referring to the script. (but I see how that can be
> misleading after the first button.)
> 
> > Also, what happens if I choose wait and there is still a problem? will I get 
> > another opportunity to choose stop?  
> 
> I think I recall it re-surfacing after a while. (I see this as the expert
> option for people that know what they are doing...)
> 
> > I'd like to recommend something like the following but want to make sure
> > I've covered the cases:
> > 
> > A script in <extension name> is causing <Firefox> to slow down.
> > 
> > You can disable this extension, stop the script, or wait. Learn more (links
> > to SUMO)
> > [Disable Extension][Stop Script] [Wait]
> 
> As the non-e10s hang will be much more severe, I think warrants some more
> explanation. (and if we reduce the copy of it, we should consider reducing
> the copy on scripts from web pages too. (currently the copy is very similar
> to that one)

So, for e10s I recommend: 
A script in the extension <extension name> is causing <Firefox> to slow down. <a href="https://support.mozilla.org/en-US/kb/warning-unresponsive-script?cache=no#w_other-causes" Learn more</a> 
[Temporarily Disable Extension][Stop Script] [Wait]

For non-e10s I still have a few questions:
1. Is it possible to also disable the extension, or are the only options Stop Script or Continue?
2. Does "Continue" mean wait? Does a user actually have to press the button or will the script keep running in on the page until you select the Stop Script button?  
3. I don't understand the checkbox: Prevent the add-on script from running . . .  - Does this mean "temporarily disable the whole add on (like in e10s) or something else? 
4. Why is the name of the script displayed? Can a user click on this or report it to someone? 

Thanks in advance for your help.
Hi Markus and Kris, Just NI'ing you to make sure you saw my additional questions.
Flags: needinfo?(mjaritz)
Flags: needinfo?(kmaglione+bmo)
(In reply to mheubusch from comment #35)
> 
> So, for e10s I recommend: 
> A script in the extension <extension name> is causing <Firefox> to slow
> down. <a
> href="https://support.mozilla.org/en-US/kb/warning-unresponsive-
> script?cache=no#w_other-causes" Learn more</a> 
> [Temporarily Disable Extension][Stop Script] [Wait]

Great. Thanks.

> For non-e10s I still have a few questions:

Non-e10s looks rather complicated, but with us expanding e10s more and more, it will soon be gone for good. 
And as it is basically the same thing that people have seen for slow scripts from websites for years, I think it is something people know (even if it is not the nicest of dialogues).
Therefor I did not put a lot of focus on it and would like to keep the effort for changes low. 
(If you think we should re-word, we should also make sure to update the non-e10s slow script dialog for websites.)

> 1. Is it possible to also disable the extension, or are the only options
> Stop Script or Continue?

The checkbox to block the script blocks a good part of the extension. If it is the extensions main script, it would be effectively be blocking the extension, but if it is only one of many scripts in the extension, I assume not.

> 2. Does "Continue" mean wait? 

Yes.

> Does a user actually have to press the button
> or will the script keep running in on the page until you select the Stop
> Script button?  

I am not sure I know what you mean.

> 3. I don't understand the checkbox: Prevent the add-on script from running .
> . .  - Does this mean "temporarily disable the whole add on (like in e10s)
> or something else?

This probably depends on what script is running slow.

> 4. Why is the name of the script displayed? Can a user click on this or
> report it to someone? 

It can be helpful for the developer to know what caused the slow-down, but a user can not click on it.
(this would be a relic from a different approach to error messages as we do them now.)

But I accepted it as it follows the pattern of the slow script warnings websites gave for a long time, and as it will only be seen by people that have no e10s yet.
(I should have expressed that earlier, when asking for your feedback Michelle, to give some sense of scope.)

> Thanks in advance for your help.
Flags: needinfo?(mjaritz)
See Also: → 1365648
As Andy asked me in SF about this, I want to summarize what we should do here:

We should follow Michelle's copy for e10s:

(In reply to mheubusch from comment #35)
> So, for e10s I recommend: 
> A script in the extension <extension name> is causing <Firefox> to slow down. <a href="https://support.mozilla.org/en-US/kb/warning-unresponsive-script?cache=no#w_other-causes" Learn more</a> 
[Temporarily Disable Extension][Stop Script] [Wait]


and for non-e10s we should keep what we have, as non-e10s is not a priority and transitioning users to e10s should be complete by 57.
Blocks: 1369274
Flags: needinfo?(kmaglione+bmo)
Sorry I haven't had much chance to get back to this the past few months. Given bugs like bug 1369274, though, it's back at the top of my priority list.

I'm dropping part 2, since nuking the sandbox should be enough in most cases, and it looks like Scriptability::Block() should work even for principals that are immune to script policies.

I updated the slow script info bar with the suggested text and documentation link (screenshot attached).
Attachment #8858666 - Attachment is obsolete: true
Comment on attachment 8858667 [details]
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID.

This part had to change pretty significantly after the rebase. Needs a new review.
Attachment #8858667 - Flags: review+ → review?(wmccloskey)
Attachment #8858665 - Flags: review?(wmccloskey) → review+
Attachment #8858667 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c880eca810ac03509cc3102bd421b5f386e82cb
Bug 1356334: Part 1 - Add helpers for safely casting principals without COM overhead. r=bholley

https://hg.mozilla.org/integration/mozilla-inbound/rev/805c568069301ae91ead5780cdc118af73907229
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a663ffd1446f9f70b220866855a0ecb3503761
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/b83aea215a82d44bec7443b69e60feef32f5fb2c
Bug 1356334: Part 5 - Add add-on name to slow script messages. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/d832803270ac831fd760356f36e16ef2a2d6d45b
Bug 1356334: Part 6 - Allow completely terminating a slow content script sandbox. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5bcd2b2dc695397c97f0c1391b66c6f8251939
Bug 1356334: Part 7 - Destroy content script sandboxes after slow script termination. r=mixedpuppy
Backed out for making test_ext_contentscript_async_loading.html nearly permafail on Android debug. Please be sure to fold those follow-up bustage fixes into the revised patches too while you're at it.
https://hg.mozilla.org/integration/mozilla-inbound/rev/399db8547abb4549b05ffd419fab29c8cbd6c2ae

https://treeherder.mozilla.org/logviewer.html#?job_id=124157172&repo=mozilla-inbound
Flags: needinfo?(kmaglione+bmo)
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dde6ae5176d04e63905eb5c7c47178371d94260
Bug 1356334: Part 1 - Add helpers for safely casting principals without COM overhead. r=bholley

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9185a04fe62a75a5a5b97947cb47f81ba2bfd3
Bug 1356334: Part 3 - Enforce a stricter slow script timeout for extension content scripts. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ebfb0df0958366c63048e2b943ce4f3827fbcf0
Bug 1356334: Part 4 - Add a helper to synchronously get an extension's name from its ID. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/6aa43c3fc0c5fa4cf17714bbe8617990503ab1ef
Bug 1356334: Part 5 - Add add-on name to slow script messages. r=billm

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f57bca8977c2106570607f76c69a68ffae8f52b
Bug 1356334: Part 6 - Allow completely terminating a slow content script sandbox. r=billm
Depends on: 1391933
(In reply to Kris Maglione [:kmag] from comment #47)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d832803270ac831fd760356f36e16ef2a2d6d45b
> +KillAddonScriptGlobalMessage=Prevent the add-on script from running on this page until it next reloads.

If this is a checkbox with a user-selectable option, it should not contain a period.
Flags: needinfo?(kmaglione+bmo)
Depends on: 1402503
Hello Kris, 

Can you please attach some test webextensions which cause the long running script warnings? Also if you can provide some STRs, it will be more clear for me how to exactly test this issue.

Thanks,
Victor
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo) → qe-verify-
Product: Toolkit → WebExtensions
Performance Impact: --- → P1
Whiteboard: [qf:p1], triaged → , triaged
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: