[Sync] Componentize local_device_info_provider_impl and test

[Sync] Componentize local_device_info_provider_impl and test

Pass channel and version info in at construction time, and the blocking pool at initialize
time.

BUG=396136
Committed: https://crrev.com/035008523b829d1f62db6b2ec825b8ca2aa4736d
Cr-Commit-Position: refs/heads/master@{#356447}

[Nicolas Zea, 2015-10-08 22:49:04]

zea@chromium.org changed reviewers:
+ stanisc@chromium.org

[Nicolas Zea, 2015-10-08 22:49:05]

+Stan, PTAL

[stanisc, 2015-10-09 17:18:03]

Looks good overall.
The only issue I found is that LocalDeviceProviderImpl is still in the old
namespace.

https://codereview.chromium.org/1397913002/diff/20001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/20001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_components_factory_impl.cc:452:
chrome::GetChannel(), chrome::GetVersionString()));
Nice way of separating the components code and the browser specific code!

https://codereview.chromium.org/1397913002/diff/20001/components/sync_driver/...
File components/sync_driver/local_device_info_provider_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/20001/components/sync_driver/...
components/sync_driver/local_device_info_provider_impl.cc:13: namespace
browser_sync {
Should the namespace be changed to sync_driver?

https://codereview.chromium.org/1397913002/diff/20001/components/sync_driver/...
File components/sync_driver/local_device_info_provider_unittest.cc (right):

https://codereview.chromium.org/1397913002/diff/20001/components/sync_driver/...
components/sync_driver/local_device_info_provider_unittest.cc:15: namespace
browser_sync {
Should the namespace change as well?

[Nicolas Zea, 2015-10-09 17:49:16]

Right now, the plan with componentization is to do the namespace conversions at
the end, once everything is componentized. This minimizes the code churn
involved (needing to update the namespace in all call sites).

[stanisc, 2015-10-09 18:00:45]

lgtm

[Nicolas Zea, 2015-10-09 18:02:52]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2015-10-09 18:07:32]

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

[commit-bot: I haz the power, 2015-10-09 18:14:26]

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

[commit-bot: I haz the power, 2015-10-09 18:14:26]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Nicolas Zea, 2015-10-09 21:20:14]

zea@chromium.org changed reviewers:
+ avi@chromium.org

[Nicolas Zea, 2015-10-09 21:20:14]

+Avi for sync_driver DEP on ui/base

[Avi (use Gerrit), 2015-10-10 03:52:51]

https://codereview.chromium.org/1397913002/diff/40001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/40001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_components_factory_impl.cc:425:
chrome::GetChannel(), chrome::GetVersionString()));
Why don't you also pass in the is_tablet boolean here...

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
File components/sync_driver/local_device_info_provider_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
components/sync_driver/local_device_info_provider_impl.cc:11: #include
"ui/base/device_form_factor.h"
... and then not have to worry about depending on ui/base here ...

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
components/sync_driver/local_device_info_provider_impl.cc:20: }
... since this is the only place that uses it, and you're already passing in
other stuff?

[Nicolas Zea, 2015-10-12 17:32:34]

Patchset #4 (id:60001) has been deleted

[Nicolas Zea, 2015-10-12 17:44:02]

Removed the new ui/ dependency.

Stan, could you take another look? I had to update the tests as well.

https://codereview.chromium.org/1397913002/diff/40001/chrome/browser/sync/pro...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/40001/chrome/browser/sync/pro...
chrome/browser/sync/profile_sync_components_factory_impl.cc:425:
chrome::GetChannel(), chrome::GetVersionString()));
On 2015/10/10 03:52:51, Avi wrote:
> Why don't you also pass in the is_tablet boolean here...

Mainly from assuming we're going to eventually use more ui/ signals to decide
device type in the future. But yeah, given it's the only bit of data from ui/
we're using right now, may as well just plumb it in. Done.

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
File components/sync_driver/local_device_info_provider_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
components/sync_driver/local_device_info_provider_impl.cc:11: #include
"ui/base/device_form_factor.h"
On 2015/10/10 03:52:51, Avi wrote:
> ... and then not have to worry about depending on ui/base here ...

Done.

https://codereview.chromium.org/1397913002/diff/40001/components/sync_driver/...
components/sync_driver/local_device_info_provider_impl.cc:20: }
On 2015/10/10 03:52:51, Avi wrote:
> ... since this is the only place that uses it, and you're already passing in
> other stuff?

Done.

[Nicolas Zea, 2015-10-20 17:28:49]

zea@chromium.org changed reviewers:
- avi@chromium.org

[blundell, 2015-10-27 16:17:52]

blundell@chromium.org changed reviewers:
+ blundell@chromium.org

[blundell, 2015-10-27 16:17:52]

What's the status of this CL?

[Nicolas Zea, 2015-10-27 21:23:48]

On 2015/10/27 16:17:52, blundell wrote:
> What's the status of this CL?

Chatted with Stan and he's good with it. Landing.

[Nicolas Zea, 2015-10-27 21:23:52]

The CQ bit was checked by zea@chromium.org

[Nicolas Zea, 2015-10-27 21:23:52]

The patchset sent to the CQ was uploaded after l-g-t-m from stanisc@chromium.org
Link to the patchset: https://codereview.chromium.org/1397913002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2015-10-27 21:25:40]

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

[commit-bot: I haz the power, 2015-10-27 21:46:28]

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

[commit-bot: I haz the power, 2015-10-27 21:46:29]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)

[stanisc, 2015-10-27 22:32:43]

https://codereview.chromium.org/1397913002/diff/100001/components/sync_driver...
File components/sync_driver/local_device_info_provider_impl.cc (right):

https://codereview.chromium.org/1397913002/diff/100001/components/sync_driver...
components/sync_driver/local_device_info_provider_impl.cc:26: return
IsTabletUI() ? sync_pb::SyncEnums_DeviceType_TYPE_TABLET
Missed this during review: IsTabletUI() should be replaced with is_tablet_

[Nicolas Zea, 2015-10-27 22:41:58]

The CQ bit was checked by zea@chromium.org

[Nicolas Zea, 2015-10-27 22:41:58]

The patchset sent to the CQ was uploaded after l-g-t-m from stanisc@chromium.org
Link to the patchset: https://codereview.chromium.org/1397913002/#ps120001
(title: "Fix android")

[commit-bot: I haz the power, 2015-10-27 22:43:24]

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

[commit-bot: I haz the power, 2015-10-28 00:16:18]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-10-28 00:17:10]

Patchset 6 (id:??) landed as
https://crrev.com/035008523b829d1f62db6b2ec825b8ca2aa4736d
Cr-Commit-Position: refs/heads/master@{#356447}