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

@@ -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

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__

layout/painting/RetainedDisplayListBuilder.cpp

@@ -1302,23 +1302,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) {}
@@ -1574,8 +1557,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.

layout/painting/RetainedDisplayListBuilder.h

@@ -272,7 +272,6 @@ class RetainedDisplayListBuilder {
 
   nsDisplayListBuilder mBuilder;
   RetainedDisplayList mList;
-  WeakFrame mPreviousCaret;
   RetainedDisplayListMetrics mMetrics;
   RetainedDisplayListData mData;
 };

layout/painting/nsDisplayList.cpp

@@ -649,7 +649,6 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame,
       mCurrentContainerASR(nullptr),
       mCurrentFrame(aReferenceFrame),
       mCurrentReferenceFrame(aReferenceFrame),
-      mCaretFrame(nullptr),
       mScrollInfoItemsForHoisting(nullptr),
       mFirstClipChainToDestroy(nullptr),
       mTableBackgroundSet(nullptr),
@@ -717,21 +716,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<nsIDocShell> focusedDocShell = focusedWnd->GetDocShell();
-  if (!focusedDocShell) {
-    return nullptr;
-  }
-
-  return focusedDocShell->GetPresShell();
-}
-
 void nsDisplayListBuilder::BeginFrame() {
   nsCSSRendering::BeginFrameTreesLocked();
 
@@ -741,26 +725,6 @@ void nsDisplayListBuilder::BeginFrame() {
   mInTransform = false;
   mInFilter = false;
   mSyncDecodeImages = false;
-
-  if (!mBuildCaret) {
-    return;
-  }
-
-  RefPtr<PresShell> presShell = GetFocusedPresShell();
-  if (presShell) {
-    RefPtr<nsCaret> 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,
@@ -800,7 +764,6 @@ void nsDisplayListBuilder::EndFrame() {
   FreeClipChains();
   FreeTemporaryItems();
   nsCSSRendering::EndFrameTreesLocked();
-  mCaretFrame = nullptr;
 }
 
 void nsDisplayListBuilder::MarkFrameForDisplay(nsIFrame* aFrame,
@@ -1135,11 +1098,27 @@ void nsDisplayListBuilder::EnterPresShell(const nsIFrame* aReferenceFrame,
     return;
   }
 
+  RefPtr<nsCaret> 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);
   }
 }
 
@@ -4128,8 +4107,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;
   }
 

layout/painting/nsDisplayList.h

@@ -656,7 +656,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.
    */
@@ -1726,6 +1726,7 @@ class nsDisplayListBuilder {
     bool mInsidePointerEventsNoneDoc;
     bool mTouchEventPrefEnabledDoc;
     nsIFrame* mPresShellIgnoreScrollFrame;
+    nsIFrame* mCaretFrame = nullptr;
   };
 
   PresShellState* CurrentPresShellState() {
@@ -1760,7 +1761,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.