Use RegistryOverrideManager properly in RLZ tests.

Use RegistryOverrideManager properly in RLZ tests.

Previously, rlz/test/rlz_test_helpers.cc and
chrome/browser/rlz/rlz_unittest.cc each did somewhat different things to
prepare the registry for tests. Now, rlz_test_helpers.cc does the prep
the right way using RegistryOverrideManager and rlz_unittests.cc uses
rlz_test_helpers.cc.

This unblocks the refactor in r234367 that was reverted in r234627.

BUG=314800
R=rogerta@chromium.org, tommycli@chromium.org

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

[grt (UTC plus 2), 2013-11-13 04:23:47]

tryjobs are running on patchset 1.

https://codereview.chromium.org/63153009/diff/40001/rlz/lib/rlz_lib.h
File rlz/lib/rlz_lib.h (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/lib/rlz_lib.h#newcode55
rlz/lib/rlz_lib.h:55: // All functions return true on success and false on
error.
rogerta: i extracted this comment from win/lib/rlz_lib.h (which i've deleted).
it seemed to fit here.

[tommycli, 2013-11-13 16:50:32]

lgtm except below:

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers.cc
File rlz/test/rlz_test_helpers.cc (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:46: void ReadRegistryTree(const base::win::RegKey&
src, RegistryKeyData* data) {
This is great. I hope that future callers that use a technique like this rather
than try to do what was done before.

Maybe something like this should be in test_reg_util_win...

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:144: #endif  // defined(OS_WIN)
// defined (OS_POSIX)

[Roger Tawa OOO till Jul 10th, 2013-11-13 18:39:31]

lgtm

https://codereview.chromium.org/63153009/diff/40001/rlz/lib/rlz_lib.h
File rlz/lib/rlz_lib.h (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/lib/rlz_lib.h#newcode55
rlz/lib/rlz_lib.h:55: // All functions return true on success and false on
error.
On 2013/11/13 04:23:47, grt wrote:
> rogerta: i extracted this comment from win/lib/rlz_lib.h (which i've deleted).
> it seemed to fit here.

Looks fine.  This file still contains a lot of windows specific comments even
though its used on all platforms.

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers.cc
File rlz/test/rlz_test_helpers.cc (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:46: void ReadRegistryTree(const base::win::RegKey&
src, RegistryKeyData* data) {
On 2013/11/13 16:50:32, tommycli wrote:
> This is great. I hope that future callers that use a technique like this
rather
> than try to do what was done before.
> 
> Maybe something like this should be in test_reg_util_win...

Why is the better than before?

[tommycli, 2013-11-13 18:53:20]

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers.cc
File rlz/test/rlz_test_helpers.cc (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:46: void ReadRegistryTree(const base::win::RegKey&
src, RegistryKeyData* data) {
On 2013/11/13 18:39:31, Roger Tawa wrote:
> On 2013/11/13 16:50:32, tommycli wrote:
> > This is great. I hope that future callers that use a technique like this
> rather
> > than try to do what was done before.
> > 
> > Maybe something like this should be in test_reg_util_win...
> 
> Why is the better than before?

By reading the reg data into memory temporarily, it avoids trying to find out
RegistryOverrideManager's temporary key paths.

This was necessary because the (reverted) refactored RegistryOverrideManager no
longer exposes the temporary key paths it uses.

[grt (UTC plus 2), 2013-11-14 05:22:14]

Thanks for the reviews.

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers.cc
File rlz/test/rlz_test_helpers.cc (right):

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:46: void ReadRegistryTree(const base::win::RegKey&
src, RegistryKeyData* data) {
On 2013/11/13 16:50:32, tommycli wrote:
> This is great. I hope that future callers that use a technique like this
rather
> than try to do what was done before.
> 
> Maybe something like this should be in test_reg_util_win...

I'd like to wait to generalize it (and write additional unit tests for the test
code) until there's another consumer. If you feel strongly, this can be done in
a followon CL.

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:46: void ReadRegistryTree(const base::win::RegKey&
src, RegistryKeyData* data) {
On 2013/11/13 18:53:20, tommycli wrote:
> On 2013/11/13 18:39:31, Roger Tawa wrote:
> > On 2013/11/13 16:50:32, tommycli wrote:
> > > This is great. I hope that future callers that use a technique like this
> > rather
> > > than try to do what was done before.
> > > 
> > > Maybe something like this should be in test_reg_util_win...
> > 
> > Why is the better than before?
> 
> By reading the reg data into memory temporarily, it avoids trying to find out
> RegistryOverrideManager's temporary key paths.
> 
> This was necessary because the (reverted) refactored RegistryOverrideManager
no
> longer exposes the temporary key paths it uses.

Right. And the reason the new RegistryOverrideManager behavior is important is
that it allows tests using the ROM to run in parallel. Hooray for faster tests!

https://codereview.chromium.org/63153009/diff/40001/rlz/test/rlz_test_helpers...
rlz/test/rlz_test_helpers.cc:144: #endif  // defined(OS_WIN)
On 2013/11/13 16:50:32, tommycli wrote:
> // defined (OS_POSIX)

Done.

[commit-bot: I haz the power, 2013-11-14 05:22:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grt@chromium.org/63153009/150001

[commit-bot: I haz the power, 2013-11-14 07:43:25]

Change committed as 235090