Bug 1312690: Load content scripts asynchronously when possible. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 02 Nov 2016 13:57:19 -0700
changeset 432815 79b8c08a8879f6ab0516222506e94f7bea4efe92
parent 432814 a91bb22b40fe35baa8088d229d41052d81321a0d
child 432816 4e946e7685bcc7863a9aef8edb0d9bdd1cc296f5
push id34435
push usermaglione.k@gmail.com
push dateWed, 02 Nov 2016 20:58:46 +0000
reviewersaswan
bugs1312690
milestone52.0a1
Bug 1312690: Load content scripts asynchronously when possible. r?aswan MozReview-Commit-ID: BzpZA4stbCI
js/xpconnect/idl/mozIJSSubScriptLoader.idl
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/test/mochitest/test_ext_contentscript.html
toolkit/components/extensions/test/mochitest/test_ext_contentscript_context.html
--- a/js/xpconnect/idl/mozIJSSubScriptLoader.idl
+++ b/js/xpconnect/idl/mozIJSSubScriptLoader.idl
@@ -34,16 +34,19 @@ interface mozIJSSubScriptLoader : nsISup
      * In JS, the signature looks like:
      * rv = loadSubScript (url, optionsObject)
      * @param url the url of the sub-script, it MUST be either a file:,
      *            resource:, or chrome: url, and MUST be local.
      * @param optionsObject an object with parameters. Valid parameters are:
      *                      - charset: specifying the character encoding of the file (default: ASCII)
      *                      - target:  an object to evaluate onto (default: global object of the caller)
      *                      - ignoreCache: if set to true, will bypass the cache for reading the file.
+     *                      - async: if set to true, the script will be loaded
+     *                        asynchronously, and a Promise is returned which
+     *                        resolves to its result when execution is complete.
      * @retval rv the value returned by the sub-script
      */
     [implicit_jscontext]
     jsval loadSubScriptWithOptions(in AString url, in jsval options);
 
     /*
      * Compiles a JS script off the main thread and calls back the
      * observer once it's done.
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -178,17 +178,42 @@ Script.prototype = {
                            .getInterface(Ci.nsIDOMWindowUtils);
 
       for (let url of this.cssURLs) {
         runSafeSyncWithoutClone(winUtils.removeSheetUsingURIString, url, winUtils.AUTHOR_SHEET);
       }
     }
   },
 
-  tryInject(window, sandbox, shouldRun) {
+  /**
+   * Tries to inject this script into the given window and sandbox, if
+   * there are pending operations for the window's current load state.
+   *
+   * @param {Window} window
+   *        The DOM Window to inject the scripts and CSS into.
+   * @param {Sandbox} sandbox
+   *        A Sandbox inheriting from `window` in which to evaluate the
+   *        injected scripts.
+   * @param {function} shouldRun
+   *        A function which, when passed the document load state that a
+   *        script is expected to run at, returns `true` if we should
+   *        currently be injecting scripts for that load state.
+   *
+   *        For initial injection of a script, this function should
+   *        return true if the document is currently in or has already
+   *        passed through the given state. For injections triggered by
+   *        document state changes, it should only return true if the
+   *        given state exactly matches the state that triggered the
+   *        change.
+   * @param {string} when
+   *        The document's current load state, or if triggered by a
+   *        document state change, the new document state that triggered
+   *        the injection.
+   */
+  tryInject(window, sandbox, shouldRun, when) {
     if (!this.matches(window)) {
       this.deferred.reject({message: "No matching window"});
       return;
     }
 
     if (shouldRun("document_start")) {
       let winUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
                            .getInterface(Ci.nsIDOMWindowUtils);
@@ -208,17 +233,19 @@ Script.prototype = {
     let scheduled = this.run_at || "document_idle";
     if (shouldRun(scheduled)) {
       for (let url of this.js) {
         url = this.extension.baseURI.resolve(url);
 
         let options = {
           target: sandbox,
           charset: "UTF-8",
-          async: false,
+          // Inject asynchronously unless we're expected to inject before any
+          // page scripts have run, and we haven't already missed that boat.
+          async: this.run_at !== "document_start" || when !== "document_start",
         };
         try {
           result = Services.scriptloader.loadSubScriptWithOptions(url, options);
         } catch (e) {
           Cu.reportError(e);
           this.deferred.reject(e);
         }
       }
@@ -353,34 +380,34 @@ class ExtensionContext extends BaseConte
                                "chrome", () => this.chromeObj);
     }
   }
 
   get cloneScope() {
     return this.sandbox;
   }
 
-  execute(script, shouldRun) {
-    script.tryInject(this.contentWindow, this.sandbox, shouldRun);
+  execute(script, shouldRun, when) {
+    script.tryInject(this.contentWindow, this.sandbox, shouldRun, when);
   }
 
-  addScript(script) {
+  addScript(script, when) {
     let state = DocumentManager.getWindowState(this.contentWindow);
-    this.execute(script, scheduled => isWhenBeforeOrSame(scheduled, state));
+    this.execute(script, scheduled => isWhenBeforeOrSame(scheduled, state), when);
 
     // Save the script in case it has pending operations in later load
     // states, but only if we're before document_idle, or require cleanup.
     if (state != "document_idle" || script.requiresCleanup) {
       this.scripts.push(script);
     }
   }
 
   triggerScripts(documentState) {
     for (let script of this.scripts) {
-      this.execute(script, scheduled => scheduled == documentState);
+      this.execute(script, scheduled => scheduled == documentState, documentState);
     }
     if (documentState == "document_idle") {
       // Don't bother saving scripts after document_idle.
       this.scripts = this.scripts.filter(script => script.requiresCleanup);
     }
   }
 
   close() {
@@ -681,17 +708,17 @@ DocumentManager = {
   },
 
   trigger(when, window) {
     if (when == "document_start") {
       for (let extension of ExtensionManager.extensions.values()) {
         for (let script of extension.scripts) {
           if (script.matches(window)) {
             let context = this.getContentScriptContext(extension, window);
-            context.addScript(script);
+            context.addScript(script, when);
           }
         }
       }
     } else {
       let contexts = this.contentScriptWindows.get(getInnerWindowID(window)) || new Map();
       for (let context of contexts.values()) {
         context.triggerScripts(when);
       }
--- a/toolkit/components/extensions/test/mochitest/test_ext_contentscript.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript.html
@@ -10,36 +10,37 @@
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
 add_task(function* test_contentscript() {
   function background() {
-    browser.runtime.onMessage.addListener(([msg, expectedState, readyState], sender) => {
+    browser.runtime.onMessage.addListener(([msg, expectedStates, readyState], sender) => {
       if (msg == "chrome-namespace-ok") {
         browser.test.sendMessage(msg);
         return;
       }
 
-      browser.test.assertEq(msg, "script-run", "message type is correct");
-      browser.test.assertEq(readyState, expectedState, "readyState is correct");
-      browser.test.sendMessage("script-run-" + expectedState);
+      browser.test.assertEq("script-run", msg, "message type is correct");
+      browser.test.assertTrue(expectedStates.includes(readyState),
+                              `readyState "${readyState}" is one of [${expectedStates}]`);
+      browser.test.sendMessage("script-run-" + expectedStates[0]);
     });
   }
 
   function contentScriptStart() {
-    browser.runtime.sendMessage(["script-run", "loading", document.readyState]);
+    browser.runtime.sendMessage(["script-run", ["loading"], document.readyState]);
   }
   function contentScriptEnd() {
-    browser.runtime.sendMessage(["script-run", "interactive", document.readyState]);
+    browser.runtime.sendMessage(["script-run", ["interactive", "complete"], document.readyState]);
   }
   function contentScriptIdle() {
-    browser.runtime.sendMessage(["script-run", "complete", document.readyState]);
+    browser.runtime.sendMessage(["script-run", ["complete"], document.readyState]);
   }
 
   function contentScript() {
     let manifest = browser.runtime.getManifest();
     void manifest.applications.gecko.id;
     chrome.runtime.sendMessage(["chrome-namespace-ok"]);
   }
 
--- a/toolkit/components/extensions/test/mochitest/test_ext_contentscript_context.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript_context.html
@@ -27,16 +27,17 @@ add_task(function* test_contentscript_co
     });
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       content_scripts: [{
         "matches": ["http://example.com/"],
         "js": ["content_script.js"],
+        "run_at": "document_start",
       }],
     },
 
     files: {
       "content_script.js": contentScript,
     },
   });