Separate debugging and developer accelerators

Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945, 642551
Committed: https://crrev.com/cd09f64d921e02333652d2fae387b8b406201cf8
Cr-Commit-Position: refs/heads/master@{#419301}

[afakhry, 2016-09-08 17:47:40]

Patchset #1 (id:1) has been deleted

[afakhry, 2016-09-08 17:47:58]

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

[commit-bot: I haz the power, 2016-09-08 17:48:19]

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-09-08 18:34:01]

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

[commit-bot: I haz the power, 2016-09-08 18:34:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[afakhry, 2016-09-08 18:40:37]

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

[afakhry, 2016-09-08 18:40:56]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-09-08 18:41:12]

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

[afakhry, 2016-09-08 18:41:24]

Oshima-san, please review. Thank you!

[afakhry, 2016-09-08 18:41:41]

afakhry@chromium.org changed reviewers:
+ oshima@chromium.org

[afakhry, 2016-09-08 18:41:42]

Oshima-san, please review. Thank you!

[commit-bot: I haz the power, 2016-09-08 19:36:48]

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

[commit-bot: I haz the power, 2016-09-08 19:36:49]

Dry run: This issue passed the CQ dry run.

[afakhry, 2016-09-14 00:50:48]

Oshima, friendly ping.

[oshima, 2016-09-14 16:34:10]

https://codereview.chromium.org/2323863002/diff/40001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.h (right):

https://codereview.chromium.org/2323863002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:65: DEBUG_TOGGLE_WALLPAPER_MODE,
can you rename dev specific actions to DEV_?

https://codereview.chromium.org/2323863002/diff/40001/ash/common/ash_switches.cc
File ash/common/ash_switches.cc (right):

https://codereview.chromium.org/2323863002/diff/40001/ash/common/ash_switches...
ash/common/ash_switches.cc:31: const char kAshDeveloperShortcuts[] =
"ash-developer-shortcuts";
I have slight preference to ash-dev-shortcuts, as dev is pretty common.
you may leave it if you prefer this way though.

[afakhry, 2016-09-15 20:25:29]

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

[commit-bot: I haz the power, 2016-09-15 20:25:51]

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

[afakhry, 2016-09-15 20:26:51]

https://codereview.chromium.org/2323863002/diff/40001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.h (right):

https://codereview.chromium.org/2323863002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:65: DEBUG_TOGGLE_WALLPAPER_MODE,
On 2016/09/14 16:34:10, oshima wrote:
> can you rename dev specific actions to DEV_?

Done.

https://codereview.chromium.org/2323863002/diff/40001/ash/common/ash_switches.cc
File ash/common/ash_switches.cc (right):

https://codereview.chromium.org/2323863002/diff/40001/ash/common/ash_switches...
ash/common/ash_switches.cc:31: const char kAshDeveloperShortcuts[] =
"ash-developer-shortcuts";
On 2016/09/14 16:34:10, oshima wrote:
> I have slight preference to ash-dev-shortcuts, as dev is pretty common.
> you may leave it if you prefer this way though.

Done. ash-dev-shortcuts is shorter, hence is better. Thanks!

[oshima, 2016-09-15 20:55:53]

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
File ash/accelerators/accelerator_controller_delegate_aura.cc (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
ash/accelerators/accelerator_controller_delegate_aura.cc:277: case
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
nit: sort

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
ash/accelerators/accelerator_controller_delegate_aura.cc:385: case
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
ditto

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.cc:279: {true, ui::VKEY_T,
kDebugModifier, DEV_TOGGLE_TOUCH_VIEW},
Sorry, I missed this in the first review. This should be DEBUG
(crbug.com/642551)

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.cc:396: DEV_ADD_REMOVE_DISPLAY,
sort

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.h (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:60:
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN,
sort

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:217: // --ash-developer-shortcuts.
They are always run similar to reserved actions.
-dev-

[commit-bot: I haz the power, 2016-09-15 21:09:22]

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

[commit-bot: I haz the power, 2016-09-15 21:09:23]

Dry run: This issue passed the CQ dry run.

[afakhry, 2016-09-15 21:57:33]

Description was changed from

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945
==========

to

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945,642551
==========

[afakhry, 2016-09-15 21:57:40]

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
File ash/accelerators/accelerator_controller_delegate_aura.cc (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
ash/accelerators/accelerator_controller_delegate_aura.cc:277: case
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
On 2016/09/15 20:55:52, oshima wrote:
> nit: sort

Done.

https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
ash/accelerators/accelerator_controller_delegate_aura.cc:385: case
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
On 2016/09/15 20:55:53, oshima wrote:
> ditto

Done.

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.cc:279: {true, ui::VKEY_T,
kDebugModifier, DEV_TOGGLE_TOUCH_VIEW},
On 2016/09/15 20:55:53, oshima wrote:
> Sorry, I missed this in the first review. This should be DEBUG
> (crbug.com/642551)

Moved it back to debug.

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.cc:396: DEV_ADD_REMOVE_DISPLAY,
On 2016/09/15 20:55:53, oshima wrote:
> sort

Done.

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
File ash/common/accelerators/accelerator_table.h (right):

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:60:
DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN,
On 2016/09/15 20:55:53, oshima wrote:
> sort

Done.

https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
ash/common/accelerators/accelerator_table.h:217: // --ash-developer-shortcuts.
They are always run similar to reserved actions.
On 2016/09/15 20:55:53, oshima wrote:
> -dev-

Done.

[afakhry, 2016-09-16 20:09:34]

On 2016/09/15 21:57:40, afakhry wrote:
>
https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
> File ash/accelerators/accelerator_controller_delegate_aura.cc (right):
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
> ash/accelerators/accelerator_controller_delegate_aura.cc:277: case
> DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
> On 2016/09/15 20:55:52, oshima wrote:
> > nit: sort
> 
> Done.
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/accelerators/accele...
> ash/accelerators/accelerator_controller_delegate_aura.cc:385: case
> DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN:
> On 2016/09/15 20:55:53, oshima wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> File ash/common/accelerators/accelerator_table.cc (right):
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> ash/common/accelerators/accelerator_table.cc:279: {true, ui::VKEY_T,
> kDebugModifier, DEV_TOGGLE_TOUCH_VIEW},
> On 2016/09/15 20:55:53, oshima wrote:
> > Sorry, I missed this in the first review. This should be DEBUG
> > (crbug.com/642551)
> 
> Moved it back to debug.
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> ash/common/accelerators/accelerator_table.cc:396: DEV_ADD_REMOVE_DISPLAY,
> On 2016/09/15 20:55:53, oshima wrote:
> > sort
> 
> Done.
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> File ash/common/accelerators/accelerator_table.h (right):
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> ash/common/accelerators/accelerator_table.h:60:
> DEV_TOGGLE_ROOT_WINDOW_FULL_SCREEN,
> On 2016/09/15 20:55:53, oshima wrote:
> > sort
> 
> Done.
> 
>
https://codereview.chromium.org/2323863002/diff/60001/ash/common/accelerators...
> ash/common/accelerators/accelerator_table.h:217: // --ash-developer-shortcuts.
> They are always run similar to reserved actions.
> On 2016/09/15 20:55:53, oshima wrote:
> > -dev-
> 
> Done.

Oshima?

[oshima, 2016-09-16 20:34:06]

lgtm

[afakhry, 2016-09-16 21:35:51]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2016-09-16 21:36:37]

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-09-16 22:27:14]

Description was changed from

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945,642551
==========

to

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945,642551
==========

[commit-bot: I haz the power, 2016-09-16 22:27:15]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-09-16 22:29:18]

Description was changed from

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945,642551
==========

to

==========
Separate debugging and developer accelerators

Hide developer accelerators behind --ash-developer-shortcuts command line
switch.

BUG=642945,642551

Committed: https://crrev.com/cd09f64d921e02333652d2fae387b8b406201cf8
Cr-Commit-Position: refs/heads/master@{#419301}
==========

[commit-bot: I haz the power, 2016-09-16 22:29:19]

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

[msw, 2016-10-05 23:01:57]

msw@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2016-10-05 23:01:58]

https://codereview.chromium.org/2323863002/diff/80001/ash/common/ash_switches.h
File ash/common/ash_switches.h (right):

https://codereview.chromium.org/2323863002/diff/80001/ash/common/ash_switches...
ash/common/ash_switches.h:24: ASH_EXPORT extern const char
kAshDeveloperShortcuts[];
Why do we want a separate switch for developer shortcuts versus debug shortcuts?
It seems like this just makes it more confusing for developers for provide the
right flag to access the shortcut they want.