Mark calls of {previous,next}PositionOf with invalid VisiblePosition deprecated

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

Index: third_party/WebKit/Source/core/editing/VisibleUnits.cpp
diff --git a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
index 6ab9347e7956955e656efcf9ca7369271f385b7e..59f2fb836c6e708d81ac73d78fbf43b24563cb95 100644
--- a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
+++ b/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
@@ -3391,9 +3391,8 @@ VisiblePositionInFlatTree rightPositionOf(const VisiblePositionInFlatTree& visib
 template <typename Strategy>
 static VisiblePositionTemplate<Strategy> nextPositionOfAlgorithm(const VisiblePositionTemplate<Strategy>& visiblePosition, EditingBoundaryCrossingRule rule)
 {
-    // TODO(xiaochengh): Ensure that this function is called with a valid
-    // |visiblePosition|, and add |DCHECK(visiblePosition.isValid())|;
-    const VisiblePositionTemplate<Strategy> next = createVisiblePositionDeprecated(nextVisuallyDistinctCandidate(visiblePosition.deepEquivalent()), visiblePosition.affinity());
+    DCHECK(visiblePosition.isValid()) << visiblePosition;
+    const VisiblePositionTemplate<Strategy> next = createVisiblePosition(nextVisuallyDistinctCandidate(visiblePosition.deepEquivalent()), visiblePosition.affinity());
 
     switch (rule) {
     case CanCrossEditingBoundary:
@@ -3418,6 +3417,33 @@ VisiblePositionInFlatTree nextPositionOf(const VisiblePositionInFlatTree& visibl
 }
 
 template <typename Strategy>
+static VisiblePositionTemplate<Strategy> nextPositionOfAlgorithmDeprecated(const VisiblePositionTemplate<Strategy>& visiblePosition, EditingBoundaryCrossingRule rule)

[yosin_UTC9, 2016/09/27 10:05:39]

How about making nextPositionOfAlgorithm to take PositionWithAffinity?
We would like to avoid copy & paste since we can't guarantee to change this
function until we get rid of.

[Xiaocheng, 2016/09/27 12:27:08]

Good idea. Done.

+{
+    const VisiblePositionTemplate<Strategy> next = createVisiblePositionDeprecated(nextVisuallyDistinctCandidate(visiblePosition.deepEquivalent()), visiblePosition.affinity());
+
+    switch (rule) {
+    case CanCrossEditingBoundary:
+        return next;
+    case CannotCrossEditingBoundary:
+        return honorEditingBoundaryAtOrAfter(next, visiblePosition.deepEquivalent());
+    case CanSkipOverEditingBoundary:
+        return skipToEndOfEditingBoundary(next, visiblePosition.deepEquivalent());
+    }
+    NOTREACHED();
+    return honorEditingBoundaryAtOrAfter(next, visiblePosition.deepEquivalent());
+}
+
+VisiblePosition nextPositionOfDeprecated(const VisiblePosition& visiblePosition, EditingBoundaryCrossingRule rule)
+{
+    return nextPositionOfAlgorithmDeprecated<EditingStrategy>(visiblePosition, rule);
+}
+
+VisiblePositionInFlatTree nextPositionOfDeprecated(const VisiblePositionInFlatTree& visiblePosition, EditingBoundaryCrossingRule rule)
+{
+    return nextPositionOfAlgorithmDeprecated<EditingInFlatTreeStrategy>(visiblePosition, rule);
+}
+
+template <typename Strategy>
 static VisiblePositionTemplate<Strategy> skipToStartOfEditingBoundary(const VisiblePositionTemplate<Strategy>& pos, const PositionTemplate<Strategy>& anchor)
 {
     DCHECK(pos.isValid()) << pos;
@@ -3445,8 +3471,7 @@ static VisiblePositionTemplate<Strategy> skipToStartOfEditingBoundary(const Visi
 template <typename Strategy>
 static VisiblePositionTemplate<Strategy> previousPositionOfAlgorithm(const VisiblePositionTemplate<Strategy>& visiblePosition, EditingBoundaryCrossingRule rule)
 {
-    // TODO(xiaochengh): Ensure that this function is called with a valid
-    // |visiblePosition|, and add |DCHECK(visiblePosition.isValid())|;
+    DCHECK(visiblePosition.isValid()) << visiblePosition;
     const PositionTemplate<Strategy> pos = previousVisuallyDistinctCandidate(visiblePosition.deepEquivalent());
 
     // return null visible position if there is no previous visible position
@@ -3457,7 +3482,7 @@ static VisiblePositionTemplate<Strategy> previousPositionOfAlgorithm(const Visib
     // because going previous from an |TextAffinity::Upstream| position can
     // never yield another |TextAffinity::Upstream position| (unless line wrap
     // length is 0!).
-    const VisiblePositionTemplate<Strategy> prev = createVisiblePositionDeprecated(pos);
+    const VisiblePositionTemplate<Strategy> prev = createVisiblePosition(pos);
     if (prev.deepEquivalent() == visiblePosition.deepEquivalent())
         return VisiblePositionTemplate<Strategy>();
 
@@ -3484,4 +3509,44 @@ VisiblePositionInFlatTree previousPositionOf(const VisiblePositionInFlatTree& vi
     return previousPositionOfAlgorithm<EditingInFlatTreeStrategy>(visiblePosition, rule);
 }
 
+template <typename Strategy>
+static VisiblePositionTemplate<Strategy> previousPositionOfAlgorithmDeprecated(const VisiblePositionTemplate<Strategy>& visiblePosition, EditingBoundaryCrossingRule rule)

[yosin_UTC9, 2016/09/27 10:05:39]

Ditto as nextPositionOfAlgorithm.

[Xiaocheng, 2016/09/27 12:27:08]

Done.

+{
+    const PositionTemplate<Strategy> pos = previousVisuallyDistinctCandidate(visiblePosition.deepEquivalent());
+
+    // return null visible position if there is no previous visible position
+    if (pos.atStartOfTree())
+        return VisiblePositionTemplate<Strategy>();
+
+    // we should always be able to make the affinity |TextAffinity::Downstream|,
+    // because going previous from an |TextAffinity::Upstream| position can
+    // never yield another |TextAffinity::Upstream position| (unless line wrap
+    // length is 0!).
+    const VisiblePositionTemplate<Strategy> prev = createVisiblePositionDeprecated(pos);
+    if (prev.deepEquivalent() == visiblePosition.deepEquivalent())
+        return VisiblePositionTemplate<Strategy>();
+
+    switch (rule) {
+    case CanCrossEditingBoundary:
+        return prev;
+    case CannotCrossEditingBoundary:
+        return honorEditingBoundaryAtOrBefore(prev, visiblePosition.deepEquivalent());
+    case CanSkipOverEditingBoundary:
+        return skipToStartOfEditingBoundary(prev, visiblePosition.deepEquivalent());
+    }
+
+    NOTREACHED();
+    return honorEditingBoundaryAtOrBefore(prev, visiblePosition.deepEquivalent());
+}
+
+VisiblePosition previousPositionOfDeprecated(const VisiblePosition& visiblePosition, EditingBoundaryCrossingRule rule)
+{
+    return previousPositionOfAlgorithmDeprecated<EditingStrategy>(visiblePosition, rule);
+}
+
+VisiblePositionInFlatTree previousPositionOfDeprecated(const VisiblePositionInFlatTree& visiblePosition, EditingBoundaryCrossingRule rule)
+{
+    return previousPositionOfAlgorithmDeprecated<EditingInFlatTreeStrategy>(visiblePosition, rule);
+}
+
 } // namespace blink