From 76eeeaccc9faaf6403addaf1ee829a8b0b238957 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 14 May 2018 20:38:21 +0200 Subject: [PATCH] feat: Added support for the sendResponse callback in the runtime.onMessage listeners (#97) --- src/browser-polyfill.js | 47 +++++- .../runtime-messaging-extension/background.js | 47 +++++- .../runtime-messaging-extension/content.js | 26 +++- .../test-runtime-messaging-on-chrome.js | 8 +- test/test-runtime-onMessage.js | 143 ++++++++++++------ 5 files changed, 201 insertions(+), 70 deletions(-) diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index f17c619..c6cefc9 100644 --- a/src/browser-polyfill.js +++ b/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; - if (isThenable(result)) { + 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 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; }; }); diff --git a/test/fixtures/runtime-messaging-extension/background.js b/test/fixtures/runtime-messaging-extension/background.js index f6bb3da..2045878 100644 --- a/test/fixtures/runtime-messaging-extension/background.js +++ b/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("background page reply"); + 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 true; }); console.log(name, "background page ready to receive a content script message..."); diff --git a/test/fixtures/runtime-messaging-extension/content.js b/test/fixtures/runtime-messaging-extension/content.js index 435d931..80d4182 100644 --- a/test/fixtures/runtime-messaging-extension/content.js +++ b/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"); diff --git a/test/integration/test-runtime-messaging-on-chrome.js b/test/integration/test-runtime-messaging-on-chrome.js index b5de93e..64ab17b 100644 --- a/test/integration/test-runtime-messaging-on-chrome.js +++ b/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(); diff --git a/test/test-runtime-onMessage.js b/test/test-runtime-onMessage.js index 0e42899..0351372 100644 --- a/test/test-runtime-onMessage.js +++ b/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"); - const sendResponseSpy = sinon.spy(); + return Promise.resolve("listener resolved value"); + }); - messageListener - .onFirstCall().returns(firstResponse) - .onSecondCall().returns(secondResponse) - .onThirdCall().returns(thirdResponse); + const sendResponseSpy = sinon.spy(); - let wrappedListener; + 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"); + + // Wait a promise to be resolved and then check the wrapped listener behaviors. + return Promise.resolve().then(() => { + ok(sendResponseSpy.calledOnce, "sendResponse callback called once"); + + 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(), + }, + }, + }; + + 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"); + })); - ok(fakeChrome.runtime.onMessage.addListener.calledOnce); + return false; + }); - wrappedListener = fakeChrome.runtime.onMessage.addListener.firstCall.args[0]; + const listenerReturnsValue = sinon.spy((msg, sender, sendResponse) => { + waitPromises.push(Promise.resolve().then(() => { + sendResponse("Ignored sendReponse callback on non boolean/thenable return values"); + })); - wrappedListener("fake message", {name: "fake sender"}, sendResponseSpy); + // 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"; + }); - 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"); + const listenerReturnsTrue = sinon.spy((msg, sender, sendResponse) => { + waitPromises.push(Promise.resolve().then(() => { + sendResponse("expected sendResponse value"); + })); - 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"); + // Expect the asynchronous sendResponse call to be used to send a response + // when the listener returns true. + return true; + }); - wrappedListener("fake message2", {name: "fake sender2"}, sendResponseSpy); + const sendResponseSpy = sinon.spy(); - // 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"); + window.browser.runtime.onMessage.addListener(listenerReturnsFalse); + window.browser.runtime.onMessage.addListener(listenerReturnsValue); + window.browser.runtime.onMessage.addListener(listenerReturnsTrue); - 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); + equal(fakeChrome.runtime.onMessage.addListener.callCount, 3, + "runtime.onMessage.addListener should have been called 3 times"); - // 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"); + 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]; - 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"); + 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"); + + equal(sendResponseSpy.firstCall.args[0], "expected sendResponse value", + "sendResponse has been called with the expected value"); }); }); });