[tor-browser/tor-browser-91.6.0esr-11.5-1] Revert "Bug 1724777, optimize suppressed MicroTask handling, r=mccr8 a=RyanVM"
commit 9901ee290ce9171d5487690c1dfa26935027dcc3 Author: Georg Koppen gk@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. However, it is a temporary workaround, that we should revert once https://bugzilla.mozilla.org/show_bug.cgi?id=1744719 is fixed. --- 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 a58e76cb5258..0ef4b3236477 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -15633,18 +15633,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 7165496397f3..69e59d09b924 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 506ef24185c4..6907a3ae816c 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::UniquePtrJS::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_AddRefeddom::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.
tbb-commits@lists.torproject.org
-
richard＠torproject.org