Bug 1903652 Part 1 - Revert Bug 1901515 Part 3 to rework the performance issue. r=dholbert

This patch is a revert of
https://hg.mozilla.org/mozilla-central/rev/8ea9b9da6922 because the correctness
assumption described in the commit message was wrong. Note that this patch does
not revert the documentation improvement for
`nsSplittableFrame::RemoveFromFlow()` because it is worth keeping.

During frame destruction, `nsBlockFrame::MarkIntrinsicISizesDirty()` can be
called and it accesses `FirstContinuation()`. Thus, it is not acceptable to
allow a stale first-continuation cache during frame destruction.

Differential Revision: https://phabricator.services.mozilla.com/D214658
This commit is contained in:
Ting-Yu Lin
2024-06-23 04:31:36 +00:00
parent 7f38322cc8
commit 415f61e85f
5 changed files with 18 additions and 60 deletions

View File

@@ -4115,7 +4115,7 @@ void nsTextFrame::Destroy(DestroyContext& aContext) {
// type might be changing. Not clear whether it's worth it.
ClearTextRuns();
if (mNextContinuation) {
mNextContinuation->SetPrevInFlowWithoutUpdatingCache(nullptr);
mNextContinuation->SetPrevInFlow(nullptr);
}
// Let the base class destroy the frame
nsIFrame::Destroy(aContext);
@@ -4164,8 +4164,7 @@ class nsContinuingTextFrame final : public nsTextFrame {
nsTextFrame* GetPrevContinuation() const final { return mPrevContinuation; }
void SetPrevContinuationWithoutUpdatingCache(
nsIFrame* aPrevContinuation) final {
void SetPrevContinuation(nsIFrame* aPrevContinuation) final {
NS_ASSERTION(!aPrevContinuation || Type() == aPrevContinuation->Type(),
"setting a prev continuation with incorrect type!");
NS_ASSERTION(
@@ -4173,10 +4172,6 @@ class nsContinuingTextFrame final : public nsTextFrame {
"creating a loop in continuation chain!");
mPrevContinuation = static_cast<nsTextFrame*>(aPrevContinuation);
RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
void SetPrevContinuation(nsIFrame* aPrevContinuation) final {
SetPrevContinuationWithoutUpdatingCache(aPrevContinuation);
UpdateCachedContinuations();
}
@@ -4185,7 +4180,7 @@ class nsContinuingTextFrame final : public nsTextFrame {
: nullptr;
}
void SetPrevInFlowWithoutUpdatingCache(nsIFrame* aPrevInFlow) final {
void SetPrevInFlow(nsIFrame* aPrevInFlow) final {
NS_ASSERTION(!aPrevInFlow || Type() == aPrevInFlow->Type(),
"setting a prev in flow with incorrect type!");
NS_ASSERTION(
@@ -4193,10 +4188,6 @@ class nsContinuingTextFrame final : public nsTextFrame {
"creating a loop in continuation chain!");
mPrevContinuation = static_cast<nsTextFrame*>(aPrevInFlow);
AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION);
}
void SetPrevInFlow(nsIFrame* aPrevInFlow) final {
SetPrevInFlowWithoutUpdatingCache(aPrevInFlow);
UpdateCachedContinuations();
}
@@ -9197,11 +9188,13 @@ static void RemoveEmptyInFlows(nsTextFrame* aFrame,
prevContinuation->SetNextInFlow(aFirstToNotRemove);
aFirstToNotRemove->SetPrevInFlow(prevContinuation);
// We are going to remove aFrame and all its later continuations up to
// lastRemoved. Therefore, it is OK to not update first continuation cache for
// them.
// **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!
lastRemoved->SetNextInFlow(nullptr);
aFrame->SetPrevInFlowWithoutUpdatingCache(nullptr);
aFrame->SetPrevInFlow(nullptr);
nsContainerFrame* parent = aFrame->GetParent();
nsIFrame::DestroyContext context(aFrame->PresShell());