Add application pre-authorization.

remoting/webapp/app_remoting/js/app_remoting.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
  * This class implements the functionality that is specific to application
  * remoting ("AppRemoting" or AR).
  */
 
@@ skipping 53 matching lines @@
 
   this.host = {
     /** @type {string} */
     applicationId: '',
 
     /** @type {string} */
     hostId: ''};
 };
 
 /**
- * Callback for when the userinfo (email and user name) is available from
- * the identity API.
- *
- * @param {string} email The user's email address.
- * @param {string} fullName The user's full name.
- * @return {void} Nothing.
+ * Initialize the application. This is called before an OAuth token is requested
+ * and should be used for tasks such as initializing the DOM, registering event
+ * handlers, etc.
  */
-remoting.onUserInfoAvailable = function(email, fullName) {

[Jamie, 2015/03/05 01:35:55]

This was needed when fullName and email had synchronous getCached* accessors.
It's not needed any more.

-};
-
-/**
- * Initialize the application and register all event handlers. After this
- * is called, the app is running and waiting for user events.
- *
- * @param {remoting.SessionConnector} connector
- * @return {void} Nothing.
- */
-remoting.AppRemoting.prototype.init = function(connector) {
-  remoting.initGlobalObjects();
-  remoting.initIdentity(remoting.onUserInfoAvailable);

[Jamie, 2015/03/05 01:35:55]

This initialization was shared between the App- and DesktopRemoting delegate
classes, so I moved it to the Application class.

-
+remoting.AppRemoting.prototype.init = function() {
   // TODO(jamiewalch): Remove ClientSession's dependency on remoting.fullscreen
   // so that this is no longer required.
   remoting.fullscreen = new remoting.FullscreenAppsV2();
 
   var restoreHostWindows = function() {
     if (remoting.clientSession) {
       remoting.clientSession.sendClientMessage('restoreAllWindows', '');
     }
   };
   chrome.app.window.current().onRestored.addListener(restoreHostWindows);
 
   remoting.windowShape.updateClientWindowShape();
 
   // Initialize the context menus.
   if (remoting.platformIsChromeOS()) {
     var adapter = new remoting.ContextMenuChrome();
   } else {
     var adapter = new remoting.ContextMenuDom(
         document.getElementById('context-menu'));
   }
   this.contextMenu_ = new remoting.ApplicationContextMenu(adapter);
   this.keyboardLayoutsMenu_ = new remoting.KeyboardLayoutsMenu(adapter);
   this.windowActivationMenu_ = new remoting.WindowActivationMenu(adapter);
 
+  remoting.LoadingWindow.show();
+};
+
+/**
+ * Start the application. Once start() is called, the delegate can assume that
+ * the user has consented to all permissions specified in the manifest.
+ *
+ * @param {remoting.SessionConnector} connector
+ * @param {string} token An OAuth access token. The delegate should not cache
+ *     this token, but can assume that it will remain valid during application
+ *     start-up.
+ */
+remoting.AppRemoting.prototype.start = function(connector, token) {

[kelvinp, 2015/03/05 02:12:14]

Since the delegate should not cache this token, nor is it currently used in
AppRemoting.start() or DesktopRemoting.start().  Maybe we should remove this
token from the function signature to discourage the use of it.

[kelvinp, 2015/03/05 02:12:14]

onAuthenticated or onInitialized would be better name as this function is in
response to the completion of the start call.

[Jamie, 2015/03/05 02:31:27]

I want the name to reflect the fact that this is an entry point of sorts, and I
think start() does that quite well. It's also a reasonable counterpoint to
init().

[Jamie, 2015/03/05 02:31:27]

We could, but the semantics are no different from any other auth token, so I
think this is fine. The alternative is to call getToken and then throw away the
token and have start() request a new one if it needs it. There's no harm in that
(it will just grab one from the cache), but it seems a bit perverse :) I also
like the fact that it makes it clear what sort of things are expected in init
(stuff that can be done before authentication) and what should wait for start.

   /** @type {remoting.AppRemoting} */
   var that = this;
 
   /** @param {XMLHttpRequest} xhr */
   var parseAppHostResponse = function(xhr) {
     if (xhr.status == 200) {
       var response = /** @type {remoting.AppRemoting.AppHostResponse} */
           (base.jsonParseSafe(xhr.responseText));
       if (response &&
           response.status &&
@@ skipping 48 matching lines @@
       } else if (xhr.status == 403) {
         that.handleError(remoting.Error.APP_NOT_AUTHORIZED);
       } else if (xhr.status == 502 || xhr.status == 503) {
         that.handleError(remoting.Error.SERVICE_UNAVAILABLE);
       } else {
         that.handleError(remoting.Error.UNEXPECTED);
       }
     }
   };
 
-  /** @param {string} token */
-  var getAppHost = function(token) {
-    remoting.xhr.start({
-      method: 'POST',
-      url: that.runApplicationUrl(),
-      onDone: parseAppHostResponse,
-      oauthToken: token
-    });
-  };
+  remoting.xhr.start({
+    method: 'POST',
+    url: that.runApplicationUrl(),
+    onDone: parseAppHostResponse,
+    oauthToken: token
+  });
+};
 
-  /** @param {remoting.Error} error */
-  var onError = function(error) {
-    that.handleError(error);
-  };
-
-  remoting.LoadingWindow.show();
-
-  remoting.identity.getToken().then(getAppHost).
-      catch(remoting.Error.handler(onError));
-}
+/**
+ * Report an authentication error to the user. This is called in lieu of start()
+ * if the user cannot be authenticated or if they decline the app permissions.
+ *
+ * @param {remoting.Error} error The failure reason.
+ */
+remoting.AppRemoting.prototype.signInFailed = function(error) {
+  if (error == remoting.Error.CANCELLED) {
+    chrome.app.window.current().close();

[garykac, 2015/03/05 02:38:25]

We also call this in onDisconnected, but it also clears the ping timer while
this code makes sure the loading window is closed.

I think that we should add Application.Delegate.closeApp() and have it do all of
these cleanup/shutdown tasks in one place.

Then signInFailed could be moved into Application since the code would be
identical between DesktopRemoting and AppRemoting.

[Jamie, 2015/03/05 19:01:36]

That's a good idea. I think I'll make it a follow-up CL because we'd also want
to call that when the user closes the app explicitly (and maybe in some other
cases) and I don't want this CL to grow too large.

[garykac, 2015/03/06 21:16:25]

sgtm

+    remoting.LoadingWindow.close();
+  } else {
+    this.handleError(error);
+  }
+};
 
 /**
  * @return {string} Application product name to be used in UI.
  */
 remoting.AppRemoting.prototype.getApplicationName = function() {
   var manifest = chrome.runtime.getManifest();
   return manifest.name;
 };
 
 /** @return {string} */
@@ skipping 132 matching lines @@
  * @param {remoting.Error} errorTag The error to be localized and displayed.
  * @return {void} Nothing.
  */
 remoting.AppRemoting.prototype.handleError = function(errorTag) {
   console.error('Connection failed: ' + errorTag);
   remoting.LoadingWindow.close();
   remoting.MessageWindow.showErrorMessage(
       chrome.i18n.getMessage(/*i18n-content*/'CONNECTION_FAILED'),
       chrome.i18n.getMessage(/** @type {string} */ (errorTag)));
 };