Bug 1013625 - Process Promise resolution runnables outside of main event queue. r=bz,khuey

dom/base/nsContentUtils.cpp

@@ -39,6 +39,7 @@
 #include "mozilla/dom/HTMLTemplateElement.h"
 #include "mozilla/dom/HTMLContentElement.h"
 #include "mozilla/dom/HTMLShadowElement.h"
+#include "mozilla/dom/Promise.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/TextDecoder.h"
 #include "mozilla/dom/TouchEvent.h"
@@ -5081,7 +5082,7 @@ nsContentUtils::LeaveMicroTask()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (--sMicroTaskLevel == 0) {
-    nsDOMMutationObserver::HandleMutations();
+    PerformMainThreadMicroTaskCheckpoint();
     nsDocument::ProcessBaseElementQueue();
   }
 }
@@ -5107,6 +5108,14 @@ nsContentUtils::SetMicroTaskLevel(uint32_t aLevel)
   sMicroTaskLevel = aLevel;
 }
 
+void
+nsContentUtils::PerformMainThreadMicroTaskCheckpoint()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsDOMMutationObserver::HandleMutations();
+}
+
 /* 
  * Helper function for nsContentUtils::ProcessViewportInfo.
  *

dom/base/nsContentUtils.h

@@ -1542,6 +1542,8 @@ public:
   static uint32_t MicroTaskLevel();
   static void SetMicroTaskLevel(uint32_t aLevel);
 
+  static void PerformMainThreadMicroTaskCheckpoint();
+
   /* Process viewport META data. This gives us information for the scale
    * and zoom of a page on mobile devices. We stick the information in
    * the document header and use it later on after rendering.

dom/promise/AbortablePromise.cpp

@@ -100,7 +100,7 @@ AbortablePromise::Abort()
 
   nsCOMPtr<nsIRunnable> runnable =
     NS_NewRunnableMethod(this, &AbortablePromise::DoAbort);
-  Promise::DispatchToMainOrWorkerThread(runnable);
+  Promise::DispatchToMicroTask(runnable);
 }
 
 void

dom/promise/Promise.cpp

@@ -354,6 +354,23 @@ Promise::MaybeReject(JSContext* aCx,
   MaybeRejectInternal(aCx, aValue);
 }
 
+void
+Promise::PerformMicroTaskCheckpoint()
+{
+  CycleCollectedJSRuntime* runtime = CycleCollectedJSRuntime::Get();
+  nsTArray<nsRefPtr<nsIRunnable>>& microtaskQueue =
+    runtime->GetPromiseMicroTaskQueue();
+
+  while (!microtaskQueue.IsEmpty()) {
+    nsRefPtr<nsIRunnable> runnable = microtaskQueue.ElementAt(0);
+    MOZ_ASSERT(runnable);
+
+    // This function can re-enter, so we remove the element before calling.
+    microtaskQueue.RemoveElementAt(0);
+    runnable->Run();
+  }
+}
+
 /* static */ bool
 Promise::JSCallback(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
 {
@@ -954,17 +971,15 @@ private:
 };
 
 /* static */ void
-Promise::DispatchToMainOrWorkerThread(nsIRunnable* aRunnable)
+Promise::DispatchToMicroTask(nsIRunnable* aRunnable)
 {
   MOZ_ASSERT(aRunnable);
-  if (NS_IsMainThread()) {
-    NS_DispatchToCurrentThread(aRunnable);
-    return;
-  }
-  WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
-  MOZ_ASSERT(worker);
-  nsRefPtr<WrappedWorkerRunnable> task = new WrappedWorkerRunnable(worker, aRunnable);
-  task->Dispatch(worker->GetJSContext());
+
+  CycleCollectedJSRuntime* runtime = CycleCollectedJSRuntime::Get();
+  nsTArray<nsRefPtr<nsIRunnable>>& microtaskQueue =
+    runtime->GetPromiseMicroTaskQueue();
+
+  microtaskQueue.AppendElement(aRunnable);
 }
 
 void
@@ -1068,7 +1083,7 @@ Promise::ResolveInternal(JSContext* aCx,
         new PromiseInit(thenObj, mozilla::dom::GetIncumbentGlobal());
       nsRefPtr<ThenableResolverTask> task =
         new ThenableResolverTask(this, valueObj, thenCallback);
-      DispatchToMainOrWorkerThread(task);
+      DispatchToMicroTask(task);
       return;
     }
   }
@@ -1134,7 +1149,7 @@ Promise::EnqueueCallbackTasks()
   for (uint32_t i = 0; i < callbacks.Length(); ++i) {
     nsRefPtr<PromiseCallbackTask> task =
       new PromiseCallbackTask(this, callbacks[i], mResult);
-    DispatchToMainOrWorkerThread(task);
+    DispatchToMicroTask(task);
   }
 }
 

dom/promise/Promise.h

@@ -111,6 +111,9 @@ public:
                                            // specializations in the .cpp for
                                            // the T values we support.
 
+  // Called by DOM to let us execute our callbacks.  May be called recursively.
+  static void PerformMicroTaskCheckpoint();
+
   // WebIDL
 
   nsIGlobalObject* GetParentObject() const
@@ -165,9 +168,9 @@ protected:
 
   virtual ~Promise();
 
-  // Queue an async task to current main or worker thread.
+  // Queue an async microtask to current main or worker thread.
   static void
-  DispatchToMainOrWorkerThread(nsIRunnable* aRunnable);
+  DispatchToMicroTask(nsIRunnable* aRunnable);
 
   // Do JS-wrapping after Promise creation.
   void CreateWrapper(ErrorResult& aRv);

dom/promise/tests/test_promise.html

@@ -116,20 +116,57 @@ function promiseGC() {
   resolve(42);
 }
 
-function promiseAsync() {
-  var global = "foo";
-  var f = new Promise(function(r1, r2) {
-    is(global, "foo", "Global should be foo");
-    r1(42);
-    is(global, "foo", "Global should still be foo");
-    setTimeout(function() {
-      // is(global, "bar", "Global should still be bar!"); // Bug 1013625
-      runTest();
-    }, 0);
-  }).then(function() {
-    global = "bar";
+function promiseAsync_TimeoutResolveThen() {
+  var handlerExecuted = false;
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  Promise.resolve().then(function() {
+    handlerExecuted = true;
   });
-  is(global, "foo", "Global should still be foo (2)");
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
+}
+
+function promiseAsync_ResolveTimeoutThen() {
+  var handlerExecuted = false;
+
+  var promise = Promise.resolve();
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  promise.then(function() {
+    handlerExecuted = true;
+  });
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
+}
+
+function promiseAsync_ResolveThenTimeout() {
+  var handlerExecuted = false;
+
+  Promise.resolve().then(function() {
+    handlerExecuted = true;
+  });
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
 }
 
 function promiseDoubleThen() {
@@ -715,7 +752,10 @@ function promiseWrapperAsyncResolution()
 }
 
 var tests = [ promiseResolve, promiseReject,
-              promiseException, promiseGC, promiseAsync,
+              promiseException, promiseGC,
+              promiseAsync_TimeoutResolveThen,
+              promiseAsync_ResolveTimeoutThen,
+              promiseAsync_ResolveThenTimeout,
               promiseDoubleThen, promiseThenException,
               promiseThenCatchThen, promiseRejectThenCatchThen,
               promiseRejectThenCatchThen2,

dom/workers/WorkerPrivate.cpp

@@ -49,6 +49,7 @@
 #include "mozilla/dom/MessageEvent.h"
 #include "mozilla/dom/MessageEventBinding.h"
 #include "mozilla/dom/MessagePortList.h"
+#include "mozilla/dom/Promise.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/StructuredClone.h"
 #include "mozilla/dom/WorkerBinding.h"
@@ -4112,6 +4113,10 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
     // Process a single runnable from the main queue.
     MOZ_ALWAYS_TRUE(NS_ProcessNextEvent(mThread, false));
 
+    // Only perform the Promise microtask checkpoint on the outermost event
+    // loop.  Don't run it, for example, during sync XHR or importScripts.
+    Promise::PerformMicroTaskCheckpoint();
+
     if (NS_HasPendingEvents(mThread)) {
       // Now *might* be a good time to GC. Let the JS engine make the decision.
       if (workerCompartment) {

dom/workers/test/promise_worker.js

@@ -95,20 +95,81 @@ function promiseException() {
   });
 }
 
-function promiseAsync() {
-  var global = "foo";
-  var f = new Promise(function(r1, r2) {
-    is(global, "foo", "Global should be foo");
-    r1(42);
-    is(global, "foo", "Global should still be foo");
-    setTimeout(function() {
-      is(global, "bar", "Global should still be bar!");
-      runTest();
-    }, 0);
-  }).then(function() {
-    global = "bar";
+function promiseAsync_TimeoutResolveThen() {
+  var handlerExecuted = false;
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  Promise.resolve().then(function() {
+    handlerExecuted = true;
   });
-  is(global, "foo", "Global should still be foo (2)");
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
+}
+
+function promiseAsync_ResolveTimeoutThen() {
+  var handlerExecuted = false;
+
+  var promise = Promise.resolve();
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  promise.then(function() {
+    handlerExecuted = true;
+  });
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
+}
+
+function promiseAsync_ResolveThenTimeout() {
+  var handlerExecuted = false;
+
+  Promise.resolve().then(function() {
+    handlerExecuted = true;
+  });
+
+  setTimeout(function() {
+    ok(handlerExecuted, "Handler should have been called before the timeout.");
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  }, 0);
+
+  ok(!handlerExecuted, "Handlers are not called before 'then' returns.");
+}
+
+function promiseAsync_SyncHXRAndImportScripts()
+{
+  var handlerExecuted = false;
+
+  Promise.resolve().then(function() {
+    handlerExecuted = true;
+
+    // Allow other assertions to run so the test could fail before the next one.
+    setTimeout(runTest, 0);
+  });
+
+  ok(!handlerExecuted, "Handlers are not called until the next microtask.");
+
+  var xhr = new XMLHttpRequest();
+  xhr.open("GET", "testXHR.txt", false);
+  xhr.send(null);
+
+  ok(!handlerExecuted, "Sync XHR should not trigger microtask execution.");
+
+  importScripts("relativeLoad_import.js");
+
+  ok(!handlerExecuted, "importScripts should not trigger microtask execution.");
 }
 
 function promiseDoubleThen() {
@@ -683,11 +744,53 @@ function promiseResolveThenableCleanStack() {
   },1000);
 }
 
+// Bug 1062323
+function promiseWrapperAsyncResolution()
+{
+  var p = new Promise(function(resolve, reject){
+    resolve();
+  });
+
+  var results = [];
+  var q = p.then(function () {
+    results.push("1-1");
+  }).then(function () {
+    results.push("1-2");
+  }).then(function () {
+    results.push("1-3");
+  });
+
+  var r = p.then(function () {
+    results.push("2-1");
+  }).then(function () {
+    results.push("2-2");
+  }).then(function () {
+    results.push("2-3");
+  });
+
+  Promise.all([q, r]).then(function() {
+    var match = results[0] == "1-1" &&
+                results[1] == "2-1" &&
+                results[2] == "1-2" &&
+                results[3] == "2-2" &&
+                results[4] == "1-3" &&
+                results[5] == "2-3";
+    ok(match, "Chained promises should resolve asynchronously.");
+    runTest();
+  }, function() {
+    ok(false, "promiseWrapperAsyncResolution: One of the promises failed.");
+    runTest();
+  });
+}
+
 var tests = [
     promiseResolve,
     promiseReject,
     promiseException,
-    promiseAsync,
+    promiseAsync_TimeoutResolveThen,
+    promiseAsync_ResolveTimeoutThen,
+    promiseAsync_ResolveThenTimeout,
+    promiseAsync_SyncHXRAndImportScripts,
     promiseDoubleThen,
     promiseThenException,
     promiseThenCatchThen,
@@ -729,6 +832,8 @@ var tests = [
     promiseResolvePromise,
 
     promiseResolveThenableCleanStack,
+
+    promiseWrapperAsyncResolution,
 ];
 
 function runTest() {

js/xpconnect/src/nsXPConnect.cpp

@@ -22,7 +22,7 @@
 
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/Exceptions.h"
-#include "mozilla/dom/PromiseBinding.h"
+#include "mozilla/dom/Promise.h"
 #include "mozilla/dom/TextDecoderBinding.h"
 #include "mozilla/dom/TextEncoderBinding.h"
 #include "mozilla/dom/DOMErrorBinding.h"
@@ -1053,7 +1053,10 @@ nsXPConnect::AfterProcessNextEvent(nsIThreadInternal *aThread,
     // Call cycle collector occasionally.
     MOZ_ASSERT(NS_IsMainThread());
     nsJSContext::MaybePokeCC();
-    nsDOMMutationObserver::HandleMutations();
+
+    nsContentUtils::PerformMainThreadMicroTaskCheckpoint();
+
+    Promise::PerformMicroTaskCheckpoint();
 
     PopJSContextNoScriptContext();
 

xpcom/base/CycleCollectedJSRuntime.cpp

@@ -955,6 +955,12 @@ CycleCollectedJSRuntime::SetPendingException(nsIException* aException)
   mPendingException = aException;
 }
 
+nsTArray<nsRefPtr<nsIRunnable>>&
+CycleCollectedJSRuntime::GetPromiseMicroTaskQueue()
+{
+  return mPromiseMicroTaskQueue;
+}
+
 nsCycleCollectionParticipant*
 CycleCollectedJSRuntime::GCThingParticipant()
 {

xpcom/base/CycleCollectedJSRuntime.h

@@ -18,6 +18,7 @@
 
 class nsCycleCollectionNoteRootCallback;
 class nsIException;
+class nsIRunnable;
 
 namespace js {
 struct Class;
@@ -257,6 +258,8 @@ public:
   already_AddRefed<nsIException> GetPendingException() const;
   void SetPendingException(nsIException* aException);
 
+  nsTArray<nsRefPtr<nsIRunnable>>& GetPromiseMicroTaskQueue();
+
   nsCycleCollectionParticipant* GCThingParticipant();
   nsCycleCollectionParticipant* ZoneParticipant();
 
@@ -306,6 +309,8 @@ private:
 
   nsCOMPtr<nsIException> mPendingException;
 
+  nsTArray<nsRefPtr<nsIRunnable>> mPromiseMicroTaskQueue;
+
   OOMState mOutOfMemoryState;
   OOMState mLargeAllocationFailureState;
 };