Add AcceleratorType to EventMatcher

Add AcceleratorType to EventMatcher

AcceleratorType defines pre-target and post-target types for EventMatcher. Pre-target accelerators will be dispatched inplace of the target window. Post-target accelerators will be dispatched after the window has a chance to consume the event. A following review will implement this.

This change also update EventMatcher to use AcceleratorType. Currently mus client will default to creating pre-target accelerators.

TEST=EventDispatcherTest.PostTargetAccelerator
BUG=560478
Committed: https://crrev.com/4d34a5d6983598edf98941672211734eca04caa9
Cr-Commit-Position: refs/heads/master@{#379579}

[jonross, 2016-03-03 20:56:00]

Patchset #1 (id:1) has been deleted

[jonross, 2016-03-03 22:11:47]

jonross@chromium.org changed reviewers:
+ sky@chromium.org

[jonross, 2016-03-03 22:11:48]

Hi,

I have added an AcceleratorType to EventMatcher. This captures whether an event
is a pre-target or post-target event.

Could you PTAL?

Thanks,
Jon

[sky, 2016-03-03 23:41:24]

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
File components/mus/public/interfaces/input_event_constants.mojom (right):

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_constants.mojom:8: PRE_TARGET,
Document what these mean.

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
File components/mus/public/interfaces/input_event_matcher.mojom (right):

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:12: struct
AcceleratorTypeMatcher {
type->phase

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorTypeMatcher? accelerator_matcher;
Shouldn't this be required? What would it mean if you don't specify it?

[jonross, 2016-03-04 15:26:00]

Patchset #2 (id:40001) has been deleted

[jonross, 2016-03-04 15:26:37]

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
File components/mus/public/interfaces/input_event_constants.mojom (right):

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_constants.mojom:8: PRE_TARGET,
On 2016/03/03 23:41:24, sky wrote:
> Document what these mean.

Done.

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
File components/mus/public/interfaces/input_event_matcher.mojom (right):

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:12: struct
AcceleratorTypeMatcher {
On 2016/03/03 23:41:24, sky wrote:
> type->phase

Done.

https://codereview.chromium.org/1759273002/diff/20001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorTypeMatcher? accelerator_matcher;
On 2016/03/03 23:41:24, sky wrote:
> Shouldn't this be required? What would it mean if you don't specify it?

I had been setting up a default of pre-target, but I think you are correct, this
should be required. Done.

[sky, 2016-03-04 16:51:55]

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

https://codereview.chromium.org/1759273002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorPhaseMatcher accelerator_matcher;
As the phase is always required can you do away with the struct and use the enum
directly here?

[jonross, 2016-03-04 20:38:20]

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

https://codereview.chromium.org/1759273002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorPhaseMatcher accelerator_matcher;
On 2016/03/04 16:51:55, sky wrote:
> As the phase is always required can you do away with the struct and use the
enum
> directly here?

Certainly. Thought that will imply a default.

I would be fine with defaulting to pre-target phase. However we could add an
'undefined' phase as the default, and require apps to specify which one.

Thoughts?

[sky, 2016-03-04 21:36:16]

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

https://codereview.chromium.org/1759273002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorPhaseMatcher accelerator_matcher;
On 2016/03/04 20:38:20, jonross wrote:
> On 2016/03/04 16:51:55, sky wrote:
> > As the phase is always required can you do away with the struct and use the
> enum
> > directly here?
> 
> Certainly. Thought that will imply a default.
> 
> I would be fine with defaulting to pre-target phase. However we could add an
> 'undefined' phase as the default, and require apps to specify which one.
> 
> Thoughts?

Ah, I see what you're saying. I'm fine with pretarget.

[jonross, 2016-03-04 22:11:50]

Patchset #3 (id:80001) has been deleted

[jonross, 2016-03-04 22:13:03]

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

https://codereview.chromium.org/1759273002/diff/60001/components/mus/public/i...
components/mus/public/interfaces/input_event_matcher.mojom:50:
AcceleratorPhaseMatcher accelerator_matcher;
On 2016/03/04 21:36:16, sky wrote:
> On 2016/03/04 20:38:20, jonross wrote:
> > On 2016/03/04 16:51:55, sky wrote:
> > > As the phase is always required can you do away with the struct and use
the
> > enum
> > > directly here?
> > 
> > Certainly. Thought that will imply a default.
> > 
> > I would be fine with defaulting to pre-target phase. However we could add an
> > 'undefined' phase as the default, and require apps to specify which one.
> > 
> > Thoughts?
> 
> Ah, I see what you're saying. I'm fine with pretarget.

Awesome. Done.

[sky, 2016-03-04 23:01:07]

LGTM

[jonross, 2016-03-07 16:54:05]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2016-03-07 16:54:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1759273002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1759273002/100001

[commit-bot: I haz the power, 2016-03-07 17:45:51]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-03-07 17:46:46]

Description was changed from

==========
Add AcceleratorType to EventMatcher

AcceleratorType defines pre-target and post-target types for EventMatcher.
Pre-target accelerators will be dispatched inplace of the target window.
Post-target accelerators will be dispatched after the window has a chance to
consume the event. A following review will implement this.

This change also update EventMatcher to use AcceleratorType. Currently mus
client will default to creating pre-target accelerators.

TEST=EventDispatcherTest.PostTargetAccelerator
BUG=560478
==========

to

==========
Add AcceleratorType to EventMatcher

AcceleratorType defines pre-target and post-target types for EventMatcher.
Pre-target accelerators will be dispatched inplace of the target window.
Post-target accelerators will be dispatched after the window has a chance to
consume the event. A following review will implement this.

This change also update EventMatcher to use AcceleratorType. Currently mus
client will default to creating pre-target accelerators.

TEST=EventDispatcherTest.PostTargetAccelerator
BUG=560478

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

[commit-bot: I haz the power, 2016-03-07 17:46:47]

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