Adds ability for accelerators to add key/value pairs to KeyEvent

Adds ability for accelerators to add key/value pairs to KeyEvent

pre-target accelerators are run before the KeyEvent is passed to the
target. If the pre-target accelerator doesn't handle the event the
event is passed to the target. This patch adds the ability for the
window-manager to add key-value pairs to the KeyEvent during the
pre-target phase. This will be used so that ash can inform chrome of
state about the KeyEvent that is necessary during Chrome's processing
the KeyEvent.

BUG=701815
TEST=covered by tests
R=erg@chromium.org, tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2761233003
Cr-Commit-Position: refs/heads/master@{#458752}
Committed: https://chromium.googlesource.com/chromium/src/+/c0c862a8b28d6afe8880049610ecb2320fd2b5b3

[sky, 2017-03-21 23:20:08]

Description was changed from

==========
Adds ability for accelerators to add key/value pairs to KeyEvent

pre-target accelerators are run before the KeyEvent is passed to the
target. If the pre-target accelerator doesn't handle the event the
event is passed to the target. This patch adds the ability for the
window-manager to add key-value pairs to the KeyEvent during the
pre-target phase. This will be used so that ash can inform chrome of
state about the KeyEvent that is necessary during Chrome's processing
the KeyEvent.

BUG=701815
TEST=covered by tests
R=erg@chromium.org
==========

to

==========
Adds ability for accelerators to add key/value pairs to KeyEvent

pre-target accelerators are run before the KeyEvent is passed to the
target. If the pre-target accelerator doesn't handle the event the
event is passed to the target. This patch adds the ability for the
window-manager to add key-value pairs to the KeyEvent during the
pre-target phase. This will be used so that ash can inform chrome of
state about the KeyEvent that is necessary during Chrome's processing
the KeyEvent.

BUG=701815
TEST=covered by tests
R=erg@chromium.org, tsepez@chromium.org
==========

[sky, 2017-03-21 23:20:08]

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

[sky, 2017-03-21 23:20:23]

tsepez: mojom
erg: the rest

[sky, 2017-03-21 23:20:26]

The CQ bit was checked by sky@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 23:20:46]

Dry run: 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, 2017-03-21 23:31:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 23:31:40]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Elliot Glaysher, 2017-03-21 23:39:29]

here is a string of letters for the commit queue: lgtm

now that that's out of the way, i think adding a property bag on key event is
not greatest idea, but you've convinced me in person that this is cleanest way
to deal with this given current behaviour in ash.

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
services/ui/ws/window_tree.cc:560: DVLOG(3) << "dispatching accelerator,
client=" << id_;
Consistent style on these DVLOGs? Preferably using the method name as you do
latter in this file?

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
services/ui/ws/window_tree.cc:1507: DVLOG(3) << "OnWindowInputEventAck client="
<< id_;
"ck[,] client"

Add comma to match output in next DVLOG.

[Tom Sepez, 2017-03-21 23:46:49]

mojom LGTM.  This only happens from window manager, and we trust it, right?

[sky, 2017-03-21 23:49:14]

That's right, only the window-manager can do this.

On Tue, Mar 21, 2017 at 4:46 PM,  <tsepez@chromium.org> wrote:
> mojom LGTM. This only happens from window manager, and we trust it, right?
>
> https://codereview.chromium.org/2761233003/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2017-03-22 00:01:03]

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
services/ui/ws/window_tree.cc:560: DVLOG(3) << "dispatching accelerator,
client=" << id_;
On 2017/03/21 23:39:28, Elliot Glaysher wrote:
> Consistent style on these DVLOGs? Preferably using the method name as you do
> latter in this file?

Done.

https://codereview.chromium.org/2761233003/diff/20001/services/ui/ws/window_t...
services/ui/ws/window_tree.cc:1507: DVLOG(3) << "OnWindowInputEventAck client="
<< id_;
On 2017/03/21 23:39:28, Elliot Glaysher wrote:
> "ck[,] client"
> 
> Add comma to match output in next DVLOG.

I removed the comma from all of them, seemed superflous.

[sky, 2017-03-22 00:01:15]

The CQ bit was checked by sky@chromium.org

[sky, 2017-03-22 00:01:17]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
erg@chromium.org
Link to the patchset: https://codereview.chromium.org/2761233003/#ps40001
(title: "feedback and compile")

[commit-bot: I haz the power, 2017-03-22 00:01:53]

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, 2017-03-22 00:30:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 00:30:20]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sky, 2017-03-22 03:08:56]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-22 03:09:49]

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, 2017-03-22 05:38:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 05:38:22]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[sky, 2017-03-22 15:04:51]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-22 15:05:20]

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, 2017-03-22 15:14:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490195091916420,
"parent_rev": "a491c34db6d1cdd8eceece02c8b5251d67c007df", "commit_rev":
"c0c862a8b28d6afe8880049610ecb2320fd2b5b3"}

[commit-bot: I haz the power, 2017-03-22 15:15:03]

Description was changed from

==========
Adds ability for accelerators to add key/value pairs to KeyEvent

pre-target accelerators are run before the KeyEvent is passed to the
target. If the pre-target accelerator doesn't handle the event the
event is passed to the target. This patch adds the ability for the
window-manager to add key-value pairs to the KeyEvent during the
pre-target phase. This will be used so that ash can inform chrome of
state about the KeyEvent that is necessary during Chrome's processing
the KeyEvent.

BUG=701815
TEST=covered by tests
R=erg@chromium.org, tsepez@chromium.org
==========

to

==========
Adds ability for accelerators to add key/value pairs to KeyEvent

pre-target accelerators are run before the KeyEvent is passed to the
target. If the pre-target accelerator doesn't handle the event the
event is passed to the target. This patch adds the ability for the
window-manager to add key-value pairs to the KeyEvent during the
pre-target phase. This will be used so that ash can inform chrome of
state about the KeyEvent that is necessary during Chrome's processing
the KeyEvent.

BUG=701815
TEST=covered by tests
R=erg@chromium.org, tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2761233003
Cr-Commit-Position: refs/heads/master@{#458752}
Committed:
https://chromium.googlesource.com/chromium/src/+/c0c862a8b28d6afe8880049610ec...
==========

[commit-bot: I haz the power, 2017-03-22 15:15:06]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c0c862a8b28d6afe8880049610ec...