Updated handling of GCD devices.

Updated handling of GCD devices.

- GCD has begun enforcing stricter requirements for the |state| field
  of devices; this change restores compatibility.

- The GCD device ID is now used as the primary key for hosts when GCD
  support is enabled.

- Hosts registered in GCD now have a magic UUID stored in their tags
  so that Chromoting devices can be distinguished non-Chromoting devices
  stored in GCD.

- The |displayName| field has been officially deprecated.  With this
  change, the user-visible name of a host is stored in the |name|
  field, and the legacy host ID is stored as part of the |state|
  field.

- HostController.getLocalHostId is now aware of the difference between
  GCD and legacy hosts.  Depending on which field is stored in the
  local host config file, it returns the legacy ID |host_id|, or the
  GCD ID |gcd_device_id|.  This logic will move to HostListApi in
  an upcoming patch.

This CL contains some design decisions that may need to be re-visited
before launch.  In particular, there are fields on GCD that may be
more appropriate to use than nonstandard fields in |state|:

- The legacy ID, stored as |state.base._legacyId|, could be stored as
  the |serialNumber| field.  The GCD docs, however, indicate that this
  field is not writable, and it seems wise to store the legacy ID in a
  field that can be removed once migration to GCD is complete.

- Instead of a magic tag, it may make more sense to store a special
  value in |modelManfiest| or |modelManifesetId|.  Unfortunately these
  fields are largely undocumented.

- The host version, stored as |state.base._hostVersion|, could be
  stored as |state.base.firmwareVersion|.  The implications of doing
  so are unclear.

BUG=471928
Committed: https://crrev.com/5617fc4828bc1bf1729127964dbaffeb86a14024
Cr-Commit-Position: refs/heads/master@{#333589}

[John Williams, 2015-06-06 00:14:09]

jrw@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2015-06-08 19:39:03]

I'll take a look at the code in a minute, but I wanted to comment on the CL
description before I forget:

> - GCD has begun enforcing stricter requirements for the |state| field
>   of devices; this change restores compatibility.

I'm concerned that we've already had a breaking server-side change before we've
even released this. Can we get automated tests into GCD's test suite to prevent
this sort of thing in the future?

> - Hosts registered in GCD now have a magic UUID stored in their tags
>   so that Chromoting devices can be distinguished from non-GCD
>   devices.

s/non-GCD/non-Chromoting/? Or have I misunderstood what classes of things you're
trying to disambiguate?

[Jamie, 2015-06-08 21:44:34]

LGTM with nits.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/host_controller.js (right):

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_controller.js:169: var hostRegistered = false;
I don't think you need this variable any more.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_controller.js:416: hostId = base.getStringAttr(
Add a comment along the lines of "Use the gcd_device_id field if it exists, or
the host_id field otherwise." Bonus points for replacing "if it exists" with
something explaining under what conditions it is expected to exist.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/host_list_api_gcd_impl.js (right):

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:38: 'base': {
Elsewhere in this file there are no quotes around object keys. I'm not sure
what's more common in our code-base, but we should aim to be consistent.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:43: '_publicKey': publicKey
What's the significance of the leading underscore?

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:128: * Tag for distinguishing
Chromoting hosts from other devices stores
s/stores/stored/

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:129: * in GCD.
Blank line between description and annotations, please (here and below).

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:132: var MAGIC_TAG =
'1ce4542c-dd87-4320-ba19-ac173f98c04e';
Please give this a more descriptive name.

[John Williams, 2015-06-09 19:46:13]

The CQ bit was checked by jrw@chromium.org

[John Williams, 2015-06-09 19:46:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1161813006/#ps40001
(title: "requested by reviewer")

[John Williams, 2015-06-09 19:46:31]

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/host_controller.js (right):

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_controller.js:169: var hostRegistered = false;
On 2015/06/08 21:44:34, Jamie wrote:
> I don't think you need this variable any more.

Good catch. Thanks.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_controller.js:416: hostId = base.getStringAttr(
On 2015/06/08 21:44:34, Jamie wrote:
> Add a comment along the lines of "Use the gcd_device_id field if it exists, or
> the host_id field otherwise." Bonus points for replacing "if it exists" with
> something explaining under what conditions it is expected to exist.

Done.  Didn't go for bonus points because this code is about to change again
anyway.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/host_list_api_gcd_impl.js (right):

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:38: 'base': {
On 2015/06/08 21:44:34, Jamie wrote:
> Elsewhere in this file there are no quotes around object keys. I'm not sure
> what's more common in our code-base, but we should aim to be consistent.

Done.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:43: '_publicKey': publicKey
On 2015/06/08 21:44:34, Jamie wrote:
> What's the significance of the leading underscore?

Done.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:128: * Tag for distinguishing
Chromoting hosts from other devices stores
On 2015/06/08 21:44:34, Jamie wrote:
> s/stores/stored/

Done.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:129: * in GCD.
On 2015/06/08 21:44:34, Jamie wrote:
> Blank line between description and annotations, please (here and below).

Done.

https://codereview.chromium.org/1161813006/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/host_list_api_gcd_impl.js:132: var MAGIC_TAG =
'1ce4542c-dd87-4320-ba19-ac173f98c04e';
On 2015/06/08 21:44:34, Jamie wrote:
> Please give this a more descriptive name.

Done.

[commit-bot: I haz the power, 2015-06-09 19:46:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1161813006/40001

[Jamie, 2015-06-09 20:22:07]

The CQ bit was unchecked by jamiewalch@chromium.org

[Jamie, 2015-06-09 20:22:42]

On 2015/06/09 19:46:54, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1161813006/40001

Could you update the CL description along the lines I mentioned in #3 before
landing, please?

[John Williams, 2015-06-09 21:26:04]

The CQ bit was checked by jrw@chromium.org

[commit-bot: I haz the power, 2015-06-09 21:27:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1161813006/40001

[Jamie, 2015-06-09 21:37:03]

The CQ bit was unchecked by jamiewalch@chromium.org

[John Williams, 2015-06-09 21:53:16]

The CQ bit was checked by jrw@chromium.org

[commit-bot: I haz the power, 2015-06-09 21:54:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1161813006/40001

[commit-bot: I haz the power, 2015-06-09 22:16:51]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-09 22:17:52]

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