Bug 1541557: Part 6 - Read scripts for loadChromeScript in child process rather than parent. r=nika
☠☠ backed out by f9bf5e4b0b4f ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Thu, 13 Jun 2019 17:01:10 -0700
changeset 543729 c2697f04d38cf0b01b1f3e227910ab5890926a33
parent 543728 75ebd6fce136ab3bd0e591c2b8b2d06d3b5bf923
child 543730 46ff845a7b0cdabf640bb2e3c783735ab68b7cd1
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1541557
milestone69.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 1541557: Part 6 - Read scripts for loadChromeScript in child process rather than parent. r=nika `loadChromeScript` is often called with http: URLs pointing to mochitest resources, which need to be read synchronously before they can be executed. Unfortunately, while the API for this pretends to be synchronous, it really spins the event loop underneath. When we attempt to use it in the parent, that means that we spin the event loop and process messages from the child before the script has been executed. And since those messages often contain messages intended for the chrome script, that causes problems. When the chrome script APIs use sync messaging, this doesn't matter much, since the `loadChromeScript` call blocks the caller until the message handler in the parent returns. When it uses async messaging, though, we have no such luck, and the messages intended for the script get sent to the parent immediately. Loading the script contents in the child solves this problem, since it reliably blocks the child callers until the script contents are ready, and doesn't give them a chance to try to send messages to the script while it's still being read. Differential Revision: https://phabricator.services.mozilla.com/D35056
testing/specialpowers/content/SpecialPowersAPI.jsm
testing/specialpowers/content/SpecialPowersObserverAPI.jsm
--- a/testing/specialpowers/content/SpecialPowersAPI.jsm
+++ b/testing/specialpowers/content/SpecialPowersAPI.jsm
@@ -230,30 +230,78 @@ class SpecialPowersAPI {
       Services.scriptloader.loadSubScript(blobUrl, sb);
     } catch (e) {
       throw WrapPrivileged.wrap(e);
     }
 
     return mc.port2;
   }
 
+  _readUrlAsString(aUrl) {
+    // Fetch script content as we can't use scriptloader's loadSubScript
+    // to evaluate http:// urls...
+    var scriptableStream = Cc["@mozilla.org/scriptableinputstream;1"]
+                             .getService(Ci.nsIScriptableInputStream);
+
+    var channel = NetUtil.newChannel({
+      uri: aUrl,
+      loadUsingSystemPrincipal: true,
+    });
+    var input = channel.open();
+    scriptableStream.init(input);
+
+    var str;
+    var buffer = [];
+
+    while ((str = scriptableStream.read(4096))) {
+      buffer.push(str);
+    }
+
+    var output = buffer.join("");
+
+    scriptableStream.close();
+    input.close();
+
+    var status;
+    if (channel instanceof Ci.nsIHttpChannel) {
+      status = channel.responseStatus;
+    }
+
+    if (status == 404) {
+      throw new Error(
+        `Error while executing chrome script '${aUrl}':\n` +
+        "The script doesn't exist. Ensure you have registered it in " +
+        "'support-files' in your mochitest.ini.");
+    }
+
+    return output;
+  }
+
   loadChromeScript(urlOrFunction, sandboxOptions) {
     // Create a unique id for this chrome script
     let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"]
                           .getService(Ci.nsIUUIDGenerator);
     let id = uuidGenerator.generateUUID().toString();
 
     // Tells chrome code to evaluate this chrome script
     let scriptArgs = { id, sandboxOptions };
     if (typeof(urlOrFunction) == "function") {
       scriptArgs.function = {
         body: "(" + urlOrFunction.toString() + ")();",
         name: urlOrFunction.name,
       };
     } else {
+      // Note: We need to do this in the child since, even though
+      // `_readUrlAsString` pretends to be synchronous, its channel
+      // winds up spinning the event loop when loading HTTP URLs. That
+      // leads to unexpected out-of-order operations if the child sends
+      // a message immediately after loading the script.
+      scriptArgs.function = {
+        body: this._readUrlAsString(urlOrFunction),
+      };
       scriptArgs.url = urlOrFunction;
     }
     this._sendSyncMessage("SPLoadChromeScript",
                           scriptArgs);
 
     // Returns a MessageManager like API in order to be
     // able to communicate with this chrome script
     let listeners = [];
--- a/testing/specialpowers/content/SpecialPowersObserverAPI.jsm
+++ b/testing/specialpowers/content/SpecialPowersObserverAPI.jsm
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["SpecialPowersObserverAPI", "SpecialPowersError"];
 
 var {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-var {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   ExtensionData: "resource://gre/modules/Extension.jsm",
   ExtensionTestCommon: "resource://testing-common/ExtensionTestCommon.jsm",
   PerTestCoverageUtils: "resource://testing-common/PerTestCoverageUtils.jsm",
   ServiceWorkerCleanUp: "resource://gre/modules/ServiceWorkerCleanUp.jsm",
 });
@@ -201,56 +200,16 @@ class SpecialPowersObserverAPI {
     }
     return removed;
   }
 
   _getURI(url) {
     return Services.io.newURI(url);
   }
 
-  _readUrlAsString(aUrl) {
-    // Fetch script content as we can't use scriptloader's loadSubScript
-    // to evaluate http:// urls...
-    var scriptableStream = Cc["@mozilla.org/scriptableinputstream;1"]
-                             .getService(Ci.nsIScriptableInputStream);
-
-    var channel = NetUtil.newChannel({
-      uri: aUrl,
-      loadUsingSystemPrincipal: true,
-    });
-    var input = channel.open();
-    scriptableStream.init(input);
-
-    var str;
-    var buffer = [];
-
-    while ((str = scriptableStream.read(4096))) {
-      buffer.push(str);
-    }
-
-    var output = buffer.join("");
-
-    scriptableStream.close();
-    input.close();
-
-    var status;
-    if (channel instanceof Ci.nsIHttpChannel) {
-      status = channel.responseStatus;
-    }
-
-    if (status == 404) {
-      throw new SpecialPowersError(
-        "Error while executing chrome script '" + aUrl + "':\n" +
-        "The script doesn't exists. Ensure you have registered it in " +
-        "'support-files' in your mochitest.ini.");
-    }
-
-    return output;
-  }
-
   _sendReply(aMessage, aReplyName, aReplyMsg) {
     let mm = aMessage.target.frameLoader
                      .messageManager;
     mm.sendAsyncMessage(aReplyName, aReplyMsg);
   }
 
   _notifyCategoryAndObservers(subject, topic, data) {
     const serviceMarker = "service,";
@@ -441,24 +400,22 @@ class SpecialPowersObserverAPI {
           default:
             throw new SpecialPowersError("Invalid operation for SPObserverervice");
         }
         return undefined; // See comment at the beginning of this function.
       }
 
       case "SPLoadChromeScript": {
         let id = aMessage.json.id;
-        let jsScript;
         let scriptName;
 
+        let jsScript = aMessage.json.function.body;
         if (aMessage.json.url) {
-          jsScript = this._readUrlAsString(aMessage.json.url);
           scriptName = aMessage.json.url;
         } else if (aMessage.json.function) {
-          jsScript = aMessage.json.function.body;
           scriptName = aMessage.json.function.name
             || "<loadChromeScript anonymous function>";
         } else {
           throw new SpecialPowersError("SPLoadChromeScript: Invalid script");
         }
 
         // Setup a chrome sandbox that has access to sendAsyncMessage
         // and {add,remove}MessageListener in order to communicate with