Bug 1298979 - Use MessageChannel to implement runtime.Port r=billm

- Add new responseType RESPONSE_NONE to MessageChannel to signal no
  expected reply.
- Modify Port to use MessageChannel instead of message managers.
- Include the `port` object to the disconnect event of ports because
  Chrome does it too.
- Replace use of `contentWindow` with `cloneScope` to make the Port
  independent of documents.
- Move registration of context destruction from `api()` to the
  constructor to make sure that the disconnect listener is properly
  removed if for some reason the `api` method is never called.

MozReview-Commit-ID: 9LCo5x1kEbH

--HG--
extra : rebase_source : 226bb0a3cacf5ad22c1f7695f90472a57616dbd6

toolkit/components/extensions/ExtensionUtils.jsm

@@ -1149,28 +1149,51 @@ function promiseObserved(topic, test = () => true) {
 
 let gNextPortId = 1;
 
-// Abstraction for a Port object in the extension API. Each port has a unique ID.
-function Port(context, messageManager, name, id, sender) {
+/**
+ * Abstraction for a Port object in the extension API.
+ *
+ * @param {BaseContext} context The context that owns this port.
+ * @param {nsIMessageSender} senderMM The message manager to send messages to.
+ * @param {Array<nsIMessageSender>} receiverMMs Message managers to listen on.
+ * @param {string} name Arbitrary port name as defined by the addon.
+ * @param {string} id An ID that uniquely identifies this port's channel.
+ * @param {object} sender The `port.sender` property.
+ * @param {object} recipient The recipient of messages sent from this port.
+ */
+function Port(context, senderMM, receiverMMs, name, id, sender, recipient) {
   this.context = context;
-  this.messageManager = messageManager;
+  this.senderMM = senderMM;
+  this.receiverMMs = receiverMMs;
   this.name = name;
   this.id = id;
-  this.listenerName = `Extension:Port-${this.id}`;
-  this.disconnectName = `Extension:Disconnect-${this.id}`;
   this.sender = sender;
+  this.recipient = recipient;
   this.disconnected = false;
-
-  this.messageManager.addMessageListener(this.disconnectName, this, true);
   this.disconnectListeners = new Set();
+
+  // Common options for onMessage and onDisconnect.
+  this.handlerBase = {
+    messageFilterStrict: {portId: id},
+    filterMessage: (sender, recipient) => {
+      if (!sender.contextId) {
+        Cu.reportError("Missing sender.contextId in message to Port");
+        return false;
+      }
+      return sender.contextId !== this.context.contextId;
+    },
+  };
+
+  this.disconnectHandler = Object.assign({
+    receiveMessage: () => this.disconnectByOtherEnd(),
+  }, this.handlerBase);
+  MessageChannel.addListener(this.receiverMMs, "Extension:Port:Disconnect", this.disconnectHandler);
+  this.context.callOnClose(this);
 }
 
 Port.prototype = {
   api() {
     let portObj = Cu.createObjectIn(this.context.cloneScope);
 
-    // We want a close() notification when the window is destroyed.
-    this.context.callOnClose(this);
-
     let publicAPI = {
       name: this.name,
       disconnect: () => {
@@ -1178,14 +1201,15 @@ Port.prototype = {
       },
       postMessage: json => {
         if (this.disconnected) {
-          throw new this.context.contentWindow.Error("Attempt to postMessage on disconnected port");
+          throw new this.context.cloneScope.Error("Attempt to postMessage on disconnected port");
         }
-        this.messageManager.sendAsyncMessage(this.listenerName, json);
+
+        this._sendMessage("Extension:Port:PostMessage", json);
       },
       onDisconnect: new EventManager(this.context, "Port.onDisconnect", fire => {
         let listener = () => {
-          if (!this.disconnected) {
-            fire();
+          if (this.context.active && !this.disconnected) {
+            fire.withoutClone(portObj);
           }
         };
 
@@ -1195,18 +1219,17 @@ Port.prototype = {
         };
       }).api(),
       onMessage: new EventManager(this.context, "Port.onMessage", fire => {
-        let listener = ({data}) => {
-          if (!this.context.active) {
-            // TODO: Send error as a response.
-            Cu.reportError("Message received on port for an inactive content script");
-          } else if (!this.disconnected) {
-            fire(data);
-          }
-        };
+        let handler = Object.assign({
+          receiveMessage: ({data}) => {
+            if (this.context.active && !this.disconnected) {
+              fire(data);
+            }
+          },
+        }, this.handlerBase);
 
-        this.messageManager.addMessageListener(this.listenerName, listener);
+        MessageChannel.addListener(this.receiverMMs, "Extension:Port:PostMessage", handler);
         return () => {
-          this.messageManager.removeMessageListener(this.listenerName, listener);
+          MessageChannel.removeListener(this.receiverMMs, "Extension:Port:PostMessage", handler);
         };
       }).api(),
     };
@@ -1219,16 +1242,19 @@ Port.prototype = {
     return portObj;
   },
 
-  handleDisconnection() {
-    this.messageManager.removeMessageListener(this.disconnectName, this);
-    this.context.forgetOnClose(this);
-    this.disconnected = true;
+  _sendMessage(message, data) {
+    let options = {
+      recipient: Object.assign({}, this.recipient, {portId: this.id}),
+      responseType: MessageChannel.RESPONSE_NONE,
+    };
+
+    return this.context.sendMessage(this.senderMM, message, data, options);
   },
 
-  receiveMessage(msg) {
-    if (msg.name == this.disconnectName) {
-      this.disconnectByOtherEnd();
-    }
+  handleDisconnection() {
+    MessageChannel.removeListener(this.receiverMMs, "Extension:Port:Disconnect", this.disconnectHandler);
+    this.context.forgetOnClose(this);
+    this.disconnected = true;
   },
 
   disconnectByOtherEnd() {
@@ -1250,7 +1276,7 @@ Port.prototype = {
       return;
     }
     this.handleDisconnection();
-    this.messageManager.sendAsyncMessage(this.disconnectName);
+    this._sendMessage("Extension:Port:Disconnect", null);
   },
 
   close() {
@@ -1362,7 +1388,7 @@ Messenger.prototype = {
 
   connect(messageManager, name, recipient) {
     let portId = `${gNextPortId++}-${Services.appinfo.uniqueProcessID}`;
-    let port = new Port(this.context, messageManager, name, portId, null);
+    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 => port.disconnectByOtherEnd());
@@ -1379,13 +1405,18 @@ Messenger.prototype = {
           return sender.contextId !== this.context.contextId;
         },
 
-        receiveMessage: ({target, data: message, sender, recipient}) => {
+        receiveMessage: ({target, data: message, sender}) => {
           let {name, portId} = message;
           let mm = getMessageManager(target);
           if (this.delegate) {
             this.delegate.getSender(this.context, target, sender);
           }
-          let port = new Port(this.context, mm, name, portId, sender);
+          let recipient = Object.assign({}, sender);
+          if (recipient.tab) {
+            recipient.tabId = recipient.tab.id;
+            delete recipient.tab;
+          }
+          let port = new Port(this.context, mm, this.messageManagers, name, portId, sender, recipient);
           this.context.runSafeWithoutClone(callback, port.api());
           return true;
         },

toolkit/components/extensions/MessageChannel.jsm

@@ -340,6 +340,11 @@ this.MessageChannel = {
    */
   RESPONSE_ALL: 2,
 
+  /**
+   * Fire-and-forget: The sender of this message does not expect a reply.
+   */
+  RESPONSE_NONE: 3,
+
   /**
    * Initializes message handlers for the given message managers if needed.
    *
@@ -357,7 +362,7 @@ this.MessageChannel = {
   },
 
   /**
-   * Returns true if the peroperties of the `data` object match those in
+   * Returns true if the properties of the `data` object match those in
    * the `filter` object. Matching is done on a strict equality basis,
    * and the behavior varies depending on the value of the `strict`
    * parameter.
@@ -370,7 +375,7 @@ this.MessageChannel = {
    *    If true, all properties in the `filter` object have a
    *    corresponding property in `data` with the same value. If
    *    false, properties present in both objects must have the same
-   *    balue.
+   *    value.
    * @returns {boolean} True if the objects match.
    */
   matchesFilter(filter, data, strict = true) {
@@ -508,6 +513,17 @@ this.MessageChannel = {
     let channelId = `${gChannelId++}-${Services.appinfo.uniqueProcessID}`;
     let message = {messageName, channelId, sender, recipient, data, responseType};
 
+    if (responseType == this.RESPONSE_NONE) {
+      try {
+        target.sendAsyncMessage(MESSAGE_MESSAGE, message);
+      } catch (e) {
+        // Caller is not expecting a reply, so dump the error to the console.
+        Cu.reportError(e);
+        return Promise.reject(e);
+      }
+      return Promise.resolve();  // Not expecting any reply.
+    }
+
     let deferred = PromiseUtils.defer();
     deferred.sender = recipient;
     deferred.messageManager = target;
@@ -602,6 +618,19 @@ this.MessageChannel = {
       target = target.messageManager;
     }
 
+    if (data.responseType == this.RESPONSE_NONE) {
+      handlers.forEach(handler => {
+        // The sender expects no reply, so dump any errors to the console.
+        new Promise(resolve => {
+          resolve(handler.receiveMessage(data));
+        }).catch(e => {
+          Cu.reportError(e.stack ? `${e}\n${e.stack}` : e.message || e);
+        });
+      });
+      // Note: Unhandled messages are silently dropped.
+      return;
+    }
+
     let deferred = {
       sender: data.sender,
       messageManager: target,