Bug 1095443 - Ensure processNextEvent never blocks after processing a Promise microtask. r=bz
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 11 Nov 2014 13:47:28 +0000
changeset 226475 d8c5f67ac4cd8c78b7f2ed00f2362b22ea38ebe8
parent 226474 942aec7a1572dca9e2f5ebecf84a879d0eb354b9
child 226476 ebec8f452f657971ab44f84d210acbbf0b92ccc9
push id53
push userdglastonbury@mozilla.com
push dateWed, 12 Nov 2014 02:04:58 +0000
reviewersbz
bugs1095443
milestone36.0a1
Bug 1095443 - Ensure processNextEvent never blocks after processing a Promise microtask. r=bz
dom/promise/Promise.cpp
dom/promise/Promise.h
dom/workers/WorkerPrivate.cpp
js/xpconnect/src/nsXPConnect.cpp
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -355,31 +355,37 @@ Promise::MaybeReject(JSContext* aCx,
   MaybeRejectInternal(aCx, aValue);
 }
 
 void
 Promise::MaybeReject(const nsRefPtr<MediaStreamError>& aArg) {
   MaybeSomething(aArg, &Promise::MaybeReject);
 }
 
-void
+bool
 Promise::PerformMicroTaskCheckpoint()
 {
   CycleCollectedJSRuntime* runtime = CycleCollectedJSRuntime::Get();
   nsTArray<nsRefPtr<nsIRunnable>>& microtaskQueue =
     runtime->GetPromiseMicroTaskQueue();
 
-  while (!microtaskQueue.IsEmpty()) {
+  if (microtaskQueue.IsEmpty()) {
+    return false;
+  }
+
+  do {
     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();
-  }
+  } while (!microtaskQueue.IsEmpty());
+
+  return true;
 }
 
 /* static */ bool
 Promise::JSCallback(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
 {
   JS::CallArgs args = CallArgsFromVp(aArgc, aVp);
 
   JS::Rooted<JS::Value> v(aCx,
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -116,17 +116,18 @@ public:
   // require use to include DOMError.h either here or in all those translation
   // units.
   template<typename T>
   void MaybeRejectBrokenly(const T& aArg); // Not implemented by default; see
                                            // 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();
+  // Returns true if at least one microtask was processed.
+  static bool PerformMicroTaskCheckpoint();
 
   // WebIDL
 
   nsIGlobalObject* GetParentObject() const
   {
     return mGlobal;
   }
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -4238,17 +4238,17 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
     // Start the periodic GC timer if it is not already running.
     SetGCTimerMode(PeriodicTimer);
 
     // 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();
+    (void)Promise::PerformMicroTaskCheckpoint();
 
     if (NS_HasPendingEvents(mThread)) {
       // Now *might* be a good time to GC. Let the JS engine make the decision.
       if (workerCompartment) {
         JS_MaybeGC(aCx);
       }
     }
     else {
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -1003,33 +1003,50 @@ nsXPConnect::JSToVariant(JSContext* ctx,
     nsRefPtr<XPCVariant> variant = XPCVariant::newVariant(ctx, value);
     variant.forget(_retval);
     if (!(*_retval))
         return NS_ERROR_FAILURE;
 
     return NS_OK;
 }
 
+namespace {
+
+class DummyRunnable : public nsRunnable {
+public:
+    NS_IMETHOD Run() { return NS_OK; }
+};
+
+} // anonymous namespace
+
 NS_IMETHODIMP
 nsXPConnect::OnProcessNextEvent(nsIThreadInternal *aThread, bool aMayWait,
                                 uint32_t aRecursionDepth)
 {
+    MOZ_ASSERT(NS_IsMainThread());
+
     // If ProcessNextEvent was called during a Promise "then" callback, we
     // must process any pending microtasks before blocking in the event loop,
     // otherwise we may deadlock until an event enters the queue later.
     if (aMayWait) {
-        Promise::PerformMicroTaskCheckpoint();
+        if (Promise::PerformMicroTaskCheckpoint()) {
+            // If any microtask was processed, we post a dummy event in order to
+            // force the ProcessNextEvent call not to block.  This is required
+            // to support nested event loops implemented using a pattern like
+            // "while (condition) thread.processNextEvent(true)", in case the
+            // condition is triggered here by a Promise "then" callback.
+            NS_DispatchToMainThread(new DummyRunnable());
+        }
     }
 
     // Record this event.
     mEventDepth++;
 
     // Push a null JSContext so that we don't see any script during
     // event processing.
-    MOZ_ASSERT(NS_IsMainThread());
     bool ok = PushJSContextNoScriptContext(nullptr);
     NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPConnect::AfterProcessNextEvent(nsIThreadInternal *aThread,
                                    uint32_t aRecursionDepth,