From 0c6597b3e013995a0893bcc0f2459bd874db740f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 7 May 2015 14:49:31 -0400 Subject: [PATCH] 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 | 22 +++++++++++++------ .../file_promise_and_timeout_ordering.js | 18 +++++++++++++++ dom/promise/tests/mochitest.ini | 4 ++++ .../test_promise_and_timeout_ordering.html | 15 +++++++++++++ ..._promise_and_timeout_ordering_workers.html | 13 +++++++++++ dom/workers/WorkerPrivate.cpp | 4 ++++ 6 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 dom/promise/tests/file_promise_and_timeout_ordering.js create mode 100644 dom/promise/tests/test_promise_and_timeout_ordering.html create mode 100644 dom/promise/tests/test_promise_and_timeout_ordering_workers.html diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 19f1723aa5fc..cc82583b6e1f 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -12447,6 +12447,11 @@ nsGlobalWindow::RunTimeoutHandler(nsTimeout* aTimeout, 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; } @@ -12566,14 +12571,17 @@ nsGlobalWindow::RunTimeout(nsTimeout *aTimeout) 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; diff --git a/dom/promise/tests/file_promise_and_timeout_ordering.js b/dom/promise/tests/file_promise_and_timeout_ordering.js new file mode 100644 index 000000000000..c83b5a6ac0b2 --- /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); diff --git a/dom/promise/tests/mochitest.ini b/dom/promise/tests/mochitest.ini index 3b621b1e1967..3dadb7240791 100644 --- a/dom/promise/tests/mochitest.ini +++ b/dom/promise/tests/mochitest.ini @@ -7,3 +7,7 @@ [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 diff --git a/dom/promise/tests/test_promise_and_timeout_ordering.html b/dom/promise/tests/test_promise_and_timeout_ordering.html new file mode 100644 index 000000000000..1dfa13bbba3e --- /dev/null +++ b/dom/promise/tests/test_promise_and_timeout_ordering.html @@ -0,0 +1,15 @@ + + +Test for promise and timeout ordering + + +
+ + diff --git a/dom/promise/tests/test_promise_and_timeout_ordering_workers.html b/dom/promise/tests/test_promise_and_timeout_ordering_workers.html new file mode 100644 index 000000000000..122a794db645 --- /dev/null +++ b/dom/promise/tests/test_promise_and_timeout_ordering_workers.html @@ -0,0 +1,13 @@ + + +Test for promise and timeout ordering in workers + + +
+ diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 9992e71b24c6..f363e91fc8ff 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -6745,6 +6745,10 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) } } + // 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!"); }