Change HostController methods to accept onError callbacks.

The HostController methods which took a |callback| parameter now
accept an additional |onError| callback. Previously, an AsyncResult was
passed to |callback| to indicate success/failure. Instead, |callback|
takes no parameters, and |onError| is called on failure, passing in a
remoting.Error code. New error-codes have been added to correspond to
AsyncResult.CANCELED and AsyncResult.FAILED_DIRECTORY.

HostSetupDialog has been modified to use the new interface, and no
longer uses HostController.AsyncResult. In particular,
HostSetupFlow.switchToNextStep(AsyncResult result) has been split into
two methods for the success and failure cases.

BUG=249970
TEST=Verify host setup flow under normal and error conditions.
R=jamiewalch@chromium.org, sergeyu@chromium.org

Review URL: https://codereview.chromium.org/17764004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209175 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 202 insertions, 136 deletions

diff --git a/remoting/resources/remoting_strings.grd b/remoting/resources/remoting_strings.grd
index 0cfd5cd..2d0c3c6 100644
--- a/remoting/resources/remoting_strings.grd
+++ b/remoting/resources/remoting_strings.grd
@@ -393,7 +393,7 @@
       <message desc="Message shown when user has attempted to continue past the manual install dialog, but the Host components are not yet installed." name="IDR_HOST_SETUP_INSTALL_PENDING">
         Please run the installer before continuing.
       </message>
-      <message desc="Message shown when host registration fails when enabling the host on local computer." name="IDR_HOST_SETUP_REGISTRATION_FAILED">
+      <message desc="Message shown when host registration fails when enabling the host on local computer." name="IDR_ERROR_HOST_REGISTRATION_FAILED">
         Failed to register this computer.
       </message>
       <message desc="Message shown after access to local computer has been enabled successfully." name="IDR_HOST_SETUP_STARTED">
diff --git a/remoting/webapp/error.js b/remoting/webapp/error.js
index a1e3ab3..756e3e6 100644
--- a/remoting/webapp/error.js
+++ b/remoting/webapp/error.js
@@ -11,6 +11,11 @@ var remoting = remoting || {};
  * @enum {string} All error messages from messages.json
  */
 remoting.Error = {
+  // Used to signify that an operation was cancelled by the user. This should
+  // not normally cause the error text to be shown to the user, so the
+  // i18n-content prefix is not needed in this case.
+  CANCELLED: '__CANCELLED__',
+
   INVALID_ACCESS_CODE: /*i18n-content*/'ERROR_INVALID_ACCESS_CODE',
   MISSING_PLUGIN: /*i18n-content*/'ERROR_MISSING_PLUGIN',
   AUTHENTICATION_FAILED: /*i18n-content*/'ERROR_AUTHENTICATION_FAILED',
@@ -23,5 +28,6 @@ remoting.Error = {
   SERVICE_UNAVAILABLE: /*i18n-content*/'ERROR_SERVICE_UNAVAILABLE',
   NOT_AUTHENTICATED: /*i18n-content*/'ERROR_NOT_AUTHENTICATED',
   INVALID_HOST_DOMAIN: /*i18n-content*/'ERROR_INVALID_HOST_DOMAIN',
-  P2P_FAILURE: /*i18n-content*/'ERROR_P2P_FAILURE'
+  P2P_FAILURE: /*i18n-content*/'ERROR_P2P_FAILURE',
+  REGISTRATION_FAILED: /*i18n-content*/'ERROR_HOST_REGISTRATION_FAILED'
 };
diff --git a/remoting/webapp/host_controller.js b/remoting/webapp/host_controller.js
index 1529e3c..de1a0a9 100644
--- a/remoting/webapp/host_controller.js
+++ b/remoting/webapp/host_controller.js
@@ -58,12 +58,13 @@ remoting.HostController.AsyncResult = {
 };
 
 /**
- * @param {function(boolean, boolean, boolean):void} callback Callback to be
+ * @param {function(boolean, boolean, boolean):void} onDone Callback to be
  *     called when done.
+ * @param {function(remoting.Error):void} onError Callback to be called on
+ *     error.
  */
-remoting.HostController.prototype.getConsent = function(callback) {
-  this.hostDispatcher_.getUsageStatsConsent(callback,
-                                            remoting.showErrorMessage);
+remoting.HostController.prototype.getConsent = function(onDone, onError) {
+  this.hostDispatcher_.getUsageStatsConsent(onDone, onError);
 };
 
 /**
@@ -71,11 +72,13 @@ remoting.HostController.prototype.getConsent = function(callback) {
  *
  * @param {string} hostPin Host PIN.
  * @param {boolean} consent The user's consent to crash dump reporting.
- * @param {function(remoting.HostController.AsyncResult):void} callback
- * callback Callback to be called when done.
+ * @param {function():void} onDone Callback to be called when done.
+ * @param {function(remoting.Error):void} onError Callback to be called on
+ *     error.
  * @return {void} Nothing.
  */
-remoting.HostController.prototype.start = function(hostPin, consent, callback) {
+remoting.HostController.prototype.start = function(hostPin, consent, onDone,
+                                                   onError) {
   /** @type {remoting.HostController} */
   var that = this;
 
@@ -95,39 +98,28 @@ remoting.HostController.prototype.start = function(hostPin, consent, callback) {
 
   var newHostId = generateUuid();
 
-  /** @param {function(remoting.HostController.AsyncResult):void} callback
-   *  @param {remoting.HostController.AsyncResult} result
-   *  @param {string} hostName
-   *  @param {string} publicKey */
-  function onStarted(callback, result, hostName, publicKey) {
-    if (result == remoting.HostController.AsyncResult.OK) {
-      remoting.hostList.onLocalHostStarted(hostName, newHostId, publicKey);
-    } else {
-      // Unregister the host if we failed to start it.
-      remoting.HostList.unregisterHostById(newHostId);
-    }
-    callback(result);
-  };
+  /** @param {remoting.Error} error */
+  function onStartError(error) {
+    // Unregister the host if we failed to start it.
+    remoting.HostList.unregisterHostById(newHostId);
+    onError(error);
+  }
 
   /**
    * @param {string} hostName
    * @param {string} publicKey
-   * @param {string} privateKey
-   * @param {XMLHttpRequest} xhr
+   * @param {remoting.HostController.AsyncResult} result
    */
-  function onRegistered(hostName, publicKey, privateKey, xhr) {
-    var success = (xhr.status == 200);
-
-    if (success) {
-      that.hostDispatcher_.getPinHash(newHostId, hostPin,
-          startHostWithHash.bind(null, hostName, publicKey, privateKey, xhr),
-          remoting.showErrorMessage);
+  function onStarted(hostName, publicKey, result) {
+    if (result == remoting.HostController.AsyncResult.OK) {
+      remoting.hostList.onLocalHostStarted(hostName, newHostId, publicKey);
+      onDone();
+    } else if (result == remoting.HostController.AsyncResult.CANCELLED) {
+      onStartError(remoting.Error.CANCELLED);
     } else {
-      console.log('Failed to register the host. Status: ' + xhr.status +
-                  ' response: ' + xhr.responseText);
-      callback(remoting.HostController.AsyncResult.FAILED_DIRECTORY);
+      onStartError(remoting.Error.UNEXPECTED);
     }
-  };
+  }
 
   /**
    * @param {string} hostName
@@ -146,16 +138,33 @@ remoting.HostController.prototype.start = function(hostPin, consent, callback) {
         host_secret_hash: hostSecretHash,
         private_key: privateKey
     };
-    /** @param {remoting.HostController.AsyncResult} result */
-    var onStartDaemon = function(result) {
-      onStarted(callback, result, hostName, publicKey);
-    };
-    that.hostDispatcher_.startDaemon(hostConfig, consent, onStartDaemon,
-                                     remoting.showErrorMessage);
+    that.hostDispatcher_.startDaemon(hostConfig, consent,
+                                     onStarted.bind(null, hostName, publicKey),
+                                     onStartError);
   }
 
   /**
    * @param {string} hostName
+   * @param {string} publicKey
+   * @param {string} privateKey
+   * @param {XMLHttpRequest} xhr
+   */
+  function onRegistered(hostName, publicKey, privateKey, xhr) {
+    var success = (xhr.status == 200);
+
+    if (success) {
+      that.hostDispatcher_.getPinHash(newHostId, hostPin,
+          startHostWithHash.bind(null, hostName, publicKey, privateKey, xhr),
+          onError);
+    } else {
+      console.log('Failed to register the host. Status: ' + xhr.status +
+                  ' response: ' + xhr.responseText);
+      onError(remoting.Error.REGISTRATION_FAILED);
+    }
+  };
+
+  /**
+   * @param {string} hostName
    * @param {string} privateKey
    * @param {string} publicKey
    * @param {string} oauthToken
@@ -173,8 +182,7 @@ remoting.HostController.prototype.start = function(hostPin, consent, callback) {
     } };
     remoting.xhr.post(
         remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts/',
-        /** @param {XMLHttpRequest} xhr */
-        function (xhr) { onRegistered(hostName, publicKey, privateKey, xhr); },
+        onRegistered.bind(null, hostName, publicKey, privateKey),
         JSON.stringify(newHostDetails),
         headers);
   };
@@ -186,15 +194,8 @@ remoting.HostController.prototype.start = function(hostPin, consent, callback) {
    */
   function onKeyGenerated(hostName, privateKey, publicKey) {
     remoting.identity.callWithToken(
-        /** @param {string} oauthToken */
-        function(oauthToken) {
-          doRegisterHost(hostName, privateKey, publicKey, oauthToken);
-        },
-        /** @param {remoting.Error} error */
-        function(error) {
-          // TODO(jamiewalch): Have a more specific error code here?
-          callback(remoting.HostController.AsyncResult.FAILED);
-        });
+        doRegisterHost.bind(null, hostName, privateKey, publicKey),
+        onError);
   };
 
   /**
@@ -203,48 +204,43 @@ remoting.HostController.prototype.start = function(hostPin, consent, callback) {
    */
   function startWithHostname(hostName) {
     that.hostDispatcher_.generateKeyPair(onKeyGenerated.bind(null, hostName),
-                                         remoting.showErrorMessage);
+                                         onError);
   }
 
-  this.hostDispatcher_.getHostName(startWithHostname,
-                                   remoting.showErrorMessage);
+  this.hostDispatcher_.getHostName(startWithHostname, onError);
 };
 
 /**
  * Stop the daemon process.
- * @param {function(remoting.HostController.AsyncResult):void} callback
- *     Callback to be called when finished.
+ * @param {function():void} onDone Callback to be called when done.
+ * @param {function(remoting.Error):void} onError Callback to be called on
+ *     error.
  * @return {void} Nothing.
  */
-remoting.HostController.prototype.stop = function(callback) {
+remoting.HostController.prototype.stop = function(onDone, onError) {
   /** @type {remoting.HostController} */
   var that = this;
 
-  /**
-   * @param {remoting.HostController.AsyncResult} result The result of the
-   *     stopDaemon call, to be passed to the callback.
-   * @param {string?} hostId The host id of the local host.
-   */
-  function unregisterHost(result, hostId) {
+  /** @param {string?} hostId The host id of the local host. */
+  function unregisterHost(hostId) {
     if (hostId) {
       remoting.HostList.unregisterHostById(hostId);
     }
-    callback(result);
+    onDone();
   };
 
-  /**
-   * @param {remoting.HostController.AsyncResult} result The result of the
-   *     stopDaemon call, to be passed to the callback.
-   */
+  /** @param {remoting.HostController.AsyncResult} result */
   function onStopped(result) {
-    if (result != remoting.HostController.AsyncResult.OK) {
-      callback(result);
-      return;
+    if (result == remoting.HostController.AsyncResult.OK) {
+      that.getLocalHostId(unregisterHost);
+    } else if (result == remoting.HostController.AsyncResult.CANCELLED) {
+      onError(remoting.Error.CANCELLED);
+    } else {
+      onError(remoting.Error.UNEXPECTED);
     }
-    that.getLocalHostId(unregisterHost.bind(null, result));
-  };
+  }
 
-  this.hostDispatcher_.stopDaemon(onStopped, remoting.showErrorMessage);
+  this.hostDispatcher_.stopDaemon(onStopped, onError);
 };
 
 /**
@@ -260,24 +256,25 @@ function isHostConfigValid_(config) {
 
 /**
  * @param {string} newPin The new PIN to set
- * @param {function(remoting.HostController.AsyncResult):void} callback
- *     Callback to be called when finished.
+ * @param {function():void} onDone Callback to be called when done.
+ * @param {function(remoting.Error):void} onError Callback to be called on
+ *     error.
  * @return {void} Nothing.
  */
-remoting.HostController.prototype.updatePin = function(newPin, callback) {
+remoting.HostController.prototype.updatePin = function(newPin, onDone,
+                                                       onError) {
   /** @type {remoting.HostController} */
   var that = this;
 
-  /** @param {Object} config */
-  function onConfig(config) {
-    if (!isHostConfigValid_(config)) {
-      callback(remoting.HostController.AsyncResult.FAILED);
-      return;
+  /** @param {remoting.HostController.AsyncResult} result */
+  function onConfigUpdated(result) {
+    if (result == remoting.HostController.AsyncResult.OK) {
+      onDone();
+    } else if (result == remoting.HostController.AsyncResult.CANCELLED) {
+      onError(remoting.Error.CANCELLED);
+    } else {
+      onError(remoting.Error.UNEXPECTED);
     }
-    /** @type {string} */
-    var hostId = config['host_id'];
-    that.hostDispatcher_.getPinHash(hostId, newPin, updateDaemonConfigWithHash,
-                                    remoting.showErrorMessage);
   }
 
   /** @param {string} pinHash */
@@ -285,20 +282,32 @@ remoting.HostController.prototype.updatePin = function(newPin, callback) {
     var newConfig = {
       host_secret_hash: pinHash
     };
-    that.hostDispatcher_.updateDaemonConfig(newConfig, callback,
-                                            remoting.showErrorMessage);
+    that.hostDispatcher_.updateDaemonConfig(newConfig, onConfigUpdated,
+                                            onError);
+  }
+
+  /** @param {Object} config */
+  function onConfig(config) {
+    if (!isHostConfigValid_(config)) {
+      onError(remoting.Error.UNEXPECTED);
+      return;
+    }
+    /** @type {string} */
+    var hostId = config['host_id'];
+    that.hostDispatcher_.getPinHash(hostId, newPin, updateDaemonConfigWithHash,
+                                    onError);
   }
 
   // TODO(sergeyu): When crbug.com/121518 is fixed: replace this call
   // with an unprivileged version if that is necessary.
-  this.hostDispatcher_.getDaemonConfig(onConfig, remoting.showErrorMessage);
+  this.hostDispatcher_.getDaemonConfig(onConfig, onError);
 };
 
 /**
  * Get the state of the local host.
  *
- * @param {function(remoting.HostController.State):void} onDone
- *     Completion callback.
+ * @param {function(remoting.HostController.State):void} onDone Completion
+ *     callback.
  */
 remoting.HostController.prototype.getLocalHostState = function(onDone) {
   this.hostDispatcher_.getDaemonState(onDone, function() {
diff --git a/remoting/webapp/host_setup_dialog.js b/remoting/webapp/host_setup_dialog.js
index c3b76e9..eb52b7c1 100644
--- a/remoting/webapp/host_setup_dialog.js
+++ b/remoting/webapp/host_setup_dialog.js
@@ -52,31 +52,30 @@ remoting.HostSetupFlow.prototype.getState = function() {
   return this.state_;
 };
 
-/**
- * @param {remoting.HostController.AsyncResult} result Result of the
- * current step.
- * @return {remoting.HostSetupFlow.State} New state.
- */
-remoting.HostSetupFlow.prototype.switchToNextStep = function(result) {
+remoting.HostSetupFlow.prototype.switchToNextStep = function() {
   if (this.state_ == remoting.HostSetupFlow.State.NONE) {
-    return this.state_;
+    return;
   }
-  if (result == remoting.HostController.AsyncResult.OK) {
-    // If the current step was successful then switch to the next
-    // step in the sequence.
-    if (this.currentStep_ < this.sequence_.length - 1) {
-      this.currentStep_ += 1;
-      this.state_ = this.sequence_[this.currentStep_];
-    } else {
-      this.state_ = remoting.HostSetupFlow.State.NONE;
-    }
-  } else if (result == remoting.HostController.AsyncResult.CANCELLED) {
+
+  if (this.currentStep_ < this.sequence_.length - 1) {
+    this.currentStep_ += 1;
+    this.state_ = this.sequence_[this.currentStep_];
+  } else {
+    this.state_ = remoting.HostSetupFlow.State.NONE;
+  }
+};
+
+/**
+ * @param {remoting.Error} error
+ */
+remoting.HostSetupFlow.prototype.switchToErrorState = function(error) {
+  if (error == remoting.Error.CANCELLED) {
     // Stop the setup flow if user rejected one of the actions.
     this.state_ = remoting.HostSetupFlow.State.NONE;
   } else {
     // Current step failed, so switch to corresponding error state.
     if (this.state_ == remoting.HostSetupFlow.State.STARTING_HOST) {
-      if (result == remoting.HostController.AsyncResult.FAILED_DIRECTORY) {
+      if (error == remoting.Error.REGISTRATION_FAILED) {
         this.state_ = remoting.HostSetupFlow.State.REGISTRATION_FAILED;
       } else {
         this.state_ = remoting.HostSetupFlow.State.START_HOST_FAILED;
@@ -90,7 +89,6 @@ remoting.HostSetupFlow.prototype.switchToNextStep = function(result) {
       this.state_ = remoting.HostSetupFlow.State.START_HOST_FAILED;
     }
   }
-  return this.state_;
 };
 
 /**
@@ -204,14 +202,25 @@ remoting.HostSetupDialog.prototype.showForStartWithToken_ =
    * @param {boolean} set_by_policy True if crash dump reporting is controlled
    *     by policy.
    */
-  var onGetConsent = function(supported, allowed, set_by_policy) {
+  function onGetConsent(supported, allowed, set_by_policy) {
     that.usageStats_.hidden = !supported;
     that.usageStatsCheckbox_.checked = allowed;
     that.usageStatsCheckbox_.disabled = set_by_policy;
-  };
+  }
+
+  /** @param {remoting.Error} error */
+  function onError(error) {
+    console.error('Error getting consent status: ' + error);
+  }
+
   this.usageStats_.hidden = false;
   this.usageStatsCheckbox_.checked = false;
-  this.hostController_.getConsent(onGetConsent);
+
+  // Prevent user from ticking the box until the current consent status is
+  // known.
+  this.usageStatsCheckbox_.disabled = true;
+
+  this.hostController_.getConsent(onGetConsent, onError);
 
   var flow = [
       remoting.HostSetupFlow.State.ASK_PIN,
@@ -337,7 +346,7 @@ remoting.HostSetupDialog.prototype.updateState_ = function() {
   } else if (state == remoting.HostSetupFlow.State.HOST_STOPPED) {
     showDoneMessage(/*i18n-content*/'HOST_SETUP_STOPPED');
   } else if (state == remoting.HostSetupFlow.State.REGISTRATION_FAILED) {
-    showErrorMessage(/*i18n-content*/'HOST_SETUP_REGISTRATION_FAILED');
+    showErrorMessage(/*i18n-content*/'ERROR_HOST_REGISTRATION_FAILED');
   } else if (state == remoting.HostSetupFlow.State.START_HOST_FAILED) {
     showErrorMessage(/*i18n-content*/'HOST_SETUP_HOST_FAILED');
   } else if (state == remoting.HostSetupFlow.State.UPDATE_PIN_FAILED) {
@@ -356,18 +365,33 @@ remoting.HostSetupDialog.prototype.startHost_ = function() {
   /** @type {remoting.HostSetupFlow} */
   var flow = this.flow_;
 
-  /** @param {remoting.HostController.AsyncResult} result */
-  function onHostStarted(result) {
+  /** @return {boolean} */
+  function isFlowActive() {
     if (flow !== that.flow_ ||
         flow.getState() != remoting.HostSetupFlow.State.STARTING_HOST) {
       console.error('Host setup was interrupted when starting the host');
-      return;
+      return false;
     }
+    return true;
+  }
 
-    flow.switchToNextStep(result);
-    that.updateState_();
+  function onHostStarted() {
+    if (isFlowActive()) {
+      flow.switchToNextStep();
+      that.updateState_();
+    }
   }
-  this.hostController_.start(this.flow_.pin, this.flow_.consent, onHostStarted);
+
+  /** @param {remoting.Error} error */
+  function onError(error) {
+    if (isFlowActive()) {
+      flow.switchToErrorState(error);
+      that.updateState_();
+    }
+  }
+
+  this.hostController_.start(this.flow_.pin, this.flow_.consent, onHostStarted,
+                             onError);
 };
 
 remoting.HostSetupDialog.prototype.updatePin_ = function() {
@@ -376,20 +400,33 @@ remoting.HostSetupDialog.prototype.updatePin_ = function() {
   /** @type {remoting.HostSetupFlow} */
   var flow = this.flow_;
 
-  /** @param {remoting.HostController.AsyncResult} result */
-  function onPinUpdated(result) {
+  /** @return {boolean} */
+  function isFlowActive() {
     if (flow !== that.flow_ ||
         flow.getState() != remoting.HostSetupFlow.State.UPDATING_PIN) {
       console.error('Host setup was interrupted when updating PIN');
-      return;
+      return false;
     }
+    return true;
+  }
 
-    flow.switchToNextStep(result);
-    that.updateState_();
+  function onPinUpdated() {
+    if (isFlowActive()) {
+      flow.switchToNextStep();
+      that.updateState_();
+    }
   }
 
-  this.hostController_.updatePin(flow.pin, onPinUpdated);
-}
+  /** @param {remoting.Error} error */
+  function onError(error) {
+    if (isFlowActive()) {
+      flow.switchToErrorState(error);
+      that.updateState_();
+    }
+  }
+
+  this.hostController_.updatePin(flow.pin, onPinUpdated, onError);
+};
 
 /**
  * Stops the host.
@@ -400,18 +437,32 @@ remoting.HostSetupDialog.prototype.stopHost_ = function() {
   /** @type {remoting.HostSetupFlow} */
   var flow = this.flow_;
 
-  /** @param {remoting.HostController.AsyncResult} result */
-  function onHostStopped(result) {
+  /** @return {boolean} */
+  function isFlowActive() {
     if (flow !== that.flow_ ||
         flow.getState() != remoting.HostSetupFlow.State.STOPPING_HOST) {
       console.error('Host setup was interrupted when stopping the host');
-      return;
+      return false;
+    }
+    return true;
+  }
+
+  function onHostStopped() {
+    if (isFlowActive()) {
+      flow.switchToNextStep();
+      that.updateState_();
     }
+  }
 
-    flow.switchToNextStep(result);
-    that.updateState_();
+  /** @param {remoting.Error} error */
+  function onError(error) {
+    if (isFlowActive()) {
+      flow.switchToErrorState(error);
+      that.updateState_();
+    }
   }
-  this.hostController_.stop(onHostStopped);
+
+  this.hostController_.stop(onHostStopped, onError);
 };
 
 /**
@@ -452,7 +503,7 @@ remoting.HostSetupDialog.prototype.onPinSubmit_ = function() {
   this.flow_.pin = pin1;
   this.flow_.consent = !this.usageStats_.hidden &&
       this.usageStatsCheckbox_.checked;
-  this.flow_.switchToNextStep(remoting.HostController.AsyncResult.OK);
+  this.flow_.switchToNextStep();
   this.updateState_();
 };
 
@@ -501,7 +552,7 @@ remoting.HostSetupDialog.prototype.onInstallDialogOk = function() {
         state != remoting.HostController.State.NOT_INSTALLED &&
         state != remoting.HostController.State.INSTALLING;
     if (installed) {
-      that.flow_.switchToNextStep(remoting.HostController.AsyncResult.OK);
+      that.flow_.switchToNextStep();
       that.updateState_();
     } else {
       remoting.setMode(remoting.AppMode.HOST_SETUP_INSTALL_PENDING);