Make delegate_execute GetLaunchMode() check for default browser.

Make delegate_execute GetLaunchMode() check for default browser.

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

[zturner, 2014-03-06 21:37:54]

gab: chrome/installer/*
cpu: win8/*

[gab, 2014-03-06 22:20:13]

lgtm w/ comments

Cheers!
Gab

https://codereview.chromium.org/189113002/diff/20001/chrome/installer/util/sh...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/189113002/diff/20001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:1763: LOG(ERROR) << "Error getting app exe
path";
Make this a NOTREACHED();

(I know you were just copying this but might as well clean it up --
LOG(ERROR)'s, especially non-debug ones with error strings that wind up in the
Release binary, are frowned upon)

https://codereview.chromium.org/189113002/diff/20001/chrome/installer/util/sh...
File chrome/installer/util/shell_util.h (right):

https://codereview.chromium.org/189113002/diff/20001/chrome/installer/util/sh...
chrome/installer/util/shell_util.h:417: static DefaultState
GetChromeDefaultState(
Overloading is typically discouraged:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

What about GetChromeDefaultStateFromPath() or something more specific?

https://codereview.chromium.org/189113002/diff/20001/chrome/installer/util/sh...
chrome/installer/util/shell_util.h:418: const base::FilePath& chrome_exe_path);
s/chrome_exe_path/chrome_exe (to be consistent with how ShellUtil names
variables containing chrome.exe's path).

https://codereview.chromium.org/189113002/diff/20001/win8/delegate_execute/co...
File win8/delegate_execute/command_execute_impl.cc (right):

https://codereview.chromium.org/189113002/diff/20001/win8/delegate_execute/co...
win8/delegate_execute/command_execute_impl.cc:425: base::FilePath chrome_exe;
nit: empty line above.

[cpu_(ooo_6.6-7.5), 2014-03-06 23:37:32]

lgtm with nit

https://codereview.chromium.org/189113002/diff/40001/win8/delegate_execute/co...
File win8/delegate_execute/command_execute_impl.cc (right):

https://codereview.chromium.org/189113002/diff/40001/win8/delegate_execute/co...
win8/delegate_execute/command_execute_impl.cc:119: // The general flow when
chrome is the default browser is as follows:
line 119 needs rewording.

[gab, 2014-03-07 00:02:34]

https://codereview.chromium.org/189113002/diff/40001/chrome/installer/util/sh...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/189113002/diff/40001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:1805: LOG(ERROR) << "Error getting app exe
path";
NOTREACHED() here too.

[zturner, 2014-03-07 00:25:18]

The CQ bit was checked by zturner@chromium.org

[commit-bot: I haz the power, 2014-03-07 00:36:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zturner@chromium.org/189113002/60001

[commit-bot: I haz the power, 2014-03-07 03:14:25]

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

[commit-bot: I haz the power, 2014-03-07 03:14:26]

Try jobs failed on following builders: mac_chromium_rel

[zturner, 2014-03-07 03:43:00]

The CQ bit was checked by zturner@chromium.org

[commit-bot: I haz the power, 2014-03-07 03:48:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zturner@chromium.org/189113002/60001

[commit-bot: I haz the power, 2014-03-07 09:14:54]

Change committed as 255562