fix: browser.devtools.inspectedWindow.eval promise should always reso…

…lve to an array (#175) * fix: browser.devtools.inspectedWindow.eval promise should always resolve to an array * test(integration): Add test for browser.devtools.inspectedWindow.eval API
mozilla · Feb 1, 2019 · 6f178c5 · 6f178c5
1 parent e6601af
commit 6f178c5
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 4 deletions.
diff --git a/api-metadata.json b/api-metadata.json
@@ -207,7 +207,8 @@
     "inspectedWindow": {
       "eval": {
         "minArgs": 1,
-        "maxArgs": 2
+        "maxArgs": 2,
+        "singleCallbackArg": false
       }
     },
     "panels": {

diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js
@@ -92,7 +92,8 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
       return (...callbackArgs) => {
         if (extensionAPIs.runtime.lastError) {
           promise.reject(extensionAPIs.runtime.lastError);
-        } else if (metadata.singleCallbackArg || callbackArgs.length <= 1) {
+        } else if (metadata.singleCallbackArg ||
+                   (callbackArgs.length <= 1 && metadata.singleCallbackArg !== false)) {
           promise.resolve(callbackArgs[0]);
         } else {
           promise.resolve(callbackArgs);

diff --git a/test/fixtures/devtools-extension/background.js b/test/fixtures/devtools-extension/background.js
@@ -0,0 +1,12 @@
+let onDevToolsPageLoaded = new Promise(resolve => {
+  const listener = () => {
+    browser.runtime.onConnect.removeListener(listener);
+    resolve();
+  };
+  browser.runtime.onConnect.addListener(listener);
+});
+
+browser.runtime.onMessage.addListener(async msg => {
+  await onDevToolsPageLoaded;
+  return browser.runtime.sendMessage(msg);
+});
diff --git a/test/fixtures/devtools-extension/content.js b/test/fixtures/devtools-extension/content.js
@@ -0,0 +1,30 @@
+test("devtools.inspectedWindow.eval resolved with an error result", async (t) => {
+  const {evalResult} = await browser.runtime.sendMessage({
+    apiMethod: "devtools.inspectedWindow.eval",
+    params: ["throw new Error('fake error');"],
+  });
+
+  t.ok(Array.isArray(evalResult), "devtools.inspectedWindow.eval should resolve to an array");
+
+  t.equal(evalResult[0], navigator.userAgent.includes("Firefox/") ? undefined : null,
+          "the first element should be null (on chrome) or undefined (on firefox)");
+
+  t.ok(evalResult[1].isException, "the second element should represent an exception");
+  t.ok(evalResult[1].value && evalResult[1].value.includes("fake error"),
+       "the second element value property should include the expected error message");
+});
+
+test("devtools.inspectedWindow.eval resolved without an error result", async (t) => {
+  const {evalResult} = await browser.runtime.sendMessage({
+    apiMethod: "devtools.inspectedWindow.eval",
+    params: ["[document.documentElement.localName]"],
+  });
+
+  t.ok(Array.isArray(evalResult), "devtools.inspectedWindow.eval should resolve to an array");
+
+  if (navigator.userAgent.includes("Firefox/")) {
+    t.deepEqual(evalResult, [["html"], undefined], "got the expected values in the array");
+  } else {
+    t.deepEqual(evalResult, [["html"]], "got the expected values in the array");
+  }
+});
diff --git a/test/fixtures/devtools-extension/devtools_page.html b/test/fixtures/devtools-extension/devtools_page.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script src="browser-polyfill.js"></script>
+        <script src="devtools_page.js"></script>
+    </head>
+</html>
diff --git a/test/fixtures/devtools-extension/devtools_page.js b/test/fixtures/devtools-extension/devtools_page.js
@@ -0,0 +1,14 @@
+console.log("devtools page loaded");
+
+browser.runtime.onMessage.addListener(async msg => {
+  switch (msg.apiMethod) {
+    case "devtools.inspectedWindow.eval": {
+      const evalResult = await browser.devtools.inspectedWindow.eval(...msg.params);
+      return {evalResult};
+    }
+  }
+
+  throw new Error(`devtools_page received an unxpected message: ${msg}`);
+});
+
+browser.runtime.connect({name: "devtools_page"});
diff --git a/test/fixtures/devtools-extension/manifest.json b/test/fixtures/devtools-extension/manifest.json
@@ -0,0 +1,27 @@
+{
+  "manifest_version": 2,
+  "name": "test-extension-devtools-api",
+  "version": "0.1",
+  "description": "test-extension-devtools-api",
+  "content_scripts": [
+    {
+      "matches": [
+        "http://localhost/*"
+      ],
+      "js": [
+        "browser-polyfill.js",
+        "tape.js",
+        "content.js"
+      ],
+      "run_at": "document_end"
+    }
+  ],
+  "permissions": [],
+  "background": {
+    "scripts": [
+      "browser-polyfill.js",
+      "background.js"
+    ]
+  },
+  "devtools_page": "devtools_page.html"
+}
diff --git a/test/integration/setup.js b/test/integration/setup.js
@@ -18,6 +18,7 @@ const TEST_TIMEOUT = 5000;
 const launchBrowser = async (launchOptions) => {
   const browser = launchOptions.browser;
   const extensionPath = launchOptions.extensionPath;
+  const openDevTools = launchOptions.openDevTools;
 
   let driver;
 
@@ -37,6 +38,10 @@ const launchBrowser = async (launchOptions) => {
       "--no-sandbox",
     ]);
 
+    if (openDevTools) {
+      options.addArguments(["-auto-open-devtools-for-tabs"]);
+    }
+
     if (process.env.TEST_NATIVE_CRX_BINDINGS === "1") {
       console.warn("NOTE: Running tests on a Chrome instance with NativeCrxBindings enabled.");
       options.addArguments([
@@ -60,6 +65,10 @@ const launchBrowser = async (launchOptions) => {
       options.headless();
     }
 
+    if (openDevTools) {
+      options.addArguments("-devtools");
+    }
+
     driver = await new Builder()
       .forBrowser("firefox")
       .setFirefoxOptions(options)
@@ -157,7 +166,7 @@ test.onFailure(() => {
  * @param {string[]} parameters.extensions
  * @param {boolean|string|string[]} [parameters.skip]
  */
-const defineExtensionTests = ({description, extensions, skip}) => {
+const defineExtensionTests = ({description, extensions, skip, openDevTools}) => {
   for (const extensionDirName of extensions) {
     test(`${description} (test extension: ${extensionDirName})`, async (tt) => {
       let timeout;
@@ -192,7 +201,7 @@ const defineExtensionTests = ({description, extensions, skip}) => {
         await bundleTapeStandalone(extensionPath);
 
         server = await createHTTPServer(path.join(__dirname, "..", "fixtures"));
-        driver = await launchBrowser({extensionPath, browser});
+        driver = await launchBrowser({extensionPath, browser, openDevTools});
         await Promise.race([
           runExtensionTest(tt, server, driver, extensionDirName, browser),
           new Promise((resolve, reject) => {

diff --git a/test/integration/test-extensions-in-browser.js b/test/integration/test-extensions-in-browser.js
@@ -26,3 +26,9 @@ defineExtensionTests({
   description: "Instance of BrowserSetting API",
   extensions: ["privacy-setting-extension"],
 });
+
+defineExtensionTests({
+  description: "browser.devtools",
+  extensions: ["devtools-extension"],
+  openDevTools: true,
+});