Bug 1299411 - Propagate errors to port.onDisconnect via port.error r=kmag
authorRob Wu <rob@robwu.nl>
Fri, 30 Sep 2016 22:42:28 +0200
changeset 320061 0ce4a6653d1966333fa72a29f762fdde5614d02f
parent 320060 e08ee4c2b1e2a0f9020f9a02f5c0ded4b1aa4ab2
child 320062 a9c19ee017a44aeb9953858b663a100a85ccffab
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
milestone52.0a1
Bug 1299411 - Propagate errors to port.onDisconnect via port.error r=kmag In Chrome, runtime.lastError is set when the port is disconnected due to an error. Here in Firefox we choose to set a new property "error" on the port if the port disconnected due to an error. Since onDisconnect fires at most once, port.error is set only once. MozReview-Commit-ID: EPaVtV4WkcQ
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html
toolkit/components/extensions/test/mochitest/test_ext_runtime_disconnect.html
toolkit/components/extensions/test/mochitest/test_ext_unload_frame.html
toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -1238,43 +1238,51 @@ function Port(context, senderMM, receive
         Cu.reportError("Missing sender.contextId in message to Port");
         return false;
       }
       return sender.contextId !== this.context.contextId;
     },
   };
 
   this.disconnectHandler = Object.assign({
-    receiveMessage: () => this.disconnectByOtherEnd(),
+    receiveMessage: ({data}) => this.disconnectByOtherEnd(data),
   }, 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);
 
+    let portError = null;
     let publicAPI = {
       name: this.name,
       disconnect: () => {
         this.disconnect();
       },
       postMessage: json => {
         this.postMessage(json);
       },
       onDisconnect: new EventManager(this.context, "Port.onDisconnect", fire => {
-        return this.registerOnDisconnect(() => fire.withoutClone(portObj));
+        return this.registerOnDisconnect(error => {
+          portError = error && this.context.normalizeError(error);
+          fire.withoutClone(portObj);
+        });
       }).api(),
       onMessage: new EventManager(this.context, "Port.onMessage", fire => {
         return this.registerOnMessage(msg => {
           msg = Cu.cloneInto(msg, this.context.cloneScope);
           fire.withoutClone(msg, portObj);
         });
       }).api(),
+
+      get error() {
+        return portError;
+      },
     };
 
     if (this.sender) {
       publicAPI.sender = this.sender;
     }
 
     injectAPI(publicAPI, portObj);
     return portObj;
@@ -1289,22 +1297,24 @@ Port.prototype = {
   },
 
   /**
    * Register a callback that is called when the port is disconnected by the
    * *other* end. The callback is automatically unregistered when the port or
    * context is closed.
    *
    * @param {function} callback Called when the other end disconnects the port.
+   *     If the disconnect is caused by an error, the first parameter is an
+   *     object with a "message" string property that describes the cause.
    * @returns {function} Function to unregister the listener.
    */
   registerOnDisconnect(callback) {
-    let listener = () => {
+    let listener = error => {
       if (this.context.active && !this.disconnected) {
-        callback();
+        callback(error);
       }
     };
     this.disconnectListeners.add(listener);
     return () => {
       this.disconnectListeners.delete(listener);
     };
   },
 
@@ -1346,36 +1356,51 @@ Port.prototype = {
     MessageChannel.removeListener(this.receiverMMs, "Extension:Port:Disconnect", this.disconnectHandler);
     for (let unregister of this.unregisterMessageFuncs) {
       unregister();
     }
     this.context.forgetOnClose(this);
     this.disconnected = true;
   },
 
-  disconnectByOtherEnd() {
+  /**
+   * Disconnect the port from the other end (which may not even exist).
+   *
+   * @param {Error|{message: string}} [error] The reason for disconnecting,
+   *     if it is an abnormal disconnect.
+   */
+  disconnectByOtherEnd(error = null) {
     if (this.disconnected) {
       return;
     }
 
     for (let listener of this.disconnectListeners) {
-      listener();
+      listener(error);
     }
 
     this.handleDisconnection();
   },
 
-  disconnect() {
+  /**
+   * Disconnect the port from this end.
+   *
+   * @param {Error|{message: string}} [error] The reason for disconnecting,
+   *     if it is an abnormal disconnect.
+   */
+  disconnect(error = null) {
     if (this.disconnected) {
       // disconnect() may be called without side effects even after the port is
       // closed - https://developer.chrome.com/extensions/runtime#type-Port
       return;
     }
     this.handleDisconnection();
-    this._sendMessage("Extension:Port:Disconnect", null);
+    if (error) {
+      error = {message: this.context.normalizeError(error).message};
+    }
+    this._sendMessage("Extension:Port:Disconnect", error);
   },
 
   close() {
     this.disconnect();
   },
 };
 
 function getMessageManager(target) {
@@ -1490,17 +1515,24 @@ Messenger.prototype = {
     }).api();
   },
 
   connect(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 => port.disconnectByOtherEnd());
+      .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.api();
   },
 
   onConnect(name) {
     return new SingletonEventManager(this.context, name, callback => {
       let listener = {
         messageFilterPermissive: this.optionalFilter,
         messageFilterStrict: this.filter,
--- a/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html
@@ -26,16 +26,17 @@ function background() {
         port.postMessage("message 2");
         expected = "message 3";
       } else if (expected == "message 3") {
         expected = "disconnect";
         browser.test.notifyPass("runtime.connect");
       }
     });
     port.onDisconnect.addListener(() => {
+      browser.test.assertEq(null, port.error, "No error because port is closed by disconnect() at other end");
       browser.test.assertEq(expected, "disconnect", "got disconnection at right time");
     });
   });
 }
 
 function contentScript() {
   let port = browser.runtime.connect({name: "ernie"});
   port.postMessage("message 1");
--- a/toolkit/components/extensions/test/mochitest/test_ext_runtime_disconnect.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_runtime_disconnect.html
@@ -12,16 +12,17 @@
 
 <script type="text/javascript">
 "use strict";
 
 function background() {
   browser.runtime.onConnect.addListener(port => {
     browser.test.assertEq(port.name, "ernie", "port name correct");
     port.onDisconnect.addListener(() => {
+      browser.test.assertEq(null, port.error, "The port is implicitly closed without errors when the other context unloads");
       // Closing an already-disconnected port is a no-op.
       port.disconnect();
       port.disconnect();
       browser.test.sendMessage("disconnected");
     });
     browser.test.sendMessage("connected");
   });
 }
--- a/toolkit/components/extensions/test/mochitest/test_ext_unload_frame.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_unload_frame.html
@@ -41,16 +41,17 @@ function connect_background() {
     browser.test.assertEq("port from frame", port.name);
 
     let disconnected = false;
     let hasMessage = false;
     port.onDisconnect.addListener(() => {
       browser.test.assertFalse(disconnected, "onDisconnect should fire once");
       disconnected = true;
       browser.test.assertTrue(hasMessage, "Expected onMessage before onDisconnect");
+      browser.test.assertEq(null, port.error, "The port is implicitly closed without errors when the other context unloads");
       delayedNotifyPass("Received onDisconnect from closing frame");
     });
     port.onMessage.addListener(msg => {
       browser.test.assertFalse(hasMessage, "onMessage should fire once");
       hasMessage = true;
       browser.test.assertFalse(disconnected, "Should get message before disconnect");
       browser.test.assertEq("from frame", msg, "Expected message from frame");
     });
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_connect_no_receiver.js
@@ -1,16 +1,17 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 add_task(function* test_connect_without_listener() {
   function background() {
     let port = browser.runtime.connect();
     port.onDisconnect.addListener(() => {
+      browser.test.assertEq("Could not establish connection. Receiving end does not exist.", port.error && port.error.message);
       browser.test.notifyPass("port.onDisconnect was called");
     });
   }
   let extensionData = {
     background,
   };
 
   let extension = ExtensionTestUtils.loadExtension(extensionData);