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 id17269
push usermaglione.k@gmail.com
push dateThu, 28 Jan 2016 00:48:07 +0000
treeherderfx-team@261e997621c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1210583
milestone47.0a1
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) {