Bug 1974382: Avoid mixing anchors in different child lists. r=layout-anchor-positioning-reviewers,dshin,emilio

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

Author: jjjalkanen

layout/base/PresShell.cpp

@@ -12059,6 +12059,7 @@ nsIFrame* PresShell::GetAbsoluteContainingBlock(nsIFrame* aFrame) {
 nsIFrame* PresShell::GetAnchorPosAnchor(
     const nsAtom* aName, const nsIFrame* aPositionedFrame) const {
   MOZ_ASSERT(aName);
+  MOZ_ASSERT(mLazyAnchorPosAnchorChanges.IsEmpty());
   if (const auto& entry = mAnchorPosAnchors.Lookup(aName)) {
     return AnchorPositioningUtils::FindFirstAcceptableAnchor(aPositionedFrame,
                                                              entry.Data());
@@ -12067,7 +12068,8 @@ nsIFrame* PresShell::GetAnchorPosAnchor(
   return nullptr;
 }
 
-void PresShell::AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
+template <bool AreWeMerging>
+void PresShell::AddAnchorPosAnchorImpl(const nsAtom* aName, nsIFrame* aFrame) {
   MOZ_ASSERT(aName);
 
   auto& entry = mAnchorPosAnchors.LookupOrInsertWith(
@@ -12082,7 +12084,7 @@ void PresShell::AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
     nsIFrame* mFrame;
 
     int32_t operator()(nsIFrame* aOther) const {
-      return nsLayoutUtils::CompareTreePosition(aOther, mFrame, nullptr);
+      return nsLayoutUtils::CompareTreePosition(mFrame, aOther, nullptr);
     }
   };
 
@@ -12102,16 +12104,39 @@ void PresShell::AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
     }
     MOZ_ASSERT(!entry.Contains(aFrame));
 
-    MOZ_ASSERT_UNREACHABLE("Frames were in different documents or child lists");
+    if constexpr (AreWeMerging) {
+      MOZ_ASSERT_UNREACHABLE(
+          "A frame may not be in a different child list at merge time");
+    } else {
+      // nsLayoutUtils::CompareTreePosition() returns 0 when the frames are
+      // in different documents or child lists. This indicates that
+      // the tree is being restructured and we can defer anchor insertion
+      // to a MergeAnchorPosAnchors call after the restructuring is complete.
+      mLazyAnchorPosAnchorChanges.AppendElement(
+          AnchorPosAnchorChange{RefPtr<const nsAtom>(aName), aFrame});
+    }
 
     return;
   }
 
+  MOZ_ASSERT(!entry.Contains(aFrame));
   *entry.InsertElementAt(matchOrInsertionIdx) = aFrame;
 }
 
+void PresShell::AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
+  AddAnchorPosAnchorImpl</* AreWeMerging */ false>(aName, aFrame);
+}
+
 void PresShell::RemoveAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
   MOZ_ASSERT(aName);
+
+  if (!mLazyAnchorPosAnchorChanges.IsEmpty()) {
+    mLazyAnchorPosAnchorChanges.RemoveElementsBy(
+        [&](const AnchorPosAnchorChange& change) {
+          return change.mFrame == aFrame;
+        });
+  }
+
   auto entry = mAnchorPosAnchors.Lookup(aName);
   if (!entry) {
     return;  // Nothing to remove.
@@ -12129,6 +12154,14 @@ void PresShell::RemoveAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
   }
 }
 
+void PresShell::MergeAnchorPosAnchorChanges() {
+  for (const auto& [name, frame] : mLazyAnchorPosAnchorChanges) {
+    AddAnchorPosAnchorImpl</* AreWeMerging */ true>(name, frame);
+  }
+
+  mLazyAnchorPosAnchorChanges.Clear();
+}
+
 void PresShell::ActivenessMaybeChanged() {
   if (!mDocument) {
     return;

layout/base/PresShell.h

@@ -1818,9 +1818,26 @@ class PresShell final : public nsStubDocumentObserver,
    */
   static Modifiers GetCurrentModifiers() { return sCurrentModifiers; }
 
+  /**
+   * Inserts mLazyAnchorPosAnchorChanges into mAnchorPosAnchors. Because the
+   * anchor lookup uses tree-ordered sorting, we're implicitly assuming
+   * that when AddAnchorPosAnchor and RemoveAnchorPosAnchor are called,
+   * the frame tree structure is valid globally.
+   *
+   * This assumption does not always hold - e.g. when the initial construction
+   * frame tree is deferred: the frame tree can may be in an indeterminate state
+   * where a frame has a parent but the parent does not have that frame as its
+   * child. Therefore, the defer tree position comparison may be deferred to a
+   * point where we know the frame tree is stable.
+   */
+  void MergeAnchorPosAnchorChanges();
+
  private:
   ~PresShell();
 
+  template <bool AreWeMerging>
+  void AddAnchorPosAnchorImpl(const nsAtom* aName, nsIFrame* aFrame);
+
   void SetIsActive(bool aIsActive);
   bool ComputeActiveness() const;
 
@@ -3205,6 +3222,15 @@ class PresShell final : public nsStubDocumentObserver,
   // A hash table of heap allocated weak frames.
   nsTHashSet<WeakFrame*> mWeakFrames;
 
+  struct AnchorPosAnchorChange {
+    RefPtr<const nsAtom> mName;
+    nsIFrame* mFrame;
+  };
+  // Holds deferred anchor changes. These changes should be deferred when
+  // the frame tree is under construction and the tree position of an anchor
+  // cannot be determined.
+  nsTArray<AnchorPosAnchorChange> mLazyAnchorPosAnchorChanges;
+
   nsTHashMap<RefPtr<const nsAtom>, nsTArray<nsIFrame*>> mAnchorPosAnchors;
   nsTArray<nsIFrame*> mAnchorPosPositioned;
 

layout/style/RestyleManager.cpp

@@ -3341,6 +3341,9 @@ void RestyleManager::DoProcessPendingRestyles(ServoTraversalFlags aFlags) {
       IncrementRestyleGeneration();
     }
 
+    // See https://bugzilla.mozilla.org/show_bug.cgi?id=1980206
+    presContext->PresShell()->MergeAnchorPosAnchorChanges();
+
     mInStyleRefresh = false;
     presContext->UpdateContainerQueryStyles();
     mInStyleRefresh = true;

testing/web-platform/meta/css/css-anchor-position/anchor-name-001.html.ini

@@ -1,9 +1,9 @@
 [anchor-name-001.html]
   [.target 1]
-    expected: FAIL
+    expected: [PASS, FAIL]
 
   [.target 2]
-    expected: FAIL
+    expected: [PASS, FAIL]
 
   [.target 3]
-    expected: FAIL
+    expected: [PASS, FAIL]

testing/web-platform/meta/css/css-anchor-position/anchor-name-002.html.ini

@@ -1,4 +1,3 @@
 [anchor-name-002.html]
   [.target 1]
-    expected:
-      if (os != "android"): FAIL
+    expected: [PASS, FAIL]

testing/web-platform/meta/css/css-anchor-position/anchor-name-003.html.ini

@@ -1,115 +1,15 @@
 [anchor-name-003.html]
-  [.target 2]
-    expected: FAIL
-
-  [.target 4]
-    expected: FAIL
-
-  [.target 5]
-    expected: FAIL
-
-  [.target 6]
-    expected: FAIL
-
-  [.target 7]
-    expected: FAIL
-
-  [.target 9]
-    expected: FAIL
-
-  [.target 12]
-    expected: FAIL
-
-  [.target 14]
-    expected: FAIL
-
-  [.target 16]
-    expected: FAIL
-
-  [.target 19]
-    expected: FAIL
-
-  [.target 20]
-    expected: FAIL
-
-  [.target 21]
-    expected: FAIL
-
-  [.target 22]
-    expected: FAIL
-
-  [.target 8]
-    expected: FAIL
-
-  [.target 13]
-    expected: FAIL
-
-  [.target 23]
-    expected: FAIL
-
-  [.target 26]
-    expected: FAIL
-
-  [.target 27]
-    expected: FAIL
-
-  [.target 28]
-    expected: FAIL
-
-  [.target 29]
-    expected: FAIL
-
-  [.target 30]
-    expected: FAIL
-
-  [.target 31]
-    expected: FAIL
-
-  [.target 33]
-    expected: FAIL
-
-  [.target 35]
-    expected: FAIL
-
-  [.target 36]
-    expected: FAIL
-
-  [.target 37]
-    expected: FAIL
-
-  [.target 38]
-    expected: FAIL
-
-  [.target 39]
-    expected: FAIL
-
-  [.target 1]
-    expected: FAIL
-
-  [.target 15]
-    expected: FAIL
-
   [.target 3]
-    expected:
-      if os != "android": FAIL
+    expected: [PASS, FAIL]
 
   [.target 10]
-    expected:
-      if os != "android": FAIL
+    expected: [PASS, FAIL]
 
   [.target 17]
-    expected:
-      if os != "android": FAIL
+    expected: [PASS, FAIL]
 
   [.target 32]
     expected:
       if asan or tsan: [PASS, FAIL]
       if (os == "linux") and debug and fission: [FAIL, PASS]
       if os == "android": PASS
-      FAIL
-
-  [.target 24]
-    expected: FAIL
-
-  [.target 25]
-    expected: FAIL

testing/web-platform/meta/css/css-anchor-position/anchor-name-in-shadow.html.ini

@@ -3,6 +3,3 @@
     if (os == "android") and fission: [TIMEOUT, OK]
   [anchor() in shadow tree should not match host anchor-name]
     expected: FAIL
-
-  [anchor-name should not leak out of a shadow tree]
-    expected: FAIL

testing/web-platform/meta/css/css-anchor-position/anchor-name-inline-001.html.ini

@@ -4,8 +4,8 @@
   [.target 4]
     expected: FAIL
 
-  [.target 5]
+  [.target 2]
     expected: FAIL
 
-  [.target 2]
+  [.target 1]
     expected: FAIL

testing/web-platform/meta/css/css-anchor-position/anchor-position-003.html.ini

@@ -1,5 +1,3 @@
 [anchor-position-003.html]
   expected:
     if (os == "android") and fission: [OK, TIMEOUT]
-  [.target 1]
-    expected: FAIL