Invalidate the previous caret even if the parent text node is removed.

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

 /*
  * Copyright (C) 2004, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 616 matching lines @@
 
     m_caretBlinkTimer.stop();
 
     LayoutView* view = m_frame->contentLayoutObject();
     if (view)
         view->clearSelection();
 
     setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
     m_selectionEditor->dispose();
     m_previousCaretNode.clear();
+    m_parentOfPreviousCaretNode.clear();
 }
 
 void FrameSelection::setStart(const VisiblePosition &pos, EUserTriggered trigger)
 {
     if (selection().isBaseFirst())
         setBase(pos, trigger);
     else
         setExtent(pos, trigger);
 }
 
@@ skipping 59 matching lines @@
         newRect = localCaretRectOfPosition(PositionWithAffinity(selection().start(), selection().affinity()), layoutObject);
     Node* newNode = layoutObject ? layoutObject->node() : nullptr;
 
     if (!m_caretBlinkTimer.isActive()
         && newNode == m_previousCaretNode
         && newRect == m_previousCaretRect
         && caretVisibility() == m_previousCaretVisibility)
         return;
 
     LayoutView* view = m_frame->document()->layoutView();
-    if (m_previousCaretNode && (shouldRepaintCaret(*m_previousCaretNode) || shouldRepaintCaret(view)))
+    if (m_previousCaretNode && (shouldRepaintCaret(*m_previousCaretNode) || shouldRepaintCaret(view))) {
         invalidateLocalCaretRect(m_previousCaretNode.get(), m_previousCaretRect);
-    if (newNode && (shouldRepaintCaret(*newNode) || shouldRepaintCaret(view)))
+        m_previousCaretRect = LayoutRect();
+    }
+    if (newNode && (shouldRepaintCaret(*newNode) || shouldRepaintCaret(view))) {
         invalidateLocalCaretRect(newNode, newRect);
+        if (!m_previousCaretRect.isEmpty() && m_previousCaretNode && !m_previousCaretNode->parentNode()
+            && newNode == m_parentOfPreviousCaretNode.get()) {

[dshwang, 2016/02/05 10:25:47]

It means |newNode| is also detached from document. If so,
invalidateLocalCaretRect() do nothing.
the condition checks m_previousCaretNode->parentNode()
Could you review again? I improve little more.

If we decide refactoring wide range of code, I have a idea.
All editable element must has anonymous #text node from creation time. However,
there might be unexpected side effect. e.g. when javascript sets 'editable' to
any element, Blink has to create and add additional #text node for box element,
while inline element become editable by itself. caretBrowsingEnabled is also big
obstacle. To be honest, I'm a bit afraid to touch well-working legacy editor
code without concrete knowledge.

[yosin_UTC9, 2016/02/12 03:53:13]

I would like to have radical change. Current implementation of Caret handling
doesn't match rendering pipeline concept.
We don't have clear idea how caret to be implemented, but we have some idea:
 - paint caret in its own layer; we don't need to remember previous rectangle(?)
 - calculate caret height based on a character will be inserted
 - integrated into rendering pipeline and live with lifecycle == calculation of
caret rectangle just before LayoutClean
 - ... and more ...

It is a time to think about caret re-implementation. I would like to leave
current code as is until we have concrete plan.
 

+            // Must invalidate #text even if it's detached. crbug.com/528922
+            invalidateLocalCaretRect(newNode, m_previousCaretRect);
+        }
+    }
     m_previousCaretNode = newNode;
+    m_parentOfPreviousCaretNode = newNode ? newNode->parentNode() : nullptr;
     m_previousCaretRect = newRect;
     m_previousCaretVisibility = caretVisibility();
 }
 
 void FrameSelection::paintCaret(GraphicsContext& context, const LayoutPoint& paintOffset)
 {
     if (selection().isCaret() && m_shouldPaintCaret) {
         updateCaretRect(PositionWithAffinity(selection().start(), selection().affinity()));
         CaretBase::paintCaret(selection().start().anchorNode(), context, paintOffset);
     }
@@ skipping 590 matching lines @@
 #endif
 
 DEFINE_TRACE(FrameSelection)
 {
     visitor->trace(m_frame);
     visitor->trace(m_pendingSelection);
     visitor->trace(m_selectionEditor);
     visitor->trace(m_originalBase);
     visitor->trace(m_originalBaseInComposedTree);
     visitor->trace(m_previousCaretNode);
+    visitor->trace(m_parentOfPreviousCaretNode);
     visitor->trace(m_typingStyle);
 }
 
 void FrameSelection::setCaretRectNeedsUpdate()
 {
     if (m_caretRectDirty)
         return;
     m_caretRectDirty = true;
 
     scheduleVisualUpdate();
@@ skipping 83 matching lines @@
 
 void showTree(const blink::FrameSelection* sel)
 {
     if (sel)
         sel->showTreeForThis();
     else
         fprintf(stderr, "Cannot showTree for (nil) FrameSelection.\n");
 }
 
 #endif