Improve HRD first run experience

remoting/webapp/crd/js/it2me_helpee_channel.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  *
  * It2MeHelpeeChannel relays messages between the Hangouts web page (Hangouts)
  * and the It2Me Native Messaging Host (It2MeHost) for the helpee (the Hangouts
  * participant who is receiving remoting assistance).
@@ skipping 232 matching lines @@
 
 /**
  * Connects to the It2Me Native messaging Host and retrieves the access code.
  *
  * @param {{method:string, data:Object.<string,*>}} message
  * @private
  */
 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');
   }
 
   if (this.hostState_ !== remoting.HostSession.State.UNKNOWN) {
     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();
+    }
   );
 };
 
 /**
  * Prompts the user before starting the It2Me Native Messaging Host.  This
  * 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_();
   }
 };
 
 /**
  * @return {Promise} A promise that resolves to a boolean value, indicating
  *     whether the user accepts the remote assistance or not.
  * @private
  */
 remoting.It2MeHelpeeChannel.prototype.showConfirmDialogV1_ = function() {
   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 = base.escapeHTML(messageHeader) + '\n' +
                 '- ' + base.escapeHTML(message1) + '\n' +
                 '- ' + base.escapeHTML(message2) + '\n';
 
   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);
     });
-  });
 };
 
 /**
  * @return {Promise} A promise that resolves when the host is initialized.
  * @private
  */
 remoting.It2MeHelpeeChannel.prototype.initializeHost_ = function() {
   /** @type {remoting.It2MeHostFacade} */
   var host = this.host_;
 
@@ skipping 13 matching lines @@
 
 /**
  * @return {Promise} Promise that resolves with the OAuth token as the value.
  */
 remoting.It2MeHelpeeChannel.prototype.fetchOAuthToken_ = function() {
   if (base.isAppsV2()) {
     /**
      * @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);
     });
   }
 };
 
 /**
  * Connects to the It2Me Native Messaging Host and retrieves the access code
  * in the |onHostStateChanged_| callback.
  *
@@ skipping 72 matching lines @@
 
   console.error('Error responding to message method:' +
                 (incomingMessage ? incomingMessage.method : 'null') +
                 ' error:' + error);
   this.hangoutPort_.postMessage({
     method: remoting.It2MeHelpeeChannel.HangoutMessageTypes.ERROR,
     message: error,
     request: incomingMessage
   });
 };