[ash-md] Makes Ctrl+W accelerator close a selected window in overview mode

[ash-md] Makes Ctrl+W accelerator close a selected window in overview mode

Added accelerator handling:
- Ctrl+W - closes a selected window (avoiding handling 'w').
- Shift+Tab - cycles left (Shift modifier wasn't handled before).

BUG=626165
TEST=WindowSelectorTest.CloseWindowWithKey

Committed: https://crrev.com/7f53f2ef11c87d9578f55950f713628908cd5c5d
Cr-Commit-Position: refs/heads/master@{#404411}

[varkha, 2016-07-07 19:34:42]

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

[varkha, 2016-07-07 19:34:52]

varkha@chromium.org changed reviewers:
+ tdanderson@chromium.org

[varkha, 2016-07-07 19:35:22]

tdanderson@, can you please take a look? Thanks!

[commit-bot: I haz the power, 2016-07-07 19:35:32]

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, 2016-07-07 19:38:28]

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

[commit-bot: I haz the power, 2016-07-07 19:38:28]

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

[varkha, 2016-07-07 21:05:01]

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

[commit-bot: I haz the power, 2016-07-07 21:05:45]

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

[tdanderson, 2016-07-07 21:26:49]

LGTM with a few comments

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:511: // allow the textfield to handle
'W' key when not used with Ctrl.
nit: 'Allow' instead of 'allow'

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:514:
grid_list_[selected_grid_index_]->SelectedWindow()->CloseWindow();
Either here or in a follow-up CL, consider adding two user action counts: one
which fires when a window is closed by keyboard, and one which fires when a
window is closed by clicking/tapping the close button in the header. This is
cheap to do and maintains the already-excellent UMA coverage we have on overview
mode.

https://codereview.chromium.org/2131603004/diff/20001/ash/common/wm/overview/...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2131603004/diff/20001/ash/common/wm/overview/...
ash/common/wm/overview/window_selector.cc:503: (key_event.key_code() !=
ui::VKEY_TAB)) {
nit: I think this is slightly more readable: if (key_code == RIGHT ||
!shift_down)

[commit-bot: I haz the power, 2016-07-07 23:08:29]

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

[commit-bot: I haz the power, 2016-07-07 23:08:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)

[varkha, 2016-07-07 23:57:19]

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

[commit-bot: I haz the power, 2016-07-07 23:58:03]

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

[varkha, 2016-07-08 00:00:12]

varkha@chromium.org changed reviewers:
+ rkaplow@chromium.org, sky@chromium.org

[varkha, 2016-07-08 00:00:13]

+rkaplow@ for OWNERS in tools/metrics
+sky@ for OWNERS in ash/metrics (added UMA counters)

Thanks!

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:511: // allow the textfield to handle
'W' key when not used with Ctrl.
On 2016/07/07 21:26:49, tdanderson wrote:
> nit: 'Allow' instead of 'allow'

Done.

https://codereview.chromium.org/2131603004/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:514:
grid_list_[selected_grid_index_]->SelectedWindow()->CloseWindow();
On 2016/07/07 21:26:49, tdanderson wrote:
> Either here or in a follow-up CL, consider adding two user action counts: one
> which fires when a window is closed by keyboard, and one which fires when a
> window is closed by clicking/tapping the close button in the header. This is
> cheap to do and maintains the already-excellent UMA coverage we have on
overview
> mode.

Done.

https://codereview.chromium.org/2131603004/diff/20001/ash/common/wm/overview/...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2131603004/diff/20001/ash/common/wm/overview/...
ash/common/wm/overview/window_selector.cc:503: (key_event.key_code() !=
ui::VKEY_TAB)) {
On 2016/07/07 21:26:49, tdanderson wrote:
> nit: I think this is slightly more readable: if (key_code == RIGHT ||
> !shift_down)

Done.

[varkha, 2016-07-08 00:03:17]

sky@, seems ash/common/metrics should have same owners as ash/metrics, right?

[varkha, 2016-07-08 00:07:45]

Posted https://codereview.chromium.org/2131763002 to deal with owners in
ash/metrics and ash/common/metrics.

[commit-bot: I haz the power, 2016-07-08 01:03:28]

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

[commit-bot: I haz the power, 2016-07-08 01:03:29]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2016-07-08 14:31:52]

lgtm

actions lg

https://codereview.chromium.org/2131603004/diff/40001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2131603004/diff/40001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:157: <action
name="AboutFlags_document-passive-event-listeners">
FYI just approved a CL fixing this https://codereview.chromium.org/2131463004/

[varkha, 2016-07-08 15:17:47]

varkha@chromium.org changed reviewers:
- sky@chromium.org

[varkha, 2016-07-08 15:17:48]

-sky@ - already have all lgtms from owners.

https://codereview.chromium.org/2131603004/diff/40001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2131603004/diff/40001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:157: <action
name="AboutFlags_document-passive-event-listeners">
On 2016/07/08 14:31:52, rkaplow wrote:
> FYI just approved a CL fixing this https://codereview.chromium.org/2131463004/

Thanks for heads up, merged.

[varkha, 2016-07-08 15:18:00]

The CQ bit was checked by varkha@chromium.org

[varkha, 2016-07-08 15:18:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org, rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/2131603004/#ps60001
(title: "[ash-md] Makes Ctrl+W accelerator close a selected window in overview
mode (UMA)")

[commit-bot: I haz the power, 2016-07-08 15:18:26]

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-07-08 16:45:58]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-08 16:46:09]

CQ bit was unchecked.

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

Description was changed from

==========
[ash-md] Makes Ctrl+W accelerator close a selected window in overview mode

Added accelerator handling:
- Ctrl+W - closes a selected window (avoiding handling 'w').
- Shift+Tab - cycles left (Shift modifier wasn't handled before).

BUG=626165
TEST=WindowSelectorTest.CloseWindowWithKey
==========

to

==========
[ash-md] Makes Ctrl+W accelerator close a selected window in overview mode

Added accelerator handling:
- Ctrl+W - closes a selected window (avoiding handling 'w').
- Shift+Tab - cycles left (Shift modifier wasn't handled before).

BUG=626165
TEST=WindowSelectorTest.CloseWindowWithKey

Committed: https://crrev.com/7f53f2ef11c87d9578f55950f713628908cd5c5d
Cr-Commit-Position: refs/heads/master@{#404411}
==========

[commit-bot: I haz the power, 2016-07-08 16:47:07]

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