1450856 - ContentDelegate.onExternalResponse doesn't work under non-e10s

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, enhancement, P1)

Description

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

There's an exception at [1] because there is no tab child under non-e10s.

[1] https://searchfox.org/mozilla-central/rev/f5fb323246bf22a3a3b4185882a1c5d8a2c02996/mobile/android/components/geckoview/GeckoViewExternalAppService.js#30

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

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

Comment 2

[Eugen Sawin [:esawin]]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

https://reviewboard.mozilla.org/r/241526/#review247332

::: mobile/android/components/geckoview/GeckoViewExternalAppService.js:28
(Diff revision 1)
>  
>  ExternalAppService.prototype = {
>    classID: Components.ID("{a89eeec6-6608-42ee-a4f8-04d425992f45}"),
>    QueryInterface: ChromeUtils.generateQI([Ci.nsIExternalHelperAppService]),
>  
> +  getTopChromeWindow(context) {

We might already support this in GeckoViewUtils, if not we should move it there.

::: mobile/android/components/geckoview/GeckoViewExternalAppService.js:45
(Diff revision 1)
>      debug `doContent: uri=${channel.URI.displaySpec}
>                        contentType=${channel.contentType}`;
>  
> -    EventDispatcher.forMessageManager(mm).sendRequest({
> +    let dispatcher;
> +    if (Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_DEFAULT) {
> +      dispatcher = EventDispatcher.for(this.getTopChromeWindow(context));

Same as above, should be in GeckoViewUtils.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241526/diff/1-2/

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241526/diff/2-3/

Comment 5

[Eugen Sawin [:esawin]]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

https://reviewboard.mozilla.org/r/241526/#review247348

::: mobile/android/components/geckoview/GeckoViewExternalAppService.js:35
(Diff revision 3)
> +    if (GeckoViewUtils.IS_PARENT_PROCESS) {
> +      dispatcher = EventDispatcher.for(GeckoViewUtils.getChromeWindow(context));
> +    } else {
> +      const mm = context.QueryInterface(Ci.nsIDocShell).tabChild.messageManager;
> +      dispatcher = EventDispatcher.forMessageManager(mm);
> +    }

I think you can use GeckoViewUtils.getDispatcherForWindow for this?

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241526/diff/3-4/

Comment 7

[Eugen Sawin [:esawin]]

Comment on attachment 8972954 [details]
Bug 1450856 - Fix PromptDelegate.onExternalResponse() for non-e10s

https://reviewboard.mozilla.org/r/241526/#review247384

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d21cb6d11c2
Fix PromptDelegate.onExternalResponse() for non-e10s r=esawin

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7d21cb6d11c2

Comment 10

[Chris Peterson [:cpeterson]]

Moving some e10s bugs to the new GeckoView::Sandboxing component.