Make omaha, gcapi and uninstall registry accesses use Wow6432Node on 64-bit

Omaha configuration parameters now use Wow6432Node (32-bit registry) on 64-bit installs.  GCAPI configuration parameters also use these same accesses since they read/write the same data.

Also, force installation of Win64 into C:\Program Files (x86) until the code is added to support moving the files to the right place during upgrade and downgrade.  Uninstall information will remain stored in the 32-bit registry (even on 64-bit) until the installation location is changed.

This fixes system installs on Win64.

BUG=338706
R=grt@chromium.org

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

[Will Harris, 2014-05-26 17:53:23]

grt@ can you take a preliminary look at this CL - for moving the Omaha
parameters to Wow6432Node.

[grt (UTC plus 2), 2014-05-27 13:53:25]

On 2014/05/26 17:53:23, Will Harris wrote:
> grt@ can you take a preliminary look at this CL - for moving the Omaha
> parameters to Wow6432Node.

will do. please have a look at src/chrome/installer/mini_installer. it uses the
Win32 registry API directly, and will need to use the WOW64_32 flag as well.

[grt (UTC plus 2), 2014-05-27 14:03:30]

On 2014/05/27 13:53:25, grt wrote:
> On 2014/05/26 17:53:23, Will Harris wrote:
> > grt@ can you take a preliminary look at this CL - for moving the Omaha
> > parameters to Wow6432Node.
> 
> will do. please have a look at src/chrome/installer/mini_installer. it uses
the
> Win32 registry API directly, and will need to use the WOW64_32 flag as well.

oh, hah, you already did. awesome.

[grt (UTC plus 2), 2014-05-27 16:42:08]

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/mini_installer.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:136: KEY_QUERY_VALUE |
KEY_WOW64_32KEY) == ERROR_SUCCESS &&
this function is only used for a key in HKCU that isn't affected by WOW64. it
doesn't seem right to put the flag here.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:144: bool
OpenClientStateKey(HKEY root_key, const wchar_t* app_guid, REGSAM access,
since this helper accesses a key that always needs WOW64_32, how about OR-ing
the bit in on line 149 and remove it from the callers?

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:445: reg_root, uninstall_reg,
KEY_WOW64_32KEY);
as with my comment on line 734, i think this should delete from both the default
and the opposite bitness to cover the case of migrating from non-msi to msi and
from 32->64 or 64->32 at the same time.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:615: // the delegate registration key
in the wrong hive.
"in the wrong hive" makes it sound like previous installs did the wrong thing.
this double-delete needs to be here for the legitimate case of a 32->64 or
64->32 migration no matter what, no? if this is correct, please change the
comment so that it's clear that both are deleted to handle the case of migrating
from one bitness to the other on upgrade. i don't think there's a need to
reference the bug, since the comment alone justifies the double-delete.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:632: WorkItem::kWow64Default);
this and line 640 should both be KEY_WOW64_32KEY since the old registrations
only existed for 32-bit Chrome.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:734: reg_root, uninstall_reg,
KEY_WOW64_32KEY);
why force the add/remove control panel data into the 32-bit section? in
contrast, isn't the right thing to do to delete the data in the opposite-bitness
section (to handle 32->64 or 64->32 migration) and write into the default for
the current bitness?

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:801: false);
it'd be nice to preserve the InstallDate when migrating from 32->64 or back

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/u...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/u...
chrome/installer/setup/uninstall.cc:1184: KEY_WOW64_32KEY);
i think this should use the default; see comment in install_worker.cc.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
File chrome/installer/util/app_command.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
chrome/installer/util/app_command.cc:76: // Command_path is derived from
GetRegCommandKey which always returns
move comment up above AddCreteRegKeyWorkItem

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
File chrome/installer/util/compat_checks.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
chrome/installer/util/compat_checks.cc:19: KEY_READ | KEY_WOW64_32KEY);
won't this miss if Symantec ever ships a 64-bit SEP? do they ship one already
that we're missing?

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
File chrome/installer/util/installation_state.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
chrome/installer/util/installation_state.cc:43: kAccess | KEY_WOW64_32KEY) ==
ERROR_SUCCESS)
move to definition of kAccess

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
chrome/installer/util/installation_state.cc:60: KEY_QUERY_VALUE |
KEY_WOW64_32KEY) == ERROR_SUCCESS) {
define a local kAccess const with these two bits and use it in their place
throughout the function

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

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:716: KEY_READ | KEY_WOW64_32KEY);
i think this should be in the default; see other comments.

[grt (UTC plus 2), 2014-05-27 16:47:59]

have a look in chrome/installer/gcapi, too.

[Will Harris, 2014-05-27 19:25:09]

we spoke about the uninstall key.  I'm going to leave the uninstall key in the
32-bit registry, as this matches the quirky 64-bit binaries in C:\Program Files
(x86) directory.

When the CL is added to move the install location from DIR_PROGRAM_FILESX86 to
DIR_PROGRAM_FILES on both x64 and x86 then I will update the uninstall link
appropriately.

Patchset (to this CL) with gcapi changes to come soon.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/mini_installer.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:136: KEY_QUERY_VALUE |
KEY_WOW64_32KEY) == ERROR_SUCCESS &&
On 2014/05/27 16:42:08, grt wrote:
> this function is only used for a key in HKCU that isn't affected by WOW64. it
> doesn't seem right to put the flag here.

Done.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:144: bool
OpenClientStateKey(HKEY root_key, const wchar_t* app_guid, REGSAM access,
On 2014/05/27 16:42:08, grt wrote:
> since this helper accesses a key that always needs WOW64_32, how about OR-ing
> the bit in on line 149 and remove it from the callers?

Done. Good idea.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:615: // the delegate registration key
in the wrong hive.
On 2014/05/27 16:42:08, grt wrote:
> "in the wrong hive" makes it sound like previous installs did the wrong thing.
> this double-delete needs to be here for the legitimate case of a 32->64 or
> 64->32 migration no matter what, no? if this is correct, please change the
> comment so that it's clear that both are deleted to handle the case of
migrating
> from one bitness to the other on upgrade. i don't think there's a need to
> reference the bug, since the comment alone justifies the double-delete.

Done.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:632: WorkItem::kWow64Default);
On 2014/05/27 16:42:08, grt wrote:
> this and line 640 should both be KEY_WOW64_32KEY since the old registrations
> only existed for 32-bit Chrome.

Done.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:734: reg_root, uninstall_reg,
KEY_WOW64_32KEY);
On 2014/05/27 16:42:08, grt wrote:
> why force the add/remove control panel data into the 32-bit section? in
> contrast, isn't the right thing to do to delete the data in the
opposite-bitness
> section (to handle 32->64 or 64->32 migration) and write into the default for
> the current bitness?

as discussed, while we're leaving Chrome in C:\Program Files (x86) even on
64-bit platforms, I'm leaving the uninstall registry entry here too.  I'll move
this when I move the files.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:801: false);
On 2014/05/27 16:42:08, grt wrote:
> it'd be nice to preserve the InstallDate when migrating from 32->64 or back

pretty sure this code does preserve, since it's only written into the 32-bit
hive, and the 'false' on the work item means it won't be overwritten if already
existing.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
File chrome/installer/util/app_command.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
chrome/installer/util/app_command.cc:76: // Command_path is derived from
GetRegCommandKey which always returns
On 2014/05/27 16:42:08, grt wrote:
> move comment up above AddCreteRegKeyWorkItem

Done.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
File chrome/installer/util/compat_checks.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
chrome/installer/util/compat_checks.cc:19: KEY_READ | KEY_WOW64_32KEY);
On 2014/05/27 16:42:08, grt wrote:
> won't this miss if Symantec ever ships a 64-bit SEP? do they ship one already
> that we're missing?

We're only checking for anything before 11MR3, which was 32-bit only and from
2008.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
File chrome/installer/util/installation_state.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
chrome/installer/util/installation_state.cc:43: kAccess | KEY_WOW64_32KEY) ==
ERROR_SUCCESS)
On 2014/05/27 16:42:08, grt wrote:
> move to definition of kAccess

Done.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
chrome/installer/util/installation_state.cc:60: KEY_QUERY_VALUE |
KEY_WOW64_32KEY) == ERROR_SUCCESS) {
On 2014/05/27 16:42:08, grt wrote:
> define a local kAccess const with these two bits and use it in their place
> throughout the function

Done.

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

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:716: KEY_READ | KEY_WOW64_32KEY);
On 2014/05/27 16:42:08, grt wrote:
> i think this should be in the default; see other comments.

see general comment on 32/64 uninstall.

[Will Harris, 2014-05-27 19:41:42]

On 2014/05/27 19:25:09, Will Harris wrote:
> we spoke about the uninstall key.  I'm going to leave the uninstall key in the
> 32-bit registry, as this matches the quirky 64-bit binaries in C:\Program
Files
> (x86) directory.
> 
> When the CL is added to move the install location from DIR_PROGRAM_FILESX86 to
> DIR_PROGRAM_FILES on both x64 and x86 then I will update the uninstall link
> appropriately.
> 
> Patchset (to this CL) with gcapi changes to come soon.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
> File chrome/installer/mini_installer/mini_installer.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
> chrome/installer/mini_installer/mini_installer.cc:136: KEY_QUERY_VALUE |
> KEY_WOW64_32KEY) == ERROR_SUCCESS &&
> On 2014/05/27 16:42:08, grt wrote:
> > this function is only used for a key in HKCU that isn't affected by WOW64.
it
> > doesn't seem right to put the flag here.
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/mini_in...
> chrome/installer/mini_installer/mini_installer.cc:144: bool
> OpenClientStateKey(HKEY root_key, const wchar_t* app_guid, REGSAM access,
> On 2014/05/27 16:42:08, grt wrote:
> > since this helper accesses a key that always needs WOW64_32, how about
OR-ing
> > the bit in on line 149 and remove it from the callers?
> 
> Done. Good idea.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
> File chrome/installer/setup/install_worker.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
> chrome/installer/setup/install_worker.cc:615: // the delegate registration key
> in the wrong hive.
> On 2014/05/27 16:42:08, grt wrote:
> > "in the wrong hive" makes it sound like previous installs did the wrong
thing.
> > this double-delete needs to be here for the legitimate case of a 32->64 or
> > 64->32 migration no matter what, no? if this is correct, please change the
> > comment so that it's clear that both are deleted to handle the case of
> migrating
> > from one bitness to the other on upgrade. i don't think there's a need to
> > reference the bug, since the comment alone justifies the double-delete.
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
> chrome/installer/setup/install_worker.cc:632: WorkItem::kWow64Default);
> On 2014/05/27 16:42:08, grt wrote:
> > this and line 640 should both be KEY_WOW64_32KEY since the old registrations
> > only existed for 32-bit Chrome.
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
> chrome/installer/setup/install_worker.cc:734: reg_root, uninstall_reg,
> KEY_WOW64_32KEY);
> On 2014/05/27 16:42:08, grt wrote:
> > why force the add/remove control panel data into the 32-bit section? in
> > contrast, isn't the right thing to do to delete the data in the
> opposite-bitness
> > section (to handle 32->64 or 64->32 migration) and write into the default
for
> > the current bitness?
> 
> as discussed, while we're leaving Chrome in C:\Program Files (x86) even on
> 64-bit platforms, I'm leaving the uninstall registry entry here too.  I'll
move
> this when I move the files.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
> chrome/installer/setup/install_worker.cc:801: false);
> On 2014/05/27 16:42:08, grt wrote:
> > it'd be nice to preserve the InstallDate when migrating from 32->64 or back
> 
> pretty sure this code does preserve, since it's only written into the 32-bit
> hive, and the 'false' on the work item means it won't be overwritten if
already
> existing.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
> File chrome/installer/util/app_command.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/ap...
> chrome/installer/util/app_command.cc:76: // Command_path is derived from
> GetRegCommandKey which always returns
> On 2014/05/27 16:42:08, grt wrote:
> > move comment up above AddCreteRegKeyWorkItem
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
> File chrome/installer/util/compat_checks.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
> chrome/installer/util/compat_checks.cc:19: KEY_READ | KEY_WOW64_32KEY);
> On 2014/05/27 16:42:08, grt wrote:
> > won't this miss if Symantec ever ships a 64-bit SEP? do they ship one
already
> > that we're missing?
> 
> We're only checking for anything before 11MR3, which was 32-bit only and from
> 2008.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
> File chrome/installer/util/installation_state.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
> chrome/installer/util/installation_state.cc:43: kAccess | KEY_WOW64_32KEY) ==
> ERROR_SUCCESS)
> On 2014/05/27 16:42:08, grt wrote:
> > move to definition of kAccess
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/in...
> chrome/installer/util/installation_state.cc:60: KEY_QUERY_VALUE |
> KEY_WOW64_32KEY) == ERROR_SUCCESS) {
> On 2014/05/27 16:42:08, grt wrote:
> > define a local kAccess const with these two bits and use it in their place
> > throughout the function
> 
> Done.
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/sh...
> File chrome/installer/util/shell_util.cc (right):
> 
>
https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/sh...
> chrome/installer/util/shell_util.cc:716: KEY_READ | KEY_WOW64_32KEY);
> On 2014/05/27 16:42:08, grt wrote:
> > i think this should be in the default; see other comments.
> 
> see general comment on 32/64 uninstall.

added the GCAPI calls - they are all to the 32-bit registry.

[grt (UTC plus 2), 2014-05-27 22:06:18]

the formatting change mixed in with the other changes makes it difficult to
review. in the future, please separate the two in to different patchsets so it's
easier for reviewers to see what counts.

lgtm with one comment suggestion on the assumption that all changes aside from
gcapi and the things i mentioned previously are only formatting.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:734: reg_root, uninstall_reg,
KEY_WOW64_32KEY);
On 2014/05/27 19:25:10, Will Harris wrote:
> On 2014/05/27 16:42:08, grt wrote:
> > why force the add/remove control panel data into the 32-bit section? in
> > contrast, isn't the right thing to do to delete the data in the
> opposite-bitness
> > section (to handle 32->64 or 64->32 migration) and write into the default
for
> > the current bitness?
> 
> as discussed, while we're leaving Chrome in C:\Program Files (x86) even on
> 64-bit platforms, I'm leaving the uninstall registry entry here too.  I'll
move
> this when I move the files.

SGTM

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:801: false);
On 2014/05/27 19:25:10, Will Harris wrote:
> On 2014/05/27 16:42:08, grt wrote:
> > it'd be nice to preserve the InstallDate when migrating from 32->64 or back
> 
> pretty sure this code does preserve, since it's only written into the 32-bit
> hive, and the 'false' on the work item means it won't be overwritten if
already
> existing.

i was referring to the (future) case where the uninstall info is moved from the
32-bit to 64-bit area of the registry. in this case, we'll need to copy this
value over.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
File chrome/installer/util/compat_checks.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
chrome/installer/util/compat_checks.cc:19: KEY_READ | KEY_WOW64_32KEY);
On 2014/05/27 19:25:10, Will Harris wrote:
> On 2014/05/27 16:42:08, grt wrote:
> > won't this miss if Symantec ever ships a 64-bit SEP? do they ship one
already
> > that we're missing?
> 
> We're only checking for anything before 11MR3, which was 32-bit only and from
> 2008.

groovy. sounds like good info for a comment in the code. ;-)

[Will Harris, 2014-05-27 22:29:28]

Sorry about the blended formatting.  Thanks for the quick review!

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:801: false);
On 2014/05/27 22:06:19, grt wrote:
> On 2014/05/27 19:25:10, Will Harris wrote:
> > On 2014/05/27 16:42:08, grt wrote:
> > > it'd be nice to preserve the InstallDate when migrating from 32->64 or
back
> > 
> > pretty sure this code does preserve, since it's only written into the 32-bit
> > hive, and the 'false' on the work item means it won't be overwritten if
> already
> > existing.
> 
> i was referring to the (future) case where the uninstall info is moved from
the
> 32-bit to 64-bit area of the registry. in this case, we'll need to copy this
> value over.

Put a TODO so I don't forget to do this.

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
File chrome/installer/util/compat_checks.cc (right):

https://codereview.chromium.org/300593002/diff/20001/chrome/installer/util/co...
chrome/installer/util/compat_checks.cc:19: KEY_READ | KEY_WOW64_32KEY);
On 2014/05/27 22:06:19, grt wrote:
> On 2014/05/27 19:25:10, Will Harris wrote:
> > On 2014/05/27 16:42:08, grt wrote:
> > > won't this miss if Symantec ever ships a 64-bit SEP? do they ship one
> already
> > > that we're missing?
> > 
> > We're only checking for anything before 11MR3, which was 32-bit only and
from
> > 2008.
> 
> groovy. sounds like good info for a comment in the code. ;-)

Done.

[Will Harris, 2014-05-28 01:41:35]

The CQ bit was checked by wfh@chromium.org

[commit-bot: I haz the power, 2014-05-28 01:42:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wfh@chromium.org/300593002/70001

[Will Harris, 2014-05-28 01:52:06]

The CQ bit was unchecked by wfh@chromium.org

[Will Harris, 2014-05-28 01:54:49]

Committed patchset #6 manually as r273108 (presubmit successful).