Bug 1873530 Part 3 - Unify continuation linking operations by removing SetPrevContinuation() and SetPrevInFlow(). r=jfkthame

SetNextContinuation() and SetPrevContinuation() are almost always called
together when setting up a continuation link, but the callers don't call them in
particular order. We should unify them as one method so that it's more
ergonomics and robust, especially when we do more complex work such as caching
continuations. Same reason for SetNextInFlow() and SetPrevInFlow().

We choose to merge the SetPrevContinuation() code into SetNextContinuation() for
the symmetry of SetNextSibling(). (Yes, we don't have SetPrevSibling().)

This patch doesn't change behavior.

Differential Revision: https://phabricator.services.mozilla.com/D197966
This commit is contained in:
Ting-Yu Lin
2024-01-11 19:50:58 +00:00
parent fd8f769590
commit 1c5fd05aca
8 changed files with 142 additions and 157 deletions

View File

@@ -680,11 +680,9 @@ static void SplitInlineAncestors(nsContainerFrame* aParent,
static void MakeContinuationFluid(nsIFrame* aFrame, nsIFrame* aNext) {
NS_ASSERTION(!aFrame->GetNextInFlow() || aFrame->GetNextInFlow() == aNext,
"next-in-flow is not next continuation!");
aFrame->SetNextInFlow(aNext);
NS_ASSERTION(!aNext->GetPrevInFlow() || aNext->GetPrevInFlow() == aFrame,
"prev-in-flow is not prev continuation!");
aNext->SetPrevInFlow(aFrame);
aFrame->SetNextInFlow(aNext);
}
static void MakeContinuationsNonFluidUpParentChain(nsIFrame* aFrame,
@@ -697,7 +695,6 @@ static void MakeContinuationsNonFluidUpParentChain(nsIFrame* aFrame,
IsBidiSplittable(frame);
frame = frame->GetParent(), next = next->GetParent()) {
frame->SetNextContinuation(next);
next->SetPrevContinuation(frame);
}
}
@@ -1126,7 +1123,6 @@ nsresult nsBidiPresUtils::ResolveParagraph(BidiParagraphData* aBpd) {
nsIFrame* next = parent->GetNextInFlow();
if (next) {
parent->SetNextContinuation(next);
next->SetPrevContinuation(parent);
}
child = parent;
parent = child->GetParent();

View File

@@ -2224,7 +2224,6 @@ nsIFrame* nsCSSFrameConstructor::ConstructTableCol(
nsTableColFrame* newCol = NS_NewTableColFrame(mPresShell, computedStyle);
InitAndRestoreFrame(aState, content, aParentFrame, newCol, false);
aFrameList.LastChild()->SetNextContinuation(newCol);
newCol->SetPrevContinuation(aFrameList.LastChild());
aFrameList.AppendFrame(nullptr, newCol);
newCol->SetColType(eColAnonymousCol);
}
@@ -8030,10 +8029,10 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingFrame(
}
// Init() set newFrame to be a fluid continuation of aFrame.
// If we want a non-fluid continuation, we need to call SetPrevContinuation()
// to reset NS_FRAME_IS_FLUID_CONTINUATION.
// If we want a non-fluid continuation, we need to call SetNextContinuation()
// to remove NS_FRAME_IS_FLUID_CONTINUATION bit from newFrame.
if (!aIsFluid) {
newFrame->SetPrevContinuation(aFrame);
aFrame->SetNextContinuation(newFrame);
}
// If a continuing frame needs to carry frame state bits from its previous
@@ -8041,10 +8040,8 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingFrame(
// frame class's Init() if the bits are belong to specific group.
if (nextInFlow) {
nextInFlow->SetPrevInFlow(newFrame);
newFrame->SetNextInFlow(nextInFlow);
} else if (nextContinuation) {
nextContinuation->SetPrevContinuation(newFrame);
newFrame->SetNextContinuation(nextContinuation);
}

View File

@@ -7132,10 +7132,6 @@ nsresult nsIFrame::AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute,
nsIFrame* nsIFrame::GetPrevContinuation() const { return nullptr; }
void nsIFrame::SetPrevContinuation(nsIFrame* aPrevContinuation) {
MOZ_ASSERT(false, "not splittable");
}
nsIFrame* nsIFrame::GetNextContinuation() const { return nullptr; }
void nsIFrame::SetNextContinuation(nsIFrame*) {
@@ -7144,10 +7140,6 @@ void nsIFrame::SetNextContinuation(nsIFrame*) {
nsIFrame* nsIFrame::GetPrevInFlow() const { return nullptr; }
void nsIFrame::SetPrevInFlow(nsIFrame* aPrevInFlow) {
MOZ_ASSERT(false, "not splittable");
}
nsIFrame* nsIFrame::GetNextInFlow() const { return nullptr; }
void nsIFrame::SetNextInFlow(nsIFrame*) { MOZ_ASSERT(false, "not splittable"); }

View File

@@ -2438,7 +2438,6 @@ class nsIFrame : public nsQueryFrame {
* Continuation member functions
*/
virtual nsIFrame* GetPrevContinuation() const;
virtual void SetPrevContinuation(nsIFrame*);
virtual nsIFrame* GetNextContinuation() const;
virtual void SetNextContinuation(nsIFrame*);
virtual nsIFrame* FirstContinuation() const {
@@ -2459,8 +2458,6 @@ class nsIFrame : public nsQueryFrame {
* Flow member functions
*/
virtual nsIFrame* GetPrevInFlow() const;
virtual void SetPrevInFlow(nsIFrame*);
virtual nsIFrame* GetNextInFlow() const;
virtual void SetNextInFlow(nsIFrame*);

View File

@@ -24,7 +24,6 @@ void nsSplittableFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
nsIFrame* aPrevInFlow) {
if (aPrevInFlow) {
// Hook the frame into the flow
SetPrevInFlow(aPrevInFlow);
aPrevInFlow->SetNextInFlow(this);
}
nsIFrame::Init(aContent, aParent, aPrevInFlow);
@@ -44,27 +43,35 @@ nsIFrame* nsSplittableFrame::GetPrevContinuation() const {
return mPrevContinuation;
}
void nsSplittableFrame::SetPrevContinuation(nsIFrame* aFrame) {
NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
"setting a prev continuation with incorrect type!");
NS_ASSERTION(!IsInPrevContinuationChain(aFrame, this),
"creating a loop in continuation chain!");
mPrevContinuation = aFrame;
RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
nsIFrame* nsSplittableFrame::GetNextContinuation() const {
return mNextContinuation;
}
void nsSplittableFrame::SetNextContinuation(nsIFrame* aFrame) {
NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
"setting a next continuation with incorrect type!");
NS_ASSERTION(!IsInNextContinuationChain(aFrame, this),
"creating a loop in continuation chain!");
void nsSplittableFrame::SetNextContinuation(nsIFrame* aFrame, bool aIsFluid) {
MOZ_ASSERT(!aFrame || Type() == aFrame->Type(),
"Setting a next-continuation with an incorrect type!");
MOZ_ASSERT(!IsInNextContinuationChain(aFrame, this),
"We shouldn't be in aFrame's next-continuation chain!");
MOZ_ASSERT(!IsInPrevContinuationChain(this, aFrame),
"aFrame shouldn't be in our prev-continuation chain!");
if (mNextContinuation && mNextContinuation != aFrame) {
MOZ_ASSERT(mNextContinuation->GetPrevContinuation() == this,
"The existing link is wrong!");
// We have a non-null next-continuation, so break the link to make it a new
// first-continuation or first-in-flow. We want to keep its fluidity with
// its next-continuations, so don't change its
// NS_FRAME_IS_FLUID_CONTINUATION bits.
auto* next = static_cast<nsSplittableFrame*>(mNextContinuation);
next->mPrevContinuation = nullptr;
}
mNextContinuation = aFrame;
if (mNextContinuation) {
mNextContinuation->RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
mNextContinuation->AddOrRemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION,
aIsFluid);
auto* next = static_cast<nsSplittableFrame*>(mNextContinuation);
next->mPrevContinuation = this;
}
}
@@ -119,15 +126,6 @@ nsIFrame* nsSplittableFrame::GetPrevInFlow() const {
: nullptr;
}
void nsSplittableFrame::SetPrevInFlow(nsIFrame* aFrame) {
NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
"setting a prev in flow with incorrect type!");
NS_ASSERTION(!IsInPrevContinuationChain(aFrame, this),
"creating a loop in continuation chain!");
mPrevContinuation = aFrame;
AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
nsIFrame* nsSplittableFrame::GetNextInFlow() const {
return mNextContinuation && mNextContinuation->HasAnyStateBits(
NS_FRAME_IS_FLUID_CONTINUATION)
@@ -135,17 +133,6 @@ nsIFrame* nsSplittableFrame::GetNextInFlow() const {
: nullptr;
}
void nsSplittableFrame::SetNextInFlow(nsIFrame* aFrame) {
NS_ASSERTION(!aFrame || Type() == aFrame->Type(),
"setting a next in flow with incorrect type!");
NS_ASSERTION(!IsInNextContinuationChain(aFrame, this),
"creating a loop in continuation chain!");
mNextContinuation = aFrame;
if (mNextContinuation) {
mNextContinuation->AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
}
nsIFrame* nsSplittableFrame::FirstInFlow() const {
nsSplittableFrame* firstInFlow = const_cast<nsSplittableFrame*>(this);
while (nsIFrame* prev = firstInFlow->GetPrevInFlow()) {
@@ -164,35 +151,28 @@ nsIFrame* nsSplittableFrame::LastInFlow() const {
return lastInFlow;
}
// Remove this frame from the flow. Connects prev in flow and next in flow
void nsSplittableFrame::RemoveFromFlow(nsIFrame* aFrame) {
MOZ_ASSERT(aFrame);
nsIFrame* prevContinuation = aFrame->GetPrevContinuation();
nsIFrame* nextContinuation = aFrame->GetNextContinuation();
nsIFrame* prevInFlow = aFrame->GetPrevInFlow();
nsIFrame* nextInFlow = aFrame->GetNextInFlow();
// The new continuation is fluid only if the continuation on both sides
// of the removed frame was fluid
if (aFrame->GetPrevInFlow() && aFrame->GetNextInFlow()) {
if (prevContinuation) {
prevContinuation->SetNextInFlow(nextContinuation);
}
if (nextContinuation) {
nextContinuation->SetPrevInFlow(prevContinuation);
}
} else {
if (prevContinuation) {
prevContinuation->SetNextContinuation(nextContinuation);
}
if (nextContinuation) {
nextContinuation->SetPrevContinuation(prevContinuation);
}
}
// **Note: it is important here that we clear the Next link from aFrame
// BEFORE clearing its Prev link, because in nsContinuingTextFrame,
// SetPrevInFlow() would follow the Next pointers, wiping out the cached
// Note: it is important here that we disconnect aFrame and its
// next-continuation BEFORE connecting prevInFlow and nextInFlow (or
// prevContinuation and nextContinuation), because in nsContinuingTextFrame,
// SetNextInFlow() would follow the Next pointers, wiping out the cached
// mFirstContinuation field from each following frame in the list.
aFrame->SetNextInFlow(nullptr);
aFrame->SetPrevInFlow(nullptr);
// The new continuation is fluid only if the continuation on both sides of the
// removed frame was fluid.
if (prevInFlow && nextInFlow) {
prevInFlow->SetNextInFlow(nextInFlow);
} else if (prevContinuation) {
prevContinuation->SetNextContinuation(nextContinuation);
}
}
NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(ConsumedBSizeProperty, nscoord);

View File

@@ -46,9 +46,14 @@ class nsSplittableFrame : public nsIFrame {
nsIFrame* GetPrevContinuation() const final;
nsIFrame* GetNextContinuation() const final;
// Set a previous/next non-fluid continuation.
void SetPrevContinuation(nsIFrame*) final;
void SetNextContinuation(nsIFrame*) final;
// Establish a non-fluid continuation relationship by making aFrame the
// next-continuation of this frame.
//
// Note: if this frame has an existing non-null next-continuation, this method
// disconnect from it before connecting aFrame.
void SetNextContinuation(nsIFrame* aFrame) final {
SetNextContinuation(aFrame, false);
}
// Get the first/last continuation for this frame.
nsIFrame* FirstContinuation() const override;
@@ -64,9 +69,14 @@ class nsSplittableFrame : public nsIFrame {
nsIFrame* GetPrevInFlow() const final;
nsIFrame* GetNextInFlow() const final;
// Set a previous/next fluid continuation.
void SetPrevInFlow(nsIFrame*) final;
void SetNextInFlow(nsIFrame*) final;
// Establish a fluid continuation relationship by making aFrame the
// next-in-flow of this frame.
//
// Note: if this frame has an existing non-null next-in-flow, this method
// disconnect from it before connecting aFrame.
void SetNextInFlow(nsIFrame* aFrame) final {
SetNextContinuation(aFrame, true);
}
// Get the first/last frame in the current flow.
nsIFrame* FirstInFlow() const final;
@@ -140,6 +150,12 @@ class nsSplittableFrame : public nsIFrame {
return GetBlockLevelLogicalSkipSides(false);
};
/**
* A helper method implementing public SetNextInFlow() and
* SetNextContinuation().
*/
void SetNextContinuation(nsIFrame* aFrame, bool aIsFluid);
nsIFrame* mPrevContinuation = nullptr;
nsIFrame* mNextContinuation = nullptr;
};

View File

@@ -4070,7 +4070,7 @@ void nsTextFrame::Destroy(DestroyContext& aContext) {
// type might be changing. Not clear whether it's worth it.
ClearTextRuns();
if (mNextContinuation) {
mNextContinuation->SetPrevInFlow(nullptr);
SetNextInFlow(nullptr);
}
// Let the base class destroy the frame
nsIFrame::Destroy(aContext);
@@ -4109,6 +4109,7 @@ class nsContinuingTextFrame final : public nsTextFrame {
public:
NS_DECL_FRAMEARENA_HELPERS(nsContinuingTextFrame)
friend class nsTextFrame;
friend nsIFrame* NS_NewContinuingTextFrame(mozilla::PresShell* aPresShell,
ComputedStyle* aStyle);
@@ -4119,33 +4120,11 @@ class nsContinuingTextFrame final : public nsTextFrame {
nsTextFrame* GetPrevContinuation() const final { return mPrevContinuation; }
void SetPrevContinuation(nsIFrame* aPrevContinuation) final {
NS_ASSERTION(!aPrevContinuation || Type() == aPrevContinuation->Type(),
"setting a prev continuation with incorrect type!");
NS_ASSERTION(
!nsSplittableFrame::IsInPrevContinuationChain(aPrevContinuation, this),
"creating a loop in continuation chain!");
mPrevContinuation = static_cast<nsTextFrame*>(aPrevContinuation);
RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
UpdateCachedContinuations();
}
nsTextFrame* GetPrevInFlow() const final {
return HasAnyStateBits(NS_FRAME_IS_FLUID_CONTINUATION) ? mPrevContinuation
: nullptr;
}
void SetPrevInFlow(nsIFrame* aPrevInFlow) final {
NS_ASSERTION(!aPrevInFlow || Type() == aPrevInFlow->Type(),
"setting a prev in flow with incorrect type!");
NS_ASSERTION(
!nsSplittableFrame::IsInPrevContinuationChain(aPrevInFlow, this),
"creating a loop in continuation chain!");
mPrevContinuation = static_cast<nsTextFrame*>(aPrevInFlow);
AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
UpdateCachedContinuations();
}
// Call this helper to update cache after mPrevContinuation is changed.
void UpdateCachedContinuations() {
nsTextFrame* prevFirst = mFirstContinuation;
@@ -4215,7 +4194,6 @@ void nsContinuingTextFrame::Init(nsIContent* aContent,
// Hook the frame into the flow
nsTextFrame* prev = static_cast<nsTextFrame*>(aPrevInFlow);
nsTextFrame* nextContinuation = prev->GetNextContinuation();
SetPrevInFlow(aPrevInFlow);
aPrevInFlow->SetNextInFlow(this);
// NOTE: bypassing nsTextFrame::Init!!!
@@ -4247,7 +4225,6 @@ void nsContinuingTextFrame::Init(nsIContent* aContent,
if (nextContinuation) {
SetNextContinuation(nextContinuation);
nextContinuation->SetPrevContinuation(this);
// Adjust next-continuations' content offset as needed.
while (nextContinuation &&
nextContinuation->GetContentOffset() < mContentOffset) {
@@ -4384,6 +4361,73 @@ Maybe<nsIFrame::Cursor> nsTextFrame::GetCursor(const nsPoint& aPoint) {
return Some(Cursor{kind, AllowCustomCursorImage::Yes});
}
void nsTextFrame::SetNextContinuation(nsIFrame* aFrame) {
MOZ_ASSERT(!aFrame || Type() == aFrame->Type(),
"Setting a next-continuation with an incorrect type!");
MOZ_ASSERT(!nsSplittableFrame::IsInNextContinuationChain(aFrame, this),
"We shouldn't be in aFrame's next-continuation chain!");
MOZ_ASSERT(!nsSplittableFrame::IsInPrevContinuationChain(this, aFrame),
"aFrame shouldn't be in our prev-continuation chain!");
if (mNextContinuation && mNextContinuation != aFrame) {
// We have an existing non-null next-continuation. Break the link.
MOZ_ASSERT(mNextContinuation->GetPrevContinuation() == this,
"The existing link is wrong!");
auto* next = static_cast<nsContinuingTextFrame*>(mNextContinuation);
next->mPrevContinuation = nullptr;
next->UpdateCachedContinuations();
}
auto* next = static_cast<nsContinuingTextFrame*>(aFrame);
mNextContinuation = next;
if (next) {
next->RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
next->mPrevContinuation = this;
next->UpdateCachedContinuations();
}
// Setting a non-fluid continuation might affect our flow length (they're
// quite rare so we assume it always does) so we delete our cached value:
if (GetContent()->HasFlag(NS_HAS_FLOWLENGTH_PROPERTY)) {
GetContent()->RemoveProperty(nsGkAtoms::flowlength);
GetContent()->UnsetFlags(NS_HAS_FLOWLENGTH_PROPERTY);
}
}
void nsTextFrame::SetNextInFlow(nsIFrame* aFrame) {
MOZ_ASSERT(!aFrame || Type() == aFrame->Type(),
"Setting a next-in-flow with an incorrect type!");
MOZ_ASSERT(!nsSplittableFrame::IsInNextContinuationChain(aFrame, this),
"We shouldn't be in aFrame's next-continuation chain!");
MOZ_ASSERT(!nsSplittableFrame::IsInPrevContinuationChain(this, aFrame),
"aFrame shouldn't be in our prev-continuation chain!");
if (mNextContinuation && mNextContinuation != aFrame) {
// We have an existing non-null next-continuation. Break the link.
MOZ_ASSERT(mNextContinuation->GetPrevContinuation() == this,
"The existing link is wrong!");
auto* next = static_cast<nsContinuingTextFrame*>(mNextContinuation);
next->mPrevContinuation = nullptr;
next->UpdateCachedContinuations();
}
auto* next = static_cast<nsContinuingTextFrame*>(aFrame);
mNextContinuation = next;
if (next) {
if (!next->HasAnyStateBits(NS_FRAME_IS_FLUID_CONTINUATION)) {
// Changing from non-fluid to fluid continuation might affect our flow
// length, so we delete our cached value:
if (GetContent()->HasFlag(NS_HAS_FLOWLENGTH_PROPERTY)) {
GetContent()->RemoveProperty(nsGkAtoms::flowlength);
GetContent()->UnsetFlags(NS_HAS_FLOWLENGTH_PROPERTY);
}
}
next->AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
next->mPrevContinuation = this;
next->UpdateCachedContinuations();
}
}
nsTextFrame* nsTextFrame::LastInFlow() const {
nsTextFrame* lastInFlow = const_cast<nsTextFrame*>(this);
while (lastInFlow->GetNextInFlow()) {
@@ -9041,16 +9085,13 @@ static void RemoveEmptyInFlows(nsTextFrame* aFrame,
}
}
prevContinuation->SetNextInFlow(aFirstToNotRemove);
aFirstToNotRemove->SetPrevInFlow(prevContinuation);
// **Note: it is important here that we clear the Next link from lastRemoved
// BEFORE clearing the Prev link from aFrame, because SetPrevInFlow() will
// follow the Next pointers, wiping out the cached mFirstContinuation field
// from each following frame in the list. We need this to stop when it
// reaches lastRemoved!
// Note: it is important here that we disconnect lastRemoved and its
// next-continuation BEFORE connecting prevContinuation and aFirstToNotRemove,
// because SetNextInFlow() will follow the Next pointers, wiping out the
// cached mFirstContinuation field from each following frame in the list. We
// need this to stop when it reaches lastRemoved!
lastRemoved->SetNextInFlow(nullptr);
aFrame->SetPrevInFlow(nullptr);
prevContinuation->SetNextInFlow(aFirstToNotRemove);
nsContainerFrame* parent = aFrame->GetParent();
nsIFrame::DestroyContext context(aFrame->PresShell());

View File

@@ -228,48 +228,14 @@ class nsTextFrame : public nsIFrame {
}
nsTextFrame* GetPrevContinuation() const override { return nullptr; }
nsTextFrame* GetNextContinuation() const final { return mNextContinuation; }
void SetNextContinuation(nsIFrame* aNextContinuation) final {
NS_ASSERTION(!aNextContinuation || Type() == aNextContinuation->Type(),
"setting a next continuation with incorrect type!");
NS_ASSERTION(
!nsSplittableFrame::IsInNextContinuationChain(aNextContinuation, this),
"creating a loop in continuation chain!");
mNextContinuation = static_cast<nsTextFrame*>(aNextContinuation);
if (aNextContinuation)
aNextContinuation->RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
// Setting a non-fluid continuation might affect our flow length (they're
// quite rare so we assume it always does) so we delete our cached value:
if (GetContent()->HasFlag(NS_HAS_FLOWLENGTH_PROPERTY)) {
GetContent()->RemoveProperty(nsGkAtoms::flowlength);
GetContent()->UnsetFlags(NS_HAS_FLOWLENGTH_PROPERTY);
}
}
void SetNextContinuation(nsIFrame* aFrame) final;
nsTextFrame* GetNextInFlow() const final {
return mNextContinuation && mNextContinuation->HasAnyStateBits(
NS_FRAME_IS_FLUID_CONTINUATION)
? mNextContinuation
: nullptr;
}
void SetNextInFlow(nsIFrame* aNextInFlow) final {
NS_ASSERTION(!aNextInFlow || Type() == aNextInFlow->Type(),
"setting a next in flow with incorrect type!");
NS_ASSERTION(
!nsSplittableFrame::IsInNextContinuationChain(aNextInFlow, this),
"creating a loop in continuation chain!");
mNextContinuation = static_cast<nsTextFrame*>(aNextInFlow);
if (mNextContinuation &&
!mNextContinuation->HasAnyStateBits(NS_FRAME_IS_FLUID_CONTINUATION)) {
// Changing from non-fluid to fluid continuation might affect our flow
// length, so we delete our cached value:
if (GetContent()->HasFlag(NS_HAS_FLOWLENGTH_PROPERTY)) {
GetContent()->RemoveProperty(nsGkAtoms::flowlength);
GetContent()->UnsetFlags(NS_HAS_FLOWLENGTH_PROPERTY);
}
}
if (aNextInFlow) {
aNextInFlow->AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
}
void SetNextInFlow(nsIFrame* aFrame) final;
nsTextFrame* LastInFlow() const final;
nsTextFrame* LastContinuation() const final;