1450344 - <select> prompts don't work inside e10s iframes

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, enhancement, P1)

Description

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

Clicking on <select> does not show a prompt when it's inside an iframe under e10s

Comment 1

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

Attachment: Bug 1450344 - 1. Use frame script to handle input prompts;

Use a frame script to handle <select> and <input type="date/time">
prompts, in order to support e10s iframes.

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

Comment 2

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

Attachment: Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

Make getDispatcherForWindow work in the child process by returning a
dispatcher through EventDispatcher.forMessageManager.

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

Comment 3

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

Attachment: Bug 1450344 - 3. Use EventDispatcher directly for child process prompts;

We had some custom messaging code in GeckoViewPrompts for supporting
child processes, but using EventDispatcher directly works just as well.

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

Comment 4

[Code Review Bot [:reviewbot]]

Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review238580


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:231
(Diff revision 1)
> +        let mm = this.getContentFrameMessageManager(aWin.top || aWin);
> +        return mm && EventDispatcher.forMessageManager(mm);
> +      }
>        let win = this.getChromeWindow(aWin.top || aWin);
> -      let dispatcher = win.WindowEventDispatcher || EventDispatcher.for(win);
> -      if (!win.closed && dispatcher) {
> +      if (!win.closed) {
> +        return win.WindowEventDispatcher || EventDispatcher.for(win)

Error: Missing semicolon. [eslint: semi]

Comment 5

[Eugen Sawin [:esawin]]

Comment on attachment 8964403 [details]
Bug 1450344 - 1. Use frame script to handle input prompts;

https://reviewboard.mozilla.org/r/233126/#review239234

Comment 6

[Eugen Sawin [:esawin]]

Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review239238

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:229
(Diff revision 1)
>      try {
> +      if (!this.IS_PARENT_PROCESS) {
> +        let mm = this.getContentFrameMessageManager(aWin.top || aWin);
> +        return mm && EventDispatcher.forMessageManager(mm);
> +      }
>        let win = this.getChromeWindow(aWin.top || aWin);

const

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm
(Diff revision 1)
> -      let dispatcher = win.WindowEventDispatcher || EventDispatcher.for(win);
> -      if (!win.closed && dispatcher) {
> +      if (!win.closed) {
> +        return win.WindowEventDispatcher || EventDispatcher.for(win)
> -        return dispatcher;
>        }
>      } catch (e) {
> -      return null;

Don't we want to log here?

Comment 7

[Eugen Sawin [:esawin]]

Comment on attachment 8964405 [details]
Bug 1450344 - 3. Use EventDispatcher directly for child process prompts;

https://reviewboard.mozilla.org/r/233130/#review239240

Comment 8

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

Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review239238

> Don't we want to log here?

That's up to the caller. It may expect a null result.

Comment 9

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

Comment on attachment 8964403 [details]
Bug 1450344 - 1. Use frame script to handle input prompts;

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

Comment 10

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

Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

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

Comment 11

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

Comment on attachment 8964405 [details]
Bug 1450344 - 3. Use EventDispatcher directly for child process prompts;

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cfb9e997161
1. Use frame script to handle input prompts; r=esawin
https://hg.mozilla.org/integration/autoland/rev/f69c0c61aa09
2. Make getDispatcherForWindow work in child process; r=esawin
https://hg.mozilla.org/integration/autoland/rev/66f0126bec69
3. Use EventDispatcher directly for child process prompts; r=esawin

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5cfb9e997161
https://hg.mozilla.org/mozilla-central/rev/f69c0c61aa09
https://hg.mozilla.org/mozilla-central/rev/66f0126bec69

Comment 14

[Chris Peterson [:cpeterson]]

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