Prune deprecated functions from ApplyStyleCommand

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

 /*
  * Copyright (C) 2005, 2006, 2008, 2009 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 171 matching lines @@
       m_removeOnly(true),
       m_isInlineElementToRemoveFunction(isInlineElementToRemoveFunction) {}
 
 void ApplyStyleCommand::updateStartEnd(const Position& newStart,
                                        const Position& newEnd) {
   DCHECK_GE(comparePositions(newEnd, newStart), 0);
 
   if (!m_useEndingSelection && (newStart != m_start || newEnd != m_end))
     m_useEndingSelection = true;
 
-  setEndingSelection(
-      createVisibleSelectionDeprecated(newStart, newEnd, VP_DEFAULT_AFFINITY,
-                                       endingSelection().isDirectional()));
+  document().updateStyleAndLayoutIgnorePendingStylesheets();
+  setEndingSelection(createVisibleSelection(newStart, newEnd,

[yosin_UTC9, 2016/10/06 04:51:50]

BTW, we would like to have SelectionForUndoStep or SelectionRecord or whatever,
which is very similar to Selection class (http://crrev.com/1958093002).
|Selection| class is sensitive on DOM tree version but SelectionForUndoStep
doesn't. It holds positions then compute visible selection at undo/redo time
with validating whether these positions are in document.

See Editor::unappliedEditing() and Editor::reappliedEditing() for how we restore
selection from undo stack.

Given that, we don't need to hold VisibleSelection for
m_{staring,ending}Selection, since we should compute VisibleSelection at
undo/redo time.

[Xiaocheng, 2016/10/06 05:04:45]

If I understand correctly, a VisibleSelection should indicate a range of
VisiblePositions, and hence, its stored positions are kind of out-of-date when
mutations take place.

In this sense, I agree with the idea of storing two Positions (with proper
wrapping) instead of a VS. Or perhaps RelocatablePosition.

It should reduce the call sites of layout update calls, since creation of VS
requires clean layout but storing two positions does not.

+                                            VP_DEFAULT_AFFINITY,
+                                            endingSelection().isDirectional()));
   m_start = newStart;
   m_end = newEnd;
 }
 
 Position ApplyStyleCommand::startPosition() {
   if (m_useEndingSelection)
     return endingSelection().start();
 
   return m_start;
 }
@@ skipping 47 matching lines @@
 
   // get positions we want to use for applying style
   Position start = startPosition();
   Position end = endPosition();
   if (comparePositions(end, start) < 0) {
     Position swap = start;
     start = end;
     end = swap;
   }
 
-  VisiblePosition visibleStart = createVisiblePositionDeprecated(start);
-  VisiblePosition visibleEnd = createVisiblePositionDeprecated(end);
+  VisiblePosition visibleStart = createVisiblePosition(start);
+  VisiblePosition visibleEnd = createVisiblePosition(end);
 
   if (visibleStart.isNull() || visibleStart.isOrphan() || visibleEnd.isNull() ||
       visibleEnd.isOrphan())
     return;
 
   // Save and restore the selection endpoints using their indices in the document, since
   // addBlockStyleIfNeeded may moveParagraphs, which can remove these endpoints.
   // Calculate start and end indices from the start of the tree that they're in.
   Node& scope = NodeTraversal::highestAncestorOrSelf(
       *visibleStart.deepEquivalent().anchorNode());
   Range* startRange =
       Range::create(document(), Position::firstPositionInNode(&scope),
                     visibleStart.deepEquivalent().parentAnchoredEquivalent());
   Range* endRange =
       Range::create(document(), Position::firstPositionInNode(&scope),
                     visibleEnd.deepEquivalent().parentAnchoredEquivalent());
   int startIndex = TextIterator::rangeLength(startRange->startPosition(),
                                              startRange->endPosition(), true);
   int endIndex = TextIterator::rangeLength(endRange->startPosition(),
                                            endRange->endPosition(), true);
 
-  VisiblePosition paragraphStart(startOfParagraphDeprecated(visibleStart));
+  VisiblePosition paragraphStart(startOfParagraph(visibleStart));
   VisiblePosition nextParagraphStart(
-      nextPositionOf(endOfParagraphDeprecated(paragraphStart)));
-  VisiblePosition beyondEnd(
-      nextPositionOf(endOfParagraphDeprecated(visibleEnd)));
+      nextPositionOf(endOfParagraph(paragraphStart)));
+  Position beyondEnd =
+      nextPositionOf(endOfParagraph(visibleEnd)).deepEquivalent();
+  // TODO(xiaochengh): Use a saner approach (e.g., temporary Ranges) to keep
+  // these positions in document instead of iteratively performing orphan checks
+  // and recalculating them when they become orphans.
   while (paragraphStart.isNotNull() &&
-         paragraphStart.deepEquivalent() != beyondEnd.deepEquivalent()) {
+         paragraphStart.deepEquivalent() != beyondEnd) {
     DCHECK(!paragraphStart.isOrphan()) << paragraphStart;
     StyleChange styleChange(style, paragraphStart.deepEquivalent());
     if (styleChange.cssStyle().length() || m_removeOnly) {
       Element* block =
           enclosingBlock(paragraphStart.deepEquivalent().anchorNode());
       const Position& paragraphStartToMove = paragraphStart.deepEquivalent();
       if (!m_removeOnly && isEditablePosition(paragraphStartToMove)) {
         HTMLElement* newBlock = moveParagraphContentsToNewBlockIfNecessary(
             paragraphStartToMove, editingState);
         if (editingState->isAborted())
           return;
         if (newBlock) {
           block = newBlock;
-          if (paragraphStart.isOrphan())
-            paragraphStart = createVisiblePositionDeprecated(
-                Position::firstPositionInNode(newBlock));
+          if (paragraphStart.isOrphan()) {
+            document().updateStyleAndLayoutIgnorePendingStylesheets();
+            paragraphStart =
+                createVisiblePosition(Position::firstPositionInNode(newBlock));
+          }
         }
         DCHECK(!paragraphStart.isOrphan()) << paragraphStart;
       }
       if (block && block->isHTMLElement()) {
         removeCSSStyle(style, toHTMLElement(block), editingState);
         if (editingState->isAborted())
           return;
         DCHECK(!paragraphStart.isOrphan()) << paragraphStart;
         if (!m_removeOnly) {
           addBlockStyle(styleChange, toHTMLElement(block));
           DCHECK(!paragraphStart.isOrphan()) << paragraphStart;
         }
       }
 
+      document().updateStyleAndLayoutIgnorePendingStylesheets();
+
+      // Make the VisiblePositions valid again after style changes.
+      // TODO(xiaochengh): We shouldn't store VisiblePositions and inspect their
+      // properties after they have been invalidated by mutations.
       DCHECK(!paragraphStart.isOrphan()) << paragraphStart;
-      if (nextParagraphStart.isOrphan())
+      paragraphStart =
+          createVisiblePosition(paragraphStart.toPositionWithAffinity());
+      if (nextParagraphStart.isOrphan()) {
+        nextParagraphStart = nextPositionOf(endOfParagraph(paragraphStart));
+      } else {
         nextParagraphStart =
-            nextPositionOf(endOfParagraphDeprecated(paragraphStart));
+            createVisiblePosition(nextParagraphStart.toPositionWithAffinity());
+      }
     }
 
     DCHECK(!nextParagraphStart.isOrphan()) << nextParagraphStart;
     paragraphStart = nextParagraphStart;
-    nextParagraphStart =
-        nextPositionOf(endOfParagraphDeprecated(paragraphStart));
+    nextParagraphStart = nextPositionOf(endOfParagraph(paragraphStart));
   }
 
   // Update style and layout again, since added or removed styles could have
   // affected the layout. We need clean layout in order to compute
   // plain-text ranges below.
   document().updateStyleAndLayoutIgnorePendingStylesheets();
 
   EphemeralRange startEphemeralRange =
       PlainTextRange(startIndex)
           .createRangeForSelection(toContainerNode(scope));
@@ skipping 1632 matching lines @@
 
 DEFINE_TRACE(ApplyStyleCommand) {
   visitor->trace(m_style);
   visitor->trace(m_start);
   visitor->trace(m_end);
   visitor->trace(m_styledInlineElement);
   CompositeEditCommand::trace(visitor);
 }
 
 }  // namespace blink