[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor-browser/tor-browser-91.5.0esr-11.5-2] Revert "Bug 1724777, optimize suppressed MicroTask handling, r=mccr8 a=RyanVM"



commit 24528ca31fd4bf981d09cdea9aff61175245d65f
Author: Georg Koppen <gk@xxxxxxxxxxxxxx>
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 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.



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits