Add WOW64 support to the installer registry work items

Add a flag to all of the registry related work items to specify whether to operate on the 64-bit or 32-bit hive on a WOW64 system.

Make DeleteRegKeyWorkItem call down into RegKey so WOW64 support works for delete work items.

All calls changed to use the Wow64_Default (0) parameter, so the behavior is identical.

This is prep for adding 64-bit support to the installer.

BUG=348626, 338706
R=grt@chromium.org, grt
TBR=sky
TEST=installer_util_unittests

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

[Will Harris, 2014-05-16 17:29:07]

reviewing my own code is fun

https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/insta...
File chrome/installer/setup/install_worker.cc (right):

https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/insta...
chrome/installer/setup/install_worker.cc:619: #endif
This looks weird because I haven't decided how I'm going to do this.  Lots of
TODO here.

https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/unins...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/unins...
chrome/installer/setup/uninstall.cc:100: true, WorkItem::WOW64_DEFAULT);
this looks like it should be WOW64_32 as it accesses the state key

[grt (UTC plus 2), 2014-05-16 18:26:10]

https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
File chrome/installer/util/work_item_list.h (right):

https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
chrome/installer/util/work_item_list.h:51: RegWow64ViewOption reg_wow64_option);
it looks like it's going to be pretty intrusive to plumb this option through all
consumers. i think this is one of those cases where a default argument is
acceptable. does that simplify things?

[Will Harris, 2014-05-16 18:34:54]

On 2014/05/16 18:26:10, grt wrote:
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> File chrome/installer/util/work_item_list.h (right):
> 
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> chrome/installer/util/work_item_list.h:51: RegWow64ViewOption
reg_wow64_option);
> it looks like it's going to be pretty intrusive to plumb this option through
all
> consumers. i think this is one of those cases where a default argument is
> acceptable. does that simplify things?

To be honest, the plumbing is pretty much already done in this CL, and it's made
me think hard about every time the call is made.  There's actually quite a few
times when WOW64_32 is needed since it writes to the state key, and I don't want
to miss any.  I still need to add all this to the tests and when I do that I can
add a whole load of new tests, which I'm going to have to do anyway (e.g. to
test rollback of WOW64 keys)

I think at this point I'm happy to proceed with adding a new parameter, unless
you feel it makes all the calls too ugly.  WDYT?

[grt (UTC plus 2), 2014-05-20 17:48:18]

On 2014/05/16 18:34:54, Will Harris wrote:
> On 2014/05/16 18:26:10, grt wrote:
> >
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> > File chrome/installer/util/work_item_list.h (right):
> > 
> >
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> > chrome/installer/util/work_item_list.h:51: RegWow64ViewOption
> reg_wow64_option);
> > it looks like it's going to be pretty intrusive to plumb this option through
> all
> > consumers. i think this is one of those cases where a default argument is
> > acceptable. does that simplify things?
> 
> To be honest, the plumbing is pretty much already done in this CL, and it's
made
> me think hard about every time the call is made.  There's actually quite a few
> times when WOW64_32 is needed since it writes to the state key, and I don't
want
> to miss any.  I still need to add all this to the tests and when I do that I
can
> add a whole load of new tests, which I'm going to have to do anyway (e.g. to
> test rollback of WOW64 keys)
> 
> I think at this point I'm happy to proceed with adding a new parameter, unless
> you feel it makes all the calls too ugly.  WDYT?

sgtm. apologies for the delay.

[Will Harris, 2014-05-21 03:46:54]

On 2014/05/20 17:48:18, grt wrote:
> On 2014/05/16 18:34:54, Will Harris wrote:
> > On 2014/05/16 18:26:10, grt wrote:
> > >
> >
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> > > File chrome/installer/util/work_item_list.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_i...
> > > chrome/installer/util/work_item_list.h:51: RegWow64ViewOption
> > reg_wow64_option);
> > > it looks like it's going to be pretty intrusive to plumb this option
through
> > all
> > > consumers. i think this is one of those cases where a default argument is
> > > acceptable. does that simplify things?
> > 
> > To be honest, the plumbing is pretty much already done in this CL, and it's
> made
> > me think hard about every time the call is made.  There's actually quite a
few
> > times when WOW64_32 is needed since it writes to the state key, and I don't
> want
> > to miss any.  I still need to add all this to the tests and when I do that I
> can
> > add a whole load of new tests, which I'm going to have to do anyway (e.g. to
> > test rollback of WOW64 keys)
> > 
> > I think at this point I'm happy to proceed with adding a new parameter,
unless
> > you feel it makes all the calls too ugly.  WDYT?
> 
> sgtm. apologies for the delay.

This is now ready for review.  Functionality should be identical.  All functions
have the additional wow64_access flag, but 0 is being passed to each function.

Thanks!

[grt (UTC plus 2), 2014-05-21 16:41:05]

looks good.

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

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:1003: .c_str(),
please move this up to the previous line. i think clang-format was too
aggressive here

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
chrome/installer/util/install_util.cc:437: LONG
InstallUtil::DeleteRegistryKey(HKEY root_key,
previously, this would return true for both successful delete and for the case
where the key didn't exist in the first place. since this is a convenience
function, i think it makes sense for it to return a bool since that's what all
consumers care about. otherwise, consumers need to know details of how the
registry functions work.

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

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:583: wow64_access_(0),  // TODO(wfh): Add
support for WOW64 registry view.
i don't know that this class will ever need to override the default. for now,
how about making the minimal change, and just hardcoding Default in
AddToWorkItemList?

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
File chrome/installer/util/work_item.h (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
chrome/installer/util/work_item.h:113: REGSAM wow64_access);
the access is more related to key_path than to value_name. would you mind moving
it so that it comes after key_path? please do so for this and all other work
item functions. thanks.

[Will Harris, 2014-05-21 23:23:15]

new patch set will come later tonight

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

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/i...
chrome/installer/setup/install_worker.cc:1003: .c_str(),
On 2014/05/21 16:41:05, grt wrote:
> please move this up to the previous line. i think clang-format was too
> aggressive here

Done.

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
chrome/installer/util/install_util.cc:437: LONG
InstallUtil::DeleteRegistryKey(HKEY root_key,
On 2014/05/21 16:41:05, grt wrote:
> previously, this would return true for both successful delete and for the case
> where the key didn't exist in the first place. since this is a convenience
> function, i think it makes sense for it to return a bool since that's what all
> consumers care about. otherwise, consumers need to know details of how the
> registry functions work.

I changed this since it was used by the new code in DeleteRegKeyWorkItem which
needs the full code, and other places which just needs bool, so I just changed
all the other callers to handle the LONG return type and share the code.

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

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:583: wow64_access_(0),  // TODO(wfh): Add
support for WOW64 registry view.
On 2014/05/21 16:41:05, grt wrote:
> i don't know that this class will ever need to override the default. for now,
> how about making the minimal change, and just hardcoding Default in
> AddToWorkItemList?

Done.  I think 64-bit installers will have to remove the 32-bit shell
registrations, but since this CL doesn't actually expose the full functionality
out of RegistryEntry yet, it's probably worth adding that once I actually need
it.

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
File chrome/installer/util/work_item.h (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
chrome/installer/util/work_item.h:113: REGSAM wow64_access);
On 2014/05/21 16:41:05, grt wrote:
> the access is more related to key_path than to value_name. would you mind
moving
> it so that it comes after key_path? please do so for this and all other work
> item functions. thanks.

could take some time, will upload a new PS with this and other changes later
today.

[Will Harris, 2014-05-22 00:55:37]

On 2014/05/21 23:23:15, Will Harris wrote:
> new patch set will come later tonight
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/i...
> File chrome/installer/setup/install_worker.cc (right):
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/i...
> chrome/installer/setup/install_worker.cc:1003: .c_str(),
> On 2014/05/21 16:41:05, grt wrote:
> > please move this up to the previous line. i think clang-format was too
> > aggressive here
> 
> Done.
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
> File chrome/installer/util/install_util.cc (right):
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
> chrome/installer/util/install_util.cc:437: LONG
> InstallUtil::DeleteRegistryKey(HKEY root_key,
> On 2014/05/21 16:41:05, grt wrote:
> > previously, this would return true for both successful delete and for the
case
> > where the key didn't exist in the first place. since this is a convenience
> > function, i think it makes sense for it to return a bool since that's what
all
> > consumers care about. otherwise, consumers need to know details of how the
> > registry functions work.
> 
> I changed this since it was used by the new code in DeleteRegKeyWorkItem which
> needs the full code, and other places which just needs bool, so I just changed
> all the other callers to handle the LONG return type and share the code.
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/sh...
> File chrome/installer/util/shell_util.cc (right):
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/sh...
> chrome/installer/util/shell_util.cc:583: wow64_access_(0),  // TODO(wfh): Add
> support for WOW64 registry view.
> On 2014/05/21 16:41:05, grt wrote:
> > i don't know that this class will ever need to override the default. for
now,
> > how about making the minimal change, and just hardcoding Default in
> > AddToWorkItemList?
> 
> Done.  I think 64-bit installers will have to remove the 32-bit shell
> registrations, but since this CL doesn't actually expose the full
functionality
> out of RegistryEntry yet, it's probably worth adding that once I actually need
> it.
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
> File chrome/installer/util/work_item.h (right):
> 
>
https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/wo...
> chrome/installer/util/work_item.h:113: REGSAM wow64_access);
> On 2014/05/21 16:41:05, grt wrote:
> > the access is more related to key_path than to value_name. would you mind
> moving
> > it so that it comes after key_path? please do so for this and all other work
> > item functions. thanks.
> 
> could take some time, will upload a new PS with this and other changes later
> today.

Changes done. PTAL. let me know if you still want me to put a new
DeleteRegistryKey function inside DeleteRegKeyWorkItem, or rejig the code to
share the function that returns a bool.

[grt (UTC plus 2), 2014-05-22 01:37:44]

looks gtm if my DeleteRegKeyWorkItem::Do() makes sense. thanks!

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
chrome/installer/util/install_util.cc:437: LONG
InstallUtil::DeleteRegistryKey(HKEY root_key,
On 2014/05/21 23:23:16, Will Harris wrote:
> On 2014/05/21 16:41:05, grt wrote:
> > previously, this would return true for both successful delete and for the
case
> > where the key didn't exist in the first place. since this is a convenience
> > function, i think it makes sense for it to return a bool since that's what
all
> > consumers care about. otherwise, consumers need to know details of how the
> > registry functions work.
> 
> I changed this since it was used by the new code in DeleteRegKeyWorkItem which
> needs the full code,

Does it? I could be reading it wrong, but modulo the text in the log message it
looks like DeleteRegKeyWorkItem makes the same determination that the old
InstallUtil::DeleteRegistryKey made. Can you not simplify
DeleteRegKeyWorkItem::Do() so that it does:

  if (!InstallUtil::DeleteRegistryKey(...)) {
    return ignore_failure_;
  }

and DeleteRegKeyWorkItem::Rollback() becomes just:

  InstallUtil::DeleteRegistryKey(...);

I think the logs will still be good enough to tell what's going on.

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

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/sh...
chrome/installer/util/shell_util.cc:583: wow64_access_(0),  // TODO(wfh): Add
support for WOW64 registry view.
On 2014/05/21 23:23:16, Will Harris wrote:
> On 2014/05/21 16:41:05, grt wrote:
> > i don't know that this class will ever need to override the default. for
now,
> > how about making the minimal change, and just hardcoding Default in
> > AddToWorkItemList?
> 
> Done.  I think 64-bit installers will have to remove the 32-bit shell
> registrations, but since this CL doesn't actually expose the full
functionality
> out of RegistryEntry yet, it's probably worth adding that once I actually need
> it.

Ah, good point. I'd forgotten about deleting the old values.

[Will Harris, 2014-05-22 21:29:23]

PTAL.  Also found some Registry code that wasn't being passed the wow64_access,
and rearranged the parameters on a few more functions to match the new
convention of having the wow64_access parameter always follow the key name
parameter.

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/in...
chrome/installer/util/install_util.cc:437: LONG
InstallUtil::DeleteRegistryKey(HKEY root_key,
On 2014/05/22 01:37:44, grt wrote:
> On 2014/05/21 23:23:16, Will Harris wrote:
> > On 2014/05/21 16:41:05, grt wrote:
> > > previously, this would return true for both successful delete and for the
> case
> > > where the key didn't exist in the first place. since this is a convenience
> > > function, i think it makes sense for it to return a bool since that's what
> all
> > > consumers care about. otherwise, consumers need to know details of how the
> > > registry functions work.
> > 
> > I changed this since it was used by the new code in DeleteRegKeyWorkItem
which
> > needs the full code,
> 
> Does it? I could be reading it wrong, but modulo the text in the log message
it
> looks like DeleteRegKeyWorkItem makes the same determination that the old
> InstallUtil::DeleteRegistryKey made. Can you not simplify
> DeleteRegKeyWorkItem::Do() so that it does:
> 
>   if (!InstallUtil::DeleteRegistryKey(...)) {
>     return ignore_failure_;
>   }
> 
> and DeleteRegKeyWorkItem::Rollback() becomes just:
> 
>   InstallUtil::DeleteRegistryKey(...);
> 
> I think the logs will still be good enough to tell what's going on.

Done.

[grt (UTC plus 2), 2014-05-22 22:17:23]

looks like there's a compile error

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/d...
File chrome/installer/util/delete_reg_key_work_item.cc (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/d...
chrome/installer/util/delete_reg_key_work_item.cc:50: return ignore_failure_;
did "git cl format" do this? i always thought we need braces here since the
conditional spans multiple lines.

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
chrome/installer/util/install_util.cc:445: KEY_READ | KEY_WRITE | wow64_access);
nit: indentation

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
File chrome/installer/util/install_util.h (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
chrome/installer/util/install_util.h:123: REGSAM wow64_access,
nice catch!

[Will Harris, 2014-05-22 22:23:29]

the compile error is because the trybots haven't pulled in the new version of
base::win::registry yet.

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/d...
File chrome/installer/util/delete_reg_key_work_item.cc (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/d...
chrome/installer/util/delete_reg_key_work_item.cc:50: return ignore_failure_;
On 2014/05/22 22:17:23, grt wrote:
> did "git cl format" do this? i always thought we need braces here since the
> conditional spans multiple lines.

git cl format seemed happy with this even with the function call spanning
multiple lines.  I can't find anything in the style guide about this.

[grt (UTC plus 2), 2014-05-22 22:45:14]

I believe the style guide says they're required if the statement spans
multiple lines, but is vague about the conditional doing the same. I think
this was discussed on chromium-dev recently, but I'm not near a proper
computer to check right now. Personally, I prefer braces if either spans
multiple lines, but I won't force my style on you if it isn't the consensus.
On May 22, 2014 6:23 PM, <wfh@chromium.org> wrote:

> the compile error is because the trybots haven't pulled in the new version
> of
> base::win::registry yet.
>
>
> https://codereview.chromium.org/282363003/diff/100001/
> chrome/installer/util/delete_reg_key_work_item.cc
> File chrome/installer/util/delete_reg_key_work_item.cc (right):
>
> https://codereview.chromium.org/282363003/diff/100001/
> chrome/installer/util/delete_reg_key_work_item.cc#newcode50
> chrome/installer/util/delete_reg_key_work_item.cc:50: return
> ignore_failure_;
> On 2014/05/22 22:17:23, grt wrote:
>
>> did "git cl format" do this? i always thought we need braces here
>>
> since the
>
>> conditional spans multiple lines.
>>
>
> git cl format seemed happy with this even with the function call
> spanning multiple lines.  I can't find anything in the style guide about
> this.
>
> https://codereview.chromium.org/282363003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Will Harris, 2014-05-23 00:36:39]

On 2014/05/22 22:45:14, grt wrote:
> I believe the style guide says they're required if the statement spans
> multiple lines, but is vague about the conditional doing the same. I think
> this was discussed on chromium-dev recently, but I'm not near a proper
> computer to check right now. Personally, I prefer braces if either spans
> multiple lines, but I won't force my style on you if it isn't the consensus.
> On May 22, 2014 6:23 PM, <mailto:wfh@chromium.org> wrote:
> 
> > the compile error is because the trybots haven't pulled in the new version
> > of
> > base::win::registry yet.
> >
> >
> > https://codereview.chromium.org/282363003/diff/100001/
> > chrome/installer/util/delete_reg_key_work_item.cc
> > File chrome/installer/util/delete_reg_key_work_item.cc (right):
> >
> > https://codereview.chromium.org/282363003/diff/100001/
> > chrome/installer/util/delete_reg_key_work_item.cc#newcode50
> > chrome/installer/util/delete_reg_key_work_item.cc:50: return
> > ignore_failure_;
> > On 2014/05/22 22:17:23, grt wrote:
> >
> >> did "git cl format" do this? i always thought we need braces here
> >>
> > since the
> >
> >> conditional spans multiple lines.
> >>
> >
> > git cl format seemed happy with this even with the function call
> > spanning multiple lines.  I can't find anything in the style guide about
> > this.
> >
> > https://codereview.chromium.org/282363003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Yup, I agree, braces look far better.   Fired up all the trybots.

[grt (UTC plus 2), 2014-05-23 01:28:50]

lgtm w/ one nit and one code fix.

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
chrome/installer/util/install_util.cc:445: KEY_READ | KEY_WRITE | wow64_access);
On 2014/05/22 22:17:23, grt wrote:
> nit: indentation

ping! this should be de-indented one space

https://codereview.chromium.org/282363003/diff/120001/chrome/installer/util/c...
File chrome/installer/util/create_reg_key_work_item.cc (right):

https://codereview.chromium.org/282363003/diff/120001/chrome/installer/util/c...
chrome/installer/util/create_reg_key_work_item.cc:101: RegKey
key(predefined_root_, L"", KEY_WRITE & wow64_access_);
facepalm!

https://codereview.chromium.org/282363003/diff/140001/chrome/test/mini_instal...
File chrome/test/mini_installer_test/installer_test_util.cc (right):

https://codereview.chromium.org/282363003/diff/140001/chrome/test/mini_instal...
chrome/test/mini_installer_test/installer_test_util.cc:76: LONG result =
InstallUtil::DeleteRegistryKey(root, key, wow64_access);
this returns bool now, so just return InstallUtil::DeleteRegistryKey, right?

[Will Harris, 2014-05-23 03:19:21]

Will wait for the bots to finish running then dcommit.

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
File chrome/installer/util/install_util.cc (right):

https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/i...
chrome/installer/util/install_util.cc:445: KEY_READ | KEY_WRITE | wow64_access);
On 2014/05/23 01:28:50, grt wrote:
> On 2014/05/22 22:17:23, grt wrote:
> > nit: indentation
> 
> ping! this should be de-indented one space

Woops. Done.

https://codereview.chromium.org/282363003/diff/120001/chrome/installer/util/c...
File chrome/installer/util/create_reg_key_work_item.cc (right):

https://codereview.chromium.org/282363003/diff/120001/chrome/installer/util/c...
chrome/installer/util/create_reg_key_work_item.cc:101: RegKey
key(predefined_root_, L"", KEY_WRITE & wow64_access_);
On 2014/05/23 01:28:51, grt wrote:
> facepalm!

found by tests!

https://codereview.chromium.org/282363003/diff/140001/chrome/test/mini_instal...
File chrome/test/mini_installer_test/installer_test_util.cc (right):

https://codereview.chromium.org/282363003/diff/140001/chrome/test/mini_instal...
chrome/test/mini_installer_test/installer_test_util.cc:76: LONG result =
InstallUtil::DeleteRegistryKey(root, key, wow64_access);
On 2014/05/23 01:28:51, grt wrote:
> this returns bool now, so just return InstallUtil::DeleteRegistryKey, right?

Good spot, not sure how I missed that one.

[grt (UTC plus 2), 2014-05-23 03:35:02]

lgtm++

[Will Harris, 2014-05-23 03:36:55]

On 2014/05/23 03:35:02, grt wrote:
> lgtm++

sky@ grateful for owner LGTM for chrome/test changes. Thanks!

[grt (UTC plus 2), 2014-05-23 11:02:56]

On 2014/05/23 03:36:55, Will Harris wrote:
> On 2014/05/23 03:35:02, grt wrote:
> > lgtm++
> 
> sky@ grateful for owner LGTM for chrome/test changes. Thanks!

Since you're 1) plumbing through a dependent change, 2) it's installer-related
test code, 3) I'm an installer owner, and 4) I'm in the process of confirming
with QA that these tests are no longer used, I'm giving you authority to TBR sky
if you don't hear from him soon. I'll take the heat should there be any. :-)

Hope you don't mind, Scott.

[Will Harris, 2014-05-23 15:39:12]

Committed patchset #10 manually as r272539 (tree was closed).