From 61d92ea79bcd7ecce8396a6827fa02e8631cde61 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Fri, 22 Sep 2017 23:38:44 +0200 Subject: [PATCH] feat: Added devtools.inspectedWindow.eval and devtools.panels.create to wrapped APIs. To be able to wrap the devtools API namespaces, this patch applies the following changes: - fix: Prevent Proxy violation exception on the read-only/non-configurable devtools property by using an empty object with the `chrome` API object as its prototype as the root Proxy target (the Proxy instances returned for the `chrome` API object) and add a related test case. - fix: Added support for a new `singleCallbackArg` metadata property, which prevents devtools.panels.create to resolve an array of parameters (See the related Chromium issue at https://bugs.chromium.org/p/chromium/issues/detail?id=768159) and add the related test cases. - test: Changes to the test case related to proxy getter/setter behavior on non wrapped properties: in the "deletes proxy getter/setter that are not wrapped" test case from the "test-proxied-properties.js" test file, we ensure that when a getter/setter is called for a "non-wrapped" property, the getter/setter is going to affect the original target object, unfortunately this in not true anymore for the root object (the `chrome` API object) because we are using an empty object (which has the `chrome` API object as its prototype and it is not exposed outside of the polyfill sources) as the target of the Proxy instance related to it, this change to the target of the Proxy has been needed to prevent the TypeError exception raised by the Proxy instance when we try to access the "devtools" property (which is non-configurable and read-only on the `chrome` API object). --- api-metadata.json | 15 +++++++++ src/browser-polyfill.js | 21 +++++++++--- test/test-async-functions.js | 57 ++++++++++++++++++++++++++++++++- test/test-proxied-properties.js | 57 ++++++++++++++++++++++++++------- 4 files changed, 134 insertions(+), 16 deletions(-) diff --git a/api-metadata.json b/api-metadata.json index b3542cb..14a963b 100644 --- a/api-metadata.json +++ b/api-metadata.json @@ -135,6 +135,21 @@ "maxArgs": 1 } }, + "devtools": { + "inspectedWindow": { + "eval": { + "minArgs": 1, + "maxArgs": 2 + } + }, + "panels": { + "create": { + "minArgs": 3, + "maxArgs": 3, + "singleCallbackArg": true + } + } + }, "downloads": { "download": { "minArgs": 1, diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index ecf6773..955f9e3 100644 --- a/src/browser-polyfill.js +++ b/src/browser-polyfill.js @@ -76,15 +76,20 @@ if (typeof browser === "undefined") { * The promise's resolution function. * @param {function} promise.rejection * The promise's rejection function. + * @param {object} metadata + * Metadata about the wrapped method which has created the callback. + * @param {integer} metadata.maxResolvedArgs + * The maximum number of arguments which may be passed to the + * callback created by the wrapped async function. * * @returns {function} * The generated callback function. */ - const makeCallback = promise => { + const makeCallback = (promise, metadata) => { return (...callbackArgs) => { if (chrome.runtime.lastError) { promise.reject(chrome.runtime.lastError); - } else if (callbackArgs.length === 1) { + } else if (metadata.singleCallbackArg || callbackArgs.length === 1) { promise.resolve(callbackArgs[0]); } else { promise.resolve(callbackArgs); @@ -107,6 +112,9 @@ if (typeof browser === "undefined") { * The maximum number of arguments which may be passed to the * function. If called with more than this number of arguments, the * wrapper will raise an exception. + * @param {integer} metadata.maxResolvedArgs + * The maximum number of arguments which may be passed to the + * callback created by the wrapped async function. * * @returns {function(object, ...*)} * The generated wrapper function. @@ -124,7 +132,7 @@ if (typeof browser === "undefined") { } return new Promise((resolve, reject) => { - target[name](...args, makeCallback({resolve, reject})); + target[name](...args, makeCallback({resolve, reject}, metadata)); }); }; }; @@ -340,7 +348,12 @@ if (typeof browser === "undefined") { }, }; - return wrapObject(chrome, staticWrappers, apiMetadata); + // Create an object that has the real target as its prototype + // to prevent a Proxy violation exception for the devtools API getter + // (which is a read-only non-configurable property on the original target). + const targetObject = Object.create(chrome); + + return wrapObject(targetObject, staticWrappers, apiMetadata); }; // The build process adds a UMD wrapper around this file, which makes the diff --git a/test/test-async-functions.js b/test/test-async-functions.js index 836163a..0dc18a9 100644 --- a/test/test-async-functions.js +++ b/test/test-async-functions.js @@ -1,6 +1,6 @@ "use strict"; -const {deepEqual, equal, fail, throws} = require("chai").assert; +const {deepEqual, equal, fail, ok, throws} = require("chai").assert; const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); @@ -86,5 +86,60 @@ describe("browser-polyfill", () => { }, "Expected at most 3 arguments for sendMessage(), got 4"); }); }); + + it("resolves to a single parameter on singleCallbackArg metadata property", () => { + const fakeChrome = { + runtime: {lastError: null}, + devtools: { + panels: { + create: sinon.spy((title, iconPath, panelURL, cb) => { + // when the callback of a API method which specifies the "singleCallbackArg" metadata + // is called with two parameters only the first one is resolved by the returned promise. + Promise.resolve().then(() => { + cb({isFakePanel: true}, "unwanted parameter"); + }); + }), + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(({browser}) => { + return browser.devtools.panels.create("title", "icon.png", "panel.html").then(panel => { + ok(fakeChrome.devtools.panels.create.calledOnce, + "devtools.panels.create has been called once"); + + ok("isFakePanel" in panel && panel.isFakePanel, + "Got the expected result from devtools.panels.create"); + }); + }); + }); + + it("resolves to a single undefined parameter on singleCallbackArg metadata and no params", () => { + const fakeChrome = { + runtime: {lastError: null}, + devtools: { + panels: { + create: sinon.spy((title, iconPath, panelURL, cb) => { + Promise.resolve().then(() => { + // when the callback of a API method which specifies the "singleCallbackArg" metadata + // is called without any parameter, the returned promise resolves to undefined + // instead of an empty array. + cb(); + }); + }), + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(({browser}) => { + return browser.devtools.panels.create("title", "icon.png", "panel.html").then(panel => { + ok(fakeChrome.devtools.panels.create.calledOnce, + "devtools.panels.create has been called once"); + + ok(panel === undefined, + "Got the expected undefined result from devtools.panels.create"); + }); + }); + }); }); }); diff --git a/test/test-proxied-properties.js b/test/test-proxied-properties.js index 589d0f1..138c3fb 100644 --- a/test/test-proxied-properties.js +++ b/test/test-proxied-properties.js @@ -6,6 +6,36 @@ const sinon = require("sinon"); const {setupTestDOMWindow} = require("./setup"); describe("browser-polyfill", () => { + describe("proxies non-configurable read-only properties", () => { + it("creates a proxy that doesn't raise a Proxy violation exception", () => { + const fakeChrome = {}; + + Object.defineProperty(fakeChrome, "devtools", { + enumarable: true, + configurable: false, + writable: false, + value: { + inspectedWindow: { + eval: sinon.spy(), + }, + }, + }); + + return setupTestDOMWindow(fakeChrome).then(window => { + window.browser.devtools; // eslint-disable-line + + ok(window.browser.devtools.inspectedWindow, + "The non-configurable read-only property can be accessed"); + + const res = window.browser.devtools.inspectedWindow.eval("test"); + + ok(fakeChrome.devtools.inspectedWindow.eval.calledOnce, + "The target API method has been called once"); + ok(res instanceof window.Promise, "The API method has been wrapped"); + }); + }); + }); + describe("proxies non-wrapped functions", () => { it("should proxy non-wrapped methods", () => { const fakeChrome = { @@ -75,28 +105,33 @@ describe("browser-polyfill", () => { }); it("deletes proxy getter/setter that are not wrapped", () => { - const fakeChrome = {}; + const fakeChrome = {runtime: {}}; return setupTestDOMWindow(fakeChrome).then(window => { - window.browser.newns = {newkey: "test-value"}; + // Test getter/setter behavior for non wrapped properties on + // an API namespace (because the root target of the Proxy object + // is an empty object which has the chrome API object as its + // prototype and the empty object is not exposed outside of the + // polyfill sources). + window.browser.runtime.newns = {newkey: "test-value"}; - ok("newns" in window.browser, "The custom namespace is in the wrapper"); - ok("newns" in window.chrome, "The custom namespace is in the target"); + ok("newns" in window.browser.runtime, "The custom namespace is in the wrapper"); + ok("newns" in window.chrome.runtime, "The custom namespace is in the target"); - equal(window.browser.newns.newkey, "test-value", + equal(window.browser.runtime.newns.newkey, "test-value", "Got the expected result from setting a wrapped property name"); - const setRes = window.browser.newns = {newkey2: "new-value"}; - equal(window.browser.newns.newkey2, "new-value", + const setRes = window.browser.runtime.newns = {newkey2: "new-value"}; + equal(window.browser.runtime.newns.newkey2, "new-value", "The new non-wrapped getter is cached"); deepEqual(setRes, {newkey2: "new-value"}, "Got the expected result from setting a new wrapped property name"); - deepEqual(window.browser.newns, window.chrome.newns, + deepEqual(window.browser.runtime.newns, window.chrome.runtime.newns, "chrome.newns and browser.newns are the same"); - delete window.browser.newns.newkey2; - equal(window.browser.newns.newkey2, undefined, + delete window.browser.runtime.newns.newkey2; + equal(window.browser.runtime.newns.newkey2, undefined, "Got the expected result from setting a wrapped property name"); - ok(!("newkey2" in window.browser.newns), + ok(!("newkey2" in window.browser.runtime.newns), "The deleted property is not listed anymore"); }); });