Improve HRD first run experience

remoting/webapp/crd/js/it2me_helpee_channel.js

Index: remoting/webapp/crd/js/it2me_helpee_channel.js
diff --git a/remoting/webapp/crd/js/it2me_helpee_channel.js b/remoting/webapp/crd/js/it2me_helpee_channel.js
index d00e1a75aac977543f1aac468236f5446f0133dd..e540f22fa8ebb50f877a9027b579507e13294813 100644
--- a/remoting/webapp/crd/js/it2me_helpee_channel.js
+++ b/remoting/webapp/crd/js/it2me_helpee_channel.js
@@ -250,6 +250,7 @@ remoting.It2MeHelpeeChannel.prototype.onHangoutDisconnect_ = function() {
 remoting.It2MeHelpeeChannel.prototype.handleConnect_ =
     function(message) {
   var email = getStringAttr(message, 'email');
+  var bounds = /** @type {Bounds} */ (message['parentBounds']);

[Jamie, 2015/02/02 19:07:40]

getObjectAttr would be better (it does some minimal type-checking).

[kelvinp, 2015/02/03 01:15:33]

Done.

 
   if (!email) {
     throw new Error('Missing required parameter: email');
@@ -259,13 +260,20 @@ remoting.It2MeHelpeeChannel.prototype.handleConnect_ =
     throw new Error('An existing connection is in progress.');
   }
 
-  this.showConfirmDialog_().then(
+  var that = this;
+  this.showConfirmDialog_(bounds).then(
     this.initializeHost_.bind(this)
   ).then(
     this.fetchOAuthToken_.bind(this)
   ).then(
-    /** @type {function(*):void} */(this.connectToHost_.bind(this, email)),
-    /** @type {function(*):void} */(this.sendErrorResponse_.bind(this, message))
+    /** @type {function(*):void} */(this.connectToHost_.bind(this, email))

[Jamie, 2015/02/02 19:07:40]

Why does this call need a cast, but the others don't? Maybe this one can be
removed now that we're using a more modern compiler?

[kelvinp, 2015/02/03 01:15:33]

Gary added this check when we switched to the new JS Compiler.
I think the new JSCompiler has a promise definition that is too strict
(function(string):void doesn't match function(*))

[Jamie, 2015/02/03 18:22:44]

I'm still not sure why this one needs annotating, but the others done. But, if
this is what's needed to make it compile, then so be it; it certainly isn't the
ugliest piece of JSDoc in the code-base :)

[kelvinp, 2015/02/03 19:31:45]

Because initializeHost_ and fetchOAuthToken_ doesn't take any parameters and
connectToHost_ does, which doesn't match the then function signature.

+  ).catch(
+    /** @param {*} reason */
+    function(reason) {
+      var error = /** @type {Error} */ (reason);
+      that.sendErrorResponse_(message, error);
+      that.dispose();
+    }
   );
 };
 
@@ -274,13 +282,14 @@ remoting.It2MeHelpeeChannel.prototype.handleConnect_ =
  * ensures that even if Hangouts is compromised, an attacker cannot start the
  * host without explicit user confirmation.
  *
+ * @param {Bounds} bounds Bounds of the hangout window
  * @return {Promise} A promise that resolves to a boolean value, indicating
  *     whether the user accepts the remote assistance or not.
  * @private
  */
-remoting.It2MeHelpeeChannel.prototype.showConfirmDialog_ = function() {
+remoting.It2MeHelpeeChannel.prototype.showConfirmDialog_ = function(bounds) {
   if (base.isAppsV2()) {
-    return this.showConfirmDialogV2_();
+    return this.showConfirmDialogV2_(bounds);
   } else {
     return this.showConfirmDialogV1_();
   }
@@ -305,70 +314,27 @@ remoting.It2MeHelpeeChannel.prototype.showConfirmDialogV1_ = function() {
   if(window.confirm(message)) {
     return Promise.resolve();
   } else {
-    return Promise.reject(new Error(remoting.Error.CANCELLED));
+    return Promise.reject(new Error(remoting.Error.NOT_AUTHORIZED));

[Jamie, 2015/02/02 19:07:40]

I think CANCELLED is a better error here. Generally, if the user explicitly
declines something (CANCELLED), then no error should be displayed. If some
operation failed because the user doesn't have permission to do it
(NOT_AUTHORIZED) then we should show an error. Even if this code-path doesn't
touch any common error handling code where such logic might be implemented, it's
better to be consistent.

[kelvinp, 2015/02/03 01:15:33]

Done.

   }
 };
 
 /**
- * @return {Promise} A promise that resolves to a boolean value, indicating
+ * @param {Bounds} bounds the bounds of the Hangouts Window.  If set, the
+ *   confirm dialog will be centered upon bounds.
+ * @return {Promise} A promise that will resolve if the user accepts remote
+ *   assistance or reject otherwise.
+ * @private
  */
-function buildConfirmationMessage() {
+remoting.It2MeHelpeeChannel.prototype.showConfirmDialogV2_ = function(bounds) {
   var getToken =
       base.Promise.as(chrome.identity.getAuthToken, [{interactive: false}]);
+
   return getToken.then(
     /** @param {string} token */
-    function(token){
-      var messageHeader = l10n.getTranslationOrError(
-      /*i18n-content*/ 'HANGOUTS_CONFIRM_DIALOG_MESSAGE_1');
-      var message1 = l10n.getTranslationOrError(
-          /*i18n-content*/ 'HANGOUTS_CONFIRM_DIALOG_MESSAGE_2');
-      var message2 = l10n.getTranslationOrError(
-          /*i18n-content*/ 'HANGOUTS_CONFIRM_DIALOG_MESSAGE_3');
-      var message =
-          '<div>' + base.escapeHTML(messageHeader) + '</div>' +
-          '<ul class="insetList">' +
-          '<li>' + base.escapeHTML(message1) + '</li>' +
-          '<li>' + base.escapeHTML(message2) + '</li>';
-      if (!token) {
-        message += '<li>To continue you must first grant extended access permissions to your computer. You only have to do this once.</li>';
-      }
-      return message + '</ul>';
-  });
-}
-
-/**
- * @return {Promise} A promise that resolves to a boolean value, indicating
- *     whether the user accepts the remote assistance or not.
- * @private
- */
-remoting.It2MeHelpeeChannel.prototype.showConfirmDialogV2_ = function() {
-  return buildConfirmationMessage().then(
-    /** @param {string} message */
-    function(message) {
-      /**
-       * @param {function(*=):void} resolve
-       * @param {function(*=):void} reject
-       */
-      return new Promise(function(resolve, reject) {
-        /** @param {number} result */
-        function confirmDialogCallback(result) {
-          if (result === 1) {
-            resolve(true);
-          } else {
-            reject(new Error(remoting.Error.CANCELLED));
-          }
-        }
-        remoting.MessageWindow.showConfirmWindow(
-            '', // Empty string to use the package name as the dialog title.
-            message,
-            l10n.getTranslationOrError(
-                /*i18n-content*/'HANGOUTS_CONFIRM_DIALOG_ACCEPT'),
-            l10n.getTranslationOrError(
-                /*i18n-content*/'HANGOUTS_CONFIRM_DIALOG_DECLINE'),
-            confirmDialogCallback
-        );
+    function(token) {
+      return remoting.HangoutConsentDialog.getInstance().show(Boolean(token),
+                                                              bounds);
     });
-  });
 };
 
 /**
@@ -402,27 +368,25 @@ remoting.It2MeHelpeeChannel.prototype.fetchOAuthToken_ = function() {
      * @param {function(*=):void} resolve
      */
     return new Promise(function(resolve){
-      // TODO(jamiewalch): Make this work with {interactive: true} as well.
       chrome.identity.getAuthToken({'interactive': true}, resolve);
     });
   } else {
     /**
      * @param {function(*=):void} resolve
+     * @param {function(*=):void} reject
      */
-    return new Promise(function(resolve) {
+    return new Promise(function(resolve, reject) {
       /** @type {remoting.OAuth2} */
       var oauth2 = new remoting.OAuth2();
-      var onAuthenticated = function() {
-        oauth2.callWithToken(
-            resolve,
-            function() { throw new Error('Authentication failed.'); });
-      };
       /** @param {remoting.Error} error */
       var onError = function(error) {
-        if (error != remoting.Error.NOT_AUTHENTICATED) {
-          throw new Error('Unexpected error fetch auth token: ' + error);
+        if (error === remoting.Error.NOT_AUTHENTICATED) {
+          oauth2.doAuthRedirect(function() {
+            oauth2.callWithToken(resolve, reject);
+          });
+          return;
         }
-        oauth2.removeCachedAuthToken();
+        reject(new Error(remoting.Error.NOT_AUTHENTICATED));
       };
       oauth2.callWithToken(resolve, onError);
     });