Skip to content

Commit f7b4133

Browse files
author
Atila Butkovits
committed
Revert "Bug 1974382: Avoid mixing anchors in different child lists. r=layout-anchor-positioning-reviewers,dshin" for causing failures at SelectionActionDelegateTest.
This reverts commit 05b31c4. Revert "Bug 1974382: Avoid short-circuit evaluation on frame ancestor lookup. r=layout-anchor-positioning-reviewers,dshin" This reverts commit 61e3989.
1 parent 35b8cf3 commit f7b4133

File tree

11 files changed

+54
-123
lines changed

11 files changed

+54
-123
lines changed

‎layout/base/PresShell.cpp‎

Lines changed: 36 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,8 +1789,6 @@ nsresult PresShell::Initialize() {
17891789
return NS_ERROR_OUT_OF_MEMORY;
17901790
}
17911791

1792-
mPresContext->PresShell()->AssertNoAnchorPosAnchorChanges();
1793-
17941792
if (Element* root = mDocument->GetRootElement()) {
17951793
{
17961794
nsAutoCauseReflowNotifier reflowNotifier(this);
@@ -1865,8 +1863,6 @@ nsresult PresShell::Initialize() {
18651863
mShouldUnsuppressPainting = true;
18661864
}
18671865

1868-
mPresContext->PresShell()->MergeAnchorPosAnchorChanges();
1869-
18701866
return NS_OK; // XXX this needs to be real. MMP
18711867
}
18721868

@@ -4523,6 +4519,7 @@ void PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush) {
45234519

45244520
// Per our API contract, hold a strong ref to ourselves until we return.
45254521
RefPtr<PresShell> kungFuDeathGrip = this;
4522+
45264523
/**
45274524
* VERY IMPORTANT: If you add some sort of new flushing to this
45284525
* method, make sure to add the relevant SetNeedLayoutFlush or
@@ -4843,7 +4840,6 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY void PresShell::ContentWillBeRemoved(
48434840
// Notify the ESM that the content has been removed, so that
48444841
// it can clean up any state related to the content.
48454842

4846-
AssertNoAnchorPosAnchorChanges();
48474843
mPresContext->EventStateManager()->ContentRemoved(mDocument, aChild);
48484844

48494845
nsAutoCauseReflowNotifier crNotifier(this);
@@ -4863,7 +4859,6 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY void PresShell::ContentWillBeRemoved(
48634859
// display: contents / display: none, because we'd have cleared all the style
48644860
// data from there.
48654861
mPresContext->RestyleManager()->ContentWillBeRemoved(aChild);
4866-
MergeAnchorPosAnchorChanges();
48674862
}
48684863

48694864
void PresShell::NotifyCounterStylesAreDirty() {
@@ -10991,7 +10986,6 @@ bool PresShell::ProcessReflowCommands(bool aInterruptible) {
1099110986
printf("ProcessReflowCommands: begin incremental reflow\n");
1099210987
}
1099310988
#endif
10994-
AssertNoAnchorPosAnchorChanges();
1099510989

1099610990
// If reflow is interruptible, then make a note of our deadline.
1099710991
const PRIntervalTime deadline =
@@ -11025,8 +11019,6 @@ bool PresShell::ProcessReflowCommands(bool aInterruptible) {
1102511019
} while (!interrupted && !mDirtyRoots.IsEmpty() &&
1102611020
(!aInterruptible || PR_IntervalNow() < deadline));
1102711021

11028-
MergeAnchorPosAnchorChanges();
11029-
1103011022
interrupted = !mDirtyRoots.IsEmpty();
1103111023

1103211024
overflowTracker.Flush();
@@ -12025,8 +12017,6 @@ nsIFrame* PresShell::GetAbsoluteContainingBlock(nsIFrame* aFrame) {
1202512017
nsIFrame* PresShell::GetAnchorPosAnchor(
1202612018
const nsAtom* aName, const nsIFrame* aPositionedFrame) const {
1202712019
MOZ_ASSERT(aName);
12028-
MOZ_ASSERT(mLazyAnchorPosAnchorChanges.IsEmpty());
12029-
1203012020
if (const auto& entry = mAnchorPosAnchors.Lookup(aName)) {
1203112021
return AnchorPositioningUtils::FindFirstAcceptableAnchor(aPositionedFrame,
1203212022
entry.Data());
@@ -12035,10 +12025,41 @@ nsIFrame* PresShell::GetAnchorPosAnchor(
1203512025
return nullptr;
1203612026
}
1203712027

12038-
static void RemoveAnchorPosAnchorFromOrderedCache(
12039-
const RefPtr<const nsAtom>& aName, nsIFrame* aFrame,
12040-
nsTHashMap<RefPtr<const nsAtom>, nsTArray<nsIFrame*>>& aAnchorPosAnchors) {
12041-
auto entry = aAnchorPosAnchors.Lookup(aName);
12028+
void PresShell::AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
12029+
MOZ_ASSERT(aName);
12030+
12031+
auto& entry = mAnchorPosAnchors.LookupOrInsertWith(
12032+
aName, []() { return nsTArray<nsIFrame*>(); });
12033+
12034+
if (entry.IsEmpty()) {
12035+
entry.AppendElement(aFrame);
12036+
return;
12037+
}
12038+
12039+
struct FrameTreeComparator {
12040+
nsIFrame* mFrame;
12041+
12042+
int32_t operator()(nsIFrame* aOther) const {
12043+
return nsLayoutUtils::CompareTreePosition(aOther, mFrame, nullptr);
12044+
}
12045+
};
12046+
12047+
FrameTreeComparator cmp{aFrame};
12048+
12049+
size_t matchOrInsertionIdx = entry.Length();
12050+
// If the same element is already in the array,
12051+
// someone forgot to call RemoveAnchorPosAnchor.
12052+
if (BinarySearchIf(entry, 0, entry.Length(), cmp, &matchOrInsertionIdx)) {
12053+
MOZ_ASSERT_UNREACHABLE("Anchor added already");
12054+
return;
12055+
}
12056+
12057+
*entry.InsertElementAt(matchOrInsertionIdx) = aFrame;
12058+
}
12059+
12060+
void PresShell::RemoveAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
12061+
MOZ_ASSERT(aName);
12062+
auto entry = mAnchorPosAnchors.Lookup(aName);
1204212063
if (!entry) {
1204312064
return; // Nothing to remove.
1204412065
}
@@ -12048,67 +12069,13 @@ static void RemoveAnchorPosAnchorFromOrderedCache(
1204812069
// XXX: Once the implementation is more complete,
1204912070
// we should probably assert here that anchorArray
1205012071
// is not empty and aFrame is in it.
12051-
MOZ_ASSERT(!anchorArray.IsEmpty());
1205212072

1205312073
anchorArray.RemoveElement(aFrame);
1205412074
if (anchorArray.IsEmpty()) {
1205512075
entry.Remove();
1205612076
}
1205712077
}
1205812078

12059-
void PresShell::RemoveAnchorPosAnchorNow(const RefPtr<const nsAtom>& aName,
12060-
nsIFrame* aFrame) {
12061-
// nsCSSFrameConstructor will destroy, remove and add frames back,
12062-
// in no particular order during reflow. Removing the elements from
12063-
// mLazyAnchorPosAnchorChanges here is necessary and for performance
12064-
// reasons it would be an advantage to not let mLazyAnchorPosAnchorChanges
12065-
// grow too much.
12066-
mLazyAnchorPosAnchorChanges.RemoveElementsBy(
12067-
[&](const AnchorPosAnchorChange& change) {
12068-
return change.mFrame == aFrame;
12069-
});
12070-
12071-
RemoveAnchorPosAnchorFromOrderedCache(aName, aFrame, mAnchorPosAnchors);
12072-
}
12073-
12074-
void PresShell::MergeAnchorPosAnchorChanges() {
12075-
for (const auto& [name, frame, isRemoval] : mLazyAnchorPosAnchorChanges) {
12076-
if (isRemoval) {
12077-
RemoveAnchorPosAnchorFromOrderedCache(name, frame, mAnchorPosAnchors);
12078-
} else {
12079-
auto& entry = mAnchorPosAnchors.LookupOrInsertWith(
12080-
name, []() { return nsTArray<nsIFrame*>(); });
12081-
12082-
if (entry.IsEmpty()) {
12083-
entry.AppendElement(frame);
12084-
continue;
12085-
}
12086-
12087-
struct FrameTreeComparator {
12088-
nsIFrame* mFrame;
12089-
12090-
int32_t operator()(nsIFrame* aOther) const {
12091-
return nsLayoutUtils::CompareTreePosition(mFrame, aOther, nullptr);
12092-
}
12093-
};
12094-
12095-
FrameTreeComparator cmp{frame};
12096-
12097-
size_t matchOrInsertionIdx = entry.Length();
12098-
// If the same element is already in the array,
12099-
// someone forgot to call RemoveAnchorPosAnchor.
12100-
if (BinarySearchIf(entry, 0, entry.Length(), cmp, &matchOrInsertionIdx)) {
12101-
MOZ_ASSERT_UNREACHABLE("Anchor added already");
12102-
continue;
12103-
}
12104-
12105-
*entry.InsertElementAt(matchOrInsertionIdx) = frame;
12106-
}
12107-
}
12108-
12109-
mLazyAnchorPosAnchorChanges.Clear();
12110-
}
12111-
1211212079
void PresShell::ActivenessMaybeChanged() {
1211312080
if (!mDocument) {
1211412081
return;

‎layout/base/PresShell.h‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -751,34 +751,8 @@ class PresShell final : public nsStubDocumentObserver,
751751
// https://drafts.csswg.org/css-anchor-position-1/#target
752752
nsIFrame* GetAnchorPosAnchor(const nsAtom* aName,
753753
const nsIFrame* aPositionedFrame) const;
754-
755-
/**
756-
* When frame tree is reconstructed, we need to avoid ordering the frames
757-
* while they may not be part of the same child list.
758-
*/
759-
inline void AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
760-
MOZ_ASSERT(aName);
761-
mLazyAnchorPosAnchorChanges.AppendElement(
762-
AnchorPosAnchorChange{RefPtr<const nsAtom>(aName), aFrame, false});
763-
}
764-
765-
/**
766-
* When frame tree is reconstructed, we need to avoid ordering the frames
767-
* while they may not be part of the same child list.
768-
*/
769-
inline void RemoveAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame) {
770-
MOZ_ASSERT(aName);
771-
mLazyAnchorPosAnchorChanges.AppendElement(
772-
AnchorPosAnchorChange{RefPtr<const nsAtom>(aName), aFrame, true});
773-
;
774-
}
775-
776-
/**
777-
* When frame is about to be destroyed, we need to remove all references to it
778-
* without delay.
779-
*/
780-
void RemoveAnchorPosAnchorNow(const RefPtr<const nsAtom>& aName,
781-
nsIFrame* aFrame);
754+
void AddAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame);
755+
void RemoveAnchorPosAnchor(const nsAtom* aName, nsIFrame* aFrame);
782756

783757
#ifdef MOZ_REFLOW_PERF
784758
void DumpReflows();
@@ -1834,12 +1808,6 @@ class PresShell final : public nsStubDocumentObserver,
18341808
*/
18351809
static Modifiers GetCurrentModifiers() { return sCurrentModifiers; }
18361810

1837-
inline void AssertNoAnchorPosAnchorChanges() const {
1838-
MOZ_ASSERT(mLazyAnchorPosAnchorChanges.IsEmpty());
1839-
}
1840-
1841-
void MergeAnchorPosAnchorChanges();
1842-
18431811
private:
18441812
~PresShell();
18451813

@@ -3229,13 +3197,6 @@ class PresShell final : public nsStubDocumentObserver,
32293197

32303198
nsTHashMap<RefPtr<const nsAtom>, nsTArray<nsIFrame*>> mAnchorPosAnchors;
32313199

3232-
struct AnchorPosAnchorChange {
3233-
RefPtr<const nsAtom> mName;
3234-
nsIFrame* mFrame;
3235-
bool mIsRemoval;
3236-
};
3237-
nsTArray<AnchorPosAnchorChange> mLazyAnchorPosAnchorChanges;
3238-
32393200
// Reflow roots that need to be reflowed.
32403201
DepthOrderedFrameList mDirtyRoots;
32413202

‎layout/base/RestyleManager.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,7 +1616,7 @@ void RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) {
16161616
nsCSSFrameConstructor* frameConstructor = presContext->FrameConstructor();
16171617

16181618
bool didUpdateCursor = false;
1619-
presContext->PresShell()->AssertNoAnchorPosAnchorChanges();
1619+
16201620
for (size_t i = 0; i < aChangeList.Length(); ++i) {
16211621
// Collect and coalesce adjacent siblings for lazy frame construction.
16221622
// Eventually it would be even better to make RecreateFramesForContent
@@ -1928,7 +1928,6 @@ void RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) {
19281928
frame->UpdateVisibleDescendantsState();
19291929
}
19301930
}
1931-
presContext->PresShell()->MergeAnchorPosAnchorChanges();
19321931

19331932
aChangeList.Clear();
19341933
FlushOverflowChangedTracker();
@@ -3277,7 +3276,6 @@ void RestyleManager::DoProcessPendingRestyles(ServoTraversalFlags aFlags) {
32773276
// those into a secondary queue and iterate until there's nothing left.
32783277
ReentrantChangeList newChanges;
32793278
mReentrantChanges = &newChanges;
3280-
presShell->AssertNoAnchorPosAnchorChanges();
32813279

32823280
{
32833281
DocumentStyleRootIterator iter(doc->GetServoRestyleRoot());
@@ -3301,8 +3299,6 @@ void RestyleManager::DoProcessPendingRestyles(ServoTraversalFlags aFlags) {
33013299
}
33023300
}
33033301

3304-
presShell->MergeAnchorPosAnchorChanges();
3305-
33063302
doc->ClearServoRestyleRoot();
33073303
ClearSnapshots();
33083304

‎layout/base/nsLayoutUtils.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,9 +1161,8 @@ int32_t nsLayoutUtils::DoCompareTreePosition(
11611161
}
11621162

11631163
AutoTArray<const nsIFrame*, 20> frame1Ancestors;
1164-
const nsIFrame* frame1CommonAncestor =
1165-
FillAncestors(aFrame1, aCommonAncestor, &frame1Ancestors);
1166-
if (aCommonAncestor && !frame1CommonAncestor) {
1164+
if (aCommonAncestor &&
1165+
!FillAncestors(aFrame1, aCommonAncestor, &frame1Ancestors)) {
11671166
// We reached the root of the frame tree ... if aCommonAncestor was set,
11681167
// it is wrong
11691168
return DoCompareTreePosition(aFrame1, aFrame2, nullptr);

‎layout/generic/nsIFrame.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ void nsIFrame::Destroy(DestroyContext& aContext) {
887887

888888
if (HasAnchorPosName()) {
889889
for (auto& name : disp->mAnchorName.AsSpan()) {
890-
PresShell()->RemoveAnchorPosAnchorNow(name.AsAtom(), this);
890+
PresShell()->RemoveAnchorPosAnchor(name.AsAtom(), this);
891891
}
892892
}
893893

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@
33
if (os == "android") and fission: [TIMEOUT, OK]
44
[anchor() in shadow tree should not match host anchor-name]
55
expected: FAIL
6+
7+
[anchor-name should not leak out of a shadow tree]
8+
expected: FAIL

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
[anchor-position-003.html]
22
expected:
33
if (os == "android") and fission: [OK, TIMEOUT]
4+
[.target 1]
5+
expected: FAIL

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[anchor-position-circular.html]
2+
expected: FAIL

‎testing/web-platform/meta/css/css-anchor-position/anchor-scope-basic.html.ini‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
[Sibling can not anchor into anchor-scope, even when anchor-name present]
2121
expected: FAIL
2222

23-
[anchor-scope:all scopes multiple names]
23+
[anchor-scope:all on common ancestor]
2424
expected: FAIL
2525

26-
[anchor-scope:--a,--b scopes --a and --b]
26+
[anchor-scope:--a on common ancestor]
2727
expected: FAIL

‎testing/web-platform/meta/css/css-anchor-position/anchor-scope-dynamic.html.ini‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,6 @@
77

88
[anchor-scope:--a appearing dynamically scopes only --a]
99
expected: FAIL
10+
11+
[anchor-scope:--b appearing dynamically (--b never referenced)]
12+
expected: FAIL

0 commit comments

Comments
 (0)