Fix caret paint invalidation when moving between blocks

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

Index: third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
diff --git a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
index cbd1befba99ac0695d7af9ab0d14047d244d78eb..7d20faae739183d871093f9d2f01d4e679382364 100644
--- a/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
+++ b/third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp
@@ -117,15 +117,17 @@ LayoutRect CaretDisplayItemClient::computeCaretRect(
 
 void CaretDisplayItemClient::updateStyleAndLayoutIfNeeded(
     const PositionWithAffinity& caretPosition) {
+  m_previousLayoutBlock = m_layoutBlock;
+  m_previousVisualRect = m_visualRect;
+
   LayoutBlock* newLayoutBlock = caretLayoutBlock(caretPosition.anchorNode());
   if (newLayoutBlock != m_layoutBlock) {
     if (m_layoutBlock)
       m_layoutBlock->setMayNeedPaintInvalidation();
-    m_previousLayoutBlock = m_layoutBlock;
     m_layoutBlock = newLayoutBlock;
-    m_needsPaintInvalidation = true;
-  } else {
-    m_previousLayoutBlock = nullptr;
+    if (newLayoutBlock)
+      m_needsPaintInvalidation = true;
+    m_visualRect = LayoutRect();
   }
 
   if (!newLayoutBlock) {
@@ -158,39 +160,44 @@ void CaretDisplayItemClient::invalidatePaintIfNeeded(
     const LayoutBlock& block,
     const PaintInvalidatorContext& context,
     PaintInvalidationReason layoutBlockPaintInvalidationReason) {
-  if (block == m_previousLayoutBlock) {
-    // Invalidate the previous caret if it was in a different block.
-    // m_previousLayoutBlock is set only when it's different from m_layoutBlock.
-    DCHECK(block != m_layoutBlock);
-
-    ObjectPaintInvalidatorWithContext objectInvalidator(*m_previousLayoutBlock,
-                                                        context);
-    if (!isImmediateFullPaintInvalidationReason(
-            layoutBlockPaintInvalidationReason)) {
-      objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret,
-                                             m_visualRect, LayoutRect());
-    }
+  if (block == m_layoutBlock) {
+    invalidatePaintInCurrentLayoutBlock(context,
+                                        layoutBlockPaintInvalidationReason);
+    return;
+  }
 
-    // If m_layoutBlock is not null, the following will be done when
-    // the new caret is invalidated in m_layoutBlock.
-    if (!m_layoutBlock) {

[Xianzhu, 2017/02/17 18:02:58]

The "will be done" assumption was incorrect because m_layoutBlock may be visited
earlier. m_layoutBlock and m_previousLayoutBlock may also in different painting
layer, so line 177 should be executed for both.

-      context.paintingLayer->setNeedsRepaint();
-      objectInvalidator.invalidateDisplayItemClient(*this,
-                                                    PaintInvalidationCaret);
-      m_visualRect = LayoutRect();

[Xianzhu, 2017/02/17 18:02:58]

This is moved into updateStyleAndLayoutIfNeeded(). The previous code may cause
unnecessary paint invalidation of the "previous visual rect in previous block"
on the current block, because m_visualRect may be the visual rect in previous
block.

-      m_needsPaintInvalidation = false;
-    }
+  if (block == m_previousLayoutBlock) {
+    invalidatePaintInPreviousLayoutBlock(context,
+                                         layoutBlockPaintInvalidationReason);
+  }
+}
 
-    m_previousLayoutBlock = nullptr;
-    return;
+void CaretDisplayItemClient::invalidatePaintInPreviousLayoutBlock(
+    const PaintInvalidatorContext& context,
+    PaintInvalidationReason layoutBlockPaintInvalidationReason) {
+  DCHECK(m_previousLayoutBlock);
+
+  ObjectPaintInvalidatorWithContext objectInvalidator(*m_previousLayoutBlock,
+                                                      context);
+  if (!isImmediateFullPaintInvalidationReason(
+          layoutBlockPaintInvalidationReason)) {
+    objectInvalidator.invalidatePaintUsingContainer(
+        *context.paintInvalidationContainer, m_previousVisualRect,
+        PaintInvalidationCaret);
   }
 
-  if (block != m_layoutBlock)
-    return;
+  context.paintingLayer->setNeedsRepaint();
+  objectInvalidator.invalidateDisplayItemClient(*this, PaintInvalidationCaret);
+  m_previousLayoutBlock = nullptr;
+}
+
+void CaretDisplayItemClient::invalidatePaintInCurrentLayoutBlock(
+    const PaintInvalidatorContext& context,
+    PaintInvalidationReason layoutBlockPaintInvalidationReason) {
+  DCHECK(m_layoutBlock);
 
-  // Invalidate the new caret, and the old caret if it was in the same block.
   LayoutRect newVisualRect;
-  if (m_layoutBlock && !m_localRect.isEmpty()) {
+  if (!m_localRect.isEmpty()) {
     newVisualRect = m_localRect;
     context.mapLocalRectToPaintInvalidationBacking(*m_layoutBlock,
                                                    newVisualRect);
@@ -204,22 +211,21 @@ void CaretDisplayItemClient::invalidatePaintIfNeeded(
     }
   }
 
-  if (m_needsPaintInvalidation || newVisualRect != m_visualRect) {
-    m_needsPaintInvalidation = false;
+  if (!m_needsPaintInvalidation && newVisualRect == m_visualRect)
+    return;
 
-    ObjectPaintInvalidatorWithContext objectInvalidator(*m_layoutBlock,
-                                                        context);
-    if (!isImmediateFullPaintInvalidationReason(
-            layoutBlockPaintInvalidationReason)) {
-      objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret,
-                                             m_visualRect, newVisualRect);
-    }
+  m_needsPaintInvalidation = false;
 
-    context.paintingLayer->setNeedsRepaint();
-    objectInvalidator.invalidateDisplayItemClient(*this,
-                                                  PaintInvalidationCaret);
+  ObjectPaintInvalidatorWithContext objectInvalidator(*m_layoutBlock, context);
+  if (!isImmediateFullPaintInvalidationReason(
+          layoutBlockPaintInvalidationReason)) {
+    objectInvalidator.fullyInvalidatePaint(PaintInvalidationCaret, m_visualRect,
+                                           newVisualRect);
   }
 
+  context.paintingLayer->setNeedsRepaint();
+  objectInvalidator.invalidateDisplayItemClient(*this, PaintInvalidationCaret);
+
   m_visualRect = newVisualRect;
 }