Adds the concept of a 'safe mode' hotkey start

Adds the concept of a 'safe mode' hotkey start

The issue is that with Aura if something goes horribly wrong with the GPU, the user can get a 'black window' with no UI.

So for ConOps it is nice to have a way to start chrome with a set of flags that we know should get you a working chrome that they can troubleshoot.

To do this we add a hotkey combo to the start menu shortcut, in this case ctrl-shift-f10 which is not taken by windows. This shortcut can start chrome in this safe mode.

BUG=271719
TEST=see bug
R=robertshield@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232825

[cpu_(ooo_6.6-7.5), 2013-11-03 05:01:48]

please take a look. I would appreciate a review before branch.

[scottmg, 2013-11-03 19:48:13]

Seems fine, except Ctrl-Shift-F10 is "Set Next Statement" in Visual Studio. So
it's a bit annoying to have that launch chrome. I guess someone's going to be
annoyed by any global shortcut though.

Ctrl-Alt-Shift-S ? (for "s"afe)

[robertshield, 2013-11-04 02:19:54]

Ignorable nits, implementation itself LGTM.

Similar concerns about global hotkey though. Would dropping a safe mode shortcut
(with the right command line args) in the start menu or some other unobtrusive,
but findable location serve as well instead?

https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc
File base/win/shortcut.cc (right):

https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc#newcode147
base/win/shortcut.cc:147:
FAILED(i_shell_link->SetHotkey(LOWORD(properties.hotkey)))) {
nit: indent

https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
File chrome/installer/setup/install.cc (right):

https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
chrome/installer/setup/install.cc:460: // The 'safe mode' hotkey launch shortcut
only for the start menu shortcut.
suggestion: The 'launch safe mode' hotkey should be set only for the start menu
shortcut.

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
File chrome/installer/util/browser_distribution.h (right):

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
chrome/installer/util/browser_distribution.h:165: virtual int
GetSafeModeHotkey();
should this return a uint32 instead?

[grt (UTC plus 2), 2013-11-04 15:25:14]

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
File chrome/installer/util/browser_distribution.h (right):

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
chrome/installer/util/browser_distribution.h:165: virtual int
GetSafeModeHotkey();
On 2013/11/04 02:19:55, robertshield wrote:
> should this return a uint32 instead?

what about uint16 (IShellLink::SetHotkey takes a WORD)?

[cpu_(ooo_6.6-7.5), 2013-11-04 19:10:57]

code changed, ptal.

I am not too happy with the global shortcut either. The final key combination
might change, if so there would be a different CL with just that.

Here are in condensed form the thought process.

- "start chrome in safe mode" shortcut:
The issue we found with Scott about the shortcut is that if you do the power
user mode to start things [winkey] + "chrome" you might end up starting the safe
mode, it depends in the locale specif sorting. Lesser but important is that
people will stumble upon it.

- launch chrome via prompt with flags
The feeling is that is too complicated for many folks. Lesser problem but has
happened: if user has UAC she might launch chrome elevated and that creates all
sorts of problems down the line.

- Modify the shortcut: also very complicated for most folks. Win 8 start screen
makes it even harder.

If you have ideas, I am open to them, but lets continue that on the bug.

https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc
File base/win/shortcut.cc (right):

https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc#newcode147
base/win/shortcut.cc:147:
FAILED(i_shell_link->SetHotkey(LOWORD(properties.hotkey)))) {
On 2013/11/04 02:19:55, robertshield wrote:
> nit: indent

Done.

https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
File chrome/installer/setup/install.cc (right):

https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
chrome/installer/setup/install.cc:460: // The 'safe mode' hotkey launch shortcut
only for the start menu shortcut.
On 2013/11/04 02:19:55, robertshield wrote:
> suggestion: The 'launch safe mode' hotkey should be set only for the start
menu
> shortcut.

Done.

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
File chrome/installer/util/browser_distribution.h (right):

https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
chrome/installer/util/browser_distribution.h:165: virtual int
GetSafeModeHotkey();
On 2013/11/04 02:19:55, robertshield wrote:
> should this return a uint32 instead?

Done.

Since I am not ifdefing for windows I rather keep it dword size, also seems that
RegisterHotkey (if we were to ever use it) is hard to pack in 16 bits (although
technically doable).

Going with Robert's uint32.

[robertshield, 2013-11-04 19:18:20]

On 2013/11/04 19:10:57, cpu wrote:
> code changed, ptal.
> 
> I am not too happy with the global shortcut either. The final key combination
> might change, if so there would be a different CL with just that.
> 
> Here are in condensed form the thought process.
> 
> - "start chrome in safe mode" shortcut:
> The issue we found with Scott about the shortcut is that if you do the power
> user mode to start things [winkey] + "chrome" you might end up starting the
safe
> mode, it depends in the locale specif sorting. Lesser but important is that
> people will stumble upon it.
> 
> - launch chrome via prompt with flags
> The feeling is that is too complicated for many folks. Lesser problem but has
> happened: if user has UAC she might launch chrome elevated and that creates
all
> sorts of problems down the line.
> 
> - Modify the shortcut: also very complicated for most folks. Win 8 start
screen
> makes it even harder.
> 
> If you have ideas, I am open to them, but lets continue that on the bug.
> 
> https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc
> File base/win/shortcut.cc (right):
> 
>
https://codereview.chromium.org/57173002/diff/1/base/win/shortcut.cc#newcode147
> base/win/shortcut.cc:147:
> FAILED(i_shell_link->SetHotkey(LOWORD(properties.hotkey)))) {
> On 2013/11/04 02:19:55, robertshield wrote:
> > nit: indent
> 
> Done.
> 
>
https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
> File chrome/installer/setup/install.cc (right):
> 
>
https://codereview.chromium.org/57173002/diff/1/chrome/installer/setup/instal...
> chrome/installer/setup/install.cc:460: // The 'safe mode' hotkey launch
shortcut
> only for the start menu shortcut.
> On 2013/11/04 02:19:55, robertshield wrote:
> > suggestion: The 'launch safe mode' hotkey should be set only for the start
> menu
> > shortcut.
> 
> Done.
> 
>
https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
> File chrome/installer/util/browser_distribution.h (right):
> 
>
https://codereview.chromium.org/57173002/diff/1/chrome/installer/util/browser...
> chrome/installer/util/browser_distribution.h:165: virtual int
> GetSafeModeHotkey();
> On 2013/11/04 02:19:55, robertshield wrote:
> > should this return a uint32 instead?
> 
> Done.
> 
> Since I am not ifdefing for windows I rather keep it dword size, also seems
that
> RegisterHotkey (if we were to ever use it) is hard to pack in 16 bits
(although
> technically doable).
> 
> Going with Robert's uint32.

Implementation still LGTM

[cpu_(ooo_6.6-7.5), 2013-11-04 22:30:57]

Committed patchset #2 manually as r232825.

[kuzmeech, 2013-11-14 23:08:07]

On 2013/11/03 05:01:48, cpu wrote:
> please take a look. I would appreciate a review before branch.

keyboard combination also overlaps with PyCharm's standard shortcut for "Run", I
wonder if I can disable it as it triggers current release of Chrome Canary each
time I expect PyCharm/IDEA to do the run...

[scottmg, 2013-11-14 23:11:19]

On 2013/11/14 23:08:07, kuzmeech wrote:
> On 2013/11/03 05:01:48, cpu wrote:
> > please take a look. I would appreciate a review before branch.
> 
> keyboard combination also overlaps with PyCharm's standard shortcut for "Run",
I
> wonder if I can disable it as it triggers current release of Chrome Canary
each
> time I expect PyCharm/IDEA to do the run...

The shortcut was in trunk for a while, but won't be shipping in a release.