From 49f27b5e78c159d2c7fae3f62c07c697e4e0cdff Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Fri, 20 Aug 2021 11:52:43 +0200 Subject: [PATCH] Backed out 3 changesets (bug 1725555) for causing crashes in nsTextFrame::FindContinuationForOffset. a=backout DONTBUILD Backed out changeset 2b74d92f7be2 (bug 1725555) Backed out changeset 054669fb1640 (bug 1725555) Backed out changeset 62ba1322b82b (bug 1725555) --- dom/base/nsRange.cpp | 10 +-- layout/generic/nsTextFrame.cpp | 123 ++++++--------------------------- layout/generic/nsTextFrame.h | 30 +------- 3 files changed, 27 insertions(+), 136 deletions(-) diff --git a/dom/base/nsRange.cpp b/dom/base/nsRange.cpp index 05ec4f76a31c..ecc6c43b365b 100644 --- a/dom/base/nsRange.cpp +++ b/dom/base/nsRange.cpp @@ -2647,16 +2647,10 @@ static nsresult GetPartialTextRect(RectCallback* aCallback, if (textFrame) { nsIFrame* relativeTo = nsLayoutUtils::GetContainingBlockForClientRect(textFrame); - - for (nsTextFrame* f = textFrame->FindContinuationForOffset(aStartOffset); f; + for (nsTextFrame* f = textFrame; f; f = static_cast(f->GetNextContinuation())) { int32_t fstart = f->GetContentOffset(), fend = f->GetContentEnd(); - if (fend <= aStartOffset) { - continue; - } - if (fstart >= aEndOffset) { - break; - } + if (fend <= aStartOffset || fstart >= aEndOffset) continue; // Calculate the text content offsets we'll need if text is requested. int32_t textContentStart = fstart; diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 41b18d352c42..dd76c77810c5 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -4384,31 +4384,6 @@ void nsTextFrame::DestroyFrom(nsIFrame* aDestructRoot, nsIFrame::DestroyFrom(aDestructRoot, aPostDestroyData); } -nsTArray* nsTextFrame::GetContinuations() { - // Only for use on the primary frame, which has no prev-continuation. - MOZ_ASSERT(!GetPrevContinuation()); - if (mHasContinuationsProperty) { - return GetProperty(ContinuationsProperty()); - } - size_t count = 0; - for (nsIFrame* f = this; f; f = f->GetNextContinuation()) { - ++count; - } - auto* continuations = new nsTArray; - if (continuations->SetCapacity(count, fallible)) { - for (nsTextFrame* f = this; f; - f = static_cast(f->GetNextContinuation())) { - continuations->AppendElement(f); - } - } else { - delete continuations; - continuations = nullptr; - } - AddProperty(ContinuationsProperty(), continuations); - mHasContinuationsProperty = true; - return continuations; -} - class nsContinuingTextFrame final : public nsTextFrame { public: NS_DECL_FRAMEARENA_HELPERS(nsContinuingTextFrame) @@ -4423,7 +4398,6 @@ class nsContinuingTextFrame final : public nsTextFrame { PostDestroyData& aPostDestroyData) final; nsTextFrame* GetPrevContinuation() const final { return mPrevContinuation; } - void SetPrevContinuation(nsIFrame* aPrevContinuation) final { NS_ASSERTION(!aPrevContinuation || Type() == aPrevContinuation->Type(), "setting a prev continuation with incorrect type!"); @@ -4432,32 +4406,11 @@ class nsContinuingTextFrame final : public nsTextFrame { "creating a loop in continuation chain!"); mPrevContinuation = static_cast(aPrevContinuation); RemoveStateBits(NS_FRAME_IS_FLUID_CONTINUATION); - nsTextFrame* prevFirst = mFirstContinuation; - if (mPrevContinuation) { - mFirstContinuation = mPrevContinuation->FirstContinuation(); - if (mFirstContinuation) { - mFirstContinuation->ClearCachedContinuations(); - } - } else { - mFirstContinuation = nullptr; - } - if (mFirstContinuation != prevFirst) { - if (prevFirst) { - prevFirst->ClearCachedContinuations(); - } - auto* f = static_cast(mNextContinuation); - while (f) { - f->mFirstContinuation = mFirstContinuation; - f = static_cast(f->mNextContinuation); - } - } } - 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!"); @@ -4466,29 +4419,9 @@ class nsContinuingTextFrame final : public nsTextFrame { "creating a loop in continuation chain!"); mPrevContinuation = static_cast(aPrevInFlow); AddStateBits(NS_FRAME_IS_FLUID_CONTINUATION); - nsTextFrame* prevFirst = mFirstContinuation; - if (mPrevContinuation) { - mFirstContinuation = mPrevContinuation->FirstContinuation(); - if (mFirstContinuation) { - mFirstContinuation->ClearCachedContinuations(); - } - } else { - mFirstContinuation = nullptr; - } - if (mFirstContinuation != prevFirst) { - if (prevFirst) { - prevFirst->ClearCachedContinuations(); - } - auto* f = static_cast(mNextContinuation); - while (f) { - f->mFirstContinuation = mFirstContinuation; - f = static_cast(f->mNextContinuation); - } - } } - nsIFrame* FirstInFlow() const final; - nsTextFrame* FirstContinuation() const final { return mFirstContinuation; }; + nsIFrame* FirstContinuation() const final; void AddInlineMinISize(gfxContext* aRenderingContext, InlineMinISizeData* aData) final; @@ -4501,7 +4434,6 @@ class nsContinuingTextFrame final : public nsTextFrame { : nsTextFrame(aStyle, aPresContext, kClassID) {} nsTextFrame* mPrevContinuation; - nsTextFrame* mFirstContinuation = nullptr; }; void nsContinuingTextFrame::Init(nsIContent* aContent, @@ -4604,6 +4536,23 @@ nsIFrame* nsContinuingTextFrame::FirstInFlow() const { return firstInFlow; } +nsIFrame* nsContinuingTextFrame::FirstContinuation() const { + // Can't cast to |nsContinuingTextFrame*| because the first one isn't. + nsIFrame *firstContinuation, + *previous = const_cast( + static_cast(mPrevContinuation)); + + NS_ASSERTION(previous, + "How can an nsContinuingTextFrame be the first continuation?"); + + do { + firstContinuation = previous; + previous = firstContinuation->GetPrevContinuation(); + } while (previous); + MOZ_ASSERT(firstContinuation, "post-condition failed"); + return firstContinuation; +} + // XXX Do we want to do all the work for the first-in-flow or do the // work for each part? (Be careful of first-letter / first-line, though, // especially first-line!) Doing all the work on the first-in-flow has @@ -7497,33 +7446,6 @@ bool nsTextFrame::IsFrameSelected() const { return mIsSelected == nsTextFrame::SelectionState::Selected; } -nsTextFrame* nsTextFrame::FindContinuationForOffset(int32_t aOffset) { - // Use a continuations array to accelerate finding the first continuation - // of interest, if possible. - MOZ_ASSERT(!GetPrevContinuation(), "should be called on the primary frame"); - auto* continuations = GetContinuations(); - nsTextFrame* f = this; - if (continuations) { - size_t index; - if (BinarySearchIf( - *continuations, 0, continuations->Length(), - [=](nsTextFrame* aFrame) -> int { - return aOffset - aFrame->GetContentOffset(); - }, - &index)) { - f = (*continuations)[index]; - } else { - f = (*continuations)[index ? index - 1 : 0]; - } - } - - while (f && f->GetContentEnd() <= aOffset) { - f = f->GetNextContinuation(); - } - - return f; -} - void nsTextFrame::SelectionStateChanged(uint32_t aStart, uint32_t aEnd, bool aSelected, SelectionType aSelectionType) { @@ -7534,11 +7456,12 @@ void nsTextFrame::SelectionStateChanged(uint32_t aStart, uint32_t aEnd, InvalidateSelectionState(); // Selection is collapsed, which can't affect text frame rendering - if (aStart == aEnd) { - return; - } + if (aStart == aEnd) return; - nsTextFrame* f = FindContinuationForOffset(aStart); + nsTextFrame* f = this; + while (f && f->GetContentEnd() <= int32_t(aStart)) { + f = f->GetNextContinuation(); + } nsPresContext* presContext = PresContext(); while (f && f->GetContentOffset() < int32_t(aEnd)) { diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h index 81e10b3546e4..f5a091dc5a49 100644 --- a/layout/generic/nsTextFrame.h +++ b/layout/generic/nsTextFrame.h @@ -214,9 +214,6 @@ class nsTextFrame : public nsIFrame { // nsQueryFrame NS_DECL_QUERYFRAME - NS_DECLARE_FRAME_PROPERTY_DELETABLE(ContinuationsProperty, - nsTArray) - // nsIFrame void BuildDisplayList(nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists) final; @@ -231,9 +228,6 @@ class nsTextFrame : public nsIFrame { nsresult CharacterDataChanged(const CharacterDataChangeInfo&) final; - nsTextFrame* FirstContinuation() const override { - return const_cast(this); - } nsTextFrame* GetPrevContinuation() const override { return nullptr; } nsTextFrame* GetNextContinuation() const final { return mNextContinuation; } void SetNextContinuation(nsIFrame* aNextContinuation) final { @@ -784,12 +778,6 @@ class nsTextFrame : public nsIFrame { nsRect WebRenderBounds(); - // Find the continuation (which may be this frame itself) containing the - // given offset. Note that this may return null, if the offset is beyond the - // text covered by the continuation chain. - // (To be used only on the first textframe in the chain.) - nsTextFrame* FindContinuationForOffset(int32_t aOffset); - protected: virtual ~nsTextFrame(); @@ -824,9 +812,6 @@ class nsTextFrame : public nsIFrame { }; mutable SelectionState mIsSelected; - // Whether a cached continuations array is present. - bool mHasContinuationsProperty = false; - /** * Return true if the frame is part of a Selection. * Helper method to implement the public IsSelected() API. @@ -1011,19 +996,6 @@ class nsTextFrame : public nsIFrame { void ClearMetrics(ReflowOutput& aMetrics); - // Return pointer to an array of all frames in the continuation chain, or - // null if we're too short of memory. - nsTArray* GetContinuations(); - - // Clear any cached continuations array; this should be called whenever the - // chain is modified. - void ClearCachedContinuations() { - if (mHasContinuationsProperty) { - RemoveProperty(ContinuationsProperty()); - mHasContinuationsProperty = false; - } - } - /** * UpdateIteratorFromOffset() updates the iterator from a given offset. * Also, aInOffset may be updated to cluster start if aInOffset isn't @@ -1035,6 +1007,8 @@ class nsTextFrame : public nsIFrame { nsPoint GetPointFromIterator(const gfxSkipCharsIterator& aIter, PropertyProvider& aProperties); + + public: }; MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsTextFrame::TrimmedOffsetFlags)