Invalidate caret during paint invalidation

third_party/WebKit/Source/core/editing/FrameCaret.cpp

Index: third_party/WebKit/Source/core/editing/FrameCaret.cpp
diff --git a/third_party/WebKit/Source/core/editing/FrameCaret.cpp b/third_party/WebKit/Source/core/editing/FrameCaret.cpp
index 2c1d4f30bf12d59b9e7c058cdbba8b87a2be4859..cdbbaa317658da7cbe0ebe598e4a340193f42035 100644
--- a/third_party/WebKit/Source/core/editing/FrameCaret.cpp
+++ b/third_party/WebKit/Source/core/editing/FrameCaret.cpp
@@ -39,7 +39,6 @@
 #include "core/layout/LayoutTheme.h"
 #include "core/layout/api/LayoutPartItem.h"
 #include "core/page/Page.h"
-#include "core/paint/PaintLayer.h"
 #include "public/platform/WebTraceLocation.h"
 
 namespace blink {
@@ -50,11 +49,9 @@ FrameCaret::FrameCaret(LocalFrame& frame,
       m_frame(frame),
       m_caretBase(new CaretDisplayItemClient()),
       m_caretVisibility(CaretVisibility::Hidden),
-      m_previousCaretVisibility(CaretVisibility::Hidden),
       m_caretBlinkTimer(TaskRunnerHelper::get(TaskType::UnspecedTimer, &frame),
                         this,
                         &FrameCaret::caretBlinkTimerFired),
-      m_caretRectDirty(true),
       m_shouldPaintCaret(true),
       m_isCaretBlinkingSuspended(false),
       m_shouldShowBlockCursor(false) {}
@@ -64,8 +61,6 @@ FrameCaret::~FrameCaret() = default;
 DEFINE_TRACE(FrameCaret) {
   visitor->trace(m_selectionEditor);
   visitor->trace(m_frame);
-  visitor->trace(m_previousCaretNode);
-  visitor->trace(m_previousCaretAnchorNode);
   SynchronousMutationObserver::trace(visitor);
 }
 
@@ -81,7 +76,11 @@ const PositionWithAffinity FrameCaret::caretPosition() const {
   return PositionWithAffinity(selection.start(), selection.affinity());
 }
 
-const DisplayItemClient& FrameCaret::displayItemClient() const {
+void FrameCaret::setVisualRect(const LayoutRect& r) {
+  m_caretBase->setVisualRect(r);
+}
+
+const DisplayItemClient& FrameCaret::getDisplayItemClient() const {
   return *m_caretBase;
 }
 
@@ -121,7 +120,7 @@ void FrameCaret::updateAppearance() {
 
 void FrameCaret::stopCaretBlinkTimer() {
   if (m_caretBlinkTimer.isActive() || m_shouldPaintCaret)
-    setCaretRectNeedsUpdate();
+    setNeedsPaintInvalidation();
   m_shouldPaintCaret = false;
   m_caretBlinkTimer.stop();
 }
@@ -136,7 +135,7 @@ void FrameCaret::startBlinkCaret() {
     m_caretBlinkTimer.startRepeating(blinkInterval, BLINK_FROM_HERE);
 
   m_shouldPaintCaret = true;
-  setCaretRectNeedsUpdate();
+  setNeedsPaintInvalidation();
 }
 
 void FrameCaret::setCaretVisibility(CaretVisibility visibility) {
@@ -148,20 +147,23 @@ void FrameCaret::setCaretVisibility(CaretVisibility visibility) {
   updateAppearance();
 }
 
-void FrameCaret::setCaretRectNeedsUpdate() {
-  if (m_caretRectDirty)
-    return;
-  m_caretRectDirty = true;
+void FrameCaret::setMayNeedPaintInvalidation() {
+  if (LayoutBlock* block = caretLayoutBlock())

[yosin_UTC9, 2017/01/31 20:12:58]

Let's use early-return style to avoid to have (single line) else-clause.

+    block->setMayNeedPaintInvalidation();
+  else
+    scheduleVisualUpdate();
+}
 
+void FrameCaret::setNeedsPaintInvalidation() {
+  if (LayoutBlock* block = caretLayoutBlock())
+    block->setCaretsNeedPaintInvalidation();

[yosin_UTC9, 2017/01/31 20:12:58]

Let's use early-return style to avoid to have (single line) else-clause.

+  else
+    scheduleVisualUpdate();
+}
+
+void FrameCaret::scheduleVisualUpdate() {
   if (Page* page = m_frame->page())
     page->animator().scheduleVisualUpdate(m_frame->localFrameRoot());
-
-  // Ensure the frame will be checked for paint invalidation during
-  // PrePaintTreeWalk.
-  if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {

[chrishtr, 2017/01/31 22:51:03]

Why is this no longer needed?

[Xianzhu, 2017/02/02 23:42:06]

Now block->setMayNeedPaintInvalidation() marks the ancestor chain.

-    if (auto layoutItem = m_frame->ownerLayoutItem())
-      layoutItem.setMayNeedPaintInvalidation();
-  }
 }
 
 bool FrameCaret::caretPositionIsValidForDocument(
@@ -172,72 +174,15 @@ bool FrameCaret::caretPositionIsValidForDocument(
   return caretPosition().document() == document && !caretPosition().isOrphan();
 }
 
-static bool shouldRepaintCaret(Node& node) {
-  // If PositionAnchorType::BeforeAnchor or PositionAnchorType::AfterAnchor,
-  // carets need to be repainted not only when the node is contentEditable but
-  // also when its parentNode() is contentEditable.
-  node.document().updateStyleAndLayoutTree();
-  return hasEditableStyle(node) ||
-         (node.parentNode() && hasEditableStyle(*node.parentNode()));
-}
-
-void FrameCaret::invalidateCaretRect(bool forceInvalidation) {
-  if (!m_caretRectDirty)
-    return;
-  m_caretRectDirty = false;
-
-  DCHECK(caretPositionIsValidForDocument(*m_frame->document()));
-  LayoutObject* layoutObject = nullptr;
-  LayoutRect newRect;
-  PositionWithAffinity currentCaretPosition = caretPosition();
-  if (isActive())
-    newRect = localCaretRectOfPosition(currentCaretPosition, layoutObject);
-  Node* newNode = layoutObject ? layoutObject->node() : nullptr;
-  // The current selected node |newNode| could be a child multiple levels below
-  // its associated "anchor node" ancestor, so we reference and keep around the
-  // anchor node for checking editability.
-  // TODO(wkorman): Consider storing previous Position, rather than Node, and
-  // making use of EditingUtilies::isEditablePosition() directly.
-  Node* newAnchorNode =
-      currentCaretPosition.position().parentAnchoredEquivalent().anchorNode();
-  if (newNode && newAnchorNode && newNode != newAnchorNode &&
-      newAnchorNode->layoutObject() && newAnchorNode->layoutObject()->isBox()) {
-    newNode->layoutObject()->mapToVisualRectInAncestorSpace(
-        toLayoutBoxModelObject(newAnchorNode->layoutObject()), newRect);
-  }
-  // It's possible for the timer to be inactive even though we want to
-  // invalidate the caret. For example, when running as a layout test the
-  // caret blink interval could be zero and thus |m_caretBlinkTimer| will
-  // never be started. We provide |forceInvalidation| for use by paint
-  // invalidation internals where we need to invalidate the caret regardless
-  // of timer state.
-  if (!forceInvalidation && !m_caretBlinkTimer.isActive() &&
-      newNode == m_previousCaretNode && newRect == m_previousCaretRect &&
-      m_caretVisibility == m_previousCaretVisibility)
-    return;
-
-  if (m_previousCaretAnchorNode &&
-      shouldRepaintCaret(*m_previousCaretAnchorNode)) {
-    m_caretBase->invalidateLocalCaretRect(m_previousCaretAnchorNode.get(),
-                                          m_previousCaretRect);
-  }
-  if (newAnchorNode && shouldRepaintCaret(*newAnchorNode))
-    m_caretBase->invalidateLocalCaretRect(newAnchorNode, newRect);
-  m_previousCaretNode = newNode;
-  m_previousCaretAnchorNode = newAnchorNode;
-  m_previousCaretRect = newRect;
-  m_previousCaretVisibility = m_caretVisibility;
+LayoutRect FrameCaret::caretLocalRect() const {
+  if (!isActive())
+    return LayoutRect();
+  return CaretDisplayItemClient::computeCaretRect(caretPosition());
 }
 
-static IntRect absoluteBoundsForLocalRect(Node* node, const LayoutRect& rect) {
-  LayoutBlock* caretPainter = CaretDisplayItemClient::caretLayoutObject(node);
-  if (!caretPainter)
-    return IntRect();
-
-  LayoutRect localRect(rect);
-  caretPainter->flipForWritingMode(localRect);
-  return caretPainter->localToAbsoluteQuad(FloatRect(localRect))
-      .enclosingBoundingBox();
+LayoutBlock* FrameCaret::caretLayoutBlock() const {
+  return CaretDisplayItemClient::caretLayoutObject(
+      caretPosition().anchorNode());
 }
 
 IntRect FrameCaret::absoluteCaretBounds() const {
@@ -247,20 +192,27 @@ IntRect FrameCaret::absoluteCaretBounds() const {
   DocumentLifecycle::DisallowTransitionScope disallowTransition(
       m_frame->document()->lifecycle());
 
-  Node* const caretNode = caretPosition().anchorNode();
   if (!isActive())
-    return absoluteBoundsForLocalRect(caretNode, LayoutRect());
+    return IntRect();
+
+  const LayoutBlock* block = caretLayoutBlock();
+  if (!block)
+    return IntRect();
+
   // TODO(yosin): We should get rid of text control short path since layout
   // tree is clean.
+  LayoutRect localRect;
   if (enclosingTextControl(caretPosition().position()) &&
       isVisuallyEquivalentCandidate(caretPosition().position())) {
-    return absoluteBoundsForLocalRect(
-        caretNode, CaretDisplayItemClient::computeCaretRect(caretPosition()));
+    localRect = this->caretLocalRect();
+  } else {
+    localRect = CaretDisplayItemClient::computeCaretRect(
+        createVisiblePosition(caretPosition()).toPositionWithAffinity());
   }
-  return absoluteBoundsForLocalRect(
-      caretNode,
-      CaretDisplayItemClient::computeCaretRect(
-          createVisiblePosition(caretPosition()).toPositionWithAffinity()));
+
+  block->flipForWritingMode(localRect);
+  return block->localToAbsoluteQuad(FloatRect(localRect))
+      .enclosingBoundingBox();
 }
 
 void FrameCaret::setShouldShowBlockCursor(bool shouldShowBlockCursor) {
@@ -279,38 +231,12 @@ void FrameCaret::paintCaret(GraphicsContext& context,
   if (!(isActive() && m_shouldPaintCaret))
     return;
 
-  const LayoutRect caretLocalRect =
-      CaretDisplayItemClient::computeCaretRect(caretPosition());
-  m_caretBase->paintCaret(caretPosition().anchorNode(), context, caretLocalRect,
-                          paintOffset, DisplayItem::kCaret);
-}
-
-void FrameCaret::dataWillChange(const CharacterData& node) {
-  if (node == m_previousCaretNode) {
-    // This invalidation is eager, and intentionally uses stale state.
-    DisableCompositingQueryAsserts disabler;
-    m_caretBase->invalidateLocalCaretRect(m_previousCaretAnchorNode.get(),
-                                          m_previousCaretRect);
-  }
-}
-
-void FrameCaret::nodeWillBeRemoved(Node& node) {
-  if (node != m_previousCaretNode && node != m_previousCaretAnchorNode)
-    return;
-  // Hits in ManualTests/caret-paint-after-last-text-is-removed.html
-  DisableCompositingQueryAsserts disabler;
-  m_caretBase->invalidateLocalCaretRect(m_previousCaretAnchorNode.get(),
-                                        m_previousCaretRect);
-  m_previousCaretNode = nullptr;
-  m_previousCaretAnchorNode = nullptr;
-  m_previousCaretRect = LayoutRect();
-  m_previousCaretVisibility = CaretVisibility::Hidden;
+  m_caretBase->paintCaret(caretPosition().anchorNode(), context,
+                          caretLocalRect(), paintOffset, DisplayItem::kCaret);

[yosin_UTC9, 2017/01/31 20:12:58]

Yes, this is what I thought. It is redundant to calculate caret rectangle twice,
one for invalidation and another for paint.

 }
 
 void FrameCaret::contextDestroyed(Document*) {
   m_caretBlinkTimer.stop();
-  m_previousCaretNode.clear();
-  m_previousCaretAnchorNode.clear();
 }
 
 bool FrameCaret::shouldBlinkCaret() const {
@@ -334,7 +260,7 @@ void FrameCaret::caretBlinkTimerFired(TimerBase*) {
   if (isCaretBlinkingSuspended() && m_shouldPaintCaret)
     return;
   m_shouldPaintCaret = !m_shouldPaintCaret;
-  setCaretRectNeedsUpdate();
+  setNeedsPaintInvalidation();
 }
 
 }  // namespace blink