[Sync] Fix namespaces for the browser_sync component.

[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw

Committed: https://crrev.com/0a37945f852bb8f479886642df5815e22879beea
Cr-Commit-Position: refs/heads/master@{#420465}

[maxbogue, 2016-09-19 17:05:55]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 17:06:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 17:26:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 17:26:30]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[maxbogue, 2016-09-19 19:50:39]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 19:51:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 20:07:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 20:07:41]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[maxbogue, 2016-09-19 20:42:47]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 20:43:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 21:22:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 21:22:27]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[maxbogue, 2016-09-20 06:39:23]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 06:39:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 08:08:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 08:08:51]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[maxbogue, 2016-09-20 16:41:37]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 16:41:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 17:34:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 17:34:38]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2016-09-22 01:26:16]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 01:26:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[maxbogue, 2016-09-22 01:35:23]

maxbogue@chromium.org changed reviewers:
+ skym@chromium.org

[maxbogue, 2016-09-22 01:35:25]

Sky, PTAL!

[commit-bot: I haz the power, 2016-09-22 02:05:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 02:05:15]

Dry run: This issue passed the CQ dry run.

[skym, 2016-09-22 17:26:00]

In retrospect, I think adding the browser_sync namespace to 
chrome/browser/sync/* before this CL would have reduced churn, but I suppose it
doesn't really matter. All inlined comments are nits. LGTM.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h:14: }
Shouldn't these have  // namespace foo upon close? Does lint catch this? Or does
lint miss the integration folder for some reason?

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/profile_sync_service_harness.cc
(right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.cc:40: 
Unbalanced newlines here look odd.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.cc:43: }
Closing namespace comment?

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/profile_sync_service_harness.h
(right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.h:20: }
Again, closing namespace comment?

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/sync_test.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/sync_test.h:49: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/app_sync_ui_state.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/app_sync_ui_state.h:23: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_sync_starter.cc:47: using
browser_sync::ProfileSyncService;
I can't seem to figure out why you sometimes go with using, and sometimes not. I
suppose it doesn't really matter.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_sync_starter.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_sync_starter.h:30: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/browsing_history_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/browsing_history_handler.h:32: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler.h:23: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc:202: mock_pss_ =
static_cast<browser_sync::ProfileSyncServiceMock*>(
I would have expected you to do a using here since it's a unit test.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/people_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/people_handler.h:28: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:204: mock_pss_ =
static_cast<browser_sync::ProfileSyncServiceMock*>(
Again, unittest, using?

https://codereview.chromium.org/2354613002/diff/100001/components/autofill/co...
File components/autofill/core/browser/webdata/autocomplete_syncable_service.h
(right):

https://codereview.chromium.org/2354613002/diff/100001/components/autofill/co...
components/autofill/core/browser/webdata/autocomplete_syncable_service.h:31: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
File components/browser_sync/abstract_profile_sync_service_test.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/abstract_profile_sync_service_test.h:27: 
Is it right to have a newline here?

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/abstract_profile_sync_service_test.h:56: // AUTO_START.
Passes |callback| down to
Why'd you strip off the namespace here?

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
File components/browser_sync/profile_sync_service.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/profile_sync_service.h:99: 
Should this newline be here?

https://codereview.chromium.org/2354613002/diff/100001/components/drive/drive...
File components/drive/drive_notification_manager.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/drive/drive...
components/drive/drive_notification_manager.h:18: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/syn...
File ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h (right):

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/syn...
ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h:24: }
Closing namespaces.

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/webui/history/browsing_history_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/webui/history/browsing_history_handler.h:30: }
Closing namespaces.

[maxbogue, 2016-09-22 19:40:14]

The CQ bit was checked by maxbogue@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 19:40:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[maxbogue, 2016-09-22 19:41:16]

Honestly I think it would make sense to have //c/b/sync in browser_sync too but
that wasn't part of my goal for this. Perhaps another time.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h:14: }
On 2016/09/22 17:25:58, skym wrote:
> Shouldn't these have  // namespace foo upon close? Does lint catch this? Or
does
> lint miss the integration folder for some reason?

Apparently it doesn't catch it. I don't even know if the style guide demands it
for forward declarations like this, but I'll add them.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/profile_sync_service_harness.cc
(right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.cc:40: 
On 2016/09/22 17:25:58, skym wrote:
> Unbalanced newlines here look odd.

That's because you're reading it wrong :P

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.cc:43: }
On 2016/09/22 17:25:58, skym wrote:
> Closing namespace comment?

This is the function close brace. The namespace closes on line 98.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/profile_sync_service_harness.h
(right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/profile_sync_service_harness.h:20: }
On 2016/09/22 17:25:58, skym wrote:
> Again, closing namespace comment?

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/sync_test.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/sync_test.h:49: }
On 2016/09/22 17:25:58, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/app_sync_ui_state.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/app_sync_ui_state.h:23: }
On 2016/09/22 17:25:58, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_sync_starter.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_sync_starter.cc:47: using
browser_sync::ProfileSyncService;
On 2016/09/22 17:25:58, skym wrote:
> I can't seem to figure out why you sometimes go with using, and sometimes not.
I
> suppose it doesn't really matter.

General rule of thumb was if the file had > ~5 uses, or used subclasses like
PSS::SyncStatusSummary, ::InitParams, etc. I don't think I was super consistent
though.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_sync_starter.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_sync_starter.h:30: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/browsing_history_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/browsing_history_handler.h:32: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler.h:23: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc:202: mock_pss_ =
static_cast<browser_sync::ProfileSyncServiceMock*>(
On 2016/09/22 17:25:59, skym wrote:
> I would have expected you to do a using here since it's a unit test.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/people_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/people_handler.h:28: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/2354613002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:204: mock_pss_ =
static_cast<browser_sync::ProfileSyncServiceMock*>(
On 2016/09/22 17:25:59, skym wrote:
> Again, unittest, using?

Done.

https://codereview.chromium.org/2354613002/diff/100001/components/autofill/co...
File components/autofill/core/browser/webdata/autocomplete_syncable_service.h
(right):

https://codereview.chromium.org/2354613002/diff/100001/components/autofill/co...
components/autofill/core/browser/webdata/autocomplete_syncable_service.h:31: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
File components/browser_sync/abstract_profile_sync_service_test.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/abstract_profile_sync_service_test.h:27: 
On 2016/09/22 17:25:59, skym wrote:
> Is it right to have a newline here?

I think having it makes it clearer that the namespace isn't /just/ for forward
declarations.

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/abstract_profile_sync_service_test.h:56: // AUTO_START.
Passes |callback| down to
On 2016/09/22 17:25:59, skym wrote:
> Why'd you strip off the namespace here?

I literally did s/browser_sync::// in this directory. I think it reads fine.
I've re-flowed this comment now though.

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
File components/browser_sync/profile_sync_service.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/browser_syn...
components/browser_sync/profile_sync_service.h:99: 
On 2016/09/22 17:25:59, skym wrote:
> Should this newline be here?

Yes.

https://codereview.chromium.org/2354613002/diff/100001/components/drive/drive...
File components/drive/drive_notification_manager.h (right):

https://codereview.chromium.org/2354613002/diff/100001/components/drive/drive...
components/drive/drive_notification_manager.h:18: }
On 2016/09/22 17:25:59, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/syn...
File ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h (right):

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/syn...
ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h:24: }
On 2016/09/22 17:26:00, skym wrote:
> Closing namespaces.

Done.

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/webui/history/browsing_history_handler.h (right):

https://codereview.chromium.org/2354613002/diff/100001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/webui/history/browsing_history_handler.h:30: }
On 2016/09/22 17:26:00, skym wrote:
> Closing namespaces.

Done.

[maxbogue, 2016-09-22 19:50:45]

Description was changed from

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
==========

to

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw
==========

[maxbogue, 2016-09-22 19:51:35]

maxbogue@chromium.org changed reviewers:
+ brettw@chromium.org

[maxbogue, 2016-09-22 19:51:37]

TBR brettw@ for all the things since this is just boring refactoring.

[maxbogue, 2016-09-22 19:51:55]

The CQ bit was unchecked by maxbogue@chromium.org

[maxbogue, 2016-09-22 19:52:02]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2016-09-22 19:52:03]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2354613002/#ps120001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-09-22 19:52:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brettw, 2016-09-22 20:06:44]

rs lgtm based on a few spot checks.

[brettw, 2016-09-22 20:06:46]

rs lgtm based on a few spot checks.

[commit-bot: I haz the power, 2016-09-22 21:35:26]

Description was changed from

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw
==========

to

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw
==========

[commit-bot: I haz the power, 2016-09-22 21:35:29]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-22 21:36:44]

Description was changed from

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw
==========

to

==========
[Sync] Fix namespaces for the browser_sync component.

Things within a component should have a namespace that matches its name.

BUG=646685
TBR=brettw

Committed: https://crrev.com/0a37945f852bb8f479886642df5815e22879beea
Cr-Commit-Position: refs/heads/master@{#420465}
==========

[commit-bot: I haz the power, 2016-09-22 21:36:47]

Patchset 7 (id:??) landed as
https://crrev.com/0a37945f852bb8f479886642df5815e22879beea
Cr-Commit-Position: refs/heads/master@{#420465}