From aff17a41ed57f81202c2ce439ff7428b52281d4b Mon Sep 17 00:00:00 2001
From: Jonathan Kew <jkew@mozilla.com>
Date: Thu, 26 May 2022 16:32:10 +0000
Subject: [PATCH] Bug 1770981 - Split the nsBlockFrame "line cursor" into
separate cursors used for display-list building vs frame queries.
r=layout-reviewers,Jamie,emilio
Differential Revision: https://phabricator.services.mozilla.com/D147203
---
layout/generic/nsBlockFrame.cpp | 71 ++++++++++++----------
layout/generic/nsBlockFrame.h | 101 ++++++++++++++++++--------------
layout/generic/nsTextFrame.cpp | 4 +-
3 files changed, 100 insertions(+), 76 deletions(-)
diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp
index 88b7a70d18601..5b1dbca5d7ad5 100644
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -472,7 +472,7 @@ void nsBlockFrame::AddSizeOfExcludingThisForTree(
void nsBlockFrame::DestroyFrom(nsIFrame* aDestructRoot,
PostDestroyData& aPostDestroyData) {
- ClearLineCursor();
+ ClearLineCursors();
DestroyAbsoluteFrames(aDestructRoot, aPostDestroyData);
mFloats.DestroyFramesFrom(aDestructRoot, aPostDestroyData);
nsPresContext* presContext = PresContext();
@@ -1338,7 +1338,7 @@ void nsBlockFrame::Reflow(nsPresContext* aPresContext, ReflowOutput& aMetrics,
// because we may invalidate the nondecreasing
// overflowArea.InkOverflow().y/yMost invariant, and we may even
// delete the line with the line cursor.
- ClearLineCursor();
+ ClearLineCursors();
if (IsFrameTreeTooDeep(*reflowInput, aMetrics, aStatus)) {
return;
@@ -1655,9 +1655,8 @@ void nsBlockFrame::Reflow(nsPresContext* aPresContext, ReflowOutput& aMetrics,
}
// Setup the line cursor here to optimize line searching for
// calculating hypothetical position of absolutely-positioned
- // frames. The line cursor is immediately cleared afterward to
- // avoid affecting the display list generation.
- AutoLineCursorSetup autoLineCursor(this);
+ // frames.
+ SetupLineCursorForQuery();
absoluteContainer->Reflow(this, aPresContext, *reflowInput,
state.mReflowStatus, containingBlock, flags,
&aMetrics.mOverflowAreas);
@@ -2231,7 +2230,7 @@ bool nsBlockFrame::ComputeCustomOverflow(OverflowAreas& aOverflowAreas) {
// Line cursor invariants depend on the overflow areas of the lines, so
// we must clear the line cursor since those areas may have changed.
- ClearLineCursor();
+ ClearLineCursors();
return nsContainerFrame::ComputeCustomOverflow(aOverflowAreas);
}
@@ -3031,8 +3030,13 @@ void nsBlockFrame::ReflowDirtyLines(BlockReflowState& aState) {
continue;
}
- if (pulledLine == nextInFlow->GetLineCursor()) {
- nextInFlow->ClearLineCursor();
+ if (nextInFlow->MaybeHasLineCursor()) {
+ if (pulledLine == nextInFlow->GetLineCursorForDisplay()) {
+ nextInFlow->ClearLineCursorForDisplay();
+ }
+ if (pulledLine == nextInFlow->GetLineCursorForQuery()) {
+ nextInFlow->ClearLineCursorForQuery();
+ }
}
ReparentFrames(pulledFrames, nextInFlow, this);
pulledLine->SetMovedFragments();
@@ -5212,7 +5216,7 @@ bool nsBlockFrame::DrainOverflowLines() {
bool didFindOverflow = false;
nsBlockFrame* prevBlock = static_cast<nsBlockFrame*>(GetPrevInFlow());
if (prevBlock) {
- prevBlock->ClearLineCursor();
+ prevBlock->ClearLineCursors();
FrameLines* overflowLines = prevBlock->RemoveOverflowLines();
if (overflowLines) {
// Make all the frames on the overflow line list mine.
@@ -5713,7 +5717,7 @@ static bool ShouldPutNextSiblingOnNewLine(nsIFrame* aLastFrame) {
void nsBlockFrame::AddFrames(nsFrameList& aFrameList, nsIFrame* aPrevSibling,
const nsLineList::iterator* aPrevSiblingLine) {
// Clear our line cursor, since our lines may change.
- ClearLineCursor();
+ ClearLineCursors();
if (aFrameList.IsEmpty()) {
return;
@@ -6093,7 +6097,7 @@ nsBlockInFlowLineIterator::nsBlockInFlowLineIterator(nsBlockFrame* aFrame,
}
// Try to use the cursor if it exists, otherwise fall back to the first line
- if (nsLineBox* const cursor = aFrame->GetLineCursor()) {
+ if (nsLineBox* const cursor = aFrame->GetLineCursorForQuery()) {
mLine = line_end;
// Perform a simultaneous forward and reverse search starting from the
// line cursor.
@@ -6123,7 +6127,7 @@ nsBlockInFlowLineIterator::nsBlockInFlowLineIterator(nsBlockFrame* aFrame,
if (mLine != line_end) {
*aFoundValidLine = true;
if (mLine != cursor) {
- aFrame->SetProperty(nsBlockFrame::LineCursorProperty(), mLine);
+ aFrame->SetProperty(nsBlockFrame::LineCursorPropertyQuery(), mLine);
}
return;
}
@@ -6244,7 +6248,7 @@ void nsBlockFrame::DoRemoveFrameInternal(nsIFrame* aDeletedFrame,
uint32_t aFlags,
PostDestroyData& aPostDestroyData) {
// Clear our line cursor, since our lines may change.
- ClearLineCursor();
+ ClearLineCursors();
if (aDeletedFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW |
NS_FRAME_IS_OVERFLOW_CONTAINER)) {
@@ -7168,7 +7172,7 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
}
if (nonDecreasingYs && lineCount >= MIN_LINES_NEEDING_CURSOR) {
- SetupLineCursor();
+ SetupLineCursorForDisplay();
}
if (!curBackplateArea.IsEmpty()) {
@@ -7243,30 +7247,31 @@ a11y::AccType nsBlockFrame::AccessibleType() {
}
#endif
-void nsBlockFrame::ClearLineCursor() {
- if (!HasAnyStateBits(NS_BLOCK_HAS_LINE_CURSOR)) {
+void nsBlockFrame::SetupLineCursorForDisplay() {
+ if (mLines.empty() || HasProperty(LineCursorPropertyDisplay())) {
return;
}
- RemoveProperty(LineCursorProperty());
- RemoveStateBits(NS_BLOCK_HAS_LINE_CURSOR);
+ SetProperty(LineCursorPropertyDisplay(), mLines.front());
+ AddStateBits(NS_BLOCK_HAS_LINE_CURSOR);
}
-void nsBlockFrame::SetupLineCursor() {
- if (HasAnyStateBits(NS_BLOCK_HAS_LINE_CURSOR) || mLines.empty()) {
+void nsBlockFrame::SetupLineCursorForQuery() {
+ if (mLines.empty() || HasProperty(LineCursorPropertyQuery())) {
return;
}
- SetProperty(LineCursorProperty(), mLines.front());
+ SetProperty(LineCursorPropertyQuery(), mLines.front());
AddStateBits(NS_BLOCK_HAS_LINE_CURSOR);
}
nsLineBox* nsBlockFrame::GetFirstLineContaining(nscoord y) {
- if (!HasAnyStateBits(NS_BLOCK_HAS_LINE_CURSOR)) {
+ // Although this looks like a "querying" method, it is used by the
+ // display-list building code, so uses the Display cursor.
+ nsLineBox* property = GetLineCursorForDisplay();
+ if (!property) {
return nullptr;
}
-
- nsLineBox* property = GetProperty(LineCursorProperty());
LineIterator cursor = mLines.begin(property);
nsRect cursorArea = cursor->InkOverflowRect();
@@ -7282,7 +7287,7 @@ nsLineBox* nsBlockFrame::GetFirstLineContaining(nscoord y) {
}
if (cursor.get() != property) {
- SetProperty(LineCursorProperty(), cursor.get());
+ SetProperty(LineCursorPropertyDisplay(), cursor.get());
}
return cursor.get();
@@ -8019,7 +8024,7 @@ void nsBlockFrame::VerifyLines(bool aFinalCheckOK) {
return;
}
- nsLineBox* cursor = GetLineCursor();
+ nsLineBox* cursor = GetLineCursorForQuery();
// Add up the counts on each line. Also validate that IsFirstLine is
// set properly.
@@ -8134,8 +8139,10 @@ void nsBlockFrame::VerifyOverflowSituation() {
overflowLines->mFrames.FirstChild(),
"bad overflow frames / lines");
}
- nsLineBox* cursor = flow->GetLineCursor();
- if (cursor) {
+ auto checkCursor = [&](nsLineBox* cursor) -> bool {
+ if (!cursor) {
+ return true;
+ }
LineIterator line = flow->LinesBegin();
LineIterator line_end = flow->LinesEnd();
for (; line != line_end && line != cursor; ++line)
@@ -8146,8 +8153,12 @@ void nsBlockFrame::VerifyOverflowSituation() {
for (; line != line_end && line != cursor; ++line)
;
}
- MOZ_ASSERT(line != line_end, "stale LineCursorProperty");
- }
+ return line != line_end;
+ };
+ MOZ_ASSERT(checkCursor(flow->GetLineCursorForDisplay()),
+ "stale LineCursorPropertyDisplay");
+ MOZ_ASSERT(checkCursor(flow->GetLineCursorForQuery()),
+ "stale LineCursorPropertyQuery");
flow = static_cast<nsBlockFrame*>(flow->GetNextInFlow());
}
}
diff --git a/layout/generic/nsBlockFrame.h b/layout/generic/nsBlockFrame.h
index 7197cf11d2949..b896a943c8c07 100644
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -175,12 +175,10 @@ class nsBlockFrame : public nsContainerFrame {
mozilla::a11y::AccType AccessibleType() override;
#endif
- // Line cursor methods to speed up line searching in which one query
- // result is expected to be close to the next in general. This is
- // mainly for searching line(s) containing a point. It is also used
- // as a cache for local computation. Use AutoLineCursorSetup for the
- // latter case so that it wouldn't interact unexpectedly with the
- // former. The basic idea for the former is that we set the cursor
+ // Line cursor methods to speed up line searching in which one query result
+ // is expected to be close to the next in general. This is mainly for
+ // searching line(s) containing a point. It is also used as a cache for local
+ // computation. The basic idea for the former is that we set the cursor
// property if the lines' overflowArea.InkOverflow().ys and
// overflowArea.InkOverflow().yMosts are non-decreasing
// (considering only non-empty overflowArea.InkOverflow()s; empty
@@ -191,45 +189,46 @@ class nsBlockFrame : public nsContainerFrame {
// "near" the cursor, then we can find those nearby lines quickly by
// starting our search at the cursor.
- // Clear out line cursor because we're disturbing the lines (i.e., Reflow)
- void ClearLineCursor();
+ // We have two independent line cursors, one used for display-list building
+ // and the other for a11y or other frame queries. Either or both may be
+ // present at any given time. When we reflow or otherwise munge the lines,
+ // both cursors will be cleared.
+ // The display cursor is only created and used if the lines satisfy the non-
+ // decreasing y-coordinate condition (see SetupLineCursorForDisplay comment
+ // below), whereas the query cursor may be created for any block. The two
+ // are separated so creating a cursor for a11y queries (eg GetRenderedText)
+ // does not risk confusing the display-list building code.
+
+ // Clear out line cursors because we're disturbing the lines (i.e., Reflow)
+ void ClearLineCursors() {
+ if (MaybeHasLineCursor()) {
+ ClearLineCursorForDisplay();
+ ClearLineCursorForQuery();
+ RemoveStateBits(NS_BLOCK_HAS_LINE_CURSOR);
+ }
+ }
+ void ClearLineCursorForDisplay() {
+ RemoveProperty(LineCursorPropertyDisplay());
+ }
+ void ClearLineCursorForQuery() { RemoveProperty(LineCursorPropertyQuery()); }
+
// Get the first line that might contain y-coord 'y', or nullptr if you must
// search all lines. If nonnull is returned then we guarantee that the lines'
// combinedArea.ys and combinedArea.yMosts are non-decreasing.
// The actual line returned might not contain 'y', but if not, it is
// guaranteed to be before any line which does contain 'y'.
nsLineBox* GetFirstLineContaining(nscoord y);
- // Set the line cursor to our first line. Only call this if you
- // guarantee that either the lines' combinedArea.ys and combinedArea.
- // yMosts are non-decreasing, or the line cursor is cleared before
- // building the display list of this frame.
- void SetupLineCursor();
- /**
- * Helper RAII class for automatically set and clear line cursor for
- * temporary use. If the frame already has line cursor, this would be
- * a no-op.
- */
- class MOZ_STACK_CLASS AutoLineCursorSetup {
- public:
- explicit AutoLineCursorSetup(nsBlockFrame* aFrame)
- : mFrame(aFrame), mOrigCursor(aFrame->GetLineCursor()) {
- if (!mOrigCursor) {
- mFrame->SetupLineCursor();
- }
- }
- ~AutoLineCursorSetup() {
- if (mOrigCursor) {
- mFrame->SetProperty(LineCursorProperty(), mOrigCursor);
- } else {
- mFrame->ClearLineCursor();
- }
- }
+ // Ensure the frame has a display-list line cursor, initializing it to the
+ // first line if it is not already present. (If there's an existing cursor,
+ // it is left untouched.) Only call this if you guarantee that the lines'
+ // combinedArea.ys and combinedArea.yMosts are non-decreasing.
+ void SetupLineCursorForDisplay();
- private:
- nsBlockFrame* mFrame;
- nsLineBox* mOrigCursor;
- };
+ // Ensure the frame has a query line cursor, initializing it to the first
+ // line if it is not already present. (If there's an existing cursor, it is
+ // left untouched.)
+ void SetupLineCursorForQuery();
void ChildIsDirty(nsIFrame* aChild) override;
@@ -424,10 +423,23 @@ class nsBlockFrame : public nsContainerFrame {
nsPresContext* aPresContext);
#endif
- NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(LineCursorProperty, nsLineBox)
- bool HasLineCursor() { return HasAnyStateBits(NS_BLOCK_HAS_LINE_CURSOR); }
- nsLineBox* GetLineCursor() {
- return HasLineCursor() ? GetProperty(LineCursorProperty()) : nullptr;
+ NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(LineCursorPropertyDisplay, nsLineBox)
+ NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(LineCursorPropertyQuery, nsLineBox)
+ // Note that the NS_BLOCK_HAS_LINE_CURSOR flag does not necessarily mean the
+ // cursor is present, as it covers both the "display" and "query" cursors,
+ // but may remain set if they have been separately deleted. In such a case,
+ // the Get* accessors will be slightly more expensive, but will still safely
+ // return null if the cursor is absent.
+ bool MaybeHasLineCursor() {
+ return HasAnyStateBits(NS_BLOCK_HAS_LINE_CURSOR);
+ }
+ nsLineBox* GetLineCursorForDisplay() {
+ return MaybeHasLineCursor() ? GetProperty(LineCursorPropertyDisplay())
+ : nullptr;
+ }
+ nsLineBox* GetLineCursorForQuery() {
+ return MaybeHasLineCursor() ? GetProperty(LineCursorPropertyQuery())
+ : nullptr;
}
nsLineBox* NewLineBox(nsIFrame* aFrame, bool aIsBlock) {
@@ -438,8 +450,11 @@ class nsBlockFrame : public nsContainerFrame {
return NS_NewLineBox(PresShell(), aFromLine, aFrame, aCount);
}
void FreeLineBox(nsLineBox* aLine) {
- if (aLine == GetLineCursor()) {
- ClearLineCursor();
+ if (aLine == GetLineCursorForDisplay()) {
+ ClearLineCursorForDisplay();
+ }
+ if (aLine == GetLineCursorForQuery()) {
+ ClearLineCursorForQuery();
}
aLine->Destroy(PresShell());
}
diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp
index 53cae3b2c16f8..6763ef401d9f7 100644
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -10199,7 +10199,6 @@ nsIFrame::RenderedText nsTextFrame::GetRenderedText(
uint32_t offsetInRenderedString = 0;
bool haveOffsets = false;
- Maybe<nsBlockFrame::AutoLineCursorSetup> autoLineCursor;
for (textFrame = this; textFrame;
textFrame = textFrame->GetNextContinuation()) {
if (textFrame->HasAnyStateBits(NS_FRAME_IS_DIRTY)) {
@@ -10225,8 +10224,7 @@ nsIFrame::RenderedText nsTextFrame::GetRenderedText(
if (thisLc != lineContainer) {
// Setup line cursor when needed.
lineContainer = thisLc;
- autoLineCursor.reset();
- autoLineCursor.emplace(lineContainer);
+ lineContainer->SetupLineCursorForQuery();
}
LineStartsOrEndsAtHardLineBreak(textFrame, lineContainer,
&startsAtHardBreak, &endsAtHardBreak);
--
GitLab