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
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 07 May 2015 14:49:31 -0400
changeset 274284 a455c8bce215d3b9361c55ee68c9d36e6f4c2379
parent 274283 b3d61b6150fd24fb7076ed49262a7df4da3b46a4
child 274285 6a92a651603a422437925421e319cef429ee7c71
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
@@ -12433,25 +12433,32 @@ nsGlobalWindow::RunTimeoutHandler(nsTime
   // Call() on a Function here since we're in a loop
   // where we're likely to be running timeouts whose OS timers
   // didn't fire in time and we don't want to not fire those timers
   // now just because execution of one timer failed. We can't
   // propagate the error to anyone who cares about it from this
   // point anyway, and the script context should have already reported
   // the script error in the usual way - so we just drop it.
 
+  // Since we might be processing more timeouts, go ahead and flush the promise
+  // queue now before we do that.  We need to do that while we're still in our
+  // "running JS is safe" state (e.g. mRunningTimeout is set, timeout->mRunning
+  // is false).
+  Promise::PerformMicroTaskCheckpoint();
+
   if (trackNestingLevel) {
     sNestingLevel = nestingLevel;
   }
 
   --mTimeoutFiringDepth;
   --gRunningTimeoutDepth;
 
   mRunningTimeout = last_running_timeout;
   timeout->mRunning = false;
+
   return timeout->mCleared;
 }
 
 bool
 nsGlobalWindow::RescheduleTimeout(nsTimeout* aTimeout, const TimeStamp& now,
                                   bool aRunningPendingTimeouts)
 {
   if (!aTimeout->mIsInterval) {
@@ -12561,24 +12568,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