Bug 1210583: Part 2 - [webext] Support callbacks in tabs.executeScript/tabs.insertCSS. r=billm
authorKris Maglione <maglione.k@gmail.com>
Mon, 25 Jan 2016 20:25:11 -0800
changeset 281860 261e997621c182a03bc330c8bd18c98eddb9c7eb
parent 281859 65b634919d299de20908a8dbffe33ba8a4ab6ad9
child 281861 0ecd7d72f232304da046b352c457e039e35ceed7
push id29949
push usercbook@mozilla.com
push dateThu, 28 Jan 2016 11:00:04 +0000
treeherdermozilla-central@0ecd7d72f232 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1210583
milestone47.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 1210583: Part 2 - [webext] Support callbacks in tabs.executeScript/tabs.insertCSS. r=billm
browser/components/extensions/ext-tabs.js
browser/components/extensions/test/browser/browser.ini
browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
browser/components/extensions/test/browser/browser_ext_tabs_insertCSS.js
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionContent.jsm
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -3,18 +3,16 @@
 "use strict";
 
 XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService",
                                    "@mozilla.org/browser/aboutnewtab-service;1",
                                    "nsIAboutNewTabService");
 
 XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
                                   "resource://gre/modules/MatchPattern.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "MessageChannel",
-                                  "resource://gre/modules/MessageChannel.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
   EventManager,
@@ -448,17 +446,16 @@ extensions.registerSchemaAPI("tabs", nul
         let mm = tab.linkedBrowser.messageManager;
 
         let options = {
           js: [],
           css: [],
         };
 
         let recipient = {
-          extensionId: extension.id,
           innerWindowID: tab.linkedBrowser.innerWindowID,
         };
 
         if (TabManager.for(extension).hasActiveTabPermission(tab)) {
           // If we have the "activeTab" permission for this tab, ignore
           // the host whitelist.
           options.matchesHost = ["<all_urls>"];
         } else {
@@ -481,26 +478,30 @@ extensions.registerSchemaAPI("tabs", nul
         }
         if (details.matchAboutBlank) {
           options.match_about_blank = details.matchAboutBlank;
         }
         if (details.runAt !== null) {
           options.run_at = details.runAt;
         }
 
-        MessageChannel.sendMessage(mm, "Extension:Execute", { options }, recipient);
-
-        // TODO: Call the callback with the result (which is what???).
+        // TODO: Set lastError.
+        context.sendMessage(mm, "Extension:Execute", { options }, recipient)
+          .then(result => {
+            if (callback) {
+              runSafe(context, callback, result);
+            }
+          });
       },
 
       executeScript: function(tabId, details, callback) {
         self.tabs._execute(tabId, details, "js", callback);
       },
 
-      insertCss: function(tabId, details, callback) {
+      insertCSS: function(tabId, details, callback) {
         self.tabs._execute(tabId, details, "css", callback);
       },
 
       connect: function(tabId, connectInfo) {
         let tab = TabManager.getTab(tabId);
         let mm = tab.linkedBrowser.messageManager;
 
         let name = "";
--- a/browser/components/extensions/test/browser/browser.ini
+++ b/browser/components/extensions/test/browser/browser.ini
@@ -15,18 +15,20 @@ support-files =
 [browser_ext_browserAction_context.js]
 [browser_ext_browserAction_disabled.js]
 [browser_ext_pageAction_context.js]
 [browser_ext_pageAction_popup.js]
 [browser_ext_browserAction_popup.js]
 [browser_ext_popup_api_injection.js]
 [browser_ext_contextMenus.js]
 [browser_ext_getViews.js]
+[browser_ext_tabs_executeScript.js]
 [browser_ext_tabs_executeScript_good.js]
 [browser_ext_tabs_executeScript_bad.js]
+[browser_ext_tabs_insertCSS.js]
 [browser_ext_tabs_query.js]
 [browser_ext_tabs_getCurrent.js]
 [browser_ext_tabs_create.js]
 [browser_ext_tabs_update.js]
 [browser_ext_tabs_onUpdated.js]
 [browser_ext_tabs_sendMessage.js]
 [browser_ext_tabs_move.js]
 [browser_ext_tabs_move_window.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
@@ -0,0 +1,66 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(function* testExecuteScript() {
+  let {MessageChannel} = Cu.import("resource://gre/modules/MessageChannel.jsm", {});
+
+  let messageManagersSize = MessageChannel.messageManagers.size;
+  let responseManagersSize = MessageChannel.responseManagers.size;
+
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/", true);
+
+  function background() {
+    browser.tabs.executeScript({
+      file: "script.js",
+      code: "42",
+    }, result => {
+      browser.test.assertEq(42, result, "Expected callback result");
+      browser.test.sendMessage("got result", result);
+    });
+
+    browser.tabs.executeScript({
+      file: "script2.js",
+    }, result => {
+      browser.test.assertEq(27, result, "Expected callback result");
+      browser.test.sendMessage("got callback", result);
+    });
+
+    browser.runtime.onMessage.addListener(message => {
+      browser.test.assertEq("script ran", message, "Expected runtime message");
+      browser.test.sendMessage("got message", message);
+    });
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["http://mochi.test/"],
+    },
+
+    background,
+
+    files: {
+      "script.js": function() {
+        browser.runtime.sendMessage("script ran");
+      },
+
+      "script2.js": "27",
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitMessage("got result");
+  yield extension.awaitMessage("got callback");
+  yield extension.awaitMessage("got message");
+
+  yield extension.unload();
+
+  yield BrowserTestUtils.removeTab(tab);
+
+  // Make sure that we're not holding on to references to closed message
+  // managers.
+  is(MessageChannel.messageManagers.size, messageManagersSize, "Message manager count");
+  is(MessageChannel.responseManagers.size, responseManagersSize, "Response manager count");
+  is(MessageChannel.pendingResponses.size, 0, "Pending response count");
+});
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_insertCSS.js
@@ -0,0 +1,106 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(function* testExecuteScript() {
+  let {MessageChannel} = Cu.import("resource://gre/modules/MessageChannel.jsm", {});
+
+  let messageManagersSize = MessageChannel.messageManagers.size;
+  let responseManagersSize = MessageChannel.responseManagers.size;
+
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/", true);
+
+  function background() {
+    let promises = [
+      {
+        background: "rgb(0, 0, 0)",
+        foreground: "rgb(255, 192, 203)",
+        promise: resolve => {
+          browser.tabs.insertCSS({
+            file: "file1.css",
+            code: "* { background: black }",
+          }, result => {
+            browser.test.assertEq(undefined, result, "Expected callback result");
+            resolve();
+          });
+        },
+      },
+      {
+        background: "rgb(0, 0, 0)",
+        foreground: "rgb(0, 113, 4)",
+        promise: resolve => {
+          browser.tabs.insertCSS({
+            file: "file2.css",
+          }, result => {
+            browser.test.assertEq(undefined, result, "Expected callback result");
+            resolve();
+          });
+        },
+      },
+      {
+        background: "rgb(42, 42, 42)",
+        foreground: "rgb(0, 113, 4)",
+        promise: resolve => {
+          browser.tabs.insertCSS({
+            code: "* { background: rgb(42, 42, 42) }",
+          }, result => {
+            browser.test.assertEq(undefined, result, "Expected callback result");
+            resolve();
+          });
+        },
+      },
+    ];
+
+    function checkCSS() {
+      let computedStyle = window.getComputedStyle(document.body);
+      return [computedStyle.backgroundColor, computedStyle.color];
+    }
+
+    function next() {
+      if (!promises.length) {
+        browser.test.notifyPass("insertCSS");
+        return;
+      }
+
+      let { promise, background, foreground } = promises.shift();
+      new Promise(promise).then(() => {
+        browser.tabs.executeScript({
+          code: `(${checkCSS})()`,
+        }, result => {
+          browser.test.assertEq(background, result[0], "Expected background color");
+          browser.test.assertEq(foreground, result[1], "Expected foreground color");
+          next();
+        });
+      });
+    }
+
+    next();
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["http://mochi.test/"],
+    },
+
+    background,
+
+    files: {
+      "file1.css": "* { color: pink }",
+      "file2.css": "* { color: rgb(0, 113, 4) }",
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitFinish("insertCSS");
+
+  yield extension.unload();
+
+  yield BrowserTestUtils.removeTab(tab);
+
+  // Make sure that we're not holding on to references to closed message
+  // managers.
+  is(MessageChannel.messageManagers.size, messageManagersSize, "Message manager count");
+  is(MessageChannel.responseManagers.size, responseManagersSize, "Response manager count");
+  is(MessageChannel.pendingResponses.size, 0, "Pending response count");
+});
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -200,16 +200,18 @@ var Management = {
     this.emitter.off(hook, callback);
   },
 };
 
 // A MessageBroker that's used to send and receive messages for
 // extension pages (which run in the chrome process).
 var globalBroker = new MessageBroker([Services.mm, Services.ppmm]);
 
+var gContextId = 0;
+
 // An extension page is an execution context for any extension content
 // that runs in the chrome process. It's used for background pages
 // (type="background"), popups (type="popup"), and any extension
 // content loaded into browser tabs (type="tab").
 //
 // |params| is an object with the following properties:
 // |type| is one of "background", "popup", or "tab".
 // |contentWindow| is the DOM window the content runs in.
@@ -219,16 +221,18 @@ var globalBroker = new MessageBroker([Se
 ExtensionPage = function(extension, params) {
   let {type, contentWindow, uri} = params;
   this.extension = extension;
   this.type = type;
   this.contentWindow = contentWindow || null;
   this.uri = uri || extension.baseURI;
   this.incognito = params.incognito || false;
   this.onClose = new Set();
+  this.contextId = gContextId++;
+  this.unloaded = false;
 
   // This is the MessageSender property passed to extension.
   // It can be augmented by the "page-open" hook.
   let sender = {id: extension.uuid};
   if (uri) {
     sender.url = uri.spec;
   }
   let delegate = {
@@ -267,16 +271,27 @@ ExtensionPage.prototype = {
     try {
       ssm.checkLoadURIStrWithPrincipal(this.principal, url, flags);
     } catch (e) {
       return false;
     }
     return true;
   },
 
+  // A wrapper around MessageChannel.sendMessage which adds the extension ID
+  // to the recipient object, and ensures replies are not processed after the
+  // context has been unloaded.
+  sendMessage(target, messageName, data, recipient = {}, sender = {}) {
+    recipient.extensionId = this.extension.id;
+    sender.extensionId = this.extension.id;
+    sender.contextId = this.contextId;
+
+    return MessageChannel.sendMessage(target, messageName, data, recipient, sender);
+  },
+
   callOnClose(obj) {
     this.onClose.add(obj);
   },
 
   forgetOnClose(obj) {
     this.onClose.delete(obj);
   },
 
@@ -284,16 +299,30 @@ ExtensionPage.prototype = {
   shutdown() {
     Management.emit("page-shutdown", this);
     this.unload();
   },
 
   // This method is called when an extension page navigates away or
   // its tab is closed.
   unload() {
+    // Note that without this guard, we end up running unload code
+    // multiple times for tab pages closed by the "page-unload" handlers
+    // triggered below.
+    if (this.unloaded) {
+      return;
+    }
+
+    this.unloaded = true;
+
+    MessageChannel.abortResponses({
+      extensionId: this.extension.id,
+      contextId: this.contextId,
+    });
+
     Management.emit("page-unload", this);
 
     this.extension.views.delete(this);
 
     for (let obj of this.onClose) {
       obj.close();
     }
   },
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -24,16 +24,18 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/AppConstants.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionManagement",
                                   "resource://gre/modules/ExtensionManagement.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
                                   "resource://gre/modules/MatchPattern.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
                                   "resource://gre/modules/PrivateBrowsingUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+                                  "resource://gre/modules/PromiseUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MessageChannel",
                                   "resource://gre/modules/MessageChannel.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   runSafeSyncWithoutClone,
   LocaleData,
   MessageBroker,
@@ -104,22 +106,24 @@ var api = context => {
       getMessage: function(messageName, substitutions) {
         return context.extension.localizeMessage(messageName, substitutions);
       },
     },
   };
 };
 
 // Represents a content script.
-function Script(options) {
+function Script(options, deferred = PromiseUtils.defer()) {
   this.options = options;
   this.run_at = this.options.run_at;
   this.js = this.options.js || [];
   this.css = this.options.css || [];
 
+  this.deferred = deferred;
+
   this.matches_ = new MatchPattern(this.options.matches);
   this.exclude_matches_ = new MatchPattern(this.options.exclude_matches || null);
   // TODO: MatchPattern should pre-mangle host-only patterns so that we
   // don't need to call a separate match function.
   this.matches_host_ = new MatchPattern(this.options.matchesHost || null);
 
   // TODO: Support glob patterns.
 }
@@ -141,16 +145,17 @@ Script.prototype = {
 
     // TODO: match_about_blank.
 
     return true;
   },
 
   tryInject(extension, window, sandbox, shouldRun) {
     if (!this.matches(window)) {
+      this.deferred.reject();
       return;
     }
 
     if (shouldRun("document_start")) {
       let winUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
                            .getInterface(Ci.nsIDOMWindowUtils);
 
       for (let url of this.css) {
@@ -159,16 +164,17 @@ Script.prototype = {
       }
 
       if (this.options.cssCode) {
         let url = "data:text/css;charset=utf-8," + encodeURIComponent(this.options.cssCode);
         runSafeSyncWithoutClone(winUtils.loadSheetUsingURIString, url, winUtils.AUTHOR_SHEET);
       }
     }
 
+    let result;
     let scheduled = this.run_at || "document_idle";
     if (shouldRun(scheduled)) {
       for (let url of this.js) {
         // On gonk we need to load the resources asynchronously because the
         // app: channels only support asyncOpen. This is safe only in the
         // `document_idle` state.
         if (AppConstants.platform == "gonk" && scheduled != "document_idle") {
           Cu.reportError(`Script injection: ignoring ${url} at ${scheduled}`);
@@ -176,23 +182,36 @@ Script.prototype = {
         }
         url = extension.baseURI.resolve(url);
 
         let options = {
           target: sandbox,
           charset: "UTF-8",
           async: AppConstants.platform == "gonk",
         };
-        runSafeSyncWithoutClone(Services.scriptloader.loadSubScriptWithOptions, url, options);
+        try {
+          result = Services.scriptloader.loadSubScriptWithOptions(url, options);
+        } catch (e) {
+          Cu.reportError(e);
+          this.deferred.reject(e.message);
+        }
       }
 
       if (this.options.jsCode) {
-        Cu.evalInSandbox(this.options.jsCode, sandbox, "latest");
+        try {
+          result = Cu.evalInSandbox(this.options.jsCode, sandbox, "latest");
+        } catch (e) {
+          Cu.reportError(e);
+          this.deferred.reject(e.message);
+        }
       }
     }
+
+    // TODO: Handle this correctly when we support runAt and allFrames.
+    this.deferred.resolve(result);
   },
 };
 
 function getWindowMessageManager(contentWindow) {
   let ir = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
                         .getInterface(Ci.nsIDocShell)
                         .QueryInterface(Ci.nsIInterfaceRequestor);
   try {
@@ -668,20 +687,23 @@ class ExtensionGlobal {
                          .getInterface(Ci.nsIDOMWindowUtils)
                          .currentInnerWindowID,
     };
   }
 
   receiveMessage({ target, messageName, recipient, data }) {
     switch (messageName) {
       case "Extension:Execute":
-        let script = new Script(data.options);
+        let deferred = PromiseUtils.defer();
+
+        let script = new Script(data.options, deferred);
         let { extensionId } = recipient;
         DocumentManager.executeScript(target, extensionId, script);
-        break;
+
+        return deferred.promise;
     }
   }
 }
 
 this.ExtensionContent = {
   globals: new Map(),
 
   init(global) {