Update CryptAuthDeviceManager to store all synced devices instead of only unlock keys.

Update CryptAuthDeviceManager to store all synced devices instead of only unlock keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263
Committed: https://crrev.com/f322da94ab81a3900e2712713bd4aa3108e78c75
Cr-Commit-Position: refs/heads/master@{#439158}

[Kyle Horimoto, 2016-12-07 22:14:44]

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

[Jeremy Klein, 2016-12-07 22:19:53]

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

[Jeremy Klein, 2016-12-07 22:19:54]

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

https://codereview.chromium.org/2561683002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:399:
unlock_keys_pref->Append(UnlockKeyToDictionary(device));
Any reason not to just use devices_as_list and remove this variable?

[Kyle Horimoto, 2016-12-07 22:25:35]

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

https://codereview.chromium.org/2561683002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.cc:399:
unlock_keys_pref->Append(UnlockKeyToDictionary(device));
On 2016/12/07 22:19:54, Jeremy Klein wrote:
> Any reason not to just use devices_as_list and remove this variable?

Done.

[Ryan Hansberry, 2016-12-07 23:51:37]

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

https://codereview.chromium.org/2561683002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.h:110: return synced_devices_;
Noob question: Why implement this method in the header file?

[Kyle Horimoto, 2016-12-08 00:11:13]

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

https://codereview.chromium.org/2561683002/diff/1/components/cryptauth/crypta...
components/cryptauth/cryptauth_device_manager.h:110: return synced_devices_;
On 2016/12/07 23:51:37, Ryan Hansberry wrote:
> Noob question: Why implement this method in the header file?

It's common to trivial getters this way to tell the compiler that you would like
the function inlined. However, the compiler can probably determine that this
should be inlined either way, so I'll just move it to the .cc file for
consistency.

[Tim Song, 2016-12-08 00:26:39]

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.cc:376:
CryptAuthDeviceManager::unlock_keys() const {
I believe the convention in Chrome is to only use underscores for trivial
getters like |synced_devices()|. For more complicated functions, use CamelCase.

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.h (right):

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:109: const
std::vector<cryptauth::ExternalDeviceInfo> synced_devices() const;
We can remove the const in the return type. It doesn't add any protection.

[Kyle Horimoto, 2016-12-08 00:35:03]

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.cc (right):

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.cc:376:
CryptAuthDeviceManager::unlock_keys() const {
On 2016/12/08 00:26:39, Tim Song wrote:
> I believe the convention in Chrome is to only use underscores for trivial
> getters like |synced_devices()|. For more complicated functions, use
CamelCase.

Done.

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.h (right):

https://codereview.chromium.org/2561683002/diff/40001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:109: const
std::vector<cryptauth::ExternalDeviceInfo> synced_devices() const;
On 2016/12/08 00:26:39, Tim Song wrote:
> We can remove the const in the return type. It doesn't add any protection.

Done.

[Tim Song, 2016-12-08 00:38:56]

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.h (right):

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:109:
std::vector<cryptauth::ExternalDeviceInfo> synced_devices() const;
I would also CamelCase this function as well since you're defining it in the .cc
file.

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:112:
std::vector<cryptauth::ExternalDeviceInfo> UnlockKeys() const;
By convention, this should be GetUnlockKeys().

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:115:
std::vector<cryptauth::ExternalDeviceInfo> TetherHosts() const;
By convention, this should be GetTetherHosts().

[Kyle Horimoto, 2016-12-08 00:46:34]

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
File components/cryptauth/cryptauth_device_manager.h (right):

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:109:
std::vector<cryptauth::ExternalDeviceInfo> synced_devices() const;
On 2016/12/08 00:38:56, Tim Song wrote:
> I would also CamelCase this function as well since you're defining it in the
.cc
> file.

Done.

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:112:
std::vector<cryptauth::ExternalDeviceInfo> UnlockKeys() const;
On 2016/12/08 00:38:56, Tim Song wrote:
> By convention, this should be GetUnlockKeys().

Done.

https://codereview.chromium.org/2561683002/diff/60001/components/cryptauth/cr...
components/cryptauth/cryptauth_device_manager.h:115:
std::vector<cryptauth::ExternalDeviceInfo> TetherHosts() const;
On 2016/12/08 00:38:56, Tim Song wrote:
> By convention, this should be GetTetherHosts().

Done.

[Kyle Horimoto, 2016-12-08 18:32:10]

Description was changed from

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()
==========

to

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672257
==========

[Kyle Horimoto, 2016-12-08 18:32:32]

Description was changed from

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672257
==========

to

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263
==========

[Tim Song, 2016-12-15 23:45:12]

lgtm

[Tim Song, 2016-12-15 23:45:13]

lgtm

[Kyle Horimoto, 2016-12-15 23:51:03]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2016-12-15 23:51:54]

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-16 02:29:14]

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

[commit-bot: I haz the power, 2016-12-16 02:29:15]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Kyle Horimoto, 2016-12-16 17:36:33]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2016-12-16 17:37:24]

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-16 18:37:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481909793439890,
"parent_rev": "723c2ce52835dfbc34aa7be436362d6b0f4435ea", "commit_rev":
"ea41d43db7fb86bca8d8f874ec305c292df4a6f2"}

[commit-bot: I haz the power, 2016-12-16 18:37:54]

Description was changed from

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263
==========

to

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263

Review-Url: https://codereview.chromium.org/2561683002
==========

[commit-bot: I haz the power, 2016-12-16 18:37:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-16 18:40:17]

Description was changed from

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263

Review-Url: https://codereview.chromium.org/2561683002
==========

to

==========
Update CryptAuthDeviceManager to store all synced devices instead of only unlock
keys.

To retrieve:
  *all devices: call synced_devices()
  *unlock keys: call unlock_keys()
  *tether hosts: call tether_hosts()

BUG=672263

Committed: https://crrev.com/f322da94ab81a3900e2712713bd4aa3108e78c75
Cr-Commit-Position: refs/heads/master@{#439158}
==========

[commit-bot: I haz the power, 2016-12-16 18:40:18]

Patchset 5 (id:??) landed as
https://crrev.com/f322da94ab81a3900e2712713bd4aa3108e78c75
Cr-Commit-Position: refs/heads/master@{#439158}