From 917ed413f62a841f84a9ac5db41c54379503017f Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 26 Sep 2017 14:32:06 +0200 Subject: [PATCH] fix: Add a new fallbackToNoCallback metadata property to customize the pageAction.show/hide wrappers behavior --- api-metadata.json | 20 +++++++- src/browser-polyfill.js | 34 +++++++++++++- test/test-async-functions.js | 88 ++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/api-metadata.json b/api-metadata.json index 1b52b28..7426764 100644 --- a/api-metadata.json +++ b/api-metadata.json @@ -295,21 +295,37 @@ "minArgs": 1, "maxArgs": 1 }, + "setPopup": { + "minArgs": 1, + "maxArgs": 1, + "fallbackToNoCallback": true + }, "getTitle": { "minArgs": 1, "maxArgs": 1 }, + "setTitle": { + "minArgs": 1, + "maxArgs": 1, + "fallbackToNoCallback": true + }, "hide": { "minArgs": 1, - "maxArgs": 1 + "maxArgs": 1, + "fallbackToNoCallback": true }, "setIcon": { "minArgs": 1, "maxArgs": 1 }, - "show": { + "getIcon": { "minArgs": 1, "maxArgs": 1 + }, + "show": { + "minArgs": 1, + "maxArgs": 1, + "fallbackToNoCallback": true } }, "runtime": { diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index 6178877..10f23ea 100644 --- a/src/browser-polyfill.js +++ b/src/browser-polyfill.js @@ -132,7 +132,39 @@ if (typeof browser === "undefined") { } return new Promise((resolve, reject) => { - target[name](...args, makeCallback({resolve, reject}, metadata)); + function callAPIWithNoCallback() { + try { + target[name](...args); + + // Update the API method metadata, so that the next API calls will not try to + // use the unsupported callback anymore. + metadata.fallbackToNoCallback = false; + metadata.noCallback = true; + + resolve(); + } catch (error) { + // Catch API parameters validation errors and rejects the promise. + reject(error); + } + } + + if (metadata.fallbackToNoCallback) { + // This API method has currently no callback on Chrome, but it return a promise on Firefox, + // and so the polyfill will try to call it with a callback first, and it will fallback + // to not passing the callback if the first call fails. + try { + target[name](...args, makeCallback({resolve, reject}, metadata)); + } catch (cbError) { + console.warn(`${name} API method doesn't seem to support the callback parameter, ` + + "falling back to call it without a callback: ", cbError); + + callAPIWithNoCallback(); + } + } else if (metadata.noCallback) { + callAPIWithNoCallback(); + } else { + target[name](...args, makeCallback({resolve, reject}, metadata)); + } }); }; }; diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 0dc18a9..dd7873a 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -141,5 +141,93 @@ describe("browser-polyfill", () => { }); }); }); + + it("returns a Promise for wrapped API methods without a callback on Chrome", () => { + const FAKE_ERROR_MSG = "API Schema validation error"; + + const fakeChrome = { + runtime: {lastError: null}, + pageAction: { + show: sinon.spy((tabId, cb) => { + if (cb) { + throw new Error("Chrome do not expect a callback"); + } + + if (tabId == null) { + throw new Error(FAKE_ERROR_MSG); + } + }), + hide: sinon.spy((tabId, cb) => { + if (cb) { + throw new Error("Chrome do not expect a callback"); + } + + if (tabId == null) { + throw new Error(FAKE_ERROR_MSG); + } + }), + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const {browser, Promise} = window; + + const pageActionShowPromise = browser.pageAction.show(1).catch(err => err); + const pageActionHidePromise = browser.pageAction.hide(undefined).catch(err => err); + + ok(pageActionShowPromise instanceof Promise, + "browser.pageAction.show returned a promise instance"); + ok(pageActionHidePromise instanceof Promise, + "browser.pageAction.hide returned a promise instance"); + + return Promise.all([ + pageActionShowPromise, pageActionHidePromise, + ]).then(([pageActionShowResolved, pageActionHideRejected]) => { + ok(fakeChrome.pageAction.show.calledTwice, "chrome.pageAction.show has been called twice"); + equal(fakeChrome.pageAction.show.firstCall.args.length, 2, + "chrome.pageAction.show first call has received a callback parameter"); + equal(fakeChrome.pageAction.show.secondCall.args.length, 1, + "chrome.pageAction.show second call has received a single parameter"); + equal(pageActionShowResolved, undefined, "pageAction.show resolved successfully"); + + ok(fakeChrome.pageAction.hide.calledTwice, "chrome.pageAction.hide has been called twice"); + equal(fakeChrome.pageAction.hide.firstCall.args.length, 2, + "chrome.pageAction.hide first call has received a callback parameter"); + equal(fakeChrome.pageAction.hide.secondCall.args.length, 1, + "chrome.pageAction.hide second call has received a single parameter"); + + ok(pageActionHideRejected instanceof Error, + "browser.pageAction.hide rejected value is an Error instance"); + equal(pageActionHideRejected.message, FAKE_ERROR_MSG, + "browser.pageAction.hide rejected error has the expected message"); + }).then(() => { + // Call pageAction.show and hide again to ensure that only after a successfull + // API call the wrapper will always call the API method without the callback parameter. + + fakeChrome.pageAction.show.reset(); + fakeChrome.pageAction.hide.reset(); + + const secondPageActionShowPromise = browser.pageAction.show(1).catch(err => err); + const secondPageActionHidePromise = browser.pageAction.hide(undefined).catch(err => err); + + return Promise.all([secondPageActionShowPromise, secondPageActionHidePromise]); + }).then(([pageActionShowResolved, pageActionHideRejected]) => { + ok(fakeChrome.pageAction.show.calledOnce, "chrome.pageAction.show has been called once"); + equal(fakeChrome.pageAction.show.firstCall.args.length, 1, + "chrome.pageAction.show call has not received a callback parameter"); + + ok(fakeChrome.pageAction.hide.calledTwice, "chrome.pageAction.hide has been called twice"); + equal(fakeChrome.pageAction.hide.firstCall.args.length, 2, + "chrome.pageAction.hide first call has received a callback parameter"); + equal(fakeChrome.pageAction.hide.secondCall.args.length, 1, + "chrome.pageAction.hide second call has received a single parameter"); + + ok(pageActionHideRejected instanceof Error, + "browser.pageAction.hide rejected value is an Error instance"); + equal(pageActionHideRejected.message, FAKE_ERROR_MSG, + "browser.pageAction.hide rejected error has the expected message"); + }); + }); + }); }); });