[Sync] DeviceInfoService should format ClientTag in backwards compatible way

[Sync] DeviceInfoService should format ClientTag in backwards compatible way

In order for existing clients to work correctly with entries from
USS based DeviceInfoService it should format ClientTag in the same way
DeviceInfoSyncService does.
In this change:
- In all interactions with sync "DeviceInfo_" prefix is used.
- plain cache_guids are used as local storage keys and keys into all_data_.

BUG=603695
R=skym@chromium.org

Committed: https://crrev.com/76aa5567e3cf4ff5dc664c5d645c65c3a0be6987
Cr-Commit-Position: refs/heads/master@{#387742}

[skym, 2016-04-14 21:08:26]

This is temporary, right? Ultimately the processor is going to speak in terms of
keys, and TagToCacheGuid(...) will be removed, and SpecificsToTag(...) will be a
ModelTypeService method, right?

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
File components/sync_driver/device_info_service.cc (right):

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:44: const char kClientTagPrefix[]
= "DeviceInfo_";
Can old and new device info services share this constant?

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:365: std::string guid =
specifics->cache_guid();
Does this do an unnecessary copy?

[skym, 2016-04-14 21:09:19]

On 2016/04/14 21:08:26, skym wrote:
> This is temporary, right? Ultimately the processor is going to speak in terms
of
> keys, and TagToCacheGuid(...) will be removed, and SpecificsToTag(...) will be
a
> ModelTypeService method, right?
> 
>
https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
> File components/sync_driver/device_info_service.cc (right):
> 
>
https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
> components/sync_driver/device_info_service.cc:44: const char
kClientTagPrefix[]
> = "DeviceInfo_";
> Can old and new device info services share this constant?
> 
>
https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
> components/sync_driver/device_info_service.cc:365: std::string guid =
> specifics->cache_guid();
> Does this do an unnecessary copy?

Errm, I meant SpecificsToTag(...) will be an unimplemented virtual
ModelTypeService method.

[maxbogue, 2016-04-14 22:30:13]

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

[maxbogue, 2016-04-14 22:30:13]

+1 to Sky's question; we're gonna have the service <-> processor interactions
also use the key aren't we? So that the data and metadata are keyed by the same
thing in the store?

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
File components/sync_driver/device_info_service.cc (right):

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:44: const char kClientTagPrefix[]
= "DeviceInfo_";
On 2016/04/14 21:08:26, skym wrote:
> Can old and new device info services share this constant?

Or at least it should be in an anonymous namespace shouldn't it?

[pavely, 2016-04-14 23:37:14]

On 2016/04/14 21:08:26, skym wrote:
> This is temporary, right? Ultimately the processor is going to speak in terms
of
> keys, and TagToCacheGuid(...) will be removed, and SpecificsToTag(...) will be
a
> ModelTypeService method, right?

Yes, this is temporary. For the proper solution I want to see how interface will
work with non-leveldb storage.
The goal of this change is to make initialzation scenario working end-to-end.

[pavely, 2016-04-14 23:37:23]

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
File components/sync_driver/device_info_service.cc (right):

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:44: const char kClientTagPrefix[]
= "DeviceInfo_";
On 2016/04/14 22:30:13, maxbogue wrote:
> On 2016/04/14 21:08:26, skym wrote:
> > Can old and new device info services share this constant?
> 
> Or at least it should be in an anonymous namespace shouldn't it?

Done.

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:44: const char kClientTagPrefix[]
= "DeviceInfo_";
On 2016/04/14 21:08:26, skym wrote:
> Can old and new device info services share this constant?

It would be nice. The best shared place for declaring this constant is
local_device_info_provider.h, this is the most natural common dependency between
DeviceInfoService and DeviceInfoSyncService. It feels like this constant is out
of place there.

Let me know if you feel strongly about it and I'll move constant there.

https://codereview.chromium.org/1888923003/diff/1/components/sync_driver/devi...
components/sync_driver/device_info_service.cc:365: std::string guid =
specifics->cache_guid();
On 2016/04/14 21:08:26, skym wrote:
> Does this do an unnecessary copy?

Done.

[skym, 2016-04-15 14:52:05]

lgtm

[maxbogue, 2016-04-15 21:37:46]

lgtm

[pavely, 2016-04-15 22:45:00]

The CQ bit was checked by pavely@chromium.org

[commit-bot: I haz the power, 2016-04-15 22:45:15]

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

[commit-bot: I haz the power, 2016-04-15 23:20:10]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-15 23:21:45]

Description was changed from

==========
[Sync] DeviceInfoService should format ClientTag in backwards compatible way

In order for existing clients to work correctly with entries from
USS based DeviceInfoService it should format ClientTag in the same way
DeviceInfoSyncService does.
In this change:
- In all interactions with sync "DeviceInfo_" prefix is used.
- plain cache_guids are used as local storage keys and keys into all_data_.

BUG=603695
R=skym@chromium.org
==========

to

==========
[Sync] DeviceInfoService should format ClientTag in backwards compatible way

In order for existing clients to work correctly with entries from
USS based DeviceInfoService it should format ClientTag in the same way
DeviceInfoSyncService does.
In this change:
- In all interactions with sync "DeviceInfo_" prefix is used.
- plain cache_guids are used as local storage keys and keys into all_data_.

BUG=603695
R=skym@chromium.org

Committed: https://crrev.com/76aa5567e3cf4ff5dc664c5d645c65c3a0be6987
Cr-Commit-Position: refs/heads/master@{#387742}
==========

[commit-bot: I haz the power, 2016-04-15 23:21:46]

Patchset 2 (id:??) landed as
https://crrev.com/76aa5567e3cf4ff5dc664c5d645c65c3a0be6987
Cr-Commit-Position: refs/heads/master@{#387742}