Handle authentication failures in the v2 app by restarting the app

remoting/webapp/base/js/remote_methods.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
+*
+* In Chrome Apps, some platform APIs can only be called from the background
+* page.  For example, you can only reload an chrome.app.AppWindow from a
+* background page.  Likewise, some chrome API's must be initiated by user
+* interaction, which can only be called from the foreground.
+*
+* This class provides helper functions to invoke methods on different pages
+* using chrome.runtime.sendMessage.  Messages are passed in the following
+* format:
+*     {requestType:{string}, params:{Array}}
+*
+* chrome.runtime.sendMessage allows multiple handlers to be registered on a
+* document, but only one handler can send a response.
+* This class uniquely identifies a request with the requestType and enforces
+* that only one handler can be registered per requestType in the document.
+*
+* For example, to call method foo() in the background page from the foreground
+* chrome.app.AppWindow, you can do the following.
+* In the background page:
+*     var remoteMethods = new base.remoteMethods();
+*     remoteMethods.register('my.service.name', foo);
+*
+* In the AppWindow document:
+*     base.remoteMethods.invoke('my.service.name', [arg1, arg2]).then(

[Jamie, 2015/01/23 23:33:26]

Would it be a lot of work to allow args to be passed without wrapping them in an
array? It seems a bit more Javascripty.

+*       function(result) {
+*         console.log('The result is ' + result);
+*     });
+*
+* This will invoke foo() with the args [arg1, arg2].
+* The return value of foo() will be passed back to the caller in the
+* form of a promise.
+*/
+
+/** @suppress {duplicate} */
+var base = base || {};
+
+(function() {
+
+'use strict';
+
+/**
+ * @constructor
+ * @implements {base.Disposable}
+ */
+base.RemoteMethods = function() {

[Jamie, 2015/01/23 23:33:26]

I suggest base.Ipc as a name instead.

[Jamie, 2015/01/23 23:33:26]

With a suitable chrome.runtime.sendMessage mock, I think this class could be
unit-tested very easily. If you do that, you might want to consider pulling this
out into a separate CL.

+  /**
+   * @type {!Object.<Function>}
+   * @private
+   */
+  this.handlers_ = {};
+  this.onMessageHandler_ = this.onMessage_.bind(this);
+  chrome.runtime.onMessage.addListener(this.onMessageHandler_);
+};
+
+
+/**
+ * @constructor
+ * @param {string} requestType
+ * @param {?Array} params
+ * @struct
+ */
+base.RemoteMethods.Request_ = function(requestType, params) {

[Jamie, 2015/01/23 23:33:26]

If this is a private type, does it need to be exposed on base.RemoteMethods?

+  this.requestType = requestType;
+  this.params = params;
+};
+
+base.RemoteMethods.prototype.dispose = function() {
+  chrome.runtime.onMessage.removeListener(this.onMessageHandler_);
+};
+
+/**
+ * @param {string} requestType

[Jamie, 2015/01/23 23:33:26]

Since we're talking about methods, I think methodName would be a more suitable
parameter name. A brief description for the first two parameters would also be
useful, particularly explaining how the method parameters are passed to the
handler.

+ * @param {Function} handler

[Jamie, 2015/01/23 23:33:26]

s/Function/function(*):void/

+ * @return {boolean} Whether the handler is successfully registered.
+ */
+base.RemoteMethods.prototype.register = function(requestType, handler) {
+  if (requestType in this.handlers_) {
+    console.error('service :' + requestType + ' is already registered.');
+    return false;
+  }
+  this.handlers_[requestType] = handler;
+  return true;
+};
+
+/**
+ * @param {string} requestType
+ */
+base.RemoteMethods.prototype.unregister = function(requestType) {
+  delete this.handlers_[requestType];
+};
+
+/**
+ * @param {base.RemoteMethods.Request_} message
+ * @param {chrome.runtime.MessageSender} sender
+ * @param {function(*): void} sendResponse
+ */
+base.RemoteMethods.prototype.onMessage_ = function(message, sender,
+                                                   sendResponse) {
+  if (sender.id !== chrome.runtime.id) {
+    // We only handle incoming requests from our extension.
+    return;
+  }
+
+  var requestType = message.requestType;
+  if (typeof requestType !== 'string') {
+    return;
+  }
+
+  var remoteMethod =
+      /** @type {function(*):void} */ (this.handlers_[requestType]);
+  if (!remoteMethod) {
+    sendResponse({error : 'Unsupported request:= ' + requestType});

[Jamie, 2015/01/23 23:33:26]

Nit: s/:=/:/

+    return;
+  }
+
+  try {
+    sendResponse(remoteMethod.apply(null, message.params));
+  } catch (/** @type {Error} */ e) {
+    sendResponse({error: e.message});
+  }
+};
+
+/**
+ * Invokes a method on a remote page

[Jamie, 2015/01/23 23:33:26]

Blank (comment) line after method description.

+ * @param {string} requestType
+ * @param {Array} params

[Jamie, 2015/01/23 23:33:26]

@return?

+ */
+base.RemoteMethods.invoke = function(requestType, params) {
+  var sendMessage = base.Promise.as(
+      chrome.runtime.sendMessage,
+      [null, new base.RemoteMethods.Request_(requestType, params)]);

[Jamie, 2015/01/23 23:33:26]

I don't think you need the leading null, since the docs say the extensionId is
optional.

+
+  return sendMessage.then(
+    /** @param {{error: Error}} response */
+    function(response) {
+      if (response.error) {
+        Promise.reject(response.error);
+      } else {
+        Promise.resolve(response);

[Jamie, 2015/01/23 23:33:26]

Don't these reject/resolves need to be returned?

+      }
+  });
+};
+
+})();

[Jamie, 2015/01/23 23:33:26]

Although we have too many globals in our code, I think this is a good example of
a class that should be a singleton, since the duplicate-checking in register
needs to be done globally. I suggest you create an instance here.