Bug 1290157 - Always pass an array to tabs.executeScript on success draft
authorRob Wu <rob@robwu.nl>
Tue, 09 Aug 2016 00:28:47 -0700
changeset 400247 b008bd3f6d7d2240ba87cb8a367534979eeb301e
parent 398491 f2c83eb65fae42e341b96d23229f163f0394676e
child 528169 3e7cfc63d2e938dc29978b5287bbd6abc5691933
push id26106
push userbmo:rob@robwu.nl
push dateFri, 12 Aug 2016 21:14:17 +0000
bugs1290157
milestone51.0a1
Bug 1290157 - Always pass an array to tabs.executeScript on success MozReview-Commit-ID: Ctw8RUtfEZC
browser/components/extensions/ext-tabs.js
browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
browser/components/extensions/test/browser/browser_ext_tabs_insertCSS.js
browser/components/extensions/test/browser/browser_ext_tabs_reload_bypass_cache.js
browser/components/extensions/test/browser/browser_ext_tabs_removeCSS.js
toolkit/components/extensions/ExtensionContent.jsm
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -808,21 +808,21 @@ extensions.registerSchemaAPI("tabs", (ex
         return context.sendMessage(mm, "Extension:Execute", {options}, {recipient});
       },
 
       executeScript: function(tabId, details) {
         return self.tabs._execute(tabId, details, "js", "executeScript");
       },
 
       insertCSS: function(tabId, details) {
-        return self.tabs._execute(tabId, details, "css", "insertCSS");
+        return self.tabs._execute(tabId, details, "css", "insertCSS").then(() => {});
       },
 
       removeCSS: function(tabId, details) {
-        return self.tabs._execute(tabId, details, "css", "removeCSS");
+        return self.tabs._execute(tabId, details, "css", "removeCSS").then(() => {});
       },
 
       connect: function(tabId, connectInfo) {
         let tab = TabManager.getTab(tabId);
         let mm = tab.linkedBrowser.messageManager;
 
         let name = "";
         if (connectInfo && connectInfo.name !== null) {
--- a/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
@@ -16,39 +16,42 @@ add_task(function* testExecuteScript() {
     browser.tabs.query({active: true, currentWindow: true}).then(tabs => {
       return browser.webNavigation.getAllFrames({tabId: tabs[0].id});
     }).then(frames => {
       browser.test.log(`FRAMES: ${frames[1].frameId} ${JSON.stringify(frames)}\n`);
       return Promise.all([
         browser.tabs.executeScript({
           code: "42",
         }).then(result => {
-          browser.test.assertEq(42, result, "Expected callback result");
+          browser.test.assertEq(1, result.length, "Expected one callback result");
+          browser.test.assertEq(42, result[0], "Expected callback result");
         }),
 
         browser.tabs.executeScript({
           file: "script.js",
           code: "42",
         }).then(result => {
           browser.test.fail("Expected not to be able to execute a script with both file and code");
         }, error => {
           browser.test.assertTrue(/a 'code' or a 'file' property, but not both/.test(error.message),
                                   "Got expected error");
         }),
 
         browser.tabs.executeScript({
           file: "script.js",
         }).then(result => {
-          browser.test.assertEq(undefined, result, "Expected callback result");
+          browser.test.assertEq(1, result.length, "Expected one callback result");
+          browser.test.assertEq(undefined, result[0], "Expected callback result");
         }),
 
         browser.tabs.executeScript({
           file: "script2.js",
         }).then(result => {
-          browser.test.assertEq(27, result, "Expected callback result");
+          browser.test.assertEq(1, result.length, "Expected one callback result");
+          browser.test.assertEq(27, result[0], "Expected callback result");
         }),
 
         browser.tabs.executeScript({
           code: "location.href;",
           allFrames: true,
         }).then(result => {
           browser.test.assertTrue(Array.isArray(result), "Result is an array");
 
@@ -57,19 +60,20 @@ add_task(function* testExecuteScript() {
           browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result[0]), "First result is correct");
           browser.test.assertEq("http://mochi.test:8888/", result[1], "Second result is correct");
         }),
 
         browser.tabs.executeScript({
           code: "location.href;",
           runAt: "document_end",
         }).then(result => {
-          browser.test.assertTrue(typeof(result) == "string", "Result is a string");
+          browser.test.assertEq(1, result.length, "Expected callback result");
+          browser.test.assertEq("string", typeof result[0], "Result is a string");
 
-          browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result), "Result is correct");
+          browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result[0]), "Result is correct");
         }),
 
         browser.tabs.executeScript({
           code: "window",
         }).then(result => {
           browser.test.fail("Expected error when returning non-structured-clonable object");
         }, error => {
           browser.test.assertEq("Script returned non-structured-clonable data",
@@ -113,17 +117,17 @@ add_task(function* testExecuteScript() {
           }).then(() => {
             return browser.tabs.remove(tab.id);
           });
         }),
 
         browser.tabs.executeScript({
           code: "Promise.resolve(42)",
         }).then(result => {
-          browser.test.assertEq(42, result, "Got expected promise resolution value as result");
+          browser.test.assertEq(42, result[0], "Got expected promise resolution value as result");
         }),
 
         browser.tabs.executeScript({
           code: "location.href;",
           runAt: "document_end",
           allFrames: true,
         }).then(result => {
           browser.test.assertTrue(Array.isArray(result), "Result is an array");
@@ -133,29 +137,31 @@ add_task(function* testExecuteScript() {
           browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result[0]), "First result is correct");
           browser.test.assertEq("http://mochi.test:8888/", result[1], "Second result is correct");
         }),
 
         browser.tabs.executeScript({
           code: "location.href;",
           frameId: frames[0].frameId,
         }).then(result => {
-          browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result), `Result for frameId[0] is correct: ${result}`);
+          browser.test.assertEq(1, result.length, "Expected one result");
+          browser.test.assertTrue(/\/file_iframe_document\.html$/.test(result[0]), `Result for frameId[0] is correct: ${result[0]}`);
         }),
 
         browser.tabs.executeScript({
           code: "location.href;",
           frameId: frames[1].frameId,
         }).then(result => {
-          browser.test.assertEq("http://mochi.test:8888/", result, "Result for frameId[1] is correct");
+          browser.test.assertEq(1, result.length, "Expected one result");
+          browser.test.assertEq("http://mochi.test:8888/", result[0], "Result for frameId[1] is correct");
         }),
 
         browser.tabs.create({url: "http://example.com/"}).then(tab => {
           return browser.tabs.executeScript(tab.id, {code: "location.href"}).then(result => {
-            browser.test.assertEq("http://example.com/", result, "Script executed correctly in new tab");
+            browser.test.assertEq("http://example.com/", result[0], "Script executed correctly in new tab");
 
             return browser.tabs.remove(tab.id);
           });
         }),
 
         new Promise(resolve => {
           browser.runtime.onMessage.addListener(message => {
             browser.test.assertEq("script ran", message, "Expected runtime message");
--- a/browser/components/extensions/test/browser/browser_ext_tabs_insertCSS.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_insertCSS.js
@@ -44,17 +44,17 @@ add_task(function* testExecuteScript() {
 
       let {promise, background, foreground} = promises.shift();
       return promise().then(result => {
         browser.test.assertEq(undefined, result, "Expected callback result");
 
         return browser.tabs.executeScript({
           code: `(${checkCSS})()`,
         });
-      }).then(result => {
+      }).then(([result]) => {
         browser.test.assertEq(background, result[0], "Expected background color");
         browser.test.assertEq(foreground, result[1], "Expected foreground color");
         return next();
       });
     }
 
     next().then(() => {
       browser.test.notifyPass("insertCSS");
--- a/browser/components/extensions/test/browser/browser_ext_tabs_reload_bypass_cache.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_reload_bypass_cache.js
@@ -28,24 +28,24 @@ add_task(function* () {
         tabId = tab.id;
         return awaitLoad(tabId);
       }).then(() => {
         return browser.tabs.reload(tabId, {bypassCache: false});
       }).then(() => {
         return awaitLoad(tabId);
       }).then(() => {
         return browser.tabs.executeScript(tabId, {code: "document.body.textContent"});
-      }).then(textContent => {
+      }).then(([textContent]) => {
         browser.test.assertEq("", textContent, "`textContent` should be empty when bypassCache=false");
         return browser.tabs.reload(tabId, {bypassCache: true});
       }).then(() => {
         return awaitLoad(tabId);
       }).then(() => {
         return browser.tabs.executeScript(tabId, {code: "document.body.textContent"});
-      }).then(textContent => {
+      }).then(([textContent]) => {
         let [pragma, cacheControl] = textContent.split(":");
         browser.test.assertEq("no-cache", pragma, "`pragma` should be set to `no-cache` when bypassCache is true");
         browser.test.assertEq("no-cache", cacheControl, "`cacheControl` should be set to `no-cache` when bypassCache is true");
         browser.tabs.remove(tabId);
         browser.test.notifyPass("tabs.reload_bypass_cache");
       }).catch(error => {
         browser.test.fail(`${error} :: ${error.stack}`);
         browser.test.notifyFail("tabs.reload_bypass_cache");
--- a/browser/components/extensions/test/browser/browser_ext_tabs_removeCSS.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_removeCSS.js
@@ -61,17 +61,17 @@ add_task(function* testExecuteScript() {
 
       let {promise, background, foreground} = promises.shift();
       return promise().then(result => {
         browser.test.assertEq(undefined, result, "Expected callback result");
 
         return browser.tabs.executeScript({
           code: `(${checkCSS})()`,
         });
-      }).then(result => {
+      }).then(([result]) => {
         browser.test.assertEq(background, result[0], "Expected background color");
         browser.test.assertEq(foreground, result[1], "Expected foreground color");
         return next();
       });
     }
 
     next().then(() => {
       browser.test.notifyPass("removeCSS");
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -635,23 +635,20 @@ DocumentManager = {
       for (let key of ["all_frames", "frame_id", "matches_about_blank", "matchesHost"]) {
         if (key in options) {
           details[key] = options[key];
         }
       }
 
       return Promise.reject({message: `No window matching ${JSON.stringify(details)}`});
     }
-    if (options.all_frames) {
-      return Promise.all(promises);
-    }
-    if (promises.length > 1) {
+    if (!options.all_frames && promises.length > 1) {
       return Promise.reject({message: `Internal error: Script matched multiple windows`});
     }
-    return promises[0];
+    return Promise.all(promises);
   },
 
   enumerateWindows: function* (docShell) {
     let window = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
                          .getInterface(Ci.nsIDOMWindow);
     yield window;
 
     for (let i = 0; i < docShell.childCount; i++) {