Create profile_sync_test_util in components

Create profile_sync_test_util in components

There are already such utils in //chrome/browser/sync and the //ios version as
well. This CL starts by pulling out some trivial parts into a common file in
//components. More will follow as ProfileSyncService unittests will get
componentised.

BUG=544972
Committed: https://crrev.com/9e9222f52c71c65bdd1cfc810f83b5ed1d1d3016
Cr-Commit-Position: refs/heads/master@{#370659}

[vabr (Chromium), 2016-01-18 16:52:13]

vabr@chromium.org changed reviewers:
+ droger@chromium.org, stanisc@chromium.org

[vabr (Chromium), 2016-01-18 16:52:14]

@droger -- please review
ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.cc

@stanisc -- please review the rest

Thanks both!
Vaclav

[droger, 2016-01-18 16:57:47]

ios_chrome_profile_sync_test_util.cc LGTM

[droger, 2016-01-18 16:58:44]

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
File components/browser_sync/browser/profile_sync_test_util.h (right):

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
components/browser_sync/browser/profile_sync_test_util.h:16: 
Should this be in the browser_sync namespace?

[vabr (Chromium), 2016-01-18 17:04:24]

Thanks, droger@!

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
File components/browser_sync/browser/profile_sync_test_util.h (right):

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
components/browser_sync/browser/profile_sync_test_util.h:16: 
On 2016/01/18 16:58:44, droger wrote:
> Should this be in the browser_sync namespace?

I was thinking about that, but the component does not seem to have a unified
style yet (most of the code has no namespace, some is in sync_driver, etc.).

I suggest going with a sync OWNER suggestion -- stanisc@, what's your take on
this?

[stanisc, 2016-01-19 19:04:00]

https://codereview.chromium.org/1607473003/diff/20001/chrome/browser/sync/tes...
File chrome/browser/sync/test_profile_sync_service.cc (right):

https://codereview.chromium.org/1607473003/diff/20001/chrome/browser/sync/tes...
chrome/browser/sync/test_profile_sync_service.cc:55: init_params.sync_client =
Is this new code a replacement for CreateProfileSyncServiceParamsForTest? What
was the reason for in-lining it?

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
File components/browser_sync/browser/profile_sync_test_util.h (right):

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
components/browser_sync/browser/profile_sync_test_util.h:16: 
On 2016/01/18 17:04:24, vabr (Chromium) wrote:
> On 2016/01/18 16:58:44, droger wrote:
> > Should this be in the browser_sync namespace?
> 
> I was thinking about that, but the component does not seem to have a unified
> style yet (most of the code has no namespace, some is in sync_driver, etc.).
> 
> I suggest going with a sync OWNER suggestion -- stanisc@, what's your take on
> this?

The code in components/browser_sync has been moved around recently and the
namespace hasn't been changed everywhere yet. The namespace should be
browser_sync going forward.

[vabr (Chromium), 2016-01-20 16:43:41]

Thanks for the review so far!

stanisc@: Comments addressed, PTAL.

Cheers,
Vaclav

https://codereview.chromium.org/1607473003/diff/20001/chrome/browser/sync/tes...
File chrome/browser/sync/test_profile_sync_service.cc (right):

https://codereview.chromium.org/1607473003/diff/20001/chrome/browser/sync/tes...
chrome/browser/sync/test_profile_sync_service.cc:55: init_params.sync_client =
On 2016/01/19 19:04:00, stanisc wrote:
> Is this new code a replacement for CreateProfileSyncServiceParamsForTest? What
> was the reason for in-lining it?

I created CreateProfileSyncServiceParamsForTest as a convenience method for
tests using Profile. However, the goal (not achieved in this CL yet) is to
componentise this test, in particular, to get rid of Profile.

Some of the init parameters sourced by CreateProfileSyncServiceParamsForTest
from the Profile have already been replaced before this CL, e.g.
oauth2_token_service, so the inlining removes sourcing them from the Profile.

And while this CL still leaves some init params sourced from Profile, it will
get changed soon in subsequent CLs, step by step.

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
File components/browser_sync/browser/profile_sync_test_util.h (right):

https://codereview.chromium.org/1607473003/diff/20001/components/browser_sync...
components/browser_sync/browser/profile_sync_test_util.h:16: 
On 2016/01/19 19:04:00, stanisc wrote:
> On 2016/01/18 17:04:24, vabr (Chromium) wrote:
> > On 2016/01/18 16:58:44, droger wrote:
> > > Should this be in the browser_sync namespace?
> > 
> > I was thinking about that, but the component does not seem to have a unified
> > style yet (most of the code has no namespace, some is in sync_driver, etc.).
> > 
> > I suggest going with a sync OWNER suggestion -- stanisc@, what's your take
on
> > this?
> 
> The code in components/browser_sync has been moved around recently and the
> namespace hasn't been changed everywhere yet. The namespace should be
> browser_sync going forward.

Thanks!
Done.

[stanisc, 2016-01-20 19:52:03]

lgtm

[vabr (Chromium), 2016-01-21 10:12:37]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2016-01-21 10:12:38]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org
Link to the patchset: https://codereview.chromium.org/1607473003/#ps80001
(title: "Fix iOS")

[commit-bot: I haz the power, 2016-01-21 10:13:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1607473003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1607473003/80001

[commit-bot: I haz the power, 2016-01-21 10:18:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-01-21 10:19:37]

Description was changed from

==========
Create profile_sync_test_util in components

There are already such utils in //chrome/browser/sync and the //ios version as
well. This CL starts by pulling out some trivial parts into a common file in
//components. More will follow as ProfileSyncService unittests will get
componentised.

BUG=544972
==========

to

==========
Create profile_sync_test_util in components

There are already such utils in //chrome/browser/sync and the //ios version as
well. This CL starts by pulling out some trivial parts into a common file in
//components. More will follow as ProfileSyncService unittests will get
componentised.

BUG=544972

Committed: https://crrev.com/9e9222f52c71c65bdd1cfc810f83b5ed1d1d3016
Cr-Commit-Position: refs/heads/master@{#370659}
==========

[commit-bot: I haz the power, 2016-01-21 10:19:38]

Patchset 5 (id:??) landed as
https://crrev.com/9e9222f52c71c65bdd1cfc810f83b5ed1d1d3016
Cr-Commit-Position: refs/heads/master@{#370659}