Bug 1162013. Process the Promise queue between adjacent setTimeout callback invocations when we're going through the callback list without returning to the event loop. r=smaug
☠☠ backed out by 8778cdfb891b ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 07 May 2015 14:49:31 -0400
changeset 274241 57f0b16030e9f7245a5cd8879e261e25354cfa7e
parent 274240 bec29299cb7ec9108297c68fade1b0ddbb0873f0
child 274242 262f4f70c56d932c1e3eaad26e7c447abae81825
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1162013
milestone40.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 1162013. Process the Promise queue between adjacent setTimeout callback invocations when we're going through the callback list without returning to the event loop. r=smaug
dom/base/nsGlobalWindow.cpp
dom/promise/tests/file_promise_and_timeout_ordering.js
dom/promise/tests/mochitest.ini
dom/promise/tests/test_promise_and_timeout_ordering.html
dom/promise/tests/test_promise_and_timeout_ordering_workers.html
dom/workers/WorkerPrivate.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -12442,16 +12442,21 @@ nsGlobalWindow::RunTimeoutHandler(nsTime
     sNestingLevel = nestingLevel;
   }
 
   --mTimeoutFiringDepth;
   --gRunningTimeoutDepth;
 
   mRunningTimeout = last_running_timeout;
   timeout->mRunning = false;
+
+  // Since we might be processing more timeouts, go ahead and flush the promise
+  // queue now before we do that.
+  Promise::PerformMicroTaskCheckpoint();
+
   return timeout->mCleared;
 }
 
 bool
 nsGlobalWindow::RescheduleTimeout(nsTimeout* aTimeout, const TimeStamp& now,
                                   bool aRunningPendingTimeouts)
 {
   if (!aTimeout->mIsInterval) {
@@ -12561,24 +12566,27 @@ nsGlobalWindow::RunTimeout(nsTimeout *aT
     // timers that *should* have fired before aTimeout *will* be fired
     // now.
 
     deadline = aTimeout->mWhen;
   } else {
     deadline = now;
   }
 
-  // The timeout list is kept in deadline order. Discover the latest
-  // timeout whose deadline has expired. On some platforms, native
-  // timeout events fire "early", so we need to test the timer as well
-  // as the deadline.
+  // The timeout list is kept in deadline order. Discover the latest timeout
+  // whose deadline has expired. On some platforms, native timeout events fire
+  // "early", but we handled that above by setting deadline to aTimeout->mWhen
+  // if the timer fired early.  So we can stop walking if we get to timeouts
+  // whose mWhen is greater than deadline, since once that happens we know
+  // nothing past that point is expired.
   last_expired_timeout = nullptr;
-  for (nsTimeout *timeout = mTimeouts.getFirst(); timeout; timeout = timeout->getNext()) {
-    if (((timeout == aTimeout) || (timeout->mWhen <= deadline)) &&
-        (timeout->mFiringDepth == 0)) {
+  for (nsTimeout *timeout = mTimeouts.getFirst();
+       timeout && timeout->mWhen <= deadline;
+       timeout = timeout->getNext()) {
+    if (timeout->mFiringDepth == 0) {
       // Mark any timeouts that are on the list to be fired with the
       // firing depth so that we can reentrantly run timeouts
       timeout->mFiringDepth = firingDepth;
       last_expired_timeout = timeout;
     }
   }
 
   // Maybe the timeout that the event was fired for has been deleted
new file mode 100644
--- /dev/null
+++ b/dom/promise/tests/file_promise_and_timeout_ordering.js
@@ -0,0 +1,18 @@
+var log = [];
+var resolvedPromise = Promise.resolve(null);
+function schedulePromiseTask(f) {
+  resolvedPromise.then(f);
+}
+
+setTimeout(function() {
+  log.push('t1start');
+  schedulePromiseTask(function() {
+    log.push('promise');
+  });
+  log.push('t1end');
+}, 10);
+
+setTimeout(function() {
+  log.push('t2');
+  postMessage(log.join(', '));
+}, 10);
--- a/dom/promise/tests/mochitest.ini
+++ b/dom/promise/tests/mochitest.ini
@@ -2,8 +2,12 @@
 
 [test_abortable_promise.html]
 [test_bug883683.html]
 [test_promise.html]
 [test_promise_utils.html]
 [test_resolve.html]
 [test_resolver_return_value.html]
 [test_thenable_vs_promise_ordering.html]
+[test_promise_and_timeout_ordering.html]
+support-files = file_promise_and_timeout_ordering.js
+[test_promise_and_timeout_ordering_workers.html]
+support-files = file_promise_and_timeout_ordering.js
new file mode 100644
--- /dev/null
+++ b/dom/promise/tests/test_promise_and_timeout_ordering.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test for promise and timeout ordering</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+var t = async_test("Promise callbacks should run immediately after the setTimeout handler that enqueues them");
+var origPostMessage = window.postMessage;
+window.postMessage = function(msg) { origPostMessage.call(window, msg, "*"); }
+window.onmessage = t.step_func_done(function(e) {
+  assert_equals(e.data, "t1start, t1end, promise, t2");
+});
+</script>
+<script src="file_promise_and_timeout_ordering.js"></script>
new file mode 100644
--- /dev/null
+++ b/dom/promise/tests/test_promise_and_timeout_ordering_workers.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test for promise and timeout ordering in workers</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+var t = async_test("Promise callbacks in workers should run immediately after the setTimeout handler that enqueues them");
+var w = new Worker("file_promise_and_timeout_ordering.js");
+w.onmessage = t.step_func_done(function(e) {
+  assert_equals(e.data, "t1start, t1end, promise, t2");
+});
+</script>
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -6740,16 +6740,20 @@ WorkerPrivate::RunExpiredTimeouts(JSCont
            !JS::Evaluate(aCx, options,
                          expression.get(), expression.Length(), &unused)) &&
           !JS_ReportPendingException(aCx)) {
         retval = false;
         break;
       }
     }
 
+    // Since we might be processing more timeouts, go ahead and flush
+    // the promise queue now before we do that.
+    Promise::PerformMicroTaskCheckpoint();
+
     NS_ASSERTION(mRunningExpiredTimeouts, "Someone changed this!");
   }
 
   // No longer possible to be called recursively.
   mRunningExpiredTimeouts = false;
 
   // Now remove canceled and expired timeouts from the main list.
   // NB: The timeouts present in expiredTimeouts must have the same order