Pass signin_scoped_device_id to DeviceInfoSpecifics.

Pass signin_scoped_device_id to DeviceInfoSpecifics.

I'm adding signin_scoped_device_id to DeviceInfoSpecifics.
This value is available from SigninClient, the rest of code is to pass
value from syncBackendHostImpl where SigninClient is availble to
SyncedDeviceTracker where value is passed to protobuf.

Also there is small change to how commandline flag is handled.

BUG=382968
R=rlarocque@chromium.org

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

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

[pavely, 2014-07-16 00:24:46]

Guys, could you review corresponding folders:

yoz@ : chrome/browser/extensions/api/*
tim@ : chrome/browser/signin/*
sky@ : chrome/browser/ui/toolbar/*
rlarocque@ : everything else

The change isn't really big, many changes are caused by git cl format.

https://codereview.chromium.org/395013002/diff/1/chrome/browser/signin/chrome...
File chrome/browser/signin/chrome_signin_client.cc (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/signin/chrome...
chrome/browser/signin/chrome_signin_client.cc:136: std::string
signin_scoped_device_id;
I made a mistake in my previous change. If commandline switch is disabled but
device_id is already stored in prefs then that device_id is returned.
I'm changing it to always return empty string if commandline switch is disabled.

[Yoyo Zhou, 2014-07-16 00:44:25]

extensions/api LGTM

[rlarocque, 2014-07-16 00:44:31]

https://codereview.chromium.org/395013002/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/signed_in_devices/id_mapping_helper_unittest.cc
(right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/signed_in_devices/id_mapping_helper_unittest.cc:39:
"device_id"));
Not that it really matters, but would it be more realistic if the "device_id"
string were different for each of these?

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_core.h (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_core.h:325: std::string
signin_scoped_device_id_;
nit: Comment this member.

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_impl.cc (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_impl.cc:140:
signin_client->GetSigninScopedDeviceId();
Is this guaranteed to succeed?  If it can fail, what does the failure path look
like?

[sky, 2014-07-16 14:47:32]

LGTM

[pavely, 2014-07-16 21:35:59]

PTAL

https://codereview.chromium.org/395013002/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/signed_in_devices/id_mapping_helper_unittest.cc
(right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/signed_in_devices/id_mapping_helper_unittest.cc:39:
"device_id"));
On 2014/07/16 00:44:31, rlarocque wrote:
> Not that it really matters, but would it be more realistic if the "device_id"
> string were different for each of these?

Done.

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_core.h (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_core.h:325: std::string
signin_scoped_device_id_;
On 2014/07/16 00:44:31, rlarocque wrote:
> nit: Comment this member.

Done.

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_impl.cc (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_impl.cc:140:
signin_client->GetSigninScopedDeviceId();
profile_ is always valid, it is dereferenced in multiple places.
signin_client creation is trivial, but let me add DCHECK for that.
GetSigninScopedDeviceId() returns either valid device_id or empty string, both
cases are fine.
On 2014/07/16 00:44:31, rlarocque wrote:
> Is this guaranteed to succeed?  If it can fail, what does the failure path
look
> like?

[rlarocque, 2014-07-16 21:39:49]

lgtm.

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host_impl.cc (right):

https://codereview.chromium.org/395013002/diff/1/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host_impl.cc:140:
signin_client->GetSigninScopedDeviceId();
On 2014/07/16 21:35:59, pavely wrote:
> profile_ is always valid, it is dereferenced in multiple places.
> signin_client creation is trivial, but let me add DCHECK for that.
> GetSigninScopedDeviceId() returns either valid device_id or empty string, both
> cases are fine.
> On 2014/07/16 00:44:31, rlarocque wrote:
> > Is this guaranteed to succeed?  If it can fail, what does the failure path
> look
> > like?
> 

I think it was the empty string case that worried me the most.  I figured there
was a chance that a check on the return value was necessary, but I guess not.

[pavely, 2014-07-18 20:15:28]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2014-07-18 20:17:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pavely@chromium.org/395013002/120001

[commit-bot: I haz the power, 2014-07-18 22:47:08]

Change committed as 284234

[Vitaly Buka (NO REVIEWS), 2014-07-18 23:28:15]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-07-18 23:29:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pavely@chromium.org/395013002/120001

[commit-bot: I haz the power, 2014-07-18 23:38:12]

Change committed as 284251