Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for wrapping the devtools APIs #57

Merged
merged 1 commit into from
Sep 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions api-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 17 additions & 4 deletions src/browser-polyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand All @@ -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));
});
};
};
Expand Down Expand Up @@ -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
Expand Down
57 changes: 56 additions & 1 deletion test/test-async-functions.js
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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");
});
});
});
});
});
57 changes: 46 additions & 11 deletions test/test-proxied-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -75,28 +105,33 @@ describe("browser-polyfill", () => {
});

it("deletes proxy getter/setter that are not wrapped", () => {
const fakeChrome = {};
const fakeChrome = {runtime: {}};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these changes? Please explain the changes in the commit description.

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