mus: Allow embedder to intercept events.

mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078
Committed: https://crrev.com/4d3588cbec47fcd629c038a0ca6aad9af58e4192
Cr-Commit-Position: refs/heads/master@{#402057}

[sadrul, 2016-06-16 16:31:24]

Description was changed from

==========
mus: Allow an embedder to intercept events targeted to the embedded window.

...

BUG=...
==========

to

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=none
==========

[sadrul, 2016-06-16 16:34:07]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-06-16 16:34:08]

I decided this makes more sense than an explicit api on mus::Window. WDYT?

[Ben Goodger (Google), 2016-06-16 16:53:49]

https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
File components/mus/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
components/mus/public/interfaces/window_tree.mojom:55: const uint32
kEmbedFlagEmbedderInterceptsEvents = 0x01;
1. Can you use an enum for this, and refer to it from Embed() below?
2. You need to document the precise behavior of this flag. Does the child
receive the event too? If not, is the embedder required to forward it? etc etc.

[sadrul, 2016-06-16 16:56:59]

https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
File components/mus/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
components/mus/public/interfaces/window_tree.mojom:55: const uint32
kEmbedFlagEmbedderInterceptsEvents = 0x01;
On 2016/06/16 16:53:49, Ben Goodger (Google) wrote:
> 1. Can you use an enum for this, and refer to it from Embed() below?
> 2. You need to document the precise behavior of this flag. Does the child
> receive the event too? If not, is the embedder required to forward it? etc
etc.

I had an enum before, but switched to using ints since it looks like that's how
flags are done in mojo (e.g. ui::EventFlag equivalents in mojom as kEventFlag*)

[Ben Goodger (Google), 2016-06-16 16:59:25]

On 2016/06/16 16:56:59, sadrul wrote:
>
https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
> File components/mus/public/interfaces/window_tree.mojom (right):
> 
>
https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
> components/mus/public/interfaces/window_tree.mojom:55: const uint32
> kEmbedFlagEmbedderInterceptsEvents = 0x01;
> On 2016/06/16 16:53:49, Ben Goodger (Google) wrote:
> > 1. Can you use an enum for this, and refer to it from Embed() below?
> > 2. You need to document the precise behavior of this flag. Does the child
> > receive the event too? If not, is the embedder required to forward it? etc
> etc.
> 
> I had an enum before, but switched to using ints since it looks like that's
how
> flags are done in mojo (e.g. ui::EventFlag equivalents in mojom as
kEventFlag*)

Ic. Can you at least improve the documentation?

[sadrul, 2016-06-16 17:16:52]

On 2016/06/16 16:59:25, Ben Goodger (Google) wrote:
> On 2016/06/16 16:56:59, sadrul wrote:
> >
>
https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
> > File components/mus/public/interfaces/window_tree.mojom (right):
> > 
> >
>
https://codereview.chromium.org/2068093002/diff/40001/components/mus/public/i...
> > components/mus/public/interfaces/window_tree.mojom:55: const uint32
> > kEmbedFlagEmbedderInterceptsEvents = 0x01;
> > On 2016/06/16 16:53:49, Ben Goodger (Google) wrote:
> > > 1. Can you use an enum for this, and refer to it from Embed() below?
> > > 2. You need to document the precise behavior of this flag. Does the child
> > > receive the event too? If not, is the embedder required to forward it? etc
> > etc.
> > 
> > I had an enum before, but switched to using ints since it looks like that's
> how
> > flags are done in mojo (e.g. ui::EventFlag equivalents in mojom as
> kEventFlag*)
> 
> Ic. Can you at least improve the documentation?

Added doc for the flag, and added a not for the doc of Embed() too.

[Ben Goodger (Google), 2016-06-16 17:34:36]

https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
File components/mus/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/window_tree.mojom:59: // to the embedded client
through other mechanism.
What other mechanism?

[sadrul, 2016-06-16 17:45:17]

https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
File components/mus/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/window_tree.mojom:59: // to the embedded client
through other mechanism.
On 2016/06/16 17:34:36, Ben Goodger (Google) wrote:
> What other mechanism?

In the case of chrome, the browser has another IPC channel to the renderer, and
it would use that channel for sending the events to the renderer. i.e. mus does
not provide a mechanism for the embedder to directly dispatch events to the
embedded client. The embedder/embeddee need to have a separate mechanism that
they decide on.

[Ben Goodger (Google), 2016-06-16 17:48:22]

On 2016/06/16 17:45:17, sadrul wrote:
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> File components/mus/public/interfaces/window_tree.mojom (right):
> 
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> components/mus/public/interfaces/window_tree.mojom:59: // to the embedded
client
> through other mechanism.
> On 2016/06/16 17:34:36, Ben Goodger (Google) wrote:
> > What other mechanism?
> 
> In the case of chrome, the browser has another IPC channel to the renderer,
and
> it would use that channel for sending the events to the renderer. i.e. mus
does
> not provide a mechanism for the embedder to directly dispatch events to the
> embedded client. The embedder/embeddee need to have a separate mechanism that
> they decide on.

Do you think it could be worth defining a standard mechanism? e.g. defining an
EventHandler interface, and having mus use it to dispatch events to clients, but
allowing clients to expose impls of it to one another for this purpose, so at
least events get transported with identical semantics?

[sadrul, 2016-06-16 19:09:25]

On 2016/06/16 17:48:22, Ben Goodger (Google) wrote:
> On 2016/06/16 17:45:17, sadrul wrote:
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > File components/mus/public/interfaces/window_tree.mojom (right):
> > 
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > components/mus/public/interfaces/window_tree.mojom:59: // to the embedded
> client
> > through other mechanism.
> > On 2016/06/16 17:34:36, Ben Goodger (Google) wrote:
> > > What other mechanism?
> > 
> > In the case of chrome, the browser has another IPC channel to the renderer,
> and
> > it would use that channel for sending the events to the renderer. i.e. mus
> does
> > not provide a mechanism for the embedder to directly dispatch events to the
> > embedded client. The embedder/embeddee need to have a separate mechanism
that
> > they decide on.
> 
> Do you think it could be worth defining a standard mechanism? e.g. defining an
> EventHandler interface, and having mus use it to dispatch events to clients,
but
> allowing clients to expose impls of it to one another for this purpose, so at
> least events get transported with identical semantics?

Hm, interesting! I think it'd be possible, yes. The client will be aware that
the input-events it is receiving is coming from the embedder, and not directly
from mus, but we should be able to hide all of that in the client-lib, so
clients wouldn't need to explicitly do anything differently.

The way I am thinking this would work for chrome is: mus sends a request for an
EventHandler to the renderer (or any embedded client whose embedder requested to
intercept events), and send the other end of it to the browser (or the
embedder). The browser can then use it to send events to the renderer. Something
like:

  interface InputEventHandler {
    OnWindowInputEvent(window_id, event) => (bool handled);
  };

  interface WindowTreeClient {
    ...
    RequestInputEventHandler(InputEventHandler& handler);
  };

  interface WindowTree {
    ...
    Embed(window_id, WindowTreeClient, flags) => (bool success,
InputEventHandler? handler);
  };

[sadrul, 2016-06-17 14:50:51]

Description was changed from

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=none
==========

to

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078
==========

[sadrul, 2016-06-23 18:02:04]

On 2016/06/16 19:09:25, sadrul wrote:
> On 2016/06/16 17:48:22, Ben Goodger (Google) wrote:
> > On 2016/06/16 17:45:17, sadrul wrote:
> > >
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > > File components/mus/public/interfaces/window_tree.mojom (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > > components/mus/public/interfaces/window_tree.mojom:59: // to the embedded
> > client
> > > through other mechanism.
> > > On 2016/06/16 17:34:36, Ben Goodger (Google) wrote:
> > > > What other mechanism?
> > > 
> > > In the case of chrome, the browser has another IPC channel to the
renderer,
> > and
> > > it would use that channel for sending the events to the renderer. i.e. mus
> > does
> > > not provide a mechanism for the embedder to directly dispatch events to
the
> > > embedded client. The embedder/embeddee need to have a separate mechanism
> that
> > > they decide on.
> > 
> > Do you think it could be worth defining a standard mechanism? e.g. defining
an
> > EventHandler interface, and having mus use it to dispatch events to clients,
> but
> > allowing clients to expose impls of it to one another for this purpose, so
at
> > least events get transported with identical semantics?
> 
> Hm, interesting! I think it'd be possible, yes. The client will be aware that
> the input-events it is receiving is coming from the embedder, and not directly
> from mus, but we should be able to hide all of that in the client-lib, so
> clients wouldn't need to explicitly do anything differently.
> 
> The way I am thinking this would work for chrome is: mus sends a request for
an
> EventHandler to the renderer (or any embedded client whose embedder requested
to
> intercept events), and send the other end of it to the browser (or the
> embedder). The browser can then use it to send events to the renderer.
Something
> like:
> 
>   interface InputEventHandler {
>     OnWindowInputEvent(window_id, event) => (bool handled);
>   };
> 
>   interface WindowTreeClient {
>     ...
>     RequestInputEventHandler(InputEventHandler& handler);
>   };
> 
>   interface WindowTree {
>     ...
>     Embed(window_id, WindowTreeClient, flags) => (bool success,
> InputEventHandler? handler);
>   };

I have started on this part of the API
(https://bugs.chromium.org/p/chromium/issues/detail?id=621085#c2), but would
still like to land this part separately. WDYT?

[Ben Goodger (Google), 2016-06-23 21:09:41]

On 2016/06/16 19:09:25, sadrul wrote:
> On 2016/06/16 17:48:22, Ben Goodger (Google) wrote:
> > On 2016/06/16 17:45:17, sadrul wrote:
> > >
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > > File components/mus/public/interfaces/window_tree.mojom (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2068093002/diff/60001/components/mus/public/i...
> > > components/mus/public/interfaces/window_tree.mojom:59: // to the embedded
> > client
> > > through other mechanism.
> > > On 2016/06/16 17:34:36, Ben Goodger (Google) wrote:
> > > > What other mechanism?
> > > 
> > > In the case of chrome, the browser has another IPC channel to the
renderer,
> > and
> > > it would use that channel for sending the events to the renderer. i.e. mus
> > does
> > > not provide a mechanism for the embedder to directly dispatch events to
the
> > > embedded client. The embedder/embeddee need to have a separate mechanism
> that
> > > they decide on.
> > 
> > Do you think it could be worth defining a standard mechanism? e.g. defining
an
> > EventHandler interface, and having mus use it to dispatch events to clients,
> but
> > allowing clients to expose impls of it to one another for this purpose, so
at
> > least events get transported with identical semantics?
> 
> Hm, interesting! I think it'd be possible, yes. The client will be aware that
> the input-events it is receiving is coming from the embedder, and not directly
> from mus, but we should be able to hide all of that in the client-lib, so
> clients wouldn't need to explicitly do anything differently.
> 
> The way I am thinking this would work for chrome is: mus sends a request for
an
> EventHandler to the renderer (or any embedded client whose embedder requested
to
> intercept events), and send the other end of it to the browser (or the
> embedder). The browser can then use it to send events to the renderer.
Something
> like:
> 
>   interface InputEventHandler {
>     OnWindowInputEvent(window_id, event) => (bool handled);
>   };
> 
>   interface WindowTreeClient {
>     ...
>     RequestInputEventHandler(InputEventHandler& handler);
>   };
> 
>   interface WindowTree {
>     ...
>     Embed(window_id, WindowTreeClient, flags) => (bool success,
> InputEventHandler? handler);
>   };


Ah. You may want RequestInputEventHandler to take an associated
InputEventHandler& to guarantee ordering if that is important.

anyway lgtm for this change.

[sadrul, 2016-06-24 07:20:54]

sadrul@chromium.org changed reviewers:
+ kenrb@chromium.org

[sadrul, 2016-06-24 07:20:55]

+kenrb@ for components/mus/public/interfaces/ changes.

[kenrb, 2016-06-24 16:00:14]

lgtm

[sadrul, 2016-06-25 07:15:53]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-06-25 07:15:54]

The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org,
ben@chromium.org
Link to the patchset: https://codereview.chromium.org/2068093002/#ps120001
(title: "tot-merge")

[commit-bot: I haz the power, 2016-06-25 07:15:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-25 08:04:29]

Description was changed from

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078
==========

to

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078
==========

[commit-bot: I haz the power, 2016-06-25 08:04:30]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-25 08:06:05]

Description was changed from

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078
==========

to

==========
mus: Allow embedder to intercept events.

Introduce mechanism so that an embedder receives the events targeted to
embedded window. This allows the browser window to receive events, even
when the renderer-content lives in a separate mus::Window.

BUG=621078

Committed: https://crrev.com/4d3588cbec47fcd629c038a0ca6aad9af58e4192
Cr-Commit-Position: refs/heads/master@{#402057}
==========

[commit-bot: I haz the power, 2016-06-25 08:06:07]

Patchset 7 (id:??) landed as
https://crrev.com/4d3588cbec47fcd629c038a0ca6aad9af58e4192
Cr-Commit-Position: refs/heads/master@{#402057}