Rewrite DocumentMarkerController to use SynchronousMutationObserver

third_party/WebKit/Source/core/editing/commands/SetCharacterDataCommand.cpp

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/editing/commands/SetCharacterDataCommand.h"
+
+#include "core/editing/EditingUtilities.h"
+#include "core/frame/Settings.h"
+#include "core/layout/LayoutText.h"
+
+namespace blink {
+
+SetCharacterDataCommand::SetCharacterDataCommand(Text* node,
+                                                 unsigned offset,
+                                                 unsigned count,
+                                                 const String& text)
+    : SimpleEditCommand(node->document()),
+      m_node(node),
+      m_offset(offset),
+      m_count(count),
+      m_newText(text) {
+  DCHECK(m_node);
+  DCHECK_LE(m_offset + m_count, m_node->length());

[yosin_UTC9, 2017/02/20 07:26:26]

Please add
DCHECK_LT(m_offset, m_node->length());

+}
+
+void SetCharacterDataCommand::doApply(EditingState*) {
+  DCHECK(m_node);

[yosin_UTC9, 2017/02/20 07:26:26]

nit: We don't need to have DCHECK(m_node). |m_node| will be const member
variable.

+
+  document().updateStyleAndLayoutTree();

[yosin_UTC9, 2017/02/20 07:26:26]

Please add following comment before L29

// TODO(editing-dev): The use of updateStyleAndLayoutTree()
// needs to be audited.  See http://crbug.com/590369 for more details.

+  if (!hasEditableStyle(*m_node))
+    return;
+
+  DummyExceptionStateForTesting exceptionState;
+  m_previousTextForUndo =
+      m_node->substringData(m_offset, m_count, exceptionState);
+  if (exceptionState.hadException())
+    return;
+
+  bool passwordEchoEnabled =

[yosin_UTC9, 2017/02/20 07:26:26]

nit: s/bool/const bool/

+      document().settings() && document().settings()->getPasswordEchoEnabled();
+
+  if (passwordEchoEnabled) {
+    LayoutText* layoutText = m_node->layoutObject();
+    if (layoutText && layoutText->isSecure()) {
+      layoutText->momentarilyRevealLastTypedCharacter(m_offset +
+                                                      m_newText.length() - 1);
+    }
+  }
+
+  m_node->replaceData(m_offset, m_count, m_newText,
+                      IGNORE_EXCEPTION_FOR_TESTING);
+  document().updateStyleAndLayout();
+}
+
+void SetCharacterDataCommand::doUnapply() {
+  DCHECK(m_node);

[yosin_UTC9, 2017/02/20 07:26:26]

nit: We don't need to have DCHECK(m_node). |m_node| will be const member
variable.

+
+  if (!hasEditableStyle(*m_node))

[yosin_UTC9, 2017/02/20 07:26:26]

Please update layout tree:

// TODO(editing-dev): The use of updateStyleAndLayoutTree()
// needs to be audited.  See http://crbug.com/590369 for more details.
 document().updateStyleAndLayoutTree();

[rlanday, 2017/02/21 20:04:06]

Can you please explain to me what this is doing and how I know where I need to
call it?

[Xiaocheng, 2017/02/22 01:49:23]

This function resolves style for all elements in the document, and calculates
their sizes and positions. It also calculates how text is wrapped into lines.

Most editing operations require clean layout, as they depend on how text is laid
out.

In most cases, you will hit a DCHECK if a call to this function is missing.

+    return;
+
+  m_node->replaceData(m_offset, m_newText.length(), m_previousTextForUndo,
+                      IGNORE_EXCEPTION_FOR_TESTING);
+  m_node->document().updateStyleAndLayout();

[yosin_UTC9, 2017/02/20 07:26:26]

Please don't call |updateStyleAndLayout()| here.

+}
+
+DEFINE_TRACE(SetCharacterDataCommand) {
+  visitor->trace(m_node);
+  SimpleEditCommand::trace(visitor);
+}
+
+}  // namespace blink