From 0d644aac47c2a9d20eb212b3bc4aff6f75cba836 Mon Sep 17 00:00:00 2001 From: Sean Feng Date: Mon, 11 Mar 2024 13:41:27 +0000 Subject: [PATCH] Bug 1860328 - Fix a bug where caret misses when dragging and dropping r=emilio Differential Revision: https://phabricator.services.mozilla.com/D192377 --- layout/base/nsCaret.cpp | 15 ++++- layout/base/nsCaret.h | 6 ++ .../painting/RetainedDisplayListBuilder.cpp | 19 ------ layout/painting/RetainedDisplayListBuilder.h | 1 - layout/painting/nsDisplayList.cpp | 61 ++++++------------- layout/painting/nsDisplayList.h | 4 +- 6 files changed, 40 insertions(+), 66 deletions(-) diff --git a/layout/base/nsCaret.cpp b/layout/base/nsCaret.cpp index d66d55d6bb2b..0cb80d4733ac 100644 --- a/layout/base/nsCaret.cpp +++ b/layout/base/nsCaret.cpp @@ -398,6 +398,10 @@ nsIFrame* nsCaret::GetGeometry(const Selection* aSelection, nsRect* aRect) { } void nsCaret::SchedulePaint(Selection* aSelection) { + if (mLastCaretFrame) { + mLastCaretFrame->MarkNeedsDisplayItemRebuild(); + } + Selection* selection; if (aSelection) { selection = aSelection; @@ -409,14 +413,16 @@ void nsCaret::SchedulePaint(Selection* aSelection) { nsIFrame* frame = GetFrameAndOffset(selection, mOverrideContent, mOverrideOffset, &frameOffset); if (!frame) { + mLastCaretFrame = nullptr; return; } if (nsIFrame* cb = GetContainingBlockIfNeeded(frame)) { - cb->SchedulePaint(); - } else { - frame->SchedulePaint(); + frame = cb; } + + mLastCaretFrame = frame; + frame->SchedulePaint(); } void nsCaret::SetVisibilityDuringSelection(bool aVisibility) { @@ -555,6 +561,9 @@ void nsCaret::PaintCaret(DrawTarget& aDrawTarget, nsIFrame* aForFrame, NS_IMETHODIMP nsCaret::NotifySelectionChanged(Document*, Selection* aDomSel, int16_t aReason, int32_t aAmount) { + if (mLastCaretFrame) { + mLastCaretFrame->MarkNeedsDisplayItemRebuild(); + } // Note that aDomSel, per the comment below may not be the same as our // selection, but that's OK since if that is the case, it wouldn't have // mattered what IsVisible() returns here, so we just opt for checking diff --git a/layout/base/nsCaret.h b/layout/base/nsCaret.h index 43329c827ffb..4283f3382b87 100644 --- a/layout/base/nsCaret.h +++ b/layout/base/nsCaret.h @@ -12,6 +12,7 @@ #include "mozilla/MemoryReporting.h" #include "mozilla/dom/Selection.h" #include "nsCoord.h" +#include "nsIFrame.h" #include "nsISelectionListener.h" #include "nsIWeakReferenceUtils.h" #include "nsPoint.h" @@ -279,6 +280,11 @@ class nsCaret final : public nsISelectionListener { * it's in non-user-modifiable content. */ bool mIgnoreUserModify; + + /** + * mLastCaretFrame is the last caret frame that was scheduled to paint. + */ + WeakFrame mLastCaretFrame; }; #endif // nsCaret_h__ diff --git a/layout/painting/RetainedDisplayListBuilder.cpp b/layout/painting/RetainedDisplayListBuilder.cpp index ca8f37a252dc..93228e1b7dd9 100644 --- a/layout/painting/RetainedDisplayListBuilder.cpp +++ b/layout/painting/RetainedDisplayListBuilder.cpp @@ -1313,23 +1313,6 @@ bool RetainedDisplayListBuilder::ShouldBuildPartial( return true; } -void RetainedDisplayListBuilder::InvalidateCaretFramesIfNeeded() { - if (mPreviousCaret == mBuilder.GetCaretFrame()) { - // The current caret frame is the same as the previous one. - return; - } - - if (mPreviousCaret) { - mPreviousCaret->MarkNeedsDisplayItemRebuild(); - } - - if (mBuilder.GetCaretFrame()) { - mBuilder.GetCaretFrame()->MarkNeedsDisplayItemRebuild(); - } - - mPreviousCaret = mBuilder.GetCaretFrame(); -} - class AutoClearFramePropsArray { public: explicit AutoClearFramePropsArray(size_t aCapacity) : mFrames(aCapacity) {} @@ -1585,8 +1568,6 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate( MarkFramesWithItemsAndImagesModified(&mList); } - InvalidateCaretFramesIfNeeded(); - // We set the override dirty regions during ComputeRebuildRegion or in // DisplayPortUtils::InvalidateForDisplayPortChange. The display port change // also marks the frame modified, so those regions are cleared here as well. diff --git a/layout/painting/RetainedDisplayListBuilder.h b/layout/painting/RetainedDisplayListBuilder.h index e9f5b56d1ba2..bb3659a8bbb8 100644 --- a/layout/painting/RetainedDisplayListBuilder.h +++ b/layout/painting/RetainedDisplayListBuilder.h @@ -272,7 +272,6 @@ class RetainedDisplayListBuilder { nsDisplayListBuilder mBuilder; RetainedDisplayList mList; - WeakFrame mPreviousCaret; RetainedDisplayListMetrics mMetrics; RetainedDisplayListData mData; }; diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp index 78388d21857d..2343bd3b58aa 100644 --- a/layout/painting/nsDisplayList.cpp +++ b/layout/painting/nsDisplayList.cpp @@ -650,7 +650,6 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame, mCurrentContainerASR(nullptr), mCurrentFrame(aReferenceFrame), mCurrentReferenceFrame(aReferenceFrame), - mCaretFrame(nullptr), mScrollInfoItemsForHoisting(nullptr), mFirstClipChainToDestroy(nullptr), mTableBackgroundSet(nullptr), @@ -718,21 +717,6 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame, mRetainingDisplayList && StaticPrefs::layout_display_list_retain_sc(); } -static PresShell* GetFocusedPresShell() { - nsPIDOMWindowOuter* focusedWnd = - nsFocusManager::GetFocusManager()->GetFocusedWindow(); - if (!focusedWnd) { - return nullptr; - } - - nsCOMPtr focusedDocShell = focusedWnd->GetDocShell(); - if (!focusedDocShell) { - return nullptr; - } - - return focusedDocShell->GetPresShell(); -} - void nsDisplayListBuilder::BeginFrame() { nsCSSRendering::BeginFrameTreesLocked(); @@ -742,26 +726,6 @@ void nsDisplayListBuilder::BeginFrame() { mInTransform = false; mInFilter = false; mSyncDecodeImages = false; - - if (!mBuildCaret) { - return; - } - - RefPtr presShell = GetFocusedPresShell(); - if (presShell) { - RefPtr caret = presShell->GetCaret(); - mCaretFrame = caret->GetPaintGeometry(&mCaretRect); - - // The focused pres shell may not be in the document that we're - // painting, or be in a popup. Check if the display root for - // the caret matches the display root that we're painting, and - // only use it if it matches. - if (mCaretFrame && - nsLayoutUtils::GetDisplayRootFrame(mCaretFrame) != - nsLayoutUtils::GetDisplayRootFrame(mReferenceFrame)) { - mCaretFrame = nullptr; - } - } } void nsDisplayListBuilder::AddEffectUpdate(dom::RemoteBrowser* aBrowser, @@ -801,7 +765,6 @@ void nsDisplayListBuilder::EndFrame() { FreeClipChains(); FreeTemporaryItems(); nsCSSRendering::EndFrameTreesLocked(); - mCaretFrame = nullptr; } void nsDisplayListBuilder::MarkFrameForDisplay(nsIFrame* aFrame, @@ -1141,11 +1104,27 @@ void nsDisplayListBuilder::EnterPresShell(const nsIFrame* aReferenceFrame, return; } + RefPtr caret = state->mPresShell->GetCaret(); + // This code run for each pres shell and caret->GetPaintGeometry + // will return nullptr for invisible caret. So only one caret + // can be painted at a time. + state->mCaretFrame = caret->GetPaintGeometry(&mCaretRect); + + // Check if the display root for the caret matches the display + // root that we're painting, and only use it if it matches. Likely + // we only need this for popup. + if (state->mCaretFrame && + nsLayoutUtils::GetDisplayRootFrame(state->mCaretFrame) != + nsLayoutUtils::GetDisplayRootFrame(aReferenceFrame)) { + state->mCaretFrame = nullptr; + } + // Caret frames add visual area to their frame, but we don't update the // overflow area. Use flags to make sure we build display items for that frame // instead. - if (mCaretFrame && mCaretFrame->PresShell() == state->mPresShell) { - MarkFrameForDisplay(mCaretFrame, aReferenceFrame); + if (state->mCaretFrame) { + MOZ_ASSERT(state->mCaretFrame->PresShell() == state->mPresShell); + MarkFrameForDisplay(state->mCaretFrame, aReferenceFrame); } } @@ -4134,8 +4113,8 @@ bool nsDisplayCaret::CreateWebRenderCommands( nscolor caretColor; nsIFrame* frame = mCaret->GetPaintGeometry(&caretRect, &hookRect, &caretColor); - MOZ_ASSERT(frame == mFrame, "We're referring different frame"); - if (!frame) { + if (NS_WARN_IF(!frame) || NS_WARN_IF(frame != mFrame)) { + NS_ASSERTION(false, "Caret invalidation bug"); return true; } diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h index 5064677cc7d1..51a46d324ed8 100644 --- a/layout/painting/nsDisplayList.h +++ b/layout/painting/nsDisplayList.h @@ -659,7 +659,7 @@ class nsDisplayListBuilder { * Get the frame that the caret is supposed to draw in. * If the caret is currently invisible, this will be null. */ - nsIFrame* GetCaretFrame() { return mCaretFrame; } + nsIFrame* GetCaretFrame() { return CurrentPresShellState()->mCaretFrame; } /** * Get the rectangle we're supposed to draw the caret into. */ @@ -1729,6 +1729,7 @@ class nsDisplayListBuilder { bool mInsidePointerEventsNoneDoc; bool mTouchEventPrefEnabledDoc; nsIFrame* mPresShellIgnoreScrollFrame; + nsIFrame* mCaretFrame = nullptr; }; PresShellState* CurrentPresShellState() { @@ -1763,7 +1764,6 @@ class nsDisplayListBuilder { // The reference frame for mCurrentFrame. const nsIFrame* mCurrentReferenceFrame; - nsIFrame* mCaretFrame; // A temporary list that we append scroll info items to while building // display items for the contents of frames with SVG effects. // Only non-null when ShouldBuildScrollInfoItemsForHoisting() is true.