Stop spinning event loop when window closes

RESOLVED FIXED in Firefox 62

Status

enhancement
P2
normal
RESOLVED FIXED
Last year
7 months ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

unspecified
mozilla62
All
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

There are a few places where we can spin the event loop for longer than the window lifetime. We should exit the loop if the window closes so the window can clean up sooner.
Comment on attachment 8973684 [details]
Bug 1459298 - Stop spinning event loop when window closes;

https://reviewboard.mozilla.org/r/242050/#review247900

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:307
(Diff revision 1)
> -          iter.getNext().QueryInterface(Ci.nsIDOMWindow));
> +      dispatcher = this.getDispatcherForWindow(win);
>        if (dispatcher) {
> -        return dispatcher;
> +        return [dispatcher, win];
>        }
>      }
> -    return null;
> +    return [null, null];

Is there an advantage of returning an array vs. object in this case?

::: mobile/android/modules/geckoview/LoadURIDelegate.jsm:20
(Diff revision 1)
>  var LoadURIDelegate = {
>    // Delegate URI loading to the app.
>    // Return whether the loading has been handled.
> -  load: function(aEventDispatcher, aUri, aWhere, aFlags, aTriggeringPrincipal) {
> +  load: function(aWindow, aEventDispatcher, aUri, aWhere, aFlags) {
> +    if (!aWindow) {
> +      return false;

This shouldn't happen, maybe throw?
Attachment #8973684 - Flags: review?(esawin) → review+
Comment on attachment 8973684 [details]
Bug 1459298 - Stop spinning event loop when window closes;

https://reviewboard.mozilla.org/r/242050/#review247900

> Is there an advantage of returning an array vs. object in this case?

I thought array was more convenient, but there's no real advantage afaik.

> This shouldn't happen, maybe throw?

I think it can (rarely) be null for frame scripts (i.e. `content` can be null in rare cases).
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd8a65154726
Stop spinning event loop when window closes; r=esawin
https://hg.mozilla.org/mozilla-central/rev/bd8a65154726
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Depends on: 1460601
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 62 → mozilla62
You need to log in before you can comment on or make changes to this bug.