Invalidate the previous caret location when editing text nodes.

third_party/WebKit/Source/core/dom/CharacterData.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2013 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 24 matching lines @@
 
 namespace blink {
 
 void CharacterData::atomize()
 {
     m_data = AtomicString(m_data);
 }
 
 void CharacterData::setData(const String& data)
 {
+    document().dataWillChange(*this);

[yosin_UTC9, 2016/04/28 01:50:28]

At glance, calling |dataWillChange()| is enough in |setDataAnUpdate()|.

Why do we need to call |dataWillChange()| for |m_data === nonNullData| case?
Is this root cause of this bug?

[chrishtr, 2016/04/28 17:22:01]

setData() is an indepenent call site, no?
Done. Also noticed that there is no early-out logic in setDataAndUpdate. I'll
try that in a followup patch.
The root cause is that if a text node is inserted by script (and not an editing
command), and then the last character of the text is deleted, the text node's
layout object
will be eagerly deleted in updateTextLayoutObject. Thus FrameSelection doesn't
have a hook to invalidate the old caret location before its old location becomes
unknown.

+
     const String& nonNullData = !data.isNull() ? data : emptyString();
     if (m_data == nonNullData)
         return;
 
     unsigned oldLength = length();
 
     setDataAndUpdate(nonNullData, 0, oldLength, nonNullData.length(), UpdateFromNonParser);
     document().didRemoveText(this, 0, oldLength);
 }
 
@@ skipping 97 matching lines @@
     return m_data.containsOnlyWhitespace();
 }
 
 void CharacterData::setNodeValue(const String& nodeValue)
 {
     setData(nodeValue);
 }
 
 void CharacterData::setDataAndUpdate(const String& newData, unsigned offsetOfReplacedData, unsigned oldLength, unsigned newLength, UpdateSource source, RecalcStyleBehavior recalcStyleBehavior)
 {
+    document().dataWillChange(*this);

[yosin_UTC9, 2016/04/28 01:50:28]

Should we call this for |source != UpdateFromParser| for paring with
|didUpdateCharacterData()| call?

[chrishtr, 2016/04/28 17:22:02]

Done.

+
     String oldData = m_data;
     m_data = newData;
 
     DCHECK(!layoutObject() || isTextNode());
     if (isTextNode())
         toText(this)->updateTextLayoutObject(offsetOfReplacedData, oldLength, recalcStyleBehavior);
 
     if (source != UpdateFromParser) {
         if (getNodeType() == PROCESSING_INSTRUCTION_NODE)
             toProcessingInstruction(this)->didAttributeChanged();
@@ skipping 26 matching lines @@
     }
     InspectorInstrumentation::characterDataModified(this);
 }
 
 int CharacterData::maxCharacterOffset() const
 {
     return static_cast<int>(length());
 }
 
 } // namespace blink