Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(18)

Issue 282363003: Add WOW64 support to the installer registry work items (Closed)

Created:
6 years, 7 months ago by Will Harris
Modified:
6 years, 7 months ago
Reviewers:
sky, grt (UTC plus 2)
CC:
chromium-reviews, grt+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 3

Patch Set 2 : work items work. use REGSAM instead. fix tests. #

Patch Set 3 : Delete CopyRegTree. Add checks for wow64_access. Change all accesses to default. #

Patch Set 4 : nits. add comment for the wow64_access option. #

Patch Set 5 : add wow64 logic to installer_util. mini_installer tests. #

Total comments: 11

Patch Set 6 : code review changes. nits. rearrange parameters. #

Patch Set 7 : code review changes. found a few APIs that needed the WOW64 flag. Fix ordering of some parameters. #

Total comments: 6

Patch Set 8 : nit: style on braces #

Total comments: 2

Patch Set 9 : typo: & should be | #

Total comments: 2

Patch Set 10 : nits. fix call to DeleteRegistryKey #

Unified diffs Side-by-side diffs Delta from patch set Stats (+778 lines, -728 lines) Patch
M chrome/chrome_installer.gypi View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/chrome_installer_util.gypi View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/installer/setup/install.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/setup/install_worker.cc View 1 2 3 4 5 26 chunks +253 lines, -101 lines 0 comments Download
M chrome/installer/setup/install_worker_unittest.cc View 1 2 3 4 5 2 chunks +6 lines, -5 lines 0 comments Download
M chrome/installer/setup/setup_main.cc View 1 2 3 4 5 1 chunk +6 lines, -3 lines 0 comments Download
M chrome/installer/setup/uninstall.cc View 1 2 3 4 5 6 18 chunks +46 lines, -26 lines 0 comments Download
M chrome/installer/util/app_command.cc View 1 2 3 4 5 2 chunks +10 lines, -6 lines 0 comments Download
M chrome/installer/util/copy_reg_key_work_item.h View 1 2 1 chunk +0 lines, -60 lines 0 comments Download
M chrome/installer/util/copy_reg_key_work_item.cc View 1 2 1 chunk +0 lines, -118 lines 0 comments Download
M chrome/installer/util/copy_reg_key_work_item_unittest.cc View 1 2 1 chunk +0 lines, -180 lines 0 comments Download
M chrome/installer/util/create_reg_key_work_item.h View 1 2 chunks +6 lines, -1 line 0 comments Download
M chrome/installer/util/create_reg_key_work_item.cc View 1 2 3 4 5 6 7 8 3 chunks +12 lines, -5 lines 0 comments Download
M chrome/installer/util/create_reg_key_work_item_unittest.cc View 1 5 chunks +10 lines, -10 lines 0 comments Download
M chrome/installer/util/delete_reg_key_work_item.h View 1 2 chunks +6 lines, -1 line 0 comments Download
M chrome/installer/util/delete_reg_key_work_item.cc View 1 2 3 4 5 6 7 4 chunks +15 lines, -13 lines 0 comments Download
M chrome/installer/util/delete_reg_key_work_item_unittest.cc View 1 4 chunks +8 lines, -8 lines 0 comments Download
M chrome/installer/util/delete_reg_value_work_item.h View 1 2 3 4 5 2 chunks +6 lines, -1 line 0 comments Download
M chrome/installer/util/delete_reg_value_work_item.cc View 1 2 3 4 5 3 chunks +11 lines, -4 lines 0 comments Download
M chrome/installer/util/delete_reg_value_work_item_unittest.cc View 1 2 3 4 5 2 chunks +10 lines, -10 lines 0 comments Download
M chrome/installer/util/google_update_settings_unittest.cc View 1 2 3 4 5 2 chunks +10 lines, -4 lines 0 comments Download
M chrome/installer/util/install_util.h View 1 2 3 4 5 6 3 chunks +6 lines, -1 line 0 comments Download
M chrome/installer/util/install_util.cc View 1 2 3 4 5 6 7 8 9 7 chunks +49 lines, -16 lines 0 comments Download
M chrome/installer/util/install_util_unittest.cc View 1 2 3 4 5 6 13 chunks +26 lines, -13 lines 0 comments Download
M chrome/installer/util/registry_key_backup.h View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/installer/util/registry_key_backup.cc View 1 2 2 chunks +14 lines, -4 lines 0 comments Download
M chrome/installer/util/registry_key_backup_unittest.cc View 1 4 chunks +19 lines, -8 lines 0 comments Download
M chrome/installer/util/set_reg_value_work_item.h View 1 2 3 4 5 2 chunks +14 lines, -4 lines 0 comments Download
M chrome/installer/util/set_reg_value_work_item.cc View 1 2 3 4 5 6 chunks +20 lines, -3 lines 0 comments Download
M chrome/installer/util/set_reg_value_work_item_unittest.cc View 1 2 3 4 5 7 chunks +64 lines, -20 lines 0 comments Download
M chrome/installer/util/shell_util.cc View 1 2 3 4 5 3 chunks +8 lines, -4 lines 0 comments Download
M chrome/installer/util/work_item.h View 1 2 3 4 5 6 chunks +23 lines, -12 lines 0 comments Download
M chrome/installer/util/work_item.cc View 1 2 3 4 5 4 chunks +34 lines, -21 lines 0 comments Download
M chrome/installer/util/work_item_list.h View 1 2 3 4 5 5 chunks +8 lines, -8 lines 0 comments Download
M chrome/installer/util/work_item_list.cc View 1 2 3 4 5 7 chunks +27 lines, -26 lines 0 comments Download
M chrome/installer/util/work_item_list_unittest.cc View 1 2 3 4 5 5 chunks +39 lines, -19 lines 0 comments Download
M chrome/test/mini_installer_test/installer_test_util.h View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/test/mini_installer_test/installer_test_util.cc View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/test/mini_installer_test/test.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
Will Harris
reviewing my own code is fun https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/install_worker.cc File chrome/installer/setup/install_worker.cc (right): https://codereview.chromium.org/282363003/diff/1/chrome/installer/setup/install_worker.cc#newcode619 chrome/installer/setup/install_worker.cc:619: #endif This looks ...
6 years, 7 months ago (2014-05-16 17:29:07 UTC) #1
grt (UTC plus 2)
https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_item_list.h File chrome/installer/util/work_item_list.h (right): https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_item_list.h#newcode51 chrome/installer/util/work_item_list.h:51: RegWow64ViewOption reg_wow64_option); it looks like it's going to be ...
6 years, 7 months ago (2014-05-16 18:26:10 UTC) #2
Will Harris
On 2014/05/16 18:26:10, grt wrote: > https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_item_list.h > File chrome/installer/util/work_item_list.h (right): > > https://codereview.chromium.org/282363003/diff/1/chrome/installer/util/work_item_list.h#newcode51 > ...
6 years, 7 months ago (2014-05-16 18:34:54 UTC) #3
grt (UTC plus 2)
On 2014/05/16 18:34:54, Will Harris wrote: > On 2014/05/16 18:26:10, grt wrote: > > > ...
6 years, 7 months ago (2014-05-20 17:48:18 UTC) #4
Will Harris
On 2014/05/20 17:48:18, grt wrote: > On 2014/05/16 18:34:54, Will Harris wrote: > > On ...
6 years, 7 months ago (2014-05-21 03:46:54 UTC) #5
grt (UTC plus 2)
looks good. https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/install_worker.cc File chrome/installer/setup/install_worker.cc (right): https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/install_worker.cc#newcode1003 chrome/installer/setup/install_worker.cc:1003: .c_str(), please move this up to the ...
6 years, 7 months ago (2014-05-21 16:41:05 UTC) #6
Will Harris
new patch set will come later tonight https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/install_worker.cc File chrome/installer/setup/install_worker.cc (right): https://codereview.chromium.org/282363003/diff/60001/chrome/installer/setup/install_worker.cc#newcode1003 chrome/installer/setup/install_worker.cc:1003: .c_str(), On ...
6 years, 7 months ago (2014-05-21 23:23:15 UTC) #7
Will Harris
On 2014/05/21 23:23:15, Will Harris wrote: > new patch set will come later tonight > ...
6 years, 7 months ago (2014-05-22 00:55:37 UTC) #8
grt (UTC plus 2)
looks gtm if my DeleteRegKeyWorkItem::Do() makes sense. thanks! https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/install_util.cc File chrome/installer/util/install_util.cc (right): https://codereview.chromium.org/282363003/diff/60001/chrome/installer/util/install_util.cc#newcode437 chrome/installer/util/install_util.cc:437: LONG ...
6 years, 7 months ago (2014-05-22 01:37:44 UTC) #9
Will Harris
PTAL. Also found some Registry code that wasn't being passed the wow64_access, and rearranged the ...
6 years, 7 months ago (2014-05-22 21:29:23 UTC) #10
grt (UTC plus 2)
looks like there's a compile error 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_; did ...
6 years, 7 months ago (2014-05-22 22:17:23 UTC) #11
Will Harris
the compile error is because the trybots haven't pulled in the new version of base::win::registry ...
6 years, 7 months ago (2014-05-22 22:23:29 UTC) #12
grt (UTC plus 2)
I believe the style guide says they're required if the statement spans multiple lines, but ...
6 years, 7 months ago (2014-05-22 22:45:14 UTC) #13
Will Harris
On 2014/05/22 22:45:14, grt wrote: > I believe the style guide says they're required if ...
6 years, 7 months ago (2014-05-23 00:36:39 UTC) #14
grt (UTC plus 2)
lgtm w/ one nit and one code fix. https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/install_util.cc File chrome/installer/util/install_util.cc (right): https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/install_util.cc#newcode445 chrome/installer/util/install_util.cc:445: KEY_READ ...
6 years, 7 months ago (2014-05-23 01:28:50 UTC) #15
Will Harris
Will wait for the bots to finish running then dcommit. https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/install_util.cc File chrome/installer/util/install_util.cc (right): https://codereview.chromium.org/282363003/diff/100001/chrome/installer/util/install_util.cc#newcode445 ...
6 years, 7 months ago (2014-05-23 03:19:21 UTC) #16
grt (UTC plus 2)
lgtm++
6 years, 7 months ago (2014-05-23 03:35:02 UTC) #17
Will Harris
On 2014/05/23 03:35:02, grt wrote: > lgtm++ sky@ grateful for owner LGTM for chrome/test changes. ...
6 years, 7 months ago (2014-05-23 03:36:55 UTC) #18
grt (UTC plus 2)
On 2014/05/23 03:36:55, Will Harris wrote: > On 2014/05/23 03:35:02, grt wrote: > > lgtm++ ...
6 years, 7 months ago (2014-05-23 11:02:56 UTC) #19
Will Harris
6 years, 7 months ago (2014-05-23 15:39:12 UTC) #20
Message was sent while issue was closed.
Committed patchset #10 manually as r272539 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698