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).
This commit is contained in:
Luca Greco 2017-09-22 23:38:44 +02:00
parent 6b3c6da659
commit 61d92ea79b
4 changed files with 134 additions and 16 deletions

View File

@ -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,

View File

@ -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

View File

@ -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");
});
});
});
});
});

View File

@ -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");
});
});