[tbb-commits] [tor-browser/tor-browser-91.4.0esr-11.5-1] Revert "Bug 1724777, optimize suppressed MicroTask handling, r=mccr8 a=RyanVM"
gk at torproject.org
gk at torproject.org
Tue Dec 7 18:39:46 UTC 2021
commit 1542d19fd888586801eba130a6750ab0cbd93e9e
Author: Georg Koppen <gk at torproject.org>
Date: Tue Dec 7 16:19:15 2021 +0000
Revert "Bug 1724777, optimize suppressed MicroTask handling, r=mccr8 a=RyanVM"
This reverts commit 1eb1364357ac5bc2a4531337fb5416af39c3793f.
This fixes tor-browser#40721, tor-browser#40698, and tor-browser#40706.
---
dom/base/Document.cpp | 12 ------
dom/base/Document.h | 8 +++-
dom/base/test/mochitest.ini | 2 -
dom/base/test/test_suppressed_microtasks.html | 62 ---------------------------
dom/workers/RuntimeService.cpp | 4 +-
dom/workers/WorkerPrivate.cpp | 2 +-
dom/worklet/WorkletThread.cpp | 2 +-
xpcom/base/CycleCollectedJSContext.cpp | 51 ++++++----------------
xpcom/base/CycleCollectedJSContext.h | 29 +++----------
9 files changed, 28 insertions(+), 144 deletions(-)
diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
index 7e103e2705d1..2973457ec3de 100644
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -15665,18 +15665,6 @@ nsAutoSyncOperation::~nsAutoSyncOperation() {
}
}
-void Document::SetIsInSyncOperation(bool aSync) {
- if (CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get()) {
- ccjs->UpdateMicroTaskSuppressionGeneration();
- }
-
- if (aSync) {
- ++mInSyncOperationCount;
- } else {
- --mInSyncOperationCount;
- }
-}
-
gfxUserFontSet* Document::GetUserFontSet() {
if (!mFontFaceSet) {
return nullptr;
diff --git a/dom/base/Document.h b/dom/base/Document.h
index c8de049526ea..7ef73651d47f 100644
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@@ -3214,7 +3214,13 @@ class Document : public nsINode,
bool IsInSyncOperation() { return mInSyncOperationCount != 0; }
- void SetIsInSyncOperation(bool aSync);
+ void SetIsInSyncOperation(bool aSync) {
+ if (aSync) {
+ ++mInSyncOperationCount;
+ } else {
+ --mInSyncOperationCount;
+ }
+ }
bool CreatingStaticClone() const { return mCreatingStaticClone; }
diff --git a/dom/base/test/mochitest.ini b/dom/base/test/mochitest.ini
index 06b5691422c5..e287a0d10ae8 100644
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -769,8 +769,6 @@ skip-if = debug == false
[test_shared_compartment2.html]
[test_structuredclone_backref.html]
[test_style_cssText.html]
-[test_suppressed_microtasks.html]
-skip-if = debug || asan || verify || toolkit == 'android' # The test needs to run reasonably fast.
[test_text_wholeText.html]
[test_textnode_normalize_in_selection.html]
[test_textnode_split_in_selection.html]
diff --git a/dom/base/test/test_suppressed_microtasks.html b/dom/base/test/test_suppressed_microtasks.html
deleted file mode 100644
index f5d333638698..000000000000
--- a/dom/base/test/test_suppressed_microtasks.html
+++ /dev/null
@@ -1,62 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
- <meta charset="utf-8">
- <title>Test microtask suppression</title>
- <script src="/tests/SimpleTest/SimpleTest.js"></script>
- <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
- <script>
- SimpleTest.waitForExplicitFinish();
-
- var previousTask = -1;
- function test() {
- let win = window.open("about:blank");
- win.onload = function() {
- win.onmessage = function() {
- win.start = win.performance.now();
- win.didRunMicrotask = false;
- win.onmessage = function() {
- ok(win.didRunMicrotask, "Should have run a microtask.");
- let period = win.performance.now() - win.start;
- win.opener.ok(
- period < 200,
- "Running a task should be fast. Took " + period + "ms.");
- win.onmessage = null;
- }
- win.queueMicrotask(function() { win.didRunMicrotask = true; });
- win.postMessage("measurementMessage", "*");
- }
- win.postMessage("initialMessage", "*");
-
- const last = 500000;
- for (let i = 0; i < last + 1; ++i) {
- window.queueMicrotask(function() {
- // Check that once microtasks are unsuppressed, they are handled in
- // the correct order.
- if (previousTask != i - 1) {
- // Explicitly optimize out cases which pass.
- ok(false, "Microtasks should be handled in order.");
- }
- previousTask = i;
- if (i == last) {
- win.close();
- SimpleTest.finish();
- }
- });
- }
-
- // Synchronous XMLHttpRequest suppresses microtasks.
- var xhr = new XMLHttpRequest();
- xhr.open("GET", "slow.sjs", false);
- xhr.send();
- is(previousTask, -1, "Shouldn't have run microtasks during a sync XHR.");
- }
- }
- </script>
-</head>
-<body onload="test()">
-<p id="display"></p>
-<div id="content" style="display: none"></div>
-<pre id="test"></pre>
-</body>
-</html>
diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp
index c3e3f56834d7..3fda0a78fd23 100644
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -931,7 +931,7 @@ class WorkerJSContext final : public mozilla::CycleCollectedJSContext {
MOZ_ASSERT(!NS_IsMainThread());
MOZ_ASSERT(runnable);
- std::deque<RefPtr<MicroTaskRunnable>>* microTaskQueue = nullptr;
+ std::queue<RefPtr<MicroTaskRunnable>>* microTaskQueue = nullptr;
JSContext* cx = Context();
NS_ASSERTION(cx, "This should never be null!");
@@ -953,7 +953,7 @@ class WorkerJSContext final : public mozilla::CycleCollectedJSContext {
}
JS::JobQueueMayNotBeEmpty(cx);
- microTaskQueue->push_back(std::move(runnable));
+ microTaskQueue->push(std::move(runnable));
}
bool IsSystemCaller() const override {
diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
index 10099edc933e..af627f33d86d 100644
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -4313,7 +4313,7 @@ void WorkerPrivate::EnterDebuggerEventLoop() {
{
MutexAutoLock lock(mMutex);
- std::deque<RefPtr<MicroTaskRunnable>>& debuggerMtQueue =
+ std::queue<RefPtr<MicroTaskRunnable>>& debuggerMtQueue =
ccjscx->GetDebuggerMicroTaskQueue();
while (mControlQueue.IsEmpty() &&
!(debuggerRunnablesPending = !mDebuggerQueue.IsEmpty()) &&
diff --git a/dom/worklet/WorkletThread.cpp b/dom/worklet/WorkletThread.cpp
index fae1a1c550d1..c672dfb21b8b 100644
--- a/dom/worklet/WorkletThread.cpp
+++ b/dom/worklet/WorkletThread.cpp
@@ -159,7 +159,7 @@ class WorkletJSContext final : public CycleCollectedJSContext {
#endif
JS::JobQueueMayNotBeEmpty(cx);
- GetMicroTaskQueue().push_back(std::move(runnable));
+ GetMicroTaskQueue().push(std::move(runnable));
}
bool IsSystemCaller() const override {
diff --git a/xpcom/base/CycleCollectedJSContext.cpp b/xpcom/base/CycleCollectedJSContext.cpp
index 0a35a5cf5524..347f15c82322 100644
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -61,7 +61,6 @@ CycleCollectedJSContext::CycleCollectedJSContext()
mDoingStableStates(false),
mTargetedMicroTaskRecursionDepth(0),
mMicroTaskLevel(0),
- mSuppressionGeneration(0),
mDebuggerRecursionDepth(0),
mMicroTaskRecursionDepth(0),
mFinalizationRegistryCleanup(this) {
@@ -292,7 +291,7 @@ class CycleCollectedJSContext::SavedMicroTaskQueue
private:
CycleCollectedJSContext* ccjs;
- std::deque<RefPtr<MicroTaskRunnable>> mQueue;
+ std::queue<RefPtr<MicroTaskRunnable>> mQueue;
};
js::UniquePtr<JS::JobQueue::SavedJobQueue>
@@ -380,13 +379,13 @@ void CycleCollectedJSContext::SetPendingException(Exception* aException) {
mPendingException = aException;
}
-std::deque<RefPtr<MicroTaskRunnable>>&
+std::queue<RefPtr<MicroTaskRunnable>>&
CycleCollectedJSContext::GetMicroTaskQueue() {
MOZ_ASSERT(mJSContext);
return mPendingMicroTaskRunnables;
}
-std::deque<RefPtr<MicroTaskRunnable>>&
+std::queue<RefPtr<MicroTaskRunnable>>&
CycleCollectedJSContext::GetDebuggerMicroTaskQueue() {
MOZ_ASSERT(mJSContext);
return mDebuggerMicroTaskQueue;
@@ -563,7 +562,7 @@ void CycleCollectedJSContext::DispatchToMicroTask(
JS::JobQueueMayNotBeEmpty(Context());
LogMicroTaskRunnable::LogDispatch(runnable.get());
- mPendingMicroTaskRunnables.push_back(std::move(runnable));
+ mPendingMicroTaskRunnables.push(std::move(runnable));
}
class AsyncMutationHandler final : public mozilla::Runnable {
@@ -582,25 +581,6 @@ class AsyncMutationHandler final : public mozilla::Runnable {
}
};
-SuppressedMicroTasks::SuppressedMicroTasks(CycleCollectedJSContext* aContext)
- : mContext(aContext),
- mSuppressionGeneration(aContext->mSuppressionGeneration) {}
-
-bool SuppressedMicroTasks::Suppressed() {
- if (mSuppressionGeneration == mContext->mSuppressionGeneration) {
- return true;
- }
-
- for (std::deque<RefPtr<MicroTaskRunnable>>::reverse_iterator it =
- mSuppressedMicroTaskRunnables.rbegin();
- it != mSuppressedMicroTaskRunnables.rend(); ++it) {
- mContext->GetMicroTaskQueue().push_front(*it);
- }
- mContext->mSuppressedMicroTasks = nullptr;
-
- return false;
-}
-
bool CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool aForce) {
if (mPendingMicroTaskRunnables.empty() && mDebuggerMicroTaskQueue.empty()) {
AfterProcessMicrotasks();
@@ -636,14 +616,15 @@ bool CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool aForce) {
bool didProcess = false;
AutoSlowOperation aso;
+ std::queue<RefPtr<MicroTaskRunnable>> suppressed;
for (;;) {
RefPtr<MicroTaskRunnable> runnable;
if (!mDebuggerMicroTaskQueue.empty()) {
runnable = std::move(mDebuggerMicroTaskQueue.front());
- mDebuggerMicroTaskQueue.pop_front();
+ mDebuggerMicroTaskQueue.pop();
} else if (!mPendingMicroTaskRunnables.empty()) {
runnable = std::move(mPendingMicroTaskRunnables.front());
- mPendingMicroTaskRunnables.pop_front();
+ mPendingMicroTaskRunnables.pop();
} else {
break;
}
@@ -654,16 +635,10 @@ bool CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool aForce) {
// all suppressed tasks in mDebuggerMicroTaskQueue unexpectedly.
MOZ_ASSERT(NS_IsMainThread());
JS::JobQueueMayNotBeEmpty(Context());
- if (runnable != mSuppressedMicroTasks) {
- if (!mSuppressedMicroTasks) {
- mSuppressedMicroTasks = new SuppressedMicroTasks(this);
- }
- mSuppressedMicroTasks->mSuppressedMicroTaskRunnables.push_back(
- runnable);
- }
+ suppressed.push(runnable);
} else {
if (mPendingMicroTaskRunnables.empty() &&
- mDebuggerMicroTaskQueue.empty() && !mSuppressedMicroTasks) {
+ mDebuggerMicroTaskQueue.empty() && suppressed.empty()) {
JS::JobQueueIsEmpty(Context());
}
didProcess = true;
@@ -678,9 +653,7 @@ bool CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool aForce) {
// Note, it is possible that we end up keeping these suppressed tasks around
// for some time, but no longer than spinning the event loop nestedly
// (sync XHR, alert, etc.)
- if (mSuppressedMicroTasks) {
- mPendingMicroTaskRunnables.push_back(mSuppressedMicroTasks);
- }
+ mPendingMicroTaskRunnables.swap(suppressed);
AfterProcessMicrotasks();
@@ -695,7 +668,7 @@ void CycleCollectedJSContext::PerformDebuggerMicroTaskCheckpoint() {
for (;;) {
// For a debugger microtask checkpoint, we always use the debugger microtask
// queue.
- std::deque<RefPtr<MicroTaskRunnable>>* microtaskQueue =
+ std::queue<RefPtr<MicroTaskRunnable>>* microtaskQueue =
&GetDebuggerMicroTaskQueue();
if (microtaskQueue->empty()) {
@@ -708,7 +681,7 @@ void CycleCollectedJSContext::PerformDebuggerMicroTaskCheckpoint() {
LogMicroTaskRunnable::Run log(runnable.get());
// This function can re-enter, so we remove the element before calling.
- microtaskQueue->pop_front();
+ microtaskQueue->pop();
if (mPendingMicroTaskRunnables.empty() && mDebuggerMicroTaskQueue.empty()) {
JS::JobQueueIsEmpty(Context());
diff --git a/xpcom/base/CycleCollectedJSContext.h b/xpcom/base/CycleCollectedJSContext.h
index 116bff1c90c8..769b000418ab 100644
--- a/xpcom/base/CycleCollectedJSContext.h
+++ b/xpcom/base/CycleCollectedJSContext.h
@@ -7,7 +7,7 @@
#ifndef mozilla_CycleCollectedJSContext_h
#define mozilla_CycleCollectedJSContext_h
-#include <deque>
+#include <queue>
#include "mozilla/Attributes.h"
#include "mozilla/MemoryReporting.h"
@@ -81,20 +81,6 @@ class MicroTaskRunnable {
virtual ~MicroTaskRunnable() = default;
};
-// Store the suppressed mictotasks in another microtask so that operations
-// for the microtask queue as a whole keep working.
-class SuppressedMicroTasks : public MicroTaskRunnable {
- public:
- explicit SuppressedMicroTasks(CycleCollectedJSContext* aContext);
-
- MOZ_CAN_RUN_SCRIPT_BOUNDARY void Run(AutoSlowOperation& aAso) final {}
- virtual bool Suppressed();
-
- CycleCollectedJSContext* mContext;
- uint64_t mSuppressionGeneration;
- std::deque<RefPtr<MicroTaskRunnable>> mSuppressedMicroTaskRunnables;
-};
-
// Support for JS FinalizationRegistry objects, which allow a JS callback to be
// registered that is called when objects die.
//
@@ -131,7 +117,6 @@ class FinalizationRegistryCleanup {
class CycleCollectedJSContext : dom::PerThreadAtomCache, private JS::JobQueue {
friend class CycleCollectedJSRuntime;
- friend class SuppressedMicroTasks;
protected:
CycleCollectedJSContext();
@@ -181,8 +166,8 @@ class CycleCollectedJSContext : dom::PerThreadAtomCache, private JS::JobQueue {
already_AddRefed<dom::Exception> GetPendingException() const;
void SetPendingException(dom::Exception* aException);
- std::deque<RefPtr<MicroTaskRunnable>>& GetMicroTaskQueue();
- std::deque<RefPtr<MicroTaskRunnable>>& GetDebuggerMicroTaskQueue();
+ std::queue<RefPtr<MicroTaskRunnable>>& GetMicroTaskQueue();
+ std::queue<RefPtr<MicroTaskRunnable>>& GetDebuggerMicroTaskQueue();
JSContext* Context() const {
MOZ_ASSERT(mJSContext);
@@ -198,8 +183,6 @@ class CycleCollectedJSContext : dom::PerThreadAtomCache, private JS::JobQueue {
mTargetedMicroTaskRecursionDepth = aDepth;
}
- void UpdateMicroTaskSuppressionGeneration() { ++mSuppressionGeneration; }
-
protected:
JSContext* MaybeContext() const { return mJSContext; }
@@ -333,10 +316,8 @@ class CycleCollectedJSContext : dom::PerThreadAtomCache, private JS::JobQueue {
uint32_t mMicroTaskLevel;
- std::deque<RefPtr<MicroTaskRunnable>> mPendingMicroTaskRunnables;
- std::deque<RefPtr<MicroTaskRunnable>> mDebuggerMicroTaskQueue;
- RefPtr<SuppressedMicroTasks> mSuppressedMicroTasks;
- uint64_t mSuppressionGeneration;
+ std::queue<RefPtr<MicroTaskRunnable>> mPendingMicroTaskRunnables;
+ std::queue<RefPtr<MicroTaskRunnable>> mDebuggerMicroTaskQueue;
// How many times the debugger has interrupted execution, possibly creating
// microtask checkpoints in places that they would not normally occur.
More information about the tbb-commits
mailing list