Refactored HostList to better support bookmarking.

remoting/webapp/me2mom/host_list.js

Index: remoting/webapp/me2mom/host_list.js
diff --git a/remoting/webapp/me2mom/host_list.js b/remoting/webapp/me2mom/host_list.js
index 5a48521601fe6c8949f05beaebd7a0446bc66d35..8efe7e2b43787495192d723cd0ea445c9d7a75ec 100644
--- a/remoting/webapp/me2mom/host_list.js
+++ b/remoting/webapp/me2mom/host_list.js
@@ -16,6 +16,7 @@ var remoting = remoting || {};
  * Create a host list consisting of the specified HTML elements, which should
  * have a common parent that contains only host-list UI as it will be hidden
  * if the host-list is empty.
+ *
  * @constructor
  * @param {Element} table The HTML <table> to contain host-list.
  * @param {Element} errorDiv The HTML <div> to display error messages.
@@ -35,18 +36,39 @@ remoting.HostList = function(table, errorDiv) {
    * @type {Array.<remoting.HostTableEntry>}
    * @private
    */
-  this.hostTableEntries_ = null;
+  this.hostTableEntries_ = [];
+  /**
+   * @type {Array.<remoting.Host>}
+   * @private
+   */
+  this.hosts_ = [];
+  /**
+   * @type {string}
+   * @private
+   */
+  this.lastError_ = '';

[Wez, 2011/12/03 01:26:33]

nit: blank line here, please

[Jamie, 2011/12/05 20:49:08]

Done.

+  // Load the cache of the last host-list, if present.
+  var cached = /** @type {string} */
+  (window.localStorage.getItem(remoting.HostList.HOSTS_KEY));

[Wez, 2011/12/03 01:26:33]

nit: indent

[Jamie, 2011/12/05 20:49:08]

Done.

+  if (cached) {
+    try {
+      this.hosts_ = /** @type {Array} */ JSON.parse(cached);
+    } catch (err) {
+      console.error('Invalid host list cache:', /** @type {*} */(err));
+    }
+  }
 };
 
 /**
  * Search the host list for a host with the specified id.
+ *
  * @param {string} hostId The unique id of the host.
- * @return {remoting.HostTableEntry?} The host table entry, if any.
+ * @return {remoting.Host?} The host, if any.
  */
 remoting.HostList.prototype.getHostForId = function(hostId) {
-  for (var i = 0; i < this.hostTableEntries_.length; ++i) {
-    if (this.hostTableEntries_[i].host.hostId == hostId) {
-      return this.hostTableEntries_[i];
+  for (var i = 0; i < this.hosts_.length; ++i) {
+    if (this.hosts_[i].hostId == hostId) {
+      return this.hosts_[i];
     }
   }
   return null;
@@ -55,16 +77,18 @@ remoting.HostList.prototype.getHostForId = function(hostId) {
 /**
  * Query the Remoting Directory for the user's list of hosts.
  *
+ * @param {function(boolean):void} onDone Callback invoked with true on success
+ *     or false on failure.
  * @return {void} Nothing.
  */
-remoting.HostList.prototype.refresh = function() {
+remoting.HostList.prototype.refresh = function(onDone) {
   /** @type {remoting.HostList} */
   var that = this;
-  /** @param {XMLHttpRequest} xhr */
+  /** @param {XMLHttpRequest} xhr The response from the server. */
   var parseHostListResponse = function(xhr) {
-    that.parseHostListResponse_(xhr);
+    that.parseHostListResponse_(xhr, onDone);
   }
-  /** @param {string} token */
+  /** @param {string} token The OAuth2 token. */
   var getHosts = function(token) {
     var headers = { 'Authorization': 'OAuth ' + token };
     remoting.xhr.get(
@@ -72,7 +96,7 @@ remoting.HostList.prototype.refresh = function() {
         parseHostListResponse, '', headers);
   };
   remoting.oauth2.callWithToken(getHosts);
-}
+};
 
 /**
  * Handle the results of the host list request.  A success response will
@@ -80,48 +104,58 @@ remoting.HostList.prototype.refresh = function() {
  * able to successfully parse it.
  *
  * @param {XMLHttpRequest} xhr The XHR object for the host list request.
+ * @param {function(boolean):void} onDone The callback passed to |refresh|.
  * @return {void} Nothing.
+ * @private
  */
-remoting.HostList.prototype.parseHostListResponse_ = function(xhr) {
+remoting.HostList.prototype.parseHostListResponse_ = function(xhr, onDone) {
   try {
     if (xhr.status == 200) {
       var parsed_response =
           /** @type {{data: {items: Array}}} */ JSON.parse(xhr.responseText);
       if (parsed_response.data && parsed_response.data.items) {
-        this.setHosts_(parsed_response.data.items);
+        this.hosts_ = parsed_response.data.items;
+        this.lastError_ = '';
       }
     } else {
       // Some other error.
       console.error('Bad status on host list query: ', xhr);
-      // For most errors in the 4xx range, tell the user to re-authorize us.
       if (xhr.status == 403) {
         // The user's account is not enabled for Me2Me, so fail silently.
-      } else if (xhr.status >= 400 && xhr.status <= 499) {
-        this.showError_(remoting.Error.GENERIC);
+        this.hosts_ = [];
+        this.lastError_ = '';
+      } else {
+        // For other errors, tell the user to re-authorize us.
+        this.hosts_ = [];
+        this.lastError_ = remoting.Error.GENERIC;

[Wez, 2011/12/03 01:26:33]

I specifically avoided sending them to the reauthorize page for non-400 errors,
since it won't help them.

[Jamie, 2011/12/05 20:49:08]

We have to show some error message in this case. I remember a conversation with
Hugh in which he requested that we never report an error without suggesting a
course of action, even if it might not work. I'd be happy to have a generic
error message that just suggests "Try again later" in this case.

Actually, talking about this makes me realize that the user has no means of
signing out from the error page, so I've fixed up the HTML to allow that.

[Wez, 2011/12/05 20:59:38]

Suggesting courses of action that we know won't help is worse than suggesting
nothing, surely?  What we really mean is that we never expect such response
codes, so if we see them then it's a bug the user should report.

       }
     }
   } catch (er) {
     var typed_er = /** @type {Object} */ (er);
     console.error('Error processing response: ', xhr, typed_er);
+    this.hosts_ = [];
+    this.lastError_ = remoting.Error.GENERIC;

[Wez, 2011/12/03 01:26:33]

I specifically avoided barfing them to the reauthorize prompt in this case,
since it's a protocol error, and so re-auth presumably won't help?

[Jamie, 2011/12/05 20:49:08]

See above.

   }
-}
+  window.localStorage.setItem(remoting.HostList.HOSTS_KEY,
+                              JSON.stringify(this.hosts_));
+  onDone(this.lastError_ == '');
+};
 
 /**
- * Refresh the host list with up-to-date details.
- * @param {Array.<remoting.Host>} hosts The new host list.
+ * Display the list of hosts or error condition.
+ *
  * @return {void} Nothing.
- * @private
  */
-remoting.HostList.prototype.setHosts_ = function(hosts) {
+remoting.HostList.prototype.display = function() {
   this.table_.innerHTML = '';
-  this.showError_(null);
+  this.errorDiv_.innerText = '';
   this.hostTableEntries_ = [];
 
   /** @type {remoting.HostList} */
   var that = this;
-  for (var i = 0; i < hosts.length; ++i) {
+  for (var i = 0; i < this.hosts_.length; ++i) {
     /** @type {remoting.Host} */
-    var host = hosts[i];
+    var host = this.hosts_[i];
     // Validate the entry to make sure it has all the fields we expect.
     if (host.hostName && host.hostId && host.status && host.jabberId &&
         host.publicKey) {
@@ -134,28 +168,16 @@ remoting.HostList.prototype.setHosts_ = function(hosts) {
     }
   }
 
-  this.showOrHide_(this.hostTableEntries_.length != 0);
-};
-
-/**
- * Display a localized error message.
- * @param {remoting.Error?} errorTag The error to display, or NULL to clear any
- *     previous error.
- * @return {void} Nothing.
- */
-remoting.HostList.prototype.showError_ = function(errorTag) {
-  this.table_.innerHTML = '';
-  if (errorTag) {
-    l10n.localizeElementFromTag(this.errorDiv_,
-                                /** @type {string} */ (errorTag));
-    this.showOrHide_(true);
-  } else {
-    this.errorDiv_.innerText = '';
+  if (this.lastError_ != '') {
+    l10n.localizeElementFromTag(this.errorDiv_, this.lastError_);
   }
+
+  this.showOrHide_(this.hosts_.length != 0 || this.lastError_ != '');
 };
 
 /**
  * Show or hide the host-list UI.
+ *
  * @param {boolean} show True to show the UI, or false to hide it.
  * @return {void} Nothing.
  * @private
@@ -173,25 +195,35 @@ remoting.HostList.prototype.showOrHide_ = function(show) {
 
 /**
  * Remove a host from the list, and deregister it.
+ *
  * @param {string} hostId The id of the host to be removed.
  * @return {void} Nothing.
  * @private
  */
 remoting.HostList.prototype.deleteHost_ = function(hostId) {
-  /** @type {remoting.HostTableEntry} */
-  var hostTableEntry = this.getHostForId(hostId);
-  if (!hostTableEntry) {
+  var host = this.getHostForId(hostId);
+  if (!host) {
     console.error('No host registered for id ' + hostId);
     return;
   }
 
-  this.table_.removeChild(hostTableEntry.tableRow);
-  var index = this.hostTableEntries_.indexOf(hostTableEntry);
+  var index = this.hosts_.indexOf(host);
   if (index != -1) {  // Since we've just found it, index must be >= 0
+    this.hosts_.splice(index, 1);
+  }
+  var hostTableEntry = null;
+  for (index = 0; index < this.hostTableEntries_.length; ++index) {
+    if (this.hostTableEntries_[index].host.hostId == hostId) {
+      hostTableEntry = this.hostTableEntries_[index];
+      break;
+    }
+  }
+  if (hostTableEntry) {
+    this.table_.removeChild(hostTableEntry.tableRow);
     this.hostTableEntries_.splice(index, 1);
   }
 
-  /** @param {string} token */
+  /** @param {string} token The OAuth2 token. */
   var deleteHost = function(token) {
     var headers = { 'Authorization': 'OAuth ' + token };
     remoting.xhr.remove(
@@ -204,28 +236,29 @@ remoting.HostList.prototype.deleteHost_ = function(hostId) {
 };
 
 /**
- * Prepare a host for renaming by replacing its name with an edit box.
+ * Rename the specified host.
+ *
  * @param {string} hostId The id of the host to be renamed.
  * @return {void} Nothing.
  * @private
  */
 remoting.HostList.prototype.renameHost_ = function(hostId) {
-  /** @type {remoting.HostTableEntry} */
-  var hostTableEntry = this.getHostForId(hostId);
-  if (!hostTableEntry) {
+  /** @type {remoting.Host} */
+  var host = this.getHostForId(hostId);
+  if (!host) {
     console.error('No host registered for id ' + hostId);
     return;
   }
-  /** @param {string} token */
+  /** @param {string} token The OAuth2 token. */
   var renameHost = function(token) {
     var headers = {
       'Authorization': 'OAuth ' + token,
       'Content-type' : 'application/json; charset=UTF-8'
     };
     var newHostDetails = { data: {
-      hostId: hostTableEntry.host.hostId,
-      hostName: hostTableEntry.host.hostName,
-      publicKey: hostTableEntry.host.publicKey
+      hostId: host.hostId,
+      hostName: host.hostName,
+      publicKey: host.publicKey
     } };
     remoting.xhr.put(
         'https://www.googleapis.com/chromoting/v1/@me/hosts/' + hostId,
@@ -242,5 +275,10 @@ remoting.HostList.prototype.renameHost_ = function(hostId) {
  */
 remoting.HostList.COLLAPSED_ = 'collapsed';
 
+/**
+ * Key name under which Me2Me hosts are cached.
+ */
+remoting.HostList.HOSTS_KEY = 'me2me-cached-hosts';
+
 /** @type {remoting.HostList} */
 remoting.hostList = null;