Ensure valid VisiblePosition input for character{Before,After}

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

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 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 2966 matching lines @@
 
     // That must mean that |pos| is not editable. Return the next position after
     // |pos| that is in the same editable region as this position
     DCHECK(highestRoot);
     return firstEditableVisiblePositionAfterPositionInRoot(pos.deepEquivalent(), *highestRoot);
 }
 
 template <typename Strategy>
 static UChar32 characterAfterAlgorithm(const VisiblePositionTemplate<Strategy>& visiblePosition)
 {
-    // TODO(xiaochengh): Ensure that this function is called with a valid
-    // |visiblePosition|, and add |DCHECK(visiblePosition.isValid())|
+    DCHECK(visiblePosition.isValid()) << visiblePosition;
     // We canonicalize to the first of two equivalent candidates, but the second
     // of the two candidates is the one that will be inside the text node
     // containing the character after this visible position.
     const PositionTemplate<Strategy> pos = mostForwardCaretPosition(visiblePosition.deepEquivalent());
     if (!pos.isOffsetInAnchor())
         return 0;
     Node* containerNode = pos.computeContainerNode();
     if (!containerNode || !containerNode->isTextNode())
         return 0;
     unsigned offset = static_cast<unsigned>(pos.offsetInContainerNode());
     Text* textNode = toText(containerNode);
     unsigned length = textNode->length();
     if (offset >= length)
         return 0;
 
     return textNode->data().characterStartingAt(offset);
 }
 
 UChar32 characterAfter(const VisiblePosition& visiblePosition)
 {
     return characterAfterAlgorithm<EditingStrategy>(visiblePosition);
 }
 
 UChar32 characterAfter(const VisiblePositionInFlatTree& visiblePosition)
 {
     return characterAfterAlgorithm<EditingInFlatTreeStrategy>(visiblePosition);
 }
 
 template <typename Strategy>
 static UChar32 characterBeforeAlgorithm(const VisiblePositionTemplate<Strategy>& visiblePosition)

[yosin_UTC9, 2016/09/27 08:30:53]

Not sure how about making |characterAfter()| to take |Position| instead of
|VisiblePosition|?
|characterAfter()| uses deep equivalent position rather than VP itself.

[Xiaocheng, 2016/09/27 08:48:51]

Talked offline. A lot of functions in VisibleUnits unnecessarily take VPs as
parameters. It's good if they get simplified.

 {
-    // TODO(xiaochengh): Ensure that this function is called with a valid
-    // |visiblePosition|, and add |DCHECK(visiblePosition.isValid())|
+    DCHECK(visiblePosition.isValid()) << visiblePosition;
     return characterAfter(previousPositionOf(visiblePosition));
 }
 
 UChar32 characterBefore(const VisiblePosition& visiblePosition)
 {
     return characterBeforeAlgorithm<EditingStrategy>(visiblePosition);
 }
 
 UChar32 characterBefore(const VisiblePositionInFlatTree& visiblePosition)
 {
@@ skipping 448 matching lines @@
 {
     return previousPositionOfAlgorithm<EditingStrategy>(visiblePosition, rule);
 }
 
 VisiblePositionInFlatTree previousPositionOf(const VisiblePositionInFlatTree& visiblePosition, EditingBoundaryCrossingRule rule)
 {
     return previousPositionOfAlgorithm<EditingInFlatTreeStrategy>(visiblePosition, rule);
 }
 
 } // namespace blink