Bug 1299411 - Move native messaging to child process r=kmag
authorRob Wu <rob@robwu.nl>
Sat, 24 Sep 2016 13:25:56 +0200
changeset 320066 ed1afd2aad61c64bb938cf1647975b4fda846d66
parent 320065 24d81c7b335e8cb1949efb2b27a978cf108de705
child 320067 c456c1797299e5dfc39187992e30c323f7d92ee1
push id20749
push userryanvm@gmail.com
push dateSat, 29 Oct 2016 13:21:21 +0000
treeherderfx-team@1b170b39ed6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1299411, 1287229
milestone52.0a1
Bug 1299411 - Move native messaging to child process r=kmag Move `runtime.connectNative` and `runtime.sendNativeMessage` to `addon_child`. Note: This does not change the behavior for launching the native app, it is still launched from the main process. Now ExtensionUtils's Port is also used for native messaging ports. Now the behavior of `runtime.connect` and `runtime.connectNative` are identical from the extension's perspective. In particular: - `disconnect()` does not throw when called again (bug 1287229). - `onDisconnect` is called with error messages (tests will be added in the next commit). MozReview-Commit-ID: AyU9amiLeoL
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/NativeMessaging.jsm
toolkit/components/extensions/ext-c-runtime.js
toolkit/components/extensions/ext-runtime.js
toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -41,16 +41,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "Log",
                                   "resource://gre/modules/Log.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MatchGlobs",
                                   "resource://gre/modules/MatchPattern.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
                                   "resource://gre/modules/MatchPattern.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MessageChannel",
                                   "resource://gre/modules/MessageChannel.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "NativeApp",
+                                  "resource://gre/modules/NativeMessaging.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
                                   "resource://gre/modules/Preferences.jsm");
@@ -104,16 +106,17 @@ const COMMENT_REGEXP = new RegExp(String
         " (?:[^"\\\n] | \\.)* "
       )*?
     )
 
     //.*
   `.replace(/\s+/g, ""), "gm");
 
 var GlobalManager;
+var ParentAPIManager;
 
 // This object loads the ext-*.js scripts that define the extension API.
 var Management = new class extends SchemaAPIManager {
   constructor() {
     super("main");
     this.initialized = null;
   }
 
@@ -177,16 +180,31 @@ let ProxyMessenger = {
 
     MessageChannel.addListener(messageManagers, "Extension:Connect", this);
     MessageChannel.addListener(messageManagers, "Extension:Message", this);
     MessageChannel.addListener(messageManagers, "Extension:Port:Disconnect", this);
     MessageChannel.addListener(messageManagers, "Extension:Port:PostMessage", this);
   },
 
   receiveMessage({target, messageName, channelId, sender, recipient, data, responseType}) {
+    if (recipient.toNativeApp) {
+      let {childId, toNativeApp} = recipient;
+      if (messageName == "Extension:Message") {
+        let context = ParentAPIManager.getContextById(childId);
+        return new NativeApp(context, toNativeApp).sendMessage(data);
+      }
+      if (messageName == "Extension:Connect") {
+        let context = ParentAPIManager.getContextById(childId);
+        NativeApp.onConnectNative(context, target.messageManager, data.portId, sender, toNativeApp);
+        return true;
+      }
+      // "Extension:Port:Disconnect" and "Extension:Port:PostMessage" for
+      // native messages are handled by NativeApp.
+      return;
+    }
     let extension = GlobalManager.extensionMap.get(sender.extensionId);
     let receiverMM = this._getMessageManagerForRecipient(recipient);
     if (!extension || !receiverMM) {
       return Promise.reject({
         result: MessageChannel.RESULT_NO_HANDLER,
         message: "No matching message handler for the given recipient.",
       });
     }
@@ -222,18 +240,16 @@ let ProxyMessenger = {
     // runtime.sendMessage / runtime.connect
     if (extensionId) {
       // TODO(robwu): map the extensionId to the addon parent process's message
       // manager when they run in a separate process.
       let pipmm = Services.ppmm.getChildAt(0);
       return pipmm;
     }
 
-    // Note: No special handling for sendNativeMessage / connectNative because
-    // native messaging runs in the chrome process, so it never needs a proxy.
     return null;
   },
 };
 
 class BrowserDocshellFollower {
   /**
    * Follows the <browser> belonging to the `xulBrowser`'s current docshell.
    *
@@ -385,17 +401,17 @@ function findPathInObject(obj, path, pri
     }
 
     obj = obj[elt];
   }
 
   return obj;
 }
 
-var ParentAPIManager = {
+ParentAPIManager = {
   proxyContexts: new Map(),
 
   init() {
     Services.obs.addObserver(this, "message-manager-close", false);
 
     Services.mm.addMessageListener("API:CreateProxyContext", this);
     Services.mm.addMessageListener("API:CloseProxyContext", this, true);
     Services.mm.addMessageListener("API:Call", this);
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -106,19 +106,17 @@ class WannabeChildAPIManager extends Chi
     // Synchronously unload the ProxyContext because we synchronously create it.
     this.context.callOnClose({close: proxyContext.unload.bind(proxyContext)});
   }
 
   getFallbackImplementation(namespace, name) {
     // This is gross and should be removed ASAP.
     let shouldSynchronouslyUseParentAPI = false;
     // Incompatible APIs are listed here.
-    if (namespace == "runtime" && name == "connectNative" || // Returns a custom Port.
-        namespace == "runtime" && name == "sendNativeMessage" || // Fix together with connectNative.
-        namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
+    if (namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
         namespace == "webRequest") { // Incompatible by design (synchronous).
       shouldSynchronouslyUseParentAPI = true;
     }
     if (shouldSynchronouslyUseParentAPI) {
       let proxyContext = ParentAPIManager.proxyContexts.get(this.id);
       let apiObj = findPathInObject(proxyContext.apiObj, namespace, false);
       if (apiObj && name in apiObj) {
         return new LocalAPIImplementation(apiObj, name, this.context);
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -1510,29 +1510,34 @@ Messenger.prototype = {
 
       MessageChannel.addListener(this.messageManagers, "Extension:Message", listener);
       return () => {
         MessageChannel.removeListener(this.messageManagers, "Extension:Message", listener);
       };
     }).api();
   },
 
-  connect(messageManager, name, recipient) {
+  connectGetRawPort(messageManager, name, recipient) {
     let portId = `${gNextPortId++}-${Services.appinfo.uniqueProcessID}`;
     let port = new Port(this.context, messageManager, this.messageManagers, name, portId, null, recipient);
     let msg = {name, portId};
     this._sendMessage(messageManager, "Extension:Connect", msg, recipient)
       .catch(e => {
         if (e.result === MessageChannel.RESULT_NO_HANDLER) {
           e = {message: "Could not establish connection. Receiving end does not exist."};
         } else if (e.result === MessageChannel.RESULT_DISCONNECTED) {
           e = null;
         }
         port.disconnectByOtherEnd(e);
       });
+    return port;
+  },
+
+  connect(messageManager, name, recipient) {
+    let port = this.connectGetRawPort(messageManager, name, recipient);
     return port.api();
   },
 
   onConnect(name) {
     return new SingletonEventManager(this.context, name, callback => {
       let listener = {
         messageFilterPermissive: this.optionalFilter,
         messageFilterStrict: this.filter,
--- a/toolkit/components/extensions/NativeMessaging.jsm
+++ b/toolkit/components/extensions/NativeMessaging.jsm
@@ -212,16 +212,40 @@ this.NativeApp = class extends EventEmit
       }).catch(err => {
         this.startupPromise = null;
         Cu.reportError(err instanceof Error ? err : err.message);
         this._cleanup(err);
       });
   }
 
   /**
+   * Open a connection to a native messaging host.
+   *
+   * @param {BaseContext} context The context associated with the port.
+   * @param {nsIMessageSender} messageManager The message manager used to send
+   *     and receive messages from the port's creator.
+   * @param {string} portId A unique internal ID that identifies the port.
+   * @param {object} sender The object describing the creator of the connection
+   *     request.
+   * @param {string} application The name of the native messaging host.
+   */
+  static onConnectNative(context, messageManager, portId, sender, application) {
+    let app = new NativeApp(context, application);
+    let port = new ExtensionUtils.Port(context, messageManager, [messageManager], "", portId, sender, sender);
+    app.once("disconnect", (what, err) => port.disconnect(err && err.message));
+
+    /* eslint-disable mozilla/balanced-listeners */
+    app.on("message", (what, msg) => port.postMessage(msg));
+    /* eslint-enable mozilla/balanced-listeners */
+
+    port.registerOnMessage(msg => app.send(msg));
+    port.registerOnDisconnect(msg => app.close());
+  }
+
+  /**
    * @param {BaseContext} context The scope from where `message` originates.
    * @param {*} message A message from the extension, meant for a native app.
    * @returns {ArrayBuffer} An ArrayBuffer that can be sent to the native app.
    */
   static encodeMessage(context, message) {
     message = context.jsonStringify(message);
     let buffer = new TextEncoder().encode(message).buffer;
     if (buffer.byteLength > NativeApp.maxWrite) {
@@ -380,59 +404,16 @@ this.NativeApp = class extends EventEmit
     }
   }
 
   // Called from Context when the extension is shut down.
   close() {
     this._cleanup();
   }
 
-  portAPI() {
-    let port = {
-      name: this.name,
-
-      disconnect: () => {
-        if (this._isDisconnected) {
-          throw new this.context.cloneScope.Error("Attempt to disconnect an already disconnected port");
-        }
-        this._cleanup();
-      },
-
-      postMessage: msg => {
-        msg = NativeApp.encodeMessage(this.context, msg);
-        this.send(msg);
-      },
-
-      onDisconnect: new ExtensionUtils.SingletonEventManager(this.context, "native.onDisconnect", fire => {
-        let listener = what => {
-          this.context.runSafeWithoutClone(fire, port);
-        };
-        this.on("disconnect", listener);
-        return () => {
-          this.off("disconnect", listener);
-        };
-      }).api(),
-
-      onMessage: new ExtensionUtils.SingletonEventManager(this.context, "native.onMessage", fire => {
-        let listener = (what, msg) => {
-          msg = Cu.cloneInto(msg, this.context.cloneScope);
-          this.context.runSafeWithoutClone(fire, msg, port);
-        };
-        this.on("message", listener);
-        return () => {
-          this.off("message", listener);
-        };
-      }).api(),
-    };
-
-    port = Cu.cloneInto(port, this.context.cloneScope, {cloneFunctions: true});
-
-    return port;
-  }
-
   sendMessage(msg) {
     let responsePromise = new Promise((resolve, reject) => {
       this.once("message", (what, msg) => { resolve(msg); });
       this.once("disconnect", (what, err) => { reject(err); });
     });
 
     let result = this.startupPromise.then(() => {
       this.send(msg);
--- a/toolkit/components/extensions/ext-c-runtime.js
+++ b/toolkit/components/extensions/ext-c-runtime.js
@@ -1,9 +1,11 @@
 "use strict";
+XPCOMUtils.defineLazyModuleGetter(this, "NativeApp",
+                                  "resource://gre/modules/NativeMessaging.jsm");
 
 function runtimeApiFactory(context) {
   let {extension} = context;
 
   return {
     runtime: {
       onConnect: context.messenger.onConnect("runtime.onConnect"),
 
@@ -50,16 +52,39 @@ function runtimeApiFactory(context) {
         // TODO(robwu): Validate option keys and values when we support it.
 
         extensionId = extensionId || extension.id;
         let recipient = {extensionId};
 
         return context.messenger.sendMessage(context.messageManager, message, recipient, responseCallback);
       },
 
+      connectNative(application) {
+        let recipient = {
+          childId: context.childManager.id,
+          toNativeApp: application,
+        };
+        let rawPort = context.messenger.connectGetRawPort(context.messageManager, "", recipient);
+        let port = rawPort.api();
+        port.postMessage = message => {
+          message = NativeApp.encodeMessage(context, message);
+          rawPort.postMessage(message);
+        };
+        return port;
+      },
+
+      sendNativeMessage(application, message) {
+        let recipient = {
+          childId: context.childManager.id,
+          toNativeApp: application,
+        };
+        message = NativeApp.encodeMessage(context, message);
+        return context.messenger.sendMessage(context.messageManager, message, recipient);
+      },
+
       get lastError() {
         return context.lastError;
       },
 
       getManifest() {
         return Cu.cloneInto(extension.manifest, context.cloneScope);
       },
 
--- a/toolkit/components/extensions/ext-runtime.js
+++ b/toolkit/components/extensions/ext-runtime.js
@@ -13,19 +13,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionManagement",
                                   "resource://gre/modules/ExtensionManagement.jsm");
 
 var {
   ignoreEvent,
   SingletonEventManager,
 } = ExtensionUtils;
 
-XPCOMUtils.defineLazyModuleGetter(this, "NativeApp",
-                                  "resource://gre/modules/NativeMessaging.jsm");
-
 extensions.registerSchemaAPI("runtime", "addon_parent", context => {
   let {extension} = context;
   return {
     runtime: {
       onStartup: ignoreEvent(context, "runtime.onStartup"),
 
       onInstalled: new SingletonEventManager(context, "runtime.onInstalled", fire => {
         let listener = () => {
@@ -70,27 +67,16 @@ extensions.registerSchemaAPI("runtime", 
         } else {
           // Otherwise, reload the current extension.
           AddonManager.getAddonByID(extension.id, addon => {
             addon.reload();
           });
         }
       },
 
-      connectNative(application) {
-        let app = new NativeApp(context, application);
-        return app.portAPI();
-      },
-
-      sendNativeMessage(application, message) {
-        let app = new NativeApp(context, application);
-        message = NativeApp.encodeMessage(context, message);
-        return app.sendMessage(message);
-      },
-
       get lastError() {
         // TODO(robwu): Figure out how to make sure that errors in the parent
         // process are propagated to the child process.
         // lastError should not be accessed from the parent.
         return context.lastError;
       },
 
       getBrowserInfo: function() {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
@@ -216,18 +216,17 @@ add_task(function* test_sendNativeMessag
 add_task(function* test_disconnect() {
   function background() {
     let port = browser.runtime.connectNative("echo");
     port.onMessage.addListener((msg, msgPort) => {
       browser.test.assertEq(port, msgPort, "onMessage handler should receive the port as the second argument");
       browser.test.sendMessage("message", msg);
     });
     port.onDisconnect.addListener(msgPort => {
-      browser.test.assertEq(port, msgPort, "onDisconnect handler should receive the port as the second argument");
-      browser.test.sendMessage("disconnected");
+      browser.test.fail("onDisconnect should not be called for disconnect()");
     });
     browser.test.onMessage.addListener((what, payload) => {
       if (what == "send") {
         if (payload._json) {
           let json = payload._json;
           payload.toJSON = () => json;
           delete payload._json;
         }
@@ -264,27 +263,24 @@ add_task(function* test_disconnect() {
 
   let procCount = yield getSubprocessCount();
   equal(procCount, 1, "subprocess is running");
 
   extension.sendMessage("disconnect");
   response = yield extension.awaitMessage("disconnect-result");
   equal(response.success, true, "disconnect succeeded");
 
-  yield extension.awaitMessage("disconnected");
-
   do_print("waiting for subprocess to exit");
   yield waitForSubprocessExit();
   procCount = yield getSubprocessCount();
   equal(procCount, 0, "subprocess is no longer running");
 
   extension.sendMessage("disconnect");
   response = yield extension.awaitMessage("disconnect-result");
-  equal(response.success, false, "second call to disconnect failed");
-  ok(/already disconnected/.test(response.errmsg), "disconnect error message is reasonable");
+  equal(response.success, true, "second call to disconnect silently ignored");
 
   yield extension.unload();
 });
 
 // Test the limit on message size for writing
 add_task(function* test_write_limit() {
   Services.prefs.setIntPref(PREF_MAX_WRITE, 10);
   function clearPref() {