feat: Added support for the sendResponse callback in the runtime.onMe…

…ssage listeners (#97)

src/browser-polyfill.js

@@ -361,18 +361,51 @@ if (typeof browser === "undefined") {
        *        yield a response. False otherwise.
        */
       return function onMessage(message, sender, sendResponse) {
-        let result = listener(message, sender);
+        let didCallSendResponse = false;
+
+        let wrappedSendResponse;
+        let sendResponsePromise = new Promise(resolve => {
+          wrappedSendResponse = function(response) {
+            didCallSendResponse = true;
+            resolve(response);
+          };
+        });
+
+        let result = listener(message, sender, wrappedSendResponse);
+
+        const isResultThenable = result !== true && isThenable(result);
 
-        if (isThenable(result)) {
+        // If the listener didn't returned true or a Promise, or called
+        // wrappedSendResponse synchronously, we can exit earlier
+        // because there will be no response sent from this listener.
+        if (result !== true && !isResultThenable && !didCallSendResponse) {
+          return false;
+        }
+
+        // If the listener returned a Promise, send the resolved value as a
+        // result, otherwise wait the promise related to the wrappedSendResponse
+        // callback to resolve and send it as a response.
+        if (isResultThenable) {
           result.then(sendResponse, error => {
             console.error(error);
-            sendResponse(error);
+            // TODO: the error object is not serializable and so for now we just send
+            // `undefined`. Nevertheless, as being discussed in #97, this is not yet
+            // providing the expected behavior (the promise received from the sender should
+            // be rejected when the promise returned by the listener is being rejected).
+            sendResponse(undefined);
+          });
+        } else {
+          sendResponsePromise.then(sendResponse, error => {
+            console.error(error);
+            // TODO: same as above, we are currently sending `undefined` in this scenario
+            // because the error oject is not serializable, but it is not yet the behavior
+            // that this scenario should present.
+            sendResponse(undefined);
           });
-
-          return true;
-        } else if (result !== undefined) {
-          sendResponse(result);
         }
+
+        // Let Chrome know that the listener is replying.
+        return true;
       };
     });
 

test/fixtures/runtime-messaging-extension/background.js

@@ -2,16 +2,49 @@ const {name} = browser.runtime.getManifest();
 
 console.log(name, "background page loaded");
 
-browser.runtime.onMessage.addListener((msg, sender) => {
+browser.runtime.onMessage.addListener((msg, sender, sendResponse) => {
   console.log(name, "background received msg", {msg, sender});
 
-  try {
-    browser.pageAction.show(sender.tab.id);
-  } catch (err) {
-    return Promise.resolve(`Unexpected error on pageAction.show: ${err}`);
+  switch (msg) {
+    case "test - sendMessage with returned Promise reply":
+      try {
+        browser.pageAction.show(sender.tab.id);
+      } catch (err) {
+        return Promise.resolve(`Unexpected error on pageAction.show: ${err}`);
+      }
+
+      return Promise.resolve("bg page reply 1");
+
+    case "test - sendMessage with returned value reply":
+      // This is supposed to be ignored and the sender should receive
+      // a reply from the second listener.
+      return "Unexpected behavior: a plain return value should not be sent as a result";
+
+    case "test - sendMessage with synchronous sendResponse":
+      sendResponse("bg page reply 3");
+      return "value returned after calling sendResponse synchrously";
+
+    case "test - sendMessage with asynchronous sendResponse":
+      setTimeout(() => sendResponse("bg page reply 4"), 50);
+      return true;
+
+    case "test - second listener if the first does not reply":
+      // This is supposed to be ignored and the sender should receive
+      // a reply from the second listener.
+      return false;
+
+    default:
+      return Promise.resolve(
+        `Unxpected message received by the background page: ${JSON.stringify(msg)}\n`);
   }
+});
+
+browser.runtime.onMessage.addListener((msg, sender, sendResponse) => {
+  setTimeout(() => {
+    sendResponse("second listener reply");
+  }, 100);
 
-  return Promise.resolve("background page reply");
+  return true;
 });
 
 console.log(name, "background page ready to receive a content script message...");

test/fixtures/runtime-messaging-extension/content.js

@@ -1,9 +1,27 @@
 const {name} = browser.runtime.getManifest();
 
+async function runTest() {
+  let reply;
+  reply = await browser.runtime.sendMessage("test - sendMessage with returned Promise reply");
+  console.log(name, "test - returned resolved Promise - received", reply);
+
+  reply = await browser.runtime.sendMessage("test - sendMessage with returned value reply");
+  console.log(name, "test - returned value - received", reply);
+
+  reply = await browser.runtime.sendMessage("test - sendMessage with synchronous sendResponse");
+  console.log(name, "test - synchronous sendResponse - received", reply);
+
+  reply = await browser.runtime.sendMessage("test - sendMessage with asynchronous sendResponse");
+  console.log(name, "test - asynchronous sendResponse - received", reply);
+
+  reply = await browser.runtime.sendMessage("test - second listener if the first does not reply");
+  console.log(name, "test - second listener sendResponse - received", reply);
+
+  console.log(name, "content script messages sent");
+}
+
 console.log(name, "content script loaded");
 
-browser.runtime.sendMessage("content script message").then(reply => {
-  console.log(name, "content script received reply", {reply});
+runTest().catch((err) => {
+  console.error("content script error", err);
 });
-
-console.log(name, "content script message sent");

test/integration/test-runtime-messaging-on-chrome.js

@@ -40,8 +40,12 @@ describe("browser.runtime.onMessage/sendMessage", function() {
 
     const expectedConsoleMessages = [
       [extensionName, "content script loaded"],
-      [extensionName, "content script message sent"],
-      [extensionName, "content script received reply", {"reply": "background page reply"}],
+      [extensionName, "test - returned resolved Promise - received", "bg page reply 1"],
+      [extensionName, "test - returned value - received", "second listener reply"],
+      [extensionName, "test - synchronous sendResponse - received", "bg page reply 3"],
+      [extensionName, "test - asynchronous sendResponse - received", "bg page reply 4"],
+      [extensionName, "test - second listener sendResponse - received", "second listener reply"],
+      [extensionName, "content script messages sent"],
     ];
 
     const lastExpectedMessage = expectedConsoleMessages.slice(-1).pop();

test/test-runtime-onMessage.js

@@ -117,8 +117,10 @@ describe("browser-polyfill", () => {
         equal(secondMessageListener.firstCall.args[0], "call second wrapper");
       });
     });
+  });
 
-    it("sends the returned value as a message response", () => {
+  describe("sendResponse callback", () => {
+    it("ignores the sendResponse calls when the listener returns a promise", () => {
       const fakeChrome = {
         runtime: {
           lastError: null,
@@ -128,70 +130,111 @@ describe("browser-polyfill", () => {
         },
       };
 
-      // Plain value returned.
-      const messageListener = sinon.stub();
-      const firstResponse = "fake reply";
-      // Resolved Promise returned.
-      const secondResponse = Promise.resolve("fake reply 2");
-      // Rejected Promise returned.
-      const thirdResponse = Promise.reject("fake error 3");
+      return setupTestDOMWindow(fakeChrome).then(window => {
+        const listener = sinon.spy((msg, sender, sendResponse) => {
+          sendResponse("Ignored sendReponse callback on returned Promise");
+
+          return Promise.resolve("listener resolved value");
+        });
+
+        const sendResponseSpy = sinon.spy();
+
+        window.browser.runtime.onMessage.addListener(listener);
+
+        ok(fakeChrome.runtime.onMessage.addListener.calledOnce,
+           "runtime.onMessage.addListener should have been called once");
+
+        let wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0];
+
+        let returnedValue = wrappedListener("test message", {name: "fake sender"}, sendResponseSpy);
+        equal(returnedValue, true, "the wrapped listener should have returned true");
+
+        ok(listener.calledOnce, "listener has been called once");
 
-      const sendResponseSpy = sinon.spy();
+        // Wait a promise to be resolved and then check the wrapped listener behaviors.
+        return Promise.resolve().then(() => {
+          ok(sendResponseSpy.calledOnce, "sendResponse callback called once");
 
-      messageListener
-        .onFirstCall().returns(firstResponse)
-        .onSecondCall().returns(secondResponse)
-        .onThirdCall().returns(thirdResponse);
+          equal(sendResponseSpy.firstCall.args[0], "listener resolved value",
+                "sendResponse has been called with the expected value");
+        });
+      });
+    });
+
+    it("ignores asynchronous sendResponse calls if the listener does not return true", () => {
+      const fakeChrome = {
+        runtime: {
+          lastError: null,
+          onMessage: {
+            addListener: sinon.spy(),
+          },
+        },
+      };
 
-      let wrappedListener;
+      const waitPromises = [];
 
       return setupTestDOMWindow(fakeChrome).then(window => {
-        window.browser.runtime.onMessage.addListener(messageListener);
+        const listenerReturnsFalse = sinon.spy((msg, sender, sendResponse) => {
+          waitPromises.push(Promise.resolve().then(() => {
+            sendResponse("Ignored sendReponse callback on returned false");
+          }));
+
+          return false;
+        });
+
+        const listenerReturnsValue = sinon.spy((msg, sender, sendResponse) => {
+          waitPromises.push(Promise.resolve().then(() => {
+            sendResponse("Ignored sendReponse callback on non boolean/thenable return values");
+          }));
+
+          // Any return value that is not a promise should not be sent as a response,
+          // and any return value that is not true should make the sendResponse
+          // calls to be ignored.
+          return "Ignored return value";
+        });
+
+        const listenerReturnsTrue = sinon.spy((msg, sender, sendResponse) => {
+          waitPromises.push(Promise.resolve().then(() => {
+            sendResponse("expected sendResponse value");
+          }));
+
+          // Expect the asynchronous sendResponse call to be used to send a response
+          // when the listener returns true.
+          return true;
+        });
+
+        const sendResponseSpy = sinon.spy();
+
+        window.browser.runtime.onMessage.addListener(listenerReturnsFalse);
+        window.browser.runtime.onMessage.addListener(listenerReturnsValue);
+        window.browser.runtime.onMessage.addListener(listenerReturnsTrue);
+
+        equal(fakeChrome.runtime.onMessage.addListener.callCount, 3,
+              "runtime.onMessage.addListener should have been called 3 times");
+
+        let wrappedListenerReturnsFalse = fakeChrome.runtime.onMessage.addListener.firstCall.args[0];
+        let wrappedListenerReturnsValue = fakeChrome.runtime.onMessage.addListener.secondCall.args[0];
+        let wrappedListenerReturnsTrue = fakeChrome.runtime.onMessage.addListener.thirdCall.args[0];
+
+        let returnedValue = wrappedListenerReturnsFalse("test message", {name: "fake sender"}, sendResponseSpy);
+        equal(returnedValue, false, "the first wrapped listener should return false");
+
+        returnedValue = wrappedListenerReturnsValue("test message2", {name: "fake sender"}, sendResponseSpy);
+        equal(returnedValue, false, "the second wrapped listener should return false");
+
+        returnedValue = wrappedListenerReturnsTrue("test message3", {name: "fake sender"}, sendResponseSpy);
+        equal(returnedValue, true, "the third wrapped listener should return true");
+
+        ok(listenerReturnsFalse.calledOnce, "first listener has been called once");
+        ok(listenerReturnsValue.calledOnce, "second listener has been called once");
+        ok(listenerReturnsTrue.calledOnce, "third listener has been called once");
+
+        // Wait all the collected promises to be resolved and then check the wrapped listener behaviors.
+        return Promise.all(waitPromises).then(() => {
+          ok(sendResponseSpy.calledOnce, "sendResponse callback should have been called once");
 
-        ok(fakeChrome.runtime.onMessage.addListener.calledOnce);
-
-        wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0];
-
-        wrappedListener("fake message", {name: "fake sender"}, sendResponseSpy);
-
-        ok(messageListener.calledOnce, "The unwrapped message listener has been called");
-        deepEqual(messageListener.firstCall.args,
-                  ["fake message", {name: "fake sender"}],
-                  "The unwrapped message listener has received the expected parameters");
-
-        ok(sendResponseSpy.calledOnce, "The sendResponse function has been called");
-        equal(sendResponseSpy.firstCall.args[0], "fake reply",
-              "sendResponse callback has been called with the expected parameters");
-
-        wrappedListener("fake message2", {name: "fake sender2"}, sendResponseSpy);
-
-        // Wait the second response promise to be resolved.
-        return secondResponse;
-      }).then(() => {
-        ok(messageListener.calledTwice,
-           "The unwrapped message listener has been called");
-        deepEqual(messageListener.secondCall.args,
-                  ["fake message2", {name: "fake sender2"}],
-                  "The unwrapped listener has received the expected parameters");
-
-        ok(sendResponseSpy.calledTwice, "The sendResponse function has been called");
-        equal(sendResponseSpy.secondCall.args[0], "fake reply 2",
-              "sendResponse callback has been called with the expected parameters");
-      }).then(() => {
-        wrappedListener("fake message3", {name: "fake sender3"}, sendResponseSpy);
-
-        // Wait the third response promise to be rejected.
-        return thirdResponse.catch(err => {
-          equal(messageListener.callCount, 3,
-                "The unwrapped message listener has been called");
-          deepEqual(messageListener.thirdCall.args,
-                    ["fake message3", {name: "fake sender3"}],
-                    "The unwrapped listener has received the expected parameters");
-
-          equal(sendResponseSpy.callCount, 3,
-                "The sendResponse function has been called");
-          equal(sendResponseSpy.thirdCall.args[0], err,
-                "sendResponse callback has been called with the expected parameters");
+          equal(sendResponseSpy.firstCall.args[0], "expected sendResponse value",
+                "sendResponse has been called with the expected value");
         });
       });
     });