Basic webapp UI for host-offline-reason.

Basic webapp UI for host-offline-reason.

This changelist:

Adding a basic UI for displaying host-offline-reason:
- When no host-offline-reason is provided, old behavior is preserved

- When a host-offline-reason is provided, the text used to present a host
  says:        "<hostname> (reported an error <time>)"
  rather than: "<hostname> (last online 11/11/2014)"

- When a known host-offline-reason is provided, a tooltip is added to the host
  giving localized description of the specific host-offline-reason.  Example
  tooltip: "Invalid host configuration."

- When an unknown host-offline-reason is provided (one without a localized
  description text), a generic tooltip is added saying:
  "Unrecognized error: INVALID_HOST_CONFIGURATION."

Context:

After crrev.com/719983002 Chromoting host sends an host-offline-reason string
before exiting.  The host-offline-reason value is currently always based on
ExitCodeToString function / HostExitCodes enum, but in principle
host-offline-reason is not necessarily tied to an exit code (i.e. in the future
the host should send "INVALID_POLICY" host-offline-reason but rather than
exiting should hang around monitoring policy for changes).  The
host-offline-reason sent by the host is stored in the Chromoting directory.
Since cr/79856832the directory sends host-offline-reason to the client as
hostOfflineReason field inside a Host object.

BUG=410050, 247245
TEST=Manually tried the UI in the 3 cases: hor missing, known hor, unknown hor.

Committed: https://crrev.com/f0a7d8ada0ddf100cd2983f0ad1d9d853115d227
Cr-Commit-Position: refs/heads/master@{#315396}

[Łukasz Anforowicz, 2015-02-07 00:02:55]

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

[Łukasz Anforowicz, 2015-02-07 00:02:55]

Jamie, could you please take a look at a basic webapp UI for displaying
host-offline-reason?

To try to break my current paralysis, I am submitting what I currently have - a
basic tooltip as you suggested initially.  Once this has landed in good shape,
then I'll look at improving the UI to be more discoverable and work better with
touchscreens (a hideable div maybe).

Thanks,

Lukasz

[Jamie, 2015-02-07 00:46:46]

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
File remoting/resources/remoting_strings.grd (right):

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
remoting/resources/remoting_strings.grd:860: </message>
I don't think that this is the right message to display. I think it would be
fine to keep the same text for both cases, but perhaps add the warning icon that
we use for out-of-date hosts to highlight the ones that have gone off-line due
to an error state.

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
remoting/resources/remoting_strings.grd:866: <message desc="Description of why
host is offline"
These descriptions are shown to localizers, so they need to be a bit more
descriptive than this. I don't think it's necessary for them all to be different
(the localizers see the strings as well, of course) but a bit more context than
this is a good idea.

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
File remoting/webapp/crd/js/host_table_entry.js (right):

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_table_entry.js:369: this.host.hostOfflineReason +
'_GENERATED_AT_RUNTIME',
Rather than constructing this tag programmatically, I think it would be safer
(and easier to find where a string is used) if you implemented something like:

function offlineReasonStringToTag(reason) {
  var knownReasons = [
      /**i18n-content*/ 'INVALID_POLICY',
      /**i18n-content*/ 'DOG_CHEWED_POWER_CABLE',
      ...
  ];
  if (reason in knownReasons) {
    return reason;
  } else {
    console.error('Unknown off-line reason: ' + reason);
    return /**i18n-content*/ 'UNKNOWN_OFFLINE_REASON';
  }
}

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_table_entry.js:374: [this.host.hostOfflineReason,
formattedTime]);
The time is already included in the status string. I don't think you need to
duplicate it in the tool-tip.

[Łukasz Anforowicz, 2015-02-09 19:21:58]

Jamie,

Thanks for the feedback.  I've tried to address it in 
patchset #2.  Could you please take another look?

-Lukasz

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
File remoting/resources/remoting_strings.grd (right):

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
remoting/resources/remoting_strings.grd:860: </message>
On 2015/02/07 00:46:46, Jamie wrote:
> I don't think that this is the right message to display. I think it would be
> fine to keep the same text for both cases, but perhaps add the warning icon
that
> we use for out-of-date hosts to highlight the ones that have gone off-line due
> to an error state.

Ok - done.

https://codereview.chromium.org/882463003/diff/1/remoting/resources/remoting_...
remoting/resources/remoting_strings.grd:866: <message desc="Description of why
host is offline"
On 2015/02/07 00:46:46, Jamie wrote:
> These descriptions are shown to localizers, so they need to be a bit more
> descriptive than this. I don't think it's necessary for them all to be
different
> (the localizers see the strings as well, of course) but a bit more context
than
> this is a good idea.

Done.

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
File remoting/webapp/crd/js/host_table_entry.js (right):

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_table_entry.js:369: this.host.hostOfflineReason +
'_GENERATED_AT_RUNTIME',
On 2015/02/07 00:46:46, Jamie wrote:
> Rather than constructing this tag programmatically, I think it would be safer
> (and easier to find where a string is used) if you implemented something like:
> 
> function offlineReasonStringToTag(reason) {
>   var knownReasons = [
>       /**i18n-content*/ 'INVALID_POLICY',
>       /**i18n-content*/ 'DOG_CHEWED_POWER_CABLE',
>       ...
>   ];
>   if (reason in knownReasons) {
>     return reason;
>   } else {
>     console.error('Unknown off-line reason: ' + reason);
>     return /**i18n-content*/ 'UNKNOWN_OFFLINE_REASON';
>   }
> }

Good idea.  Done.

https://codereview.chromium.org/882463003/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_table_entry.js:374: [this.host.hostOfflineReason,
formattedTime]);
On 2015/02/07 00:46:46, Jamie wrote:
> The time is already included in the status string. I don't think you need to
> duplicate it in the tool-tip.

Ok - removed.

[Jamie, 2015-02-09 19:57:15]

lgtm

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_table_entry.js (right):

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:338: * Formats host's
host-offline-reason value (i.e. "INVALID_HOST_CONFIGURATION_")
Nit: trailing underscore.

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:345: /*i18n-content*/
"OFFLINE_REASON_INITIALIZATION_FAILED",
Single-quotes for JS strings, please.

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:354: if
(knownReasonTags.indexOf(offlineReasonTag) != (-1)) {
No need for parentheses around -1.

[Łukasz Anforowicz, 2015-02-09 20:17:49]

New patchsets have been uploaded after l-g-t-m from jamiewalch@chromium.org

[Łukasz Anforowicz, 2015-02-09 20:19:13]

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_table_entry.js (right):

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:338: * Formats host's
host-offline-reason value (i.e. "INVALID_HOST_CONFIGURATION_")
On 2015/02/09 19:57:15, Jamie wrote:
> Nit: trailing underscore.

Ooops.  Fixed.  Thanks.

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:345: /*i18n-content*/
"OFFLINE_REASON_INITIALIZATION_FAILED",
On 2015/02/09 19:57:15, Jamie wrote:
> Single-quotes for JS strings, please.

Done.

https://codereview.chromium.org/882463003/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_table_entry.js:354: if
(knownReasonTags.indexOf(offlineReasonTag) != (-1)) {
On 2015/02/09 19:57:15, Jamie wrote:
> No need for parentheses around -1.

Acknowledged.

[Łukasz Anforowicz, 2015-02-09 20:19:22]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2015-02-09 20:20:40]

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

[commit-bot: I haz the power, 2015-02-09 21:16:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-09 21:17:33]

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