CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo fields.

CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a mobile hotspot is supported for the device).

Committed: https://crrev.com/28a34f90f6bd1fe320fd9a71e3bc1f639c539ac1
Cr-Commit-Position: refs/heads/master@{#436840}

[Kyle Horimoto, 2016-11-30 02:58:16]

khorimoto@chromium.org changed reviewers:
+ tengs@chromium.org

[Tim Song, 2016-12-02 21:03:52]

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:109: if
(beacon_seed_list->GetSize() > 0) {
Why not set it even if there is nothing in the list?

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:158: const
base::DictionaryValue* seed_dictionary = nullptr;
Refactor the parsing of BeaconSeed to its own function.

[Kyle Horimoto, 2016-12-06 19:56:27]

Description was changed from

==========
CryptAuthDeviceManager: Add support for syncing BeaconSeeds.

After this change, BeaconSeeds that are synced from CryptAuth are stored in user
preferences.
==========

to

==========
CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo
fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a
mobile hotspot is supported for the device).
==========

[Kyle Horimoto, 2016-12-06 19:56:36]

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:109: if
(beacon_seed_list->GetSize() > 0) {
On 2016/12/02 21:03:52, Tim Song wrote:
> Why not set it even if there is nothing in the list?

As per our offline discussion, not having anything stored and having an empty
list stored are distinct possibilities and should be treated as much. I've
updated this entire class to reflect that with every field except public key,
which is a required field.

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:158: const
base::DictionaryValue* seed_dictionary = nullptr;
On 2016/12/02 21:03:52, Tim Song wrote:
> Refactor the parsing of BeaconSeed to its own function.

Done.

[Tim Song, 2016-12-07 00:16:08]

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:109: if
(beacon_seed_list->GetSize() > 0) {
On 2016/12/06 19:56:36, Kyle Horimoto wrote:
> On 2016/12/02 21:03:52, Tim Song wrote:
> > Why not set it even if there is nothing in the list?
> 
> As per our offline discussion, not having anything stored and having an empty
> list stored are distinct possibilities and should be treated as much. I've
> updated this entire class to reflect that with every field except public key,
> which is a required field.

I thought we decided to store an empty list in this case?

[Kyle Horimoto, 2016-12-07 01:04:50]

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2540073002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:109: if
(beacon_seed_list->GetSize() > 0) {
On 2016/12/07 00:16:08, Tim Song wrote:
> On 2016/12/06 19:56:36, Kyle Horimoto wrote:
> > On 2016/12/02 21:03:52, Tim Song wrote:
> > > Why not set it even if there is nothing in the list?
> > 
> > As per our offline discussion, not having anything stored and having an
empty
> > list stored are distinct possibilities and should be treated as much. I've
> > updated this entire class to reflect that with every field except public
key,
> > which is a required field.
> 
> I thought we decided to store an empty list in this case?

Done.

[Tim Song, 2016-12-07 01:21:11]

lgtm

[Kyle Horimoto, 2016-12-07 01:23:12]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2016-12-07 01:23:41]

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-12-07 02:20:15]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481073792336820,
"parent_rev": "c6eb9d1a1789ae25ca03b74492a3f64de1c92582", "commit_rev":
"70f8c70139983b78a6a2fd250a6d8e34d07f0331"}

[commit-bot: I haz the power, 2016-12-07 02:20:37]

Description was changed from

==========
CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo
fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a
mobile hotspot is supported for the device).
==========

to

==========
CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo
fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a
mobile hotspot is supported for the device).
==========

[commit-bot: I haz the power, 2016-12-07 02:20:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-07 02:22:36]

Description was changed from

==========
CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo
fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a
mobile hotspot is supported for the device).
==========

to

==========
CryptAuthDeviceManager: Add support for syncing all of the ExternalDeviceInfo
fields.

This includes syncing BeaconSeeds as well as other metadata (e.g., whether a
mobile hotspot is supported for the device).

Committed: https://crrev.com/28a34f90f6bd1fe320fd9a71e3bc1f639c539ac1
Cr-Commit-Position: refs/heads/master@{#436840}
==========

[commit-bot: I haz the power, 2016-12-07 02:22:37]

Patchset 3 (id:??) landed as
https://crrev.com/28a34f90f6bd1fe320fd9a71e3bc1f639c539ac1
Cr-Commit-Position: refs/heads/master@{#436840}