Bug 1290117: Call sendMessage response callback when there are no replies. r?robwu draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 06 Aug 2016 14:21:10 -0700
changeset 397570 dc6297a85062c6713fa808f935ec7051d8668dc1
parent 397569 fdbc19bfac55ae5a43b5c5cdd8cfeb52b21d0c91
child 527486 aa78e4c6d66b0c6117625bee853d6ab4c89714c2
push id25332
push usermaglione.k@gmail.com
push dateSat, 06 Aug 2016 21:21:51 +0000
reviewersrobwu
bugs1290117
milestone51.0a1
Bug 1290117: Call sendMessage response callback when there are no replies. r?robwu MozReview-Commit-ID: 1BYD1CgZmvD
browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
--- a/browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_sendMessage.js
@@ -16,42 +16,41 @@ add_task(function* tabsSendMessageReply(
 
     background: function() {
       let firstTab;
       let promiseResponse = new Promise(resolve => {
         browser.runtime.onMessage.addListener((msg, sender, respond) => {
           if (msg == "content-script-ready") {
             let tabId = sender.tab.id;
 
-            browser.tabs.sendMessage(tabId, "respond-never", response => {
-              browser.test.fail(`Got unexpected response callback: ${response}`);
-              browser.test.notifyFail("sendMessage");
-            });
-
             Promise.all([
               promiseResponse,
 
               browser.tabs.sendMessage(tabId, "respond-now"),
               browser.tabs.sendMessage(tabId, "respond-now-2"),
               new Promise(resolve => browser.tabs.sendMessage(tabId, "respond-soon", resolve)),
               browser.tabs.sendMessage(tabId, "respond-promise"),
               browser.tabs.sendMessage(tabId, "respond-never"),
+              new Promise(resolve => {
+                browser.runtime.sendMessage("respond-never", response => { resolve(response); });
+              }),
 
               browser.tabs.sendMessage(tabId, "respond-error").catch(error => Promise.resolve({error})),
               browser.tabs.sendMessage(tabId, "throw-error").catch(error => Promise.resolve({error})),
 
               browser.tabs.sendMessage(firstTab, "no-listener").catch(error => Promise.resolve({error})),
-            ]).then(([response, respondNow, respondNow2, respondSoon, respondPromise, respondNever, respondError, throwError, noListener]) => {
+            ]).then(([response, respondNow, respondNow2, respondSoon, respondPromise, respondNever, respondNever2, respondError, throwError, noListener]) => {
               browser.test.assertEq("expected-response", response, "Content script got the expected response");
 
               browser.test.assertEq("respond-now", respondNow, "Got the expected immediate response");
               browser.test.assertEq("respond-now-2", respondNow2, "Got the expected immediate response from the second listener");
               browser.test.assertEq("respond-soon", respondSoon, "Got the expected delayed response");
               browser.test.assertEq("respond-promise", respondPromise, "Got the expected promise response");
               browser.test.assertEq(undefined, respondNever, "Got the expected no-response resolution");
+              browser.test.assertEq(undefined, respondNever2, "Got the expected no-response resolution");
 
               browser.test.assertEq("respond-error", respondError.error.message, "Got the expected error response");
               browser.test.assertEq("throw-error", throwError.error.message, "Got the expected thrown error response");
 
               browser.test.assertEq("Could not establish connection. Receiving end does not exist.",
                                     noListener.error.message,
                                     "Got the expected no listener response");
 
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -1214,24 +1214,17 @@ Messenger.prototype = {
     return this.context.sendMessage(messageManager, message, data, options);
   },
 
   sendMessage(messageManager, msg, recipient, responseCallback) {
     let promise = this._sendMessage(messageManager, "Extension:Message", msg, recipient)
       .catch(error => {
         if (error.result == MessageChannel.RESULT_NO_HANDLER) {
           return Promise.reject({message: "Could not establish connection. Receiving end does not exist."});
-        } else if (error.result == MessageChannel.RESULT_NO_RESPONSE) {
-          if (responseCallback) {
-            // As a special case, we don't call the callback variant if we
-            // receive no response. So return a promise which will never
-            // resolve.
-            return new Promise(() => {});
-          }
-        } else {
+        } else if (error.result != MessageChannel.RESULT_NO_RESPONSE) {
           return Promise.reject({message: error.message});
         }
       });
 
     return this.context.wrapPromise(promise, responseCallback);
   },
 
   onMessage(name) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
@@ -24,36 +24,35 @@ add_task(function* tabsSendMessageReply(
     browser.runtime.onMessage.addListener((msg, sender, respond) => {
       if (msg == "respond-now") {
         respond("hello");
       } else if (msg == "respond-now-2") {
         respond(msg);
       }
     });
 
-    browser.runtime.sendMessage("respond-never", response => {
-      browser.test.fail(`Got unexpected response callback: ${response}`);
-      browser.test.notifyFail("sendMessage");
-    });
-
     Promise.all([
       browser.runtime.sendMessage("respond-now"),
       browser.runtime.sendMessage("respond-now-2"),
       new Promise(resolve => browser.runtime.sendMessage("respond-soon", resolve)),
       browser.runtime.sendMessage("respond-promise"),
       browser.runtime.sendMessage("respond-never"),
+      new Promise(resolve => {
+        browser.runtime.sendMessage("respond-never", response => { resolve(response); });
+      }),
 
       browser.runtime.sendMessage("respond-error").catch(error => Promise.resolve({error})),
       browser.runtime.sendMessage("throw-error").catch(error => Promise.resolve({error})),
-    ]).then(([respondNow, respondNow2, respondSoon, respondPromise, respondNever, respondError, throwError]) => {
+    ]).then(([respondNow, respondNow2, respondSoon, respondPromise, respondNever, respondNever2, respondError, throwError]) => {
       browser.test.assertEq("respond-now", respondNow, "Got the expected immediate response");
       browser.test.assertEq("respond-now-2", respondNow2, "Got the expected immediate response from the second listener");
       browser.test.assertEq("respond-soon", respondSoon, "Got the expected delayed response");
       browser.test.assertEq("respond-promise", respondPromise, "Got the expected promise response");
       browser.test.assertEq(undefined, respondNever, "Got the expected no-response resolution");
+      browser.test.assertEq(undefined, respondNever2, "Got the expected no-response resolution");
 
       browser.test.assertEq("respond-error", respondError.error.message, "Got the expected error response");
       browser.test.assertEq("throw-error", throwError.error.message, "Got the expected thrown error response");
 
       browser.test.notifyPass("sendMessage");
     }).catch(e => {
       browser.test.fail(`Error: ${e} :: ${e.stack}`);
       browser.test.notifyFail("sendMessage");