1459298 - Stop spinning event loop when window closes

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

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 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1459298 - Stop spinning event loop when window closes;

If our window closes while we're spinning the event loop, stop spinning
as soon as possible so we can continue with cleaning up the window.

Review commit: https://reviewboard.mozilla.org/r/242050/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242050/

Comment 2

[Eugen Sawin [:esawin]]

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?

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

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).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd8a65154726
Stop spinning event loop when window closes; r=esawin

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/bd8a65154726