Use PositionMoveType::GraphemeBoundary

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

Index: third_party/WebKit/Source/core/editing/EditingUtilities.cpp
diff --git a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
index bb3f0d1738dfdaa546bd416742a9a36b561bae28..74c96c46e240bbd678445ebc3a2336283fd08f79 100644
--- a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
+++ b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
@@ -64,6 +64,19 @@ namespace blink {
 
 using namespace HTMLNames;
 
+namespace {
+std::ostream& operator<<(std::ostream& os, PositionMoveType type)
+{
+    static const char* const texts[] = {
+        "CodeUnit", "CodePoint", "BackwardDeletion", "GraphemeCluster"
+    };
+    const auto& it = std::begin(texts) + static_cast<size_t>(type);
+    DCHECK_GE(it, std::begin(texts)) << "Unknown PositionMoveType value";
+    DCHECK_LT(it, std::end(texts)) << "Unknown PositionMoveType value";
+    return os << *it;
+}
+} // namespace
+
 // Atomic means that the node has no children, or has children which are ignored for the
 // purposes of editing.
 bool isAtomicNode(const Node *node)
@@ -632,10 +645,14 @@ PositionTemplate<Strategy> previousPositionOfAlgorithm(const PositionTemplate<St
         case PositionMoveType::CodeUnit:
             return PositionTemplate<Strategy>(node, offset - 1);
         case PositionMoveType::CodePoint:
-            // TODO(nona): Move to PositionMoveType::GraphemeBoundary case.
-            return PositionTemplate<Strategy>(node, previousGraphemeBoundaryOf(node, offset));
+            NOTIMPLEMENTED() << moveType << " is not yet supported.";

[yosin_UTC9, 2016/04/07 01:13:56]

Let's see results of layout test. If nothing is failed, we can get rid of
|PositionMoveType::CodePoint|. 

[Seigo Nonaka, 2016/04/08 05:47:45]

Done.

+            return PositionTemplate<Strategy>(node, offset - 1);
         case PositionMoveType::BackwardDeletion:
             return PositionTemplate<Strategy>(node, previousBackwardDeletionOffsetOf(node, offset));
+        case PositionMoveType::GraphemeCluster:
+            return PositionTemplate<Strategy>(node, previousGraphemeBoundaryOf(node, offset));
+        default:
+            NOTREACHED() << "Unhandled moveType: " << moveType;
         }
     }
 
@@ -682,9 +699,22 @@ PositionTemplate<Strategy> nextPositionOfAlgorithm(const PositionTemplate<Strate
         //      is correct.
         //   2) The new offset is a bogus offset like (<br>, 1), and there is no
         //      child. Going from 0 to 1 is correct.
-        // TODO(nona): Call nextGraphemeBoundaryOf if
-        //             moveType == PositionMoveType::GraphemeBoundary
-        return PositionTemplate<Strategy>::editingPositionOf(node, (moveType == PositionMoveType::CodePoint) ? nextGraphemeBoundaryOf(node, offset) : offset + 1);
+        switch (moveType) {
+        case PositionMoveType::CodeUnit:
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::CodePoint:

[yosin_UTC9, 2016/04/08 00:51:57]

Since, nobody use |PositionMoveType::CodePoint|, could you get rid of this
rather than |NOTIMPLEMENTED()|?

[Seigo Nonaka, 2016/04/08 05:47:45]

Done.

+            NOTIMPLEMENTED() << moveType << " is not yet supported.";
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::BackwardDeletion:
+            NOTREACHED()
+                << "BackwardDeletion is only available for prevPositionOf "
+                << "functions.";
+            return PositionTemplate<Strategy>::editingPositionOf(node, offset + 1);
+        case PositionMoveType::GraphemeCluster:
+            return PositionTemplate<Strategy>::editingPositionOf(node, nextGraphemeBoundaryOf(node, offset));
+        default:
+            NOTREACHED() << "Unhandled moveType: " << moveType;
+        }
     }
 
     if (ContainerNode* parent = Strategy::parent(*node))