Implement the persistent metro mode setting (win8)

Implement the persistent metro mode setting (win8)

We use HKCU\Software\<product>\Metro registry path and 'launch_mode'
value to store the last 'forced' mode.

Unless chrome is already running we use the stored mode if it is found
else we use a "tabletness" heuristic to determine the launch mode.

BUG=141032
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=156708

[grt (UTC plus 2), 2012-09-13 04:33:15]

http://codereview.chromium.org/10928172/diff/1/chrome/common/chrome_constants.cc
File chrome/common/chrome_constants.cc (right):

http://codereview.chromium.org/10928172/diff/1/chrome/common/chrome_constants...
chrome/common/chrome_constants.cc:219: const wchar_t
kMetroChromeUserDataSubDir[] = L"Metro";
as an aside, should we remove this now (in other CLs, of course)?

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
File win8/delegate_execute/command_execute_impl.cc (right):

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:405: KEY_ALL_ACCESS) != 0) {
please use ERROR_SUCCESS rather than 0 for consistency with the codebase

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:406: AtlTrace("Failed to open HKCU
%ls key\n", chrome::kMetroRegistryPath);
might be helpful to log the result of reg_key.Create() here

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:416:
reg_key.WriteValue(chrome::kLaunchModeValue,
please add code to chrome/installer/setup/uninstall.cc to delete this value, and
the key as well if it's empty.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:422: if
(reg_key.ReadValueDW(chrome::kLaunchModeValue, &reg_value) != 0) {
please use ERROR_SUCCESS rather than 0

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:423: launch_mode =
delegate_execute::IsMachineATablet() ?
i'm curious: is this ever called when chrome isn't the default browser?

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:426: } else if (reg_value >
ECHUIM_SYSTEM_LAUNCHER) {
should this be >=?  is it valid for the launch mode to be set to SYSTEM_LAUNCHER
in the registry?

[cpu_(ooo_6.6-7.5), 2012-09-13 20:56:56]

code changed please take a look again.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
File win8/delegate_execute/command_execute_impl.cc (right):

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:405: KEY_ALL_ACCESS) != 0) {
On 2012/09/13 04:33:16, grt wrote:
> please use ERROR_SUCCESS rather than 0 for consistency with the codebase

Done.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:406: AtlTrace("Failed to open HKCU
%ls key\n", chrome::kMetroRegistryPath);
On 2012/09/13 04:33:16, grt wrote:
> might be helpful to log the result of reg_key.Create() here

Done.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:422: if
(reg_key.ReadValueDW(chrome::kLaunchModeValue, &reg_value) != 0) {
On 2012/09/13 04:33:16, grt wrote:
> please use ERROR_SUCCESS rather than 0

Done.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:423: launch_mode =
delegate_execute::IsMachineATablet() ?
On 2012/09/13 04:33:16, grt wrote:
> i'm curious: is this ever called when chrome isn't the default browser?

I'' see if it does.

http://codereview.chromium.org/10928172/diff/1/win8/delegate_execute/command_...
win8/delegate_execute/command_execute_impl.cc:426: } else if (reg_value >
ECHUIM_SYSTEM_LAUNCHER) {
I was thinking on using it experimentally but I'll experiment in my checkout.
Changed.

On 2012/09/13 04:33:16, grt wrote:
> should this be >=?  is it valid for the launch mode to be set to
SYSTEM_LAUNCHER
> in the registry?

[ananta, 2012-09-13 23:51:50]

lgtm

[grt (UTC plus 2), 2012-09-14 00:13:01]

Rubberstamp lgtm. Will reread later.
On Sep 13, 2012 7:51 PM, <ananta@chromium.org> wrote:

> lgtm
>
>
>
>
http://codereview.chromium.**org/10928172/<http://codereview.chromium.org/109...
>

[grt (UTC plus 2), 2012-09-14 01:13:42]

i have but two nits to offer you, sir.  my lgtm stays either way.

http://codereview.chromium.org/10928172/diff/8007/win8/delegate_execute/comma...
File win8/delegate_execute/command_execute_impl.cc (right):

http://codereview.chromium.org/10928172/diff/8007/win8/delegate_execute/comma...
win8/delegate_execute/command_execute_impl.cc:418: AtlTrace("Failed to open HKCU
%ls key, error 0x%x\n",
in this case, key_result is a last-error code.  i think they're base-10 in their
natural habitat (winerror.h), so i'd go with %u.

http://codereview.chromium.org/10928172/diff/8007/win8/delegate_execute/comma...
win8/delegate_execute/command_execute_impl.cc:420: key_result);
nit: move up to previous line

[memyy, 2012-09-20 03:32:05]

good tank you.   memyi04@ovi.com

[memyy, 2012-09-20 08:25:06]

http://codereview.chromium.org/10928172/diff/1/chrome/common/chrome_constants.cc
File chrome/common/chrome_constants.cc (right):

http://codereview.chromium.org/10928172/diff/1/chrome/common/chrome_constants...
chrome/common/chrome_constants.cc:219: const wchar_t
kMetroChromeUserDataSubDir[] = L"Metro";
On 2012/09/13 04:33:16, grt wrote:
> as an aside, should we remove this now (in other CLs, of course)?