From 451af2d29bb05ad15c644659db0acd4095a488a0 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 14 May 2018 18:59:13 +0200 Subject: [PATCH 1/3] fix: print a deprecation warning when sendResponse callback is used for the first time --- src/browser-polyfill.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index c6cefc93..3ae8c49b 100644 --- a/src/browser-polyfill.js +++ b/src/browser-polyfill.js @@ -7,6 +7,13 @@ "use strict"; if (typeof browser === "undefined") { + const SEND_RESPONSE_DEPRECATION_WARNING = ` + Returning a Promise is the preferred way to send a reply from an + onMessage/onMessageExternal listener, as the sendResponse will be + removed from the specs (See + https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onMessage) + `.replace(/\s+/g, " ").trim(); + // Wrapping the bulk of this polyfill in a one-time-use function is a minor // optimization for Firefox. Since Spidermonkey does not fully parse the // contents of a function until the first time it's called, and since it will @@ -338,6 +345,9 @@ if (typeof browser === "undefined") { }, }); + // Keep track if the deprecation warning has been logged at least once. + let loggedSendResponseDeprecationWarning = false; + const onMessageWrappers = new DefaultWeakMap(listener => { if (typeof listener !== "function") { return listener; @@ -366,6 +376,10 @@ if (typeof browser === "undefined") { let wrappedSendResponse; let sendResponsePromise = new Promise(resolve => { wrappedSendResponse = function(response) { + if (!loggedSendResponseDeprecationWarning) { + console.warn(SEND_RESPONSE_DEPRECATION_WARNING, new Error().stack); + loggedSendResponseDeprecationWarning = true; + } didCallSendResponse = true; resolve(response); }; From 4c4b63569fd6de215acaa9dba6efc58066855cd7 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 14 May 2018 19:02:59 +0200 Subject: [PATCH 2/3] feat: Reject sendMessage returned promise when a onMessage listener returns a rejected promise. --- src/browser-polyfill.js | 90 +++++++++++++++---- .../runtime-messaging-extension/background.js | 6 ++ .../runtime-messaging-extension/content.js | 25 ++++++ 3 files changed, 103 insertions(+), 18 deletions(-) diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index 3ae8c49b..f5cd05e2 100644 --- a/src/browser-polyfill.js +++ b/src/browser-polyfill.js @@ -104,6 +104,8 @@ if (typeof browser === "undefined") { }; }; + const pluralizeArguments = (numArgs) => numArgs == 1 ? "argument" : "arguments"; + /** * Creates a wrapper function for a method with the given name and metadata. * @@ -127,8 +129,6 @@ if (typeof browser === "undefined") { * The generated wrapper function. */ const wrapAsyncFunction = (name, metadata) => { - const pluralizeArguments = (numArgs) => numArgs == 1 ? "argument" : "arguments"; - return function asyncFunctionWrapper(target, ...args) { if (args.length < metadata.minArgs) { throw new Error(`Expected at least ${metadata.minArgs} ${pluralizeArguments(metadata.minArgs)} for ${name}(), got ${args.length}`); @@ -385,7 +385,12 @@ if (typeof browser === "undefined") { }; }); - let result = listener(message, sender, wrappedSendResponse); + let result; + try { + result = listener(message, sender, wrappedSendResponse); + } catch (err) { + result = Promise.reject(err); + } const isResultThenable = result !== true && isThenable(result); @@ -396,26 +401,42 @@ if (typeof browser === "undefined") { return false; } + // A small helper to send the message if the promise resolves + // and an error if the promise rejects (a wrapped sendMessage has + // to translate the message into a resolved promise or a rejected + // promise). + const sendPromisedResult = (promise) => { + promise.then(msg => { + // send the message value. + sendResponse(msg); + }, error => { + // Send a JSON representation of the error if the rejected value + // is an instance of error, or the object itself otherwise. + let message; + if (error && (error instanceof Error || + typeof error.message === "string")) { + message = error.message; + } else { + message = "An unexpected error occurred"; + } + + sendResponse({ + __mozWebExtensionPolyfillReject__: true, + message, + }); + }).catch(err => { + // Print an error on the console if unable to send the response. + console.error("Failed to send onMessage rejected reply", err); + }); + }; + // 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); - // 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); - }); + sendPromisedResult(result); } 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); - }); + sendPromisedResult(sendResponsePromise); } // Let Chrome know that the listener is replying. @@ -423,9 +444,42 @@ if (typeof browser === "undefined") { }; }); + const wrappedSendMessageCallback = ({reject, resolve}, reply) => { + if (chrome.runtime.lastError) { + reject(chrome.runtime.lastError); + } else if (reply && reply.__mozWebExtensionPolyfillReject__) { + // Convert back the JSON representation of the error into + // an Error instance. + reject(new Error(reply.message)); + } else { + resolve(reply); + } + }; + + const wrappedSendMessage = (name, metadata, apiNamespaceObj, ...args) => { + if (args.length < metadata.minArgs) { + throw new Error(`Expected at least ${metadata.minArgs} ${pluralizeArguments(metadata.minArgs)} for ${name}(), got ${args.length}`); + } + + if (args.length > metadata.maxArgs) { + throw new Error(`Expected at most ${metadata.maxArgs} ${pluralizeArguments(metadata.maxArgs)} for ${name}(), got ${args.length}`); + } + + return new Promise((resolve, reject) => { + const wrappedCb = wrappedSendMessageCallback.bind(null, {resolve, reject}); + args.push(wrappedCb); + apiNamespaceObj.sendMessage(...args); + }); + }; + const staticWrappers = { runtime: { onMessage: wrapEvent(onMessageWrappers), + onMessageExternal: wrapEvent(onMessageWrappers), + sendMessage: wrappedSendMessage.bind(null, "sendMessage", {minArgs: 1, maxArgs: 3}), + }, + tabs: { + sendMessage: wrappedSendMessage.bind(null, "sendMessage", {minArgs: 2, maxArgs: 3}), }, }; diff --git a/test/fixtures/runtime-messaging-extension/background.js b/test/fixtures/runtime-messaging-extension/background.js index 20458786..a42b9532 100644 --- a/test/fixtures/runtime-messaging-extension/background.js +++ b/test/fixtures/runtime-messaging-extension/background.js @@ -33,6 +33,12 @@ browser.runtime.onMessage.addListener((msg, sender, sendResponse) => { // a reply from the second listener. return false; + case "test - sendMessage with returned rejected Promise with Error value": + return Promise.reject(new Error("rejected-error-value")); + + case "test - sendMessage with returned rejected Promise with non-Error value": + return Promise.reject("rejected-non-error-value"); + default: return Promise.resolve( `Unxpected message received by the background page: ${JSON.stringify(msg)}\n`); diff --git a/test/fixtures/runtime-messaging-extension/content.js b/test/fixtures/runtime-messaging-extension/content.js index 80d41827..b123505d 100644 --- a/test/fixtures/runtime-messaging-extension/content.js +++ b/test/fixtures/runtime-messaging-extension/content.js @@ -25,3 +25,28 @@ console.log(name, "content script loaded"); runTest().catch((err) => { console.error("content script error", err); }); + +test("sendMessage with returned rejected Promise with Error value", async (t) => { + try { + const reply = await browser.runtime.sendMessage( + "test - sendMessage with returned rejected Promise with Error value"); + t.fail(`Unexpected successfully reply while expecting a rejected promise`); + t.equal(reply, undefined, "Unexpected successfully reply"); + } catch (err) { + t.equal(err.message, "rejected-error-value", "Got an error rejection with the expected message"); + } +}); + +test("sendMessage with returned rejected Promise with non-Error value", async (t) => { + try { + const reply = await browser.runtime.sendMessage( + "test - sendMessage with returned rejected Promise with non-Error value"); + t.fail(`Unexpected successfully reply while expecting a rejected promise`); + t.equal(reply, undefined, "Unexpected successfully reply"); + } catch (err) { + // Unfortunately Firefox currently reject an error with an undefined + // message, in the meantime we just check that the object rejected is + // an instance of Error. + t.ok(err instanceof Error, "Got an error object as expected"); + } +}); From b16ee4f6080bf715584c0f39e1a84d26a3b7692b Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Thu, 10 May 2018 23:55:05 +0200 Subject: [PATCH 3/3] test(browsers-smoketests): Run a set of smoke tests on both Chrome and Firefox This commit introduces tape as the test framework used to define the tests in the test extension contexts and send them to the nodejs script that orchestrate the test run. The nodejs script has also been migrated from mocha to tape, it uses the custom test helpers provided to setup the test environment (e.g. create a temporary dir for the test extension, copy the last polyfill build, bundle tape to be used in the test extension, start the browser which run the test extension and finally collect the results of the test extension) and then it merges all the tap logs collected from every test extension into a single "per browser" test suite. - updated travis nodejs environment to nodejs 8 - uses tape to collect test results from inside the test extension - added test case to check polyfill 'existing browser API object' detection - added test for expected rejection on tabs.sendMessage with an invalid tabId - added test with multiple listeners which resolves to undefined and null - optionally run chrome smoketests with --enable-features=NativeCrxBindings --- .travis.yml | 12 +- package.json | 14 +- test/fixtures/browserify-tape.js | 12 ++ .../content.js | 18 ++ .../manifest.json | 19 ++ .../background.js | 39 ++++ .../content.js | 18 ++ .../manifest.json | 25 +++ .../runtime-messaging-extension/background.js | 6 + .../runtime-messaging-extension/content.js | 75 ++++--- .../runtime-messaging-extension/manifest.json | 1 + .../tabs-sendmessage-extension/background.js | 36 ++++ .../tabs-sendmessage-extension/content.js | 8 + .../tabs-sendmessage-extension/manifest.json | 25 +++ test/fixtures/tape-standalone.js | 47 +++++ test/integration/setup.js | 195 ++++++++++++++++-- .../integration/test-extensions-in-browser.js | 23 +++ .../test-runtime-messaging-on-chrome.js | 89 -------- test/run-browsers-smoketests.sh | 14 ++ test/run-chrome-smoketests.sh | 21 -- 20 files changed, 538 insertions(+), 159 deletions(-) create mode 100644 test/fixtures/browserify-tape.js create mode 100644 test/fixtures/detect-browser-api-object-in-content-script/content.js create mode 100644 test/fixtures/detect-browser-api-object-in-content-script/manifest.json create mode 100644 test/fixtures/multiple-onmessage-listeners-extension/background.js create mode 100644 test/fixtures/multiple-onmessage-listeners-extension/content.js create mode 100644 test/fixtures/multiple-onmessage-listeners-extension/manifest.json create mode 100644 test/fixtures/tabs-sendmessage-extension/background.js create mode 100644 test/fixtures/tabs-sendmessage-extension/content.js create mode 100644 test/fixtures/tabs-sendmessage-extension/manifest.json create mode 100644 test/fixtures/tape-standalone.js create mode 100644 test/integration/test-extensions-in-browser.js delete mode 100644 test/integration/test-runtime-messaging-on-chrome.js create mode 100755 test/run-browsers-smoketests.sh delete mode 100755 test/run-chrome-smoketests.sh diff --git a/.travis.yml b/.travis.yml index 5192f69f..9030a0b2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,8 @@ language: node_js sudo: false node_js: ## Some of the ES6 syntax used in the browser-polyfill sources is only supported on nodejs >= 6 -- '6' +## and the selenium-webdriver dependency used by the integration tests requires nodejs >= 8. +- '8' script: - npm run build @@ -14,7 +15,14 @@ script: - export DISPLAY=:99.0 - sh -e /etc/init.d/xvfb start - echo "RUN integration tests on chrome" && - TRAVIS_CI=true ./test/run-chrome-smoketests.sh + TRAVIS_CI=true ./test/run-browsers-smoketests.sh + +## See https://docs.travis-ci.com/user/chrome +sudo: required + +addons: + firefox: 'latest' + chrome: 'stable' after_script: npm run publish-coverage diff --git a/package.json b/package.json index 5db72286..9faccdb2 100644 --- a/package.json +++ b/package.json @@ -22,9 +22,13 @@ "babel-plugin-transform-es2015-modules-umd": "^6.24.1", "babel-preset-babili": "^0.0.10", "babel-preset-es2017": "^6.24.1", + "browserify": "^16.2.2", "chai": "^3.5.0", + "chromedriver": "^2.38.3", "eslint": "^3.9.1", "finalhandler": "^1.1.0", + "geckodriver": "^1.11.0", + "global-replaceify": "^1.0.0", "grunt": "^1.0.1", "grunt-babel": "^6.0.0", "grunt-contrib-concat": "^1.0.1", @@ -35,9 +39,13 @@ "jsdom": "^9.6.0", "mocha": "^3.1.0", "nyc": "^8.3.1", - "puppeteer": "^0.10.2", + "selenium-webdriver": "^4.0.0-alpha.1", "serve-static": "^1.13.1", - "sinon": "^1.17.6" + "shelljs": "^0.8.2", + "sinon": "^1.17.6", + "tap-nirvana": "^1.0.8", + "tape-async": "^2.3.0", + "tmp": "0.0.33" }, "nyc": { "reporter": [ @@ -54,6 +62,6 @@ "test": "mocha", "test-coverage": "COVERAGE=y nyc mocha", "test-minified": "TEST_MINIFIED_POLYFILL=1 mocha", - "test-integration": "mocha -r test/mocha-babel test/integration/test-*" + "test-integration": "tape test/integration/test-*" } } diff --git a/test/fixtures/browserify-tape.js b/test/fixtures/browserify-tape.js new file mode 100644 index 00000000..8bb9ab24 --- /dev/null +++ b/test/fixtures/browserify-tape.js @@ -0,0 +1,12 @@ +const browserify = require("browserify"); + +const b = browserify(); + +b.add("./test/fixtures/tape-standalone.js"); +b.transform("global-replaceify", { + global: true, + replacements: { + setImmediate: "require('timers').setImmediate", + }, +}); +b.bundle().pipe(process.stdout); diff --git a/test/fixtures/detect-browser-api-object-in-content-script/content.js b/test/fixtures/detect-browser-api-object-in-content-script/content.js new file mode 100644 index 00000000..99957e4e --- /dev/null +++ b/test/fixtures/detect-browser-api-object-in-content-script/content.js @@ -0,0 +1,18 @@ +test("browser api object in content script", (t) => { + t.ok(browser && browser.runtime, "a global browser API object should be defined"); + t.ok(chrome && chrome.runtime, "a global chrome API object should be defined"); + + if (navigator.userAgent.includes("Firefox/")) { + // Check that the polyfill didn't create a polyfill wrapped browser API object on Firefox. + t.equal(browser.runtime, chrome.runtime, "browser.runtime and chrome.runtime should be equal on Firefox"); + // On Firefox, window is not the global object for content scripts, and so we expect window.browser to not + // be defined. + t.equal(window.browser, undefined, "window.browser is expected to be undefined on Firefox"); + } else { + // Check that the polyfill has created a wrapped API namespace as expected. + t.notEqual(browser.runtime, chrome.runtime, "browser.runtime and chrome.runtime should not be equal"); + // On chrome, window is the global object and so the polyfilled browser API should + // be also equal to window.browser. + t.equal(browser, window.browser, "browser and window.browser should be the same object"); + } +}); diff --git a/test/fixtures/detect-browser-api-object-in-content-script/manifest.json b/test/fixtures/detect-browser-api-object-in-content-script/manifest.json new file mode 100644 index 00000000..582316bf --- /dev/null +++ b/test/fixtures/detect-browser-api-object-in-content-script/manifest.json @@ -0,0 +1,19 @@ +{ + "manifest_version": 2, + "name": "test-detect-browser-api-object-in-content-script", + "version": "0.1", + "description": "test-detect-browser-api-object-in-content-script", + "content_scripts": [ + { + "matches": [ + "http://localhost/*" + ], + "js": [ + "browser-polyfill.js", + "tape.js", + "content.js" + ] + } + ], + "permissions": [] +} diff --git a/test/fixtures/multiple-onmessage-listeners-extension/background.js b/test/fixtures/multiple-onmessage-listeners-extension/background.js new file mode 100644 index 00000000..40a1ae6d --- /dev/null +++ b/test/fixtures/multiple-onmessage-listeners-extension/background.js @@ -0,0 +1,39 @@ +console.log(name, "background page loaded"); + +async function testMessageHandler(msg, sender) { + console.log(name, "background received msg", {msg, sender}); + + // We only expect messages coming from a content script in this test. + if (!sender.tab || !msg.startsWith("test-multiple-onmessage-listeners:")) { + return { + success: false, + failureReason: `An unexpected message has been received: ${JSON.stringify({msg, sender})}`, + }; + } + + if (msg.endsWith(":resolve-to-undefined")) { + return undefined; + } + + if (msg.endsWith(":resolve-to-null")) { + return null; + } + + return { + success: false, + failureReason: `An unexpected message has been received: ${JSON.stringify({msg, sender})}`, + }; +} + +// Register the same message handler twice. +browser.runtime.onMessage.addListener(testMessageHandler); +browser.runtime.onMessage.addListener(testMessageHandler); + +// Register an additional message handler that always reply after +// a small latency time. +browser.runtime.onMessage.addListener(async (msg, sender) => { + await new Promise(resolve => setTimeout(resolve, 100)); + return "resolved-to-string-with-latency"; +}); + +console.log(name, "background page ready to receive a content script message..."); diff --git a/test/fixtures/multiple-onmessage-listeners-extension/content.js b/test/fixtures/multiple-onmessage-listeners-extension/content.js new file mode 100644 index 00000000..f38c3eab --- /dev/null +++ b/test/fixtures/multiple-onmessage-listeners-extension/content.js @@ -0,0 +1,18 @@ +test("Multiple runtime.onmessage listeners which resolve to undefined", async (t) => { + const res = await browser.runtime.sendMessage("test-multiple-onmessage-listeners:resolve-to-undefined"); + + if (navigator.userAgent.includes("Firefox/")) { + t.deepEqual(res, undefined, "Got an undefined value as expected"); + } else { + // NOTE: When an onMessage listener sends `undefined` in a response, + // Chrome internally converts it to null and the receiver receives it + // as a null object. + t.deepEqual(res, null, "Got a null value as expected on Chrome"); + } +}); + +test("Multiple runtime.onmessage listeners which resolve to null", async (t) => { + const res = await browser.runtime.sendMessage("test-multiple-onmessage-listeners:resolve-to-null"); + + t.deepEqual(res, null, "Got a null value as expected"); +}); diff --git a/test/fixtures/multiple-onmessage-listeners-extension/manifest.json b/test/fixtures/multiple-onmessage-listeners-extension/manifest.json new file mode 100644 index 00000000..e090814d --- /dev/null +++ b/test/fixtures/multiple-onmessage-listeners-extension/manifest.json @@ -0,0 +1,25 @@ +{ + "manifest_version": 2, + "name": "test-multiple-onmessage-listeners", + "version": "0.1", + "description": "test-multiple-onmessage-listeners", + "content_scripts": [ + { + "matches": [ + "http://localhost/*" + ], + "js": [ + "browser-polyfill.js", + "tape.js", + "content.js" + ] + } + ], + "permissions": [], + "background": { + "scripts": [ + "browser-polyfill.js", + "background.js" + ] + } +} diff --git a/test/fixtures/runtime-messaging-extension/background.js b/test/fixtures/runtime-messaging-extension/background.js index a42b9532..c090a8e5 100644 --- a/test/fixtures/runtime-messaging-extension/background.js +++ b/test/fixtures/runtime-messaging-extension/background.js @@ -39,6 +39,12 @@ browser.runtime.onMessage.addListener((msg, sender, sendResponse) => { case "test - sendMessage with returned rejected Promise with non-Error value": return Promise.reject("rejected-non-error-value"); + case "test - sendMessage with returned rejected Promise with non-Error value with message property": + return Promise.reject({message: "rejected-non-error-message"}); + + case "test - sendMessage with listener callback throws": + throw new Error("listener throws"); + default: return Promise.resolve( `Unxpected message received by the background page: ${JSON.stringify(msg)}\n`); diff --git a/test/fixtures/runtime-messaging-extension/content.js b/test/fixtures/runtime-messaging-extension/content.js index b123505d..5773af24 100644 --- a/test/fixtures/runtime-messaging-extension/content.js +++ b/test/fixtures/runtime-messaging-extension/content.js @@ -1,29 +1,26 @@ -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); +test("sendMessage with returned Promise reply", async (t) => { + const reply = await browser.runtime.sendMessage("test - sendMessage with returned Promise reply"); + t.equal(reply, "bg page reply 1"); +}); - reply = await browser.runtime.sendMessage("test - second listener if the first does not reply"); - console.log(name, "test - second listener sendResponse - received", reply); +test("sendMessage with returned value reply", async (t) => { + const reply = await browser.runtime.sendMessage("test - sendMessage with returned value reply"); + t.equal(reply, "second listener reply"); +}); - console.log(name, "content script messages sent"); -} +test("sendMessage with synchronous sendResponse", async (t) => { + const reply = await browser.runtime.sendMessage("test - sendMessage with synchronous sendResponse"); + t.equal(reply, "bg page reply 3"); +}); -console.log(name, "content script loaded"); +test("sendMessage with asynchronous sendResponse", async (t) => { + const reply = await browser.runtime.sendMessage("test - sendMessage with asynchronous sendResponse"); + t.equal(reply, "bg page reply 4"); +}); -runTest().catch((err) => { - console.error("content script error", err); +test("second listener if the first does not reply", async (t) => { + const reply = await browser.runtime.sendMessage("test - second listener if the first does not reply"); + t.equal(reply, "second listener reply"); }); test("sendMessage with returned rejected Promise with Error value", async (t) => { @@ -33,6 +30,7 @@ test("sendMessage with returned rejected Promise with Error value", async (t) => t.fail(`Unexpected successfully reply while expecting a rejected promise`); t.equal(reply, undefined, "Unexpected successfully reply"); } catch (err) { + t.ok(err instanceof Error, "Got an error object as expected"); t.equal(err.message, "rejected-error-value", "Got an error rejection with the expected message"); } }); @@ -44,9 +42,36 @@ test("sendMessage with returned rejected Promise with non-Error value", async (t t.fail(`Unexpected successfully reply while expecting a rejected promise`); t.equal(reply, undefined, "Unexpected successfully reply"); } catch (err) { - // Unfortunately Firefox currently reject an error with an undefined - // message, in the meantime we just check that the object rejected is - // an instance of Error. + // Unfortunately Firefox currently rejects an error with an "undefined" + // message in Firefox 60 and "An unexpected error occurred" in Firefox 59, + // in the meantime we just check that the object rejected is an instance + // of Error. + t.ok(err instanceof Error, "Got an error object as expected"); + } +}); + +test("sendMessage with returned rejected Promise with non-Error value with message property", async (t) => { + try { + const reply = await browser.runtime.sendMessage( + "test - sendMessage with returned rejected Promise with non-Error value with message property"); + t.fail(`Unexpected successfully reply while expecting a rejected promise`); + t.equal(reply, undefined, "Unexpected successfully reply"); + } catch (err) { + // Firefox currently converts any rejection with a message property into an error instance + // with the value of that message property as the error message. + t.ok(err instanceof Error, "Got an error object as expected"); + t.equal(err.message, "rejected-non-error-message", "Got an error rejection with the expected message"); + } +}); + +test("sendMessage with listener callback throws", async (t) => { + try { + const reply = await browser.runtime.sendMessage( + "test - sendMessage with listener callback throws"); + t.fail(`Unexpected successfully reply while expecting a rejected promise`); + t.equal(reply, undefined, "Unexpected successfully reply"); + } catch (err) { t.ok(err instanceof Error, "Got an error object as expected"); + t.equal(err.message, "listener throws", "Got an error with the expected message"); } }); diff --git a/test/fixtures/runtime-messaging-extension/manifest.json b/test/fixtures/runtime-messaging-extension/manifest.json index e4d5aea5..839c6591 100644 --- a/test/fixtures/runtime-messaging-extension/manifest.json +++ b/test/fixtures/runtime-messaging-extension/manifest.json @@ -10,6 +10,7 @@ ], "js": [ "browser-polyfill.js", + "tape.js", "content.js" ] } diff --git a/test/fixtures/tabs-sendmessage-extension/background.js b/test/fixtures/tabs-sendmessage-extension/background.js new file mode 100644 index 00000000..2c8516f5 --- /dev/null +++ b/test/fixtures/tabs-sendmessage-extension/background.js @@ -0,0 +1,36 @@ +console.log(name, "background page loaded"); + +browser.runtime.onMessage.addListener(async (msg, sender, sendResponse) => { + console.log(name, "background received msg", {msg, sender}); + + // We only expect messages coming from a content script in this test. + if (!sender.tab || msg != "test-tabssendMessage-unknown-tabid") { + return { + success: false, + failureReason: `An unexpected message has been received: ${JSON.stringify({msg, sender})}`, + }; + } + + try { + const tabs = await browser.tabs.query({}); + const lastValidTabId = tabs.reduce((acc, tab) => { + return Math.max(acc, tab.id); + }, 0); + const INVALID_TABID = lastValidTabId + 100; + + await browser.tabs.sendMessage(INVALID_TABID, "message-to-unknown-tab"); + + return { + success: false, + failureReason: `browser.tabs.sendMessage should reject on sending messages to non-existing tab`, + }; + } catch (err) { + return { + success: true, + isRejected: true, + errorMessage: err.message, + }; + } +}); + +console.log(name, "background page ready to receive a content script message..."); diff --git a/test/fixtures/tabs-sendmessage-extension/content.js b/test/fixtures/tabs-sendmessage-extension/content.js new file mode 100644 index 00000000..bdfd3ab1 --- /dev/null +++ b/test/fixtures/tabs-sendmessage-extension/content.js @@ -0,0 +1,8 @@ +test("tabs.sendMessage reject when sending to unknown tab id", async (t) => { + const res = await browser.runtime.sendMessage("test-tabssendMessage-unknown-tabid"); + t.deepEqual(res, { + success: true, + isRejected: true, + errorMessage: "Could not establish connection. Receiving end does not exist.", + }, "The background page got a rejection as expected"); +}); diff --git a/test/fixtures/tabs-sendmessage-extension/manifest.json b/test/fixtures/tabs-sendmessage-extension/manifest.json new file mode 100644 index 00000000..16d7aa95 --- /dev/null +++ b/test/fixtures/tabs-sendmessage-extension/manifest.json @@ -0,0 +1,25 @@ +{ + "manifest_version": 2, + "name": "test-tabs-sendmessage", + "version": "0.1", + "description": "test-tabs-sendmessage", + "content_scripts": [ + { + "matches": [ + "http://localhost/*" + ], + "js": [ + "browser-polyfill.js", + "tape.js", + "content.js" + ] + } + ], + "permissions": [], + "background": { + "scripts": [ + "browser-polyfill.js", + "background.js" + ] + } +} diff --git a/test/fixtures/tape-standalone.js b/test/fixtures/tape-standalone.js new file mode 100644 index 00000000..1face237 --- /dev/null +++ b/test/fixtures/tape-standalone.js @@ -0,0 +1,47 @@ +const tape = require("tape-async"); + +const DEFAULT_TIMEOUT = 500; + +let browser = "unknown"; +if (navigator.userAgent.includes("Chrome/")) { + browser = "Chrome"; +} else if (navigator.userAgent.includes("Firefox/")) { + browser = "Firefox"; +} + +// Export as a global a wrapped test function which enforces a timeout by default. +window.test = (desc, fn) => { + tape(`${desc} (${browser})`, async (t) => { + t.timeoutAfter(DEFAULT_TIMEOUT); + await fn(t); + }); +}; + +// Export the rest of the property usually available on the tape test object. +window.test.skip = tape.skip.bind(tape); +window.test.onFinish = tape.onFinish.bind(tape); +window.test.onFailure = tape.onFailure.bind(tape); + +// Configure dump test results into an HTML pre element +// added to the test page. +const stream = tape.createStream(); +let results = ""; +stream.on("data", (result) => { + // Skip the TAP protocol version from the collected logs. + if (!result.startsWith("TAP version")) { + console.log("TAP test result:", result); + results += result; + } +}); +stream.on("end", () => { + try { + const el = document.createElement("pre"); + el.setAttribute("id", "test-results"); + el.textContent = results; + document.body.appendChild(el); + } catch (err) { + console.error(err); + } finally { + console.log("TAP tests completed."); + } +}); diff --git a/test/integration/setup.js b/test/integration/setup.js index 59934fd4..8d97cd12 100644 --- a/test/integration/setup.js +++ b/test/integration/setup.js @@ -1,12 +1,89 @@ -const finalhandler = require("finalhandler"); +"use strict"; + +const fs = require("fs"); const http = require("http"); +const path = require("path"); + +const browserify = require("browserify"); +const finalhandler = require("finalhandler"); const serveStatic = require("serve-static"); -const puppeteer = require("puppeteer"); +const {Builder, By, until} = require("selenium-webdriver"); + +const test = require("tape-async"); +const tmp = require("tmp"); +const {cp} = require("shelljs"); + +const TEST_TIMEOUT = 5000; + +const launchBrowser = async (launchOptions) => { + const browser = launchOptions.browser || process.env.TEST_BROWSER_TYPE; + const extensionPath = launchOptions.extensionPath; + + let driver; + + if (browser === "chrome") { + const chrome = require("selenium-webdriver/chrome"); + const chromedriver = require("chromedriver"); + + if (process.env.HEADLESS === "1") { + console.warn("WARN: Chrome doesn't currently support extensions in headless mode. " + + "Falling back to non-headless mode"); + } -exports.createHTTPServer = async (path) => { - var serve = serveStatic(path); + const options = new chrome.Options(); + options.addArguments([ + `--load-extension=${extensionPath}`, + // See https://docs.travis-ci.com/user/chrome and issue #85 for a rationale. + "--no-sandbox", + ]); + + if (process.env.TEST_NATIVE_CRX_BINDINGS === "1") { + console.warn("NOTE: Running tests on a Chrome instance with NativeCrxBindings enabled."); + options.addArguments([ + "--enable-features=NativeCrxBindings", + ]); + } + + driver = await new Builder() + .forBrowser("chrome") + .setChromeOptions(options) + .setChromeService(new chrome.ServiceBuilder(chromedriver.path)) + .build(); + } else if (browser === "firefox") { + const firefox = require("selenium-webdriver/firefox"); + const geckodriver = require("geckodriver"); + const {Command} = require("selenium-webdriver/lib/command"); + + const options = new firefox.Options(); + + if (process.env.HEADLESS === "1") { + options.headless(); + } + + driver = await new Builder() + .forBrowser("firefox") + .setFirefoxOptions(options) + .setFirefoxService(new firefox.ServiceBuilder(geckodriver.path)) + .build(); + + const command = new Command("install addon") + .setParameter("path", extensionPath) + .setParameter("temporary", true); + + await driver.execute(command); + } else { + const errorHelpMsg = ( + "Set a supported browser (firefox or chrome) " + + "using the TEST_BROWSER_TYPE environment var."); + throw new Error(`Target browser not supported yet: ${browser}. ${errorHelpMsg}`); + } + + return driver; +}; - var server = http.createServer((req, res) => { +const createHTTPServer = async (path) => { + const serve = serveStatic(path); + const server = http.createServer((req, res) => { serve(req, res, finalhandler(req, res)); }); @@ -21,24 +98,104 @@ exports.createHTTPServer = async (path) => { }); }; -exports.launchPuppeteer = async (puppeteerArgs) => { - if (!puppeteerArgs || !Array.isArray(puppeteerArgs)) { - throw new Error(`Invalid puppeteer arguments: ${JSON.stringify(puppeteerArgs)}`); - } +async function runExtensionTest(t, server, driver, extensionDirName) { + try { + const url = `http://localhost:${server.address().port}`; + const userAgent = await driver.executeScript(() => window.navigator.userAgent); + + t.pass(`Connected to browser: ${userAgent}"`); - const args = [].concat(puppeteerArgs); + await driver.get(url); - // Pass the --no-sandbox chrome CLI option when running the integration tests - // on Travis. - if (process.env.TRAVIS_CI) { - args.push("--no-sandbox"); + // Merge tap results from the connected browser. + const el = await driver.wait(until.elementLocated(By.id("test-results")), 10000); + const testResults = await el.getAttribute("textContent"); + console.log(testResults); + } catch (err) { + t.fail(err); } +} + +const awaitStreamEnd = (stream) => { + return new Promise((resolve, reject) => { + stream.on("end", resolve); + stream.on("error", reject); + }); +}; - return puppeteer.launch({ - // Chrome Extensions are not currently supported in headless mode. - headless: false, +const bundleTapeStandalone = async (destDir) => { + const bundleFileName = path.join(destDir, "tape.js"); + const b = browserify(); + b.add(path.join(__dirname, "..", "fixtures", "tape-standalone.js")); - // Custom chrome arguments. - args, + // Inject setImmediate (used internally by tape). + b.transform("global-replaceify", { + global: true, + replacements: { + setImmediate: "require('timers').setImmediate", + }, }); + + const stream = b.bundle(); + const onceStreamEnd = awaitStreamEnd(stream); + stream.pipe(fs.createWriteStream(bundleFileName)); + + await onceStreamEnd; +}; + +test.onFailure(() => { + process.exit(1); +}); + +const defineExtensionTests = ({description, extensions}) => { + for (const extensionDirName of extensions) { + test(`${description} (test extension: ${extensionDirName})`, async (tt) => { + let timeout; + let driver; + let server; + let tempDir; + + try { + const srcExtensionPath = path.resolve( + path.join(__dirname, "..", "fixtures", extensionDirName)); + const srcPolyfill = path.join(__dirname, "..", "..", "dist", "browser-polyfill.js"); + + const tmpDir = tmp.dirSync({unsafeCleanup: true}); + const extensionPath = path.join(tmpDir.name, extensionDirName); + + cp("-rf", srcExtensionPath, extensionPath); + cp("-f", srcPolyfill, extensionPath); + cp("-f", `${srcPolyfill}.map`, extensionPath); + await bundleTapeStandalone(extensionPath); + + server = await createHTTPServer(path.join(__dirname, "..", "fixtures")); + driver = await launchBrowser({extensionPath}); + await Promise.race([ + runExtensionTest(tt, server, driver, extensionDirName), + new Promise((resolve, reject) => { + timeout = setTimeout(() => reject(new Error(`test timeout after ${TEST_TIMEOUT}`)), TEST_TIMEOUT); + }), + ]); + } finally { + clearTimeout(timeout); + if (driver) { + await driver.quit(); + driver = null; + } + if (server) { + server.close(); + server = null; + } + if (tempDir) { + tempDir.removeCallback(); + } + } + }); + } +}; + +module.exports = { + launchBrowser, + createHTTPServer, + defineExtensionTests, }; diff --git a/test/integration/test-extensions-in-browser.js b/test/integration/test-extensions-in-browser.js new file mode 100644 index 00000000..032f34f7 --- /dev/null +++ b/test/integration/test-extensions-in-browser.js @@ -0,0 +1,23 @@ +"use strict"; + +const {defineExtensionTests} = require("./setup"); + +defineExtensionTests({ + description: "browser.runtime.onMessage/sendMessage", + extensions: ["runtime-messaging-extension"], +}); + +defineExtensionTests({ + description: "browser.runtime.onMessage/sendMessage", + extensions: ["tabs-sendmessage-extension"], +}); + +defineExtensionTests({ + description: "browser.runtime.onMessage/sendMessage", + extensions: ["multiple-onmessage-listeners-extension"], +}); + +defineExtensionTests({ + description: "polyfill should detect an existent browser API object in content scripts", + extensions: ["detect-browser-api-object-in-content-script"], +}); diff --git a/test/integration/test-runtime-messaging-on-chrome.js b/test/integration/test-runtime-messaging-on-chrome.js deleted file mode 100644 index 64ab17bb..00000000 --- a/test/integration/test-runtime-messaging-on-chrome.js +++ /dev/null @@ -1,89 +0,0 @@ -"use strict"; - -const path = require("path"); - -const waitUntil = require("async-wait-until"); -const {deepEqual} = require("chai").assert; - -const {createHTTPServer, launchPuppeteer} = require("./setup"); - -const fixtureExtensionDirName = "runtime-messaging-extension"; - -const extensionName = require(`../fixtures/${fixtureExtensionDirName}/manifest.json`).name; - -describe("browser.runtime.onMessage/sendMessage", function() { - this.timeout(10000); - - it("works as expected on Chrome", async () => { - const server = await createHTTPServer(path.join(__dirname, "..", "fixtures")); - - const url = `http://localhost:${server.address().port}`; - - const browser = await launchPuppeteer([ - `--load-extension=${process.env.TEST_EXTENSIONS_PATH}/${fixtureExtensionDirName}`, - ]); - - const page = await browser.newPage(); - - const pageConsoleMessages = []; - const pageErrors = []; - - page.on("console", (...args) => { - pageConsoleMessages.push(args); - }); - - page.on("error", (error) => { - pageErrors.push(error); - }); - - await page.goto(url); - - const expectedConsoleMessages = [ - [extensionName, "content script loaded"], - [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(); - - let unexpectedException; - - try { - // Wait until the last expected message has been received. - await waitUntil(() => { - return pageConsoleMessages.filter((msg) => { - return msg[0] === lastExpectedMessage[0] && msg[1] === lastExpectedMessage[1]; - }).length > 0; - }, 5000); - } catch (error) { - // Collect any unexpected exception (e.g. a timeout error raised by waitUntil), - // it will be part of the deepEqual assertion of the results. - unexpectedException = error; - } - - let actualResults = { - consoleMessages: pageConsoleMessages, - unexpectedException, - }; - - let expectedResults = { - consoleMessages: expectedConsoleMessages, - unexpectedException: undefined, - }; - - try { - deepEqual(actualResults, expectedResults, "Got the expected results"); - } finally { - // ensure that we close the browser and the test HTTP server before exiting - // the test, even when the assertions fails. - await Promise.all([ - browser.close(), - new Promise(resolve => server.close(resolve)), - ]); - } - }); -}); diff --git a/test/run-browsers-smoketests.sh b/test/run-browsers-smoketests.sh new file mode 100755 index 00000000..363b1e89 --- /dev/null +++ b/test/run-browsers-smoketests.sh @@ -0,0 +1,14 @@ +echo "\nTest webextension-polyfill on real browsers" +echo "============================================" + +export PATH=$PATH:./node_modules/.bin/ + +## HEADLESS=1 Enable the headless mode (currently used only on Firefox +## because Chrome doesn't currently support the extensions in headless mode) +export HEADLESS=1 + +echo "\nRun smoketests on Chrome" +TEST_BROWSER_TYPE=chrome npm run test-integration | tap-nirvana + +echo "\nRun smoketests on Firefox" +TEST_BROWSER_TYPE=firefox npm run test-integration | tap-nirvana \ No newline at end of file diff --git a/test/run-chrome-smoketests.sh b/test/run-chrome-smoketests.sh deleted file mode 100755 index f91553ed..00000000 --- a/test/run-chrome-smoketests.sh +++ /dev/null @@ -1,21 +0,0 @@ -echo "\nTest webextension-polyfill from an extension running on chrome" -echo "===============================================" - -export TEST_EXTENSIONS_PATH=/tmp/browser-polyfill-chrome-smoketests - -MARKER_FILE=$TEST_EXTENSIONS_PATH/.created-for-run-chrome-smoketests - -# Check if the marker file exists and then remove the directory. -if [ -f $MARKER_FILE ]; then - rm -fr $TEST_EXTENSIONS_PATH -fi - -## Exits immediately if the directory already exists (which can only happen in a local -## development environment, while this test will usually run on travis). -mkdir $TEST_EXTENSIONS_PATH || exit 1 -touch $MARKER_FILE - -cp -rf test/fixtures/runtime-messaging-extension $TEST_EXTENSIONS_PATH -cp -rf dist/browser-polyfill.js* $TEST_EXTENSIONS_PATH/runtime-messaging-extension/ - -npm run test-integration