Bug 1646498 - Fix GetInProcessTop usage in nsAutoSyncOperation, r=kmag

This makes it so that we correctly set nsIGlobalObject::IsInSyncOperation
for in-process descendant frames that have out-of-process ancestors.

Differential Revision: https://phabricator.services.mozilla.com/D102330

dom/base/AutoSuppressEventHandlingAndSuspend.h

@@ -7,16 +7,14 @@
 #ifndef dom_base_AutoSuppressEventHandlingAndSuspend_h
 #define dom_base_AutoSuppressEventHandlingAndSuspend_h
 
+#include "mozilla/dom/BrowsingContext.h"
+#include "mozilla/dom/BrowsingContextGroup.h"
 #include "mozilla/dom/Document.h"
 #include "nsCOMPtr.h"
 #include "nsPIDOMWindow.h"
 #include "nsTArray.h"
 
 namespace mozilla::dom {
-
-class BrowsingContext;
-class BrowsingContextGroup;
-
 /**
  * Suppresses event handling and suspends the active inner window for all
  * in-process documents in a BrowsingContextGroup. This should be used while
@@ -25,15 +23,22 @@ class BrowsingContextGroup;
  * group.
  */
 
-class MOZ_RAII AutoSuppressEventHandlingAndSuspend {
+class MOZ_RAII AutoSuppressEventHandlingAndSuspend
+    : private AutoWalkBrowsingContextGroup {
  public:
-  explicit AutoSuppressEventHandlingAndSuspend(BrowsingContextGroup* aGroup);
+  explicit AutoSuppressEventHandlingAndSuspend(BrowsingContextGroup* aGroup) {
+    if (aGroup) {
+      SuppressBrowsingContextGroup(aGroup);
+    }
+  }
+
   ~AutoSuppressEventHandlingAndSuspend();
 
- private:
-  void SuppressBrowsingContext(BrowsingContext* aBC);
+ protected:
+  void SuppressDocument(Document* aDocument) override;
+  void UnsuppressDocument(Document* aDocument) override;
 
-  AutoTArray<RefPtr<Document>, 16> mDocuments;
+ private:
   AutoTArray<nsCOMPtr<nsPIDOMWindowInner>, 16> mWindows;
 };
 }  // namespace mozilla::dom

dom/base/Document.cpp

@@ -15328,38 +15328,32 @@ already_AddRefed<Element> Document::CreateHTMLElement(nsAtom* aTag) {
   return element.forget();
 }
 
-static CallState MarkDocumentTreeToBeInSyncOperation(
-    Document& aDoc, nsTArray<RefPtr<Document>>& aDocuments) {
-  aDoc.SetIsInSyncOperation(true);
-  if (nsCOMPtr<nsPIDOMWindowInner> window = aDoc.GetInnerWindow()) {
-    window->TimeoutManager().BeginSyncOperation();
+void AutoWalkBrowsingContextGroup::SuppressBrowsingContextGroup(
+    BrowsingContextGroup* aGroup) {
+  for (const auto& bc : aGroup->Toplevels()) {
+    bc->PreOrderWalk([&](BrowsingContext* aBC) {
+      if (nsCOMPtr<nsPIDOMWindowOuter> win = aBC->GetDOMWindow()) {
+        if (RefPtr<Document> doc = win->GetExtantDoc()) {
+          SuppressDocument(doc);
+          mDocuments.AppendElement(doc);
+        }
+      }
+    });
   }
-  aDocuments.AppendElement(&aDoc);
-  auto recurse = [&aDocuments](Document& aSubDoc) {
-    return MarkDocumentTreeToBeInSyncOperation(aSubDoc, aDocuments);
-  };
-  aDoc.EnumerateSubDocuments(recurse);
-  return CallState::Continue;
 }
 
 nsAutoSyncOperation::nsAutoSyncOperation(Document* aDoc,
                                          SyncOperationBehavior aSyncBehavior)
     : mSyncBehavior(aSyncBehavior) {
   mMicroTaskLevel = 0;
-  CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get();
-  if (ccjs) {
+  if (CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get()) {
     mMicroTaskLevel = ccjs->MicroTaskLevel();
     ccjs->SetMicroTaskLevel(0);
   }
   if (aDoc) {
-    if (nsPIDOMWindowOuter* win = aDoc->GetWindow()) {
-      if (nsCOMPtr<nsPIDOMWindowOuter> top = win->GetInProcessTop()) {
-        if (RefPtr<Document> doc = top->GetExtantDoc()) {
-          MarkDocumentTreeToBeInSyncOperation(*doc, mDocuments);
-        }
-      }
+    if (auto* bcg = aDoc->GetDocGroup()->GetBrowsingContextGroup()) {
+      SuppressBrowsingContextGroup(bcg);
     }
-
     mBrowsingContext = aDoc->GetBrowsingContext();
     if (mBrowsingContext &&
         mSyncBehavior == SyncOperationBehavior::eSuspendInput &&
@@ -15369,18 +15363,26 @@ nsAutoSyncOperation::nsAutoSyncOperation(Document* aDoc,
   }
 }
 
-nsAutoSyncOperation::~nsAutoSyncOperation() {
-  for (RefPtr<Document>& doc : mDocuments) {
-    if (nsCOMPtr<nsPIDOMWindowInner> window = doc->GetInnerWindow()) {
-      window->TimeoutManager().EndSyncOperation();
-    }
-    doc->SetIsInSyncOperation(false);
+void nsAutoSyncOperation::SuppressDocument(Document* aDoc) {
+  if (nsCOMPtr<nsPIDOMWindowInner> win = aDoc->GetInnerWindow()) {
+    win->TimeoutManager().BeginSyncOperation();
   }
+  aDoc->SetIsInSyncOperation(true);
+}
+
+void nsAutoSyncOperation::UnsuppressDocument(Document* aDoc) {
+  if (nsCOMPtr<nsPIDOMWindowInner> win = aDoc->GetInnerWindow()) {
+    win->TimeoutManager().EndSyncOperation();
+  }
+  aDoc->SetIsInSyncOperation(false);
+}
+
+nsAutoSyncOperation::~nsAutoSyncOperation() {
+  UnsuppressDocuments();
   CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get();
   if (ccjs) {
     ccjs->SetMicroTaskLevel(mMicroTaskLevel);
   }
-
   if (mBrowsingContext &&
       mSyncBehavior == SyncOperationBehavior::eSuspendInput &&
       InputTaskManager::CanSuspendInputEvent()) {

dom/base/Document.h

@@ -5253,14 +5253,33 @@ class MOZ_STACK_CLASS mozAutoSubtreeModified {
 
 enum class SyncOperationBehavior { eSuspendInput, eAllowInput };
 
-class MOZ_STACK_CLASS nsAutoSyncOperation {
+class AutoWalkBrowsingContextGroup {
+ public:
+  virtual ~AutoWalkBrowsingContextGroup() = default;
+
+ protected:
+  void SuppressBrowsingContextGroup(BrowsingContextGroup* aGroup);
+  void UnsuppressDocuments() {
+    for (const auto& doc : mDocuments) {
+      UnsuppressDocument(doc);
+    }
+  }
+  virtual void SuppressDocument(Document* aDocument) = 0;
+  virtual void UnsuppressDocument(Document* aDocument) = 0;
+  AutoTArray<RefPtr<Document>, 16> mDocuments;
+};
+
+class MOZ_RAII nsAutoSyncOperation : private AutoWalkBrowsingContextGroup {
  public:
   explicit nsAutoSyncOperation(Document* aDocument,
                                SyncOperationBehavior aSyncBehavior);
   ~nsAutoSyncOperation();
 
+ protected:
+  void SuppressDocument(Document* aDocument) override;
+  void UnsuppressDocument(Document* aDocument) override;
+
  private:
-  nsTArray<RefPtr<Document>> mDocuments;
   uint32_t mMicroTaskLevel;
   const SyncOperationBehavior mSyncBehavior;
   RefPtr<BrowsingContext> mBrowsingContext;

dom/base/nsContentUtils.cpp

@@ -676,43 +676,30 @@ class SameOriginCheckerImpl final : public nsIChannelEventSink,
 
 }  // namespace
 
-AutoSuppressEventHandlingAndSuspend::AutoSuppressEventHandlingAndSuspend(
-    BrowsingContextGroup* aGroup) {
-  for (const auto& bc : aGroup->Toplevels()) {
-    SuppressBrowsingContext(bc);
+void AutoSuppressEventHandlingAndSuspend::SuppressDocument(Document* aDoc) {
+  // Note: Document::SuppressEventHandling will also automatically suppress
+  // event handling for any in-process sub-documents. However, since we need
+  // to deal with cases where remote BrowsingContexts may be interleaved
+  // with in-process ones, we still need to walk the entire tree ourselves.
+  // This may be slightly redundant in some cases, but since event handling
+  // suppressions maintain a count of current blockers, it does not cause
+  // any problems.
+  aDoc->SuppressEventHandling();
+  if (nsCOMPtr<nsPIDOMWindowInner> win = aDoc->GetInnerWindow()) {
+    win->Suspend();
+    mWindows.AppendElement(win);
   }
 }
 
-void AutoSuppressEventHandlingAndSuspend::SuppressBrowsingContext(
-    BrowsingContext* aBC) {
-  if (nsCOMPtr<nsPIDOMWindowOuter> win = aBC->GetDOMWindow()) {
-    if (RefPtr<Document> doc = win->GetExtantDoc()) {
-      mDocuments.AppendElement(doc);
-      mWindows.AppendElement(win->GetCurrentInnerWindow());
-      // Note: Document::SuppressEventHandling will also automatically suppress
-      // event handling for any in-process sub-documents. However, since we need
-      // to deal with cases where remote BrowsingContexts may be interleaved
-      // with in-process ones, we still need to walk the entire tree ourselves.
-      // This may be slightly redundant in some cases, but since event handling
-      // suppressions maintain a count of current blockers, it does not cause
-      // any problems.
-      doc->SuppressEventHandling();
-      win->GetCurrentInnerWindow()->Suspend();
-    }
-  }
-
-  for (const auto& bc : aBC->Children()) {
-    SuppressBrowsingContext(bc);
-  }
+void AutoSuppressEventHandlingAndSuspend::UnsuppressDocument(Document* aDoc) {
+  aDoc->UnsuppressEventHandlingAndFireEvents(true);
 }
 
 AutoSuppressEventHandlingAndSuspend::~AutoSuppressEventHandlingAndSuspend() {
   for (const auto& win : mWindows) {
     win->Resume();
   }
-  for (const auto& doc : mDocuments) {
-    doc->UnsuppressEventHandlingAndFireEvents(true);
-  }
+  UnsuppressDocuments();
 }
 
 /**