Bug 1290157 - Always pass an array to tabs.executeScript on success r=kmag
authorRob Wu <rob@robwu.nl>
Tue, 09 Aug 2016 00:28:47 -0700
changeset 309439 86c39a5b482dd479d7f0d63109a9117bfc90e9ee
parent 309438 a327a15f5d55fd391882a951bc936b73ef1f8504
child 309440 620e1894c6d59e2d00c6d8ad0ec5501a56722251
push id31375
push userryanvm@gmail.com
push dateTue, 16 Aug 2016 01:31:16 +0000
treeherderautoland@86c39a5b482d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1290157
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1290157 - Always pass an array to tabs.executeScript on success r=kmag 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
@@ -804,21 +804,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, context);
         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
@@ -609,23 +609,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++) {