Bug 1164564 - Refactor Promise-backend.js so it can be required as a CommonJS module on the main thread;r=paolo
authorEddy Bruël <ejpbruel@gmail.com>
Thu, 11 Jun 2015 21:24:15 +0200
changeset 279191 b93a5e3d522dd8572704e66d20405bd1fee04802
parent 279190 80e89b5bfe4f1e80b5e82284d44d04d69abc125b
child 279192 74ac345e777ad2a7a87b9aba543c4b9b07263940
child 279276 fba4dc82c9296c0c3abf5bb51c6d42806a02f269
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo
bugs1164564
milestone41.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 1164564 - Refactor Promise-backend.js so it can be required as a CommonJS module on the main thread;r=paolo
toolkit/devtools/Loader.jsm
toolkit/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js
toolkit/devtools/server/tests/unit/xpcshell.ini
toolkit/modules/Promise-backend.js
toolkit/modules/Promise.jsm
--- a/toolkit/devtools/Loader.jsm
+++ b/toolkit/devtools/Loader.jsm
@@ -25,17 +25,16 @@ this.EXPORTED_SYMBOLS = ["DevToolsLoader
 
 /**
  * Providers are different strategies for loading the devtools.
  */
 
 let loaderModules = {
   "Services": Object.create(Services),
   "toolkit/loader": loader,
-  "promise": promise,
   "PromiseDebugging": PromiseDebugging
 };
 XPCOMUtils.defineLazyGetter(loaderModules, "Debugger", () => {
   // addDebuggerToGlobal only allows adding the Debugger object to a global. The
   // this object is not guaranteed to be a global (in particular on B2G, due to
   // compartment sharing), so add the Debugger object to a sandbox instead.
   let sandbox = Cu.Sandbox(CC('@mozilla.org/systemprincipal;1', 'nsIPrincipal')());
   Cu.evalInSandbox(
@@ -91,16 +90,17 @@ BuiltinProvider.prototype = {
         "devtools/touch-events": "resource://gre/modules/devtools/touch-events",
         "devtools/client": "resource://gre/modules/devtools/client",
         "devtools/pretty-fast": "resource://gre/modules/devtools/pretty-fast.js",
         "devtools/jsbeautify": "resource://gre/modules/devtools/jsbeautify/beautify.js",
         "devtools/async-utils": "resource://gre/modules/devtools/async-utils",
         "devtools/content-observer": "resource://gre/modules/devtools/content-observer",
         "gcli": "resource://gre/modules/devtools/gcli",
         "projecteditor": "resource:///modules/devtools/projecteditor",
+        "promise": "resource://gre/modules/Promise-backend.js",
         "acorn": "resource://gre/modules/devtools/acorn",
         "acorn/util/walk": "resource://gre/modules/devtools/acorn/walk.js",
         "tern": "resource://gre/modules/devtools/tern",
         "source-map": "resource://gre/modules/devtools/SourceMap.jsm",
 
         // Allow access to xpcshell test items from the loader.
         "xpcshell-test": "resource://test"
       },
@@ -130,16 +130,17 @@ SrcdirProvider.prototype = {
   },
 
   load: function() {
     let srcdir = Services.prefs.getComplexValue("devtools.loader.srcdir",
                                                 Ci.nsISupportsString);
     srcdir = OS.Path.normalize(srcdir.data.trim());
     let devtoolsDir = OS.Path.join(srcdir, "browser", "devtools");
     let toolkitDir = OS.Path.join(srcdir, "toolkit", "devtools");
+    let modulesDir = OS.Path.join(srcdir, "toolkit", "modules");
     let mainURI = this.fileURI(OS.Path.join(devtoolsDir, "main.js"));
     let definitionsURI = this.fileURI(OS.Path.join(devtoolsDir, "definitions.js"));
     let devtoolsURI = this.fileURI(devtoolsDir);
     let toolkitURI = this.fileURI(toolkitDir);
     let serverURI = this.fileURI(OS.Path.join(toolkitDir, "server"));
     let webconsoleURI = this.fileURI(OS.Path.join(toolkitDir, "webconsole"));
     let appActorURI = this.fileURI(OS.Path.join(toolkitDir, "apps", "app-actor-front.js"));
     let cssLogicURI = this.fileURI(OS.Path.join(toolkitDir, "styleinspector", "css-logic"));
@@ -148,16 +149,17 @@ SrcdirProvider.prototype = {
     let touchEventsURI = this.fileURI(OS.Path.join(toolkitDir, "touch-events"));
     let clientURI = this.fileURI(OS.Path.join(toolkitDir, "client"));
     let prettyFastURI = this.fileURI(OS.Path.join(toolkitDir), "pretty-fast.js");
     let jsBeautifyURI = this.fileURI(OS.Path.join(toolkitDir, "jsbeautify", "beautify.js"));
     let asyncUtilsURI = this.fileURI(OS.Path.join(toolkitDir), "async-utils.js");
     let contentObserverURI = this.fileURI(OS.Path.join(toolkitDir), "content-observer.js");
     let gcliURI = this.fileURI(OS.Path.join(toolkitDir, "gcli", "source", "lib", "gcli"));
     let projecteditorURI = this.fileURI(OS.Path.join(devtoolsDir, "projecteditor"));
+    let promiseURI = this.fileURI(OS.Path.join(modulesDir, "promise-backend.js"));
     let acornURI = this.fileURI(OS.Path.join(toolkitDir, "acorn"));
     let acornWalkURI = OS.Path.join(acornURI, "walk.js");
     let ternURI = OS.Path.join(toolkitDir, "tern");
     let sourceMapURI = this.fileURI(OS.Path.join(toolkitDir), "SourceMap.jsm");
     this.loader = new loader.Loader({
       id: "fx-devtools",
       modules: loaderModules,
       paths: {
@@ -175,16 +177,17 @@ SrcdirProvider.prototype = {
         "devtools/touch-events": touchEventsURI,
         "devtools/client": clientURI,
         "devtools/pretty-fast": prettyFastURI,
         "devtools/jsbeautify": jsBeautifyURI,
         "devtools/async-utils": asyncUtilsURI,
         "devtools/content-observer": contentObserverURI,
         "gcli": gcliURI,
         "projecteditor": projecteditorURI,
+        "promise": promiseURI,
         "acorn": acornURI,
         "acorn/util/walk": acornWalkURI,
         "tern": ternURI,
         "source-map": sourceMapURI,
       },
       globals: this.globals,
       invisibleToDebugger: this.invisibleToDebugger,
       sharedGlobal: true,
deleted file mode 100644
--- a/toolkit/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js
+++ /dev/null
@@ -1,91 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-/**
- * Test that we can get the list of Promise objects that have settled from the
- * PromisesActor onPromiseSettled event handler.
- */
-
-"use strict";
-
-const { PromisesFront } = devtools.require("devtools/server/actors/promises");
-
-let events = devtools.require("sdk/event/core");
-
-add_task(function*() {
-  let client = yield startTestDebuggerServer("promises-actor-test");
-  let chromeActors = yield getChromeActors(client);
-
-  ok(Promise.toString().contains("native code"), "Expect native DOM Promise");
-
-  yield testPromisesSettled(client, chromeActors,
-    v => new Promise(resolve => resolve(v)),
-    v => new Promise((resolve, reject) => reject(v)));
-
-  let response = yield listTabs(client);
-  let targetTab = findTab(response.tabs, "promises-actor-test");
-  ok(targetTab, "Found our target tab.");
-
-  yield testPromisesSettled(client, targetTab, v => {
-    const debuggee = DebuggerServer.getTestGlobal("promises-actor-test");
-    return debuggee.Promise.resolve(v);
-  }, v => {
-    const debuggee = DebuggerServer.getTestGlobal("promises-actor-test");
-    return debuggee.Promise.reject(v);
-  });
-
-  yield close(client);
-});
-
-function* testPromisesSettled(client, form, makeResolvePromise,
-    makeRejectPromise) {
-  let front = PromisesFront(client, form);
-  let resolution = "MyLittleSecret" + Math.random();
-
-  yield front.attach();
-  yield front.listPromises();
-
-  let onPromiseSettled = oncePromiseSettled(front, resolution, true, false);
-  let resolvedPromise = makeResolvePromise(resolution);
-  let foundResolvedPromise = yield onPromiseSettled;
-  ok(foundResolvedPromise, "Found our resolved promise");
-
-  onPromiseSettled = oncePromiseSettled(front, resolution, false, true);
-  let rejectedPromise = makeRejectPromise(resolution);
-  let foundRejectedPromise = yield onPromiseSettled;
-  ok(foundRejectedPromise, "Found our rejected promise");
-
-  yield front.detach();
-  // Appease eslint
-  void resolvedPromise;
-  void rejectedPromise;
-}
-
-function oncePromiseSettled(front, resolution, resolveValue, rejectValue) {
-  return new Promise(resolve => {
-    events.on(front, "promises-settled", promises => {
-      for (let p of promises) {
-        equal(p.type, "object", "Expect type to be Object");
-        equal(p.class, "Promise", "Expect class to be Promise");
-        equal(typeof p.promiseState.creationTimestamp, "number",
-          "Expect creation timestamp to be a number");
-        equal(typeof p.promiseState.timeToSettle, "number",
-          "Expect time to settle to be a number");
-
-        if (p.promiseState.state === "fulfilled" &&
-            p.promiseState.value === resolution) {
-          equal(Math.floor(p.promiseState.timeToSettle), 0,
-            "Expect time to settle for resolved promise to be 0.");
-          resolve(resolveValue);
-        } else if (p.promiseState.state === "rejected" &&
-                   p.promiseState.reason === resolution) {
-          equal(Math.floor(p.promiseState.timeToSettle), 0,
-            "Expect time to settle for rejected promise to be 0.");
-          resolve(rejectValue);
-        } else {
-          dump("Found non-target promise\n");
-        }
-      }
-    });
-  });
-}
--- a/toolkit/devtools/server/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/server/tests/unit/xpcshell.ini
@@ -79,17 +79,16 @@ support-files =
 [test_eval-02.js]
 [test_eval-03.js]
 [test_eval-04.js]
 [test_eval-05.js]
 [test_promises_actor_attach.js]
 [test_promises_actor_exist.js]
 [test_promises_actor_list_promises.js]
 [test_promises_actor_onnewpromise.js]
-[test_promises_actor_onpromisesettled.js]
 [test_promises_client_getdependentpromises.js]
 [test_promises_object_creationtimestamp.js]
 [test_promises_object_timetosettle-01.js]
 [test_promises_object_timetosettle-02.js]
 [test_protocol_abort.js]
 [test_protocol_async.js]
 [test_protocol_children.js]
 [test_protocol_formtype.js]
--- a/toolkit/modules/Promise-backend.js
+++ b/toolkit/modules/Promise-backend.js
@@ -17,19 +17,38 @@
  * Components.utils.import("resource://gre/modules/Promise.jsm");
  *
  * More documentation can be found in the Promise.jsm module.
  */
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Globals
 
-// Do not load the FinalizationWitnessService is we are being required as a
-// CommonJS module, because the Components object is not available in workers.
-if (!isWorker) {
+// Obtain an instance of Cu. How this instance is obtained depends on how this
+// file is loaded.
+//
+// This file can be loaded in three different ways:
+// 1. As a CommonJS module, by Loader.jsm, on the main thread.
+// 2. As a CommonJS module, by worker-loader.js, on a worker thread.
+// 3. As a subscript, by Promise.jsm, on the main thread.
+//
+// If require is defined, the file is loaded as a CommonJS module. Components
+// will not be defined in that case, but we can obtain an instance of Cu from
+// the chrome module. Otherwise, this file is loaded as a subscript, and we can
+// obtain an instance of Cu from Components directly.
+//
+// If the file is loaded as a CommonJS module on a worker thread, the instance
+// of Cu obtained from the chrome module will be null. The reason for this is
+// that Components is not defined in worker threads, so no instance of Cu can
+// be obtained.
+
+let Cu = this.require ? require("chrome").Cu : Components.utils;
+
+// If Cu is defined, use it to lazily define the FinalizationWitnessService.
+if (Cu) {
   Cu.import("resource://gre/modules/Services.jsm");
   Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
   XPCOMUtils.defineLazyServiceGetter(this, "FinalizationWitnessService",
                                      "@mozilla.org/toolkit/finalizationwitness;1",
                                      "nsIFinalizationWitnessService");
 }
 
@@ -41,17 +60,17 @@ const STATUS_REJECTED = 2;
 // accessed only by this module, while still being visible on the object
 // manually when using a debugger.  This doesn't strictly guarantee that the
 // properties are inaccessible by other code, but provide enough protection to
 // avoid using them by mistake.
 const salt = Math.floor(Math.random() * 100);
 const N_INTERNALS = "{private:internals:" + salt + "}";
 
 // We use DOM Promise for scheduling the walker loop.
-const DOMPromise = isWorker ? null : Promise;
+const DOMPromise = Cu ? Promise : null;
 
 /////// Warn-upon-finalization mechanism
 //
 // One of the difficult problems with promises is locating uncaught
 // rejections. We adopt the following strategy: if a promise is rejected
 // at the time of its garbage-collection *and* if the promise is at the
 // end of a promise chain (i.e. |thatPromise.then| has never been
 // called), then we print a warning.
@@ -247,20 +266,19 @@ let PendingErrors = {
   /**
    * Remove all the observers added with addObserver
    */
   removeAllObservers: function() {
     this._observers.clear();
   }
 };
 
-// Do not initialize the warn-on-finalization mechanism if we are being required
-// as a CommonJS module by the worker loader, because the Components object (and
-// therefore the FinalizationWitnessService) is not available.
-if (!isWorker) {
+// Initialize the warn-upon-finalization mechanism if and only if Cu is defined.
+// Otherwise, FinalizationWitnessService won't be defined (see above).
+if (Cu) {
   PendingErrors.init();
 }
 
 // Default mechanism for displaying errors
 PendingErrors.addObserver(function(details) {
   const generalDescription = "A promise chain failed to handle a rejection." +
     " Did you forget to '.catch', or did you forget to 'return'?\nSee" +
     " https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise\n\n";
@@ -623,19 +641,19 @@ Promise.Debugging = {
   flushUncaughtErrors: function() {
     PendingErrors.flush();
   },
 };
 Object.freeze(Promise.Debugging);
 
 Object.freeze(Promise);
 
-// Make sure to export the Promise object if we are being required as a CommonJS
-// module by the worker loader.
-if (isWorker) {
+// If module is defined, this file is loaded as a CommonJS module. Make sure
+// Promise is exported in that case.
+if (this.module) {
   module.exports = Promise;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// PromiseWalker
 
 /**
  * This singleton object invokes the handlers registered on resolved and
@@ -680,36 +698,51 @@ this.PromiseWalker = {
       return;
     }
 
     // Change the promise status and schedule our handlers for processing.
     aPromise[N_INTERNALS].status = aStatus;
     aPromise[N_INTERNALS].value = aValue;
     if (aPromise[N_INTERNALS].handlers.length > 0) {
       this.schedulePromise(aPromise);
-    } else if (!isWorker && aStatus == STATUS_REJECTED) {
+    } else if (Cu && aStatus == STATUS_REJECTED) {
       // This is a rejection and the promise is the last in the chain.
       // For the time being we therefore have an uncaught error.
       let id = PendingErrors.register(aValue);
       let witness =
           FinalizationWitnessService.make("promise-finalization-witness", id);
       aPromise[N_INTERNALS].witness = [id, witness];
     }
   },
 
   /**
    * Sets up the PromiseWalker loop to start on the next tick of the event loop
    */
   scheduleWalkerLoop: function()
   {
     this.walkerLoopScheduled = true;
-    if (isWorker) {
+
+    // If this file is loaded on a worker thread, DOMPromise will not behave as
+    // expected: because native promises are not aware of nested event loops
+    // created by the debugger, their respective handlers will not be called
+    // until after leaving the nested event loop. The debugger server relies
+    // heavily on the use promises, so this could cause the debugger to hang.
+    //
+    // To work around this problem, any use of native promises in the debugger
+    // server should be avoided when it is running on a worker thread. Because
+    // it is still necessary to be able to schedule runnables on the event
+    // queue, the worker loader defines the function setImmediate as a
+    // per-module global for this purpose.
+    //
+    // If Cu is defined, this file is loaded on the main thread. Otherwise, it
+    // is loaded on the worker thread.
+    if (Cu) {
+      DOMPromise.resolve().then(() => this.walkerLoop());
+    } else {
       setImmediate(this.walkerLoop);
-    } else {
-      DOMPromise.resolve().then(() => this.walkerLoop());
     }
   },
 
   /**
    * Schedules the resolution or rejection handlers registered on the provided
    * promise for processing.
    *
    * @param aPromise
--- a/toolkit/modules/Promise.jsm
+++ b/toolkit/modules/Promise.jsm
@@ -91,22 +91,11 @@ this.EXPORTED_SYMBOLS = [
 
 // These constants must be defined on the "this" object for them to be visible
 // by subscripts in B2G, since "this" does not match the global scope.
 this.Cc = Components.classes;
 this.Ci = Components.interfaces;
 this.Cu = Components.utils;
 this.Cr = Components.results;
 
-// Promise-backend.js can either be loaded as a subscript by this file, or
-// required as a CommonJS module by the worker loader. Because certain APIS (in
-// particular, Components) are not available in workers, Promise-backend.js
-// behaves slightly different in the latter case.
-//
-// To distinguish between these two cases, the worker loader defines a global
-// variable isWorker, and sets it to true. When loading Promise-backend.js as
-// a subscript, we need to make sure this variable is defined as well, and set
-// it to false.
-this.isWorker = false;
-
 this.Cc["@mozilla.org/moz/jssubscript-loader;1"]
     .getService(this.Ci.mozIJSSubScriptLoader)
     .loadSubScript("resource://gre/modules/Promise-backend.js", this);