Stop using canonical versions of Previous/NextWordPosition() in SelectionModifier

This patch changes SelectionModifier to use non-canonical versions of
Previous/NextWordPosition() to reduce usage of VisiblePosition().

Now that the canonical versions are no longer used, they are also
removed by this patch.

Bug: 657237
Change-Id: Ifa908bfb6bda948e78d4afc8171be55343eaa0ea
Reviewed-on: https://chromium-review.googlesource.com/c/1315930Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605249}

third_party/blink/renderer/core/editing/selection_modifier.cc

@@ -253,15 +253,15 @@ VisiblePosition SelectionModifier::EndForPlatform() const {
   return PositionForPlatform(false);
 }
 
-VisiblePosition SelectionModifier::NextWordPositionForPlatform(
-    const VisiblePosition& original_position) {
-  VisiblePosition position_after_current_word =
-      NextWordPosition(original_position);
+Position SelectionModifier::NextWordPositionForPlatform(
+    const Position& original_position) {
+  // Next word position can't be upstream.
+  const Position position_after_current_word =
+      NextWordPosition(original_position).GetPosition();
 
   if (!GetFrame().GetEditor().Behavior().ShouldSkipSpaceWhenMovingRight())
     return position_after_current_word;
-  return CreateVisiblePosition(
-      SkipWhitespace(position_after_current_word.DeepEquivalent()));
+  return SkipWhitespace(position_after_current_word);
 }
 
 static VisiblePosition AdjustForwardPositionForUserSelectAll(
@@ -302,9 +302,11 @@ VisiblePosition SelectionModifier::ModifyExtendingRightInternal(
                                 kCanSkipOverEditingBoundary);
     case TextGranularity::kWord:
       if (DirectionOfEnclosingBlock() == TextDirection::kLtr) {
-        return NextWordPositionForPlatform(ComputeVisibleExtent(selection_));
+        return CreateVisiblePosition(NextWordPositionForPlatform(
+            ComputeVisibleExtent(selection_).DeepEquivalent()));
       }
-      return PreviousWordPosition(ComputeVisibleExtent(selection_));
+      return CreateVisiblePosition(PreviousWordPosition(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
     case TextGranularity::kLineBoundary:
       if (DirectionOfEnclosingBlock() == TextDirection::kLtr)
         return ModifyExtendingForwardInternal(granularity);
@@ -337,7 +339,8 @@ VisiblePosition SelectionModifier::ModifyExtendingForwardInternal(
       return NextPositionOf(ComputeVisibleExtent(selection_),
                             kCanSkipOverEditingBoundary);
     case TextGranularity::kWord:
-      return NextWordPositionForPlatform(ComputeVisibleExtent(selection_));
+      return CreateVisiblePosition(NextWordPositionForPlatform(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
     case TextGranularity::kSentence:
       return NextSentencePosition(ComputeVisibleExtent(selection_));
     case TextGranularity::kLine:
@@ -417,7 +420,8 @@ VisiblePosition SelectionModifier::ModifyMovingForward(
       return NextPositionOf(ComputeVisibleExtent(selection_),
                             kCanSkipOverEditingBoundary);
     case TextGranularity::kWord:
-      return NextWordPositionForPlatform(ComputeVisibleExtent(selection_));
+      return CreateVisiblePosition(NextWordPositionForPlatform(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
     case TextGranularity::kSentence:
       return NextSentencePosition(ComputeVisibleExtent(selection_));
     case TextGranularity::kLine: {
@@ -470,9 +474,11 @@ VisiblePosition SelectionModifier::ModifyExtendingLeftInternal(
                             kCanSkipOverEditingBoundary);
     case TextGranularity::kWord:
       if (DirectionOfEnclosingBlock() == TextDirection::kLtr) {
-        return PreviousWordPosition(ComputeVisibleExtent(selection_));
+        return CreateVisiblePosition(PreviousWordPosition(
+            ComputeVisibleExtent(selection_).DeepEquivalent()));
       }
-      return NextWordPositionForPlatform(ComputeVisibleExtent(selection_));
+      return CreateVisiblePosition(NextWordPositionForPlatform(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
     case TextGranularity::kLineBoundary:
       if (DirectionOfEnclosingBlock() == TextDirection::kLtr)
         return ModifyExtendingBackwardInternal(granularity);
@@ -508,7 +514,8 @@ VisiblePosition SelectionModifier::ModifyExtendingBackwardInternal(
       return PreviousPositionOf(ComputeVisibleExtent(selection_),
                                 kCanSkipOverEditingBoundary);
     case TextGranularity::kWord:
-      return PreviousWordPosition(ComputeVisibleExtent(selection_));
+      return CreateVisiblePosition(PreviousWordPosition(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
     case TextGranularity::kSentence:
       return PreviousSentencePosition(ComputeVisibleExtent(selection_));
     case TextGranularity::kLine:
@@ -589,7 +596,8 @@ VisiblePosition SelectionModifier::ModifyMovingBackward(
       }
       break;
     case TextGranularity::kWord:
-      pos = PreviousWordPosition(ComputeVisibleExtent(selection_));
+      pos = CreateVisiblePosition(PreviousWordPosition(
+          ComputeVisibleExtent(selection_).DeepEquivalent()));
       break;
     case TextGranularity::kSentence:
       pos = PreviousSentencePosition(ComputeVisibleExtent(selection_));

third_party/blink/renderer/core/editing/selection_modifier.h

@@ -98,7 +98,7 @@ class CORE_EXPORT SelectionModifier {
   VisiblePosition ModifyExtendingBackwardInternal(TextGranularity);
   VisiblePosition ModifyMovingLeft(TextGranularity);
   VisiblePosition ModifyMovingBackward(TextGranularity);
-  VisiblePosition NextWordPositionForPlatform(const VisiblePosition&);
+  Position NextWordPositionForPlatform(const Position&);
 
   // TODO(editing-dev): We should handle |skips_spaces_when_moving_right| in
   // another way, e.g. pass |EditingBehavior()|.

third_party/blink/renderer/core/editing/visible_units.h

@@ -134,11 +134,8 @@ CORE_EXPORT PositionInFlatTree
 EndOfWordPosition(const PositionInFlatTree&, EWordSide = kNextWordIfOnBoundary);
 CORE_EXPORT VisiblePositionInFlatTree
 EndOfWord(const VisiblePositionInFlatTree&, EWordSide = kNextWordIfOnBoundary);
-// TODO(crbug.com/657237): Change all callers to use non-canonical versions.
 CORE_EXPORT PositionWithAffinity PreviousWordPosition(const Position&);
-CORE_EXPORT VisiblePosition PreviousWordPosition(const VisiblePosition&);
 CORE_EXPORT PositionWithAffinity NextWordPosition(const Position&);
-CORE_EXPORT VisiblePosition NextWordPosition(const VisiblePosition&);
 
 // sentences
 CORE_EXPORT VisiblePosition StartOfSentence(const VisiblePosition&);

third_party/blink/renderer/core/editing/visible_units_word.cc

@@ -223,13 +223,6 @@ PositionWithAffinity NextWordPosition(const Position& start) {
   return ToPositionInDOMTreeWithAffinity(next);
 }
 
-// TODO(yosin): This function will be removed by replacing call sites to use
-// |Position| version. since there are only two call sites, one is in test.
-VisiblePosition NextWordPosition(const VisiblePosition& c) {
-  DCHECK(c.IsValid()) << c;
-  return CreateVisiblePosition(NextWordPosition(c.DeepEquivalent()));
-}
-
 PositionInFlatTreeWithAffinity PreviousWordPosition(
     const PositionInFlatTree& start) {
   const PositionInFlatTree prev = PreviousWordPositionInternal(start);
@@ -243,13 +236,6 @@ PositionWithAffinity PreviousWordPosition(const Position& start) {
   return ToPositionInDOMTreeWithAffinity(prev);
 }
 
-// TODO(xiaochengh): Remove this function. Change callers to use the
-// Position version as it doesn't need canonical input position.
-VisiblePosition PreviousWordPosition(const VisiblePosition& c) {
-  DCHECK(c.IsValid()) << c;
-  return CreateVisiblePosition(PreviousWordPosition(c.DeepEquivalent()));
-}
-
 Position StartOfWordPosition(const VisiblePosition& position, EWordSide side) {
   return StartOfWordAlgorithm<EditingStrategy>(position, side);
 }