Return EphemeralRange from Editor::findRangeOfString.

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

Index: third_party/WebKit/Source/core/editing/Editor.cpp
diff --git a/third_party/WebKit/Source/core/editing/Editor.cpp b/third_party/WebKit/Source/core/editing/Editor.cpp
index 7a4237d403016eafaa67be28ab835cdf0c97dda9..520fe097d851e5673ae0deb4c4086bfacb38f701 100644
--- a/third_party/WebKit/Source/core/editing/Editor.cpp
+++ b/third_party/WebKit/Source/core/editing/Editor.cpp
@@ -1145,58 +1145,78 @@ void Editor::computeAndSetTypingStyle(StylePropertySet* style, EditAction editin
 
 bool Editor::findString(const String& target, FindOptions options)
 {
-    VisibleSelection selection = frame().selection().selection();
-
-    // TODO(yosin) We should make |findRangeOfString()| to return
-    // |EphemeralRange| rather than|Range| object.
-    RefPtrWillBeRawPtr<Range> resultRange = findRangeOfString(target, EphemeralRange(selection.start(), selection.end()), static_cast<FindOptions>(options | FindAPICall));
+    const VisibleSelection& selection = frame().selection().selection();
+    const EphemeralRange& resultRange = findRangeOfString(
+        target,
+        EphemeralRange(selection.start(), selection.end()),
+        static_cast<FindOptions>(options | FindAPICall));
 
-    if (!resultRange)
+    if (resultRange.isCollapsed())
         return false;
 
-    frame().selection().setSelection(VisibleSelection(EphemeralRange(resultRange.get())));
+    frame().selection().setSelection(VisibleSelection(resultRange));
     frame().selection().revealSelection();
     return true;
 }
 
 template <typename Strategy>
-static PassRefPtrWillBeRawPtr<Range> findStringAndScrollToVisibleAlgorithm(Editor& editor, const String& target, const EphemeralRangeTemplate<Strategy>& previousMatch, FindOptions options)
-{
-    RefPtrWillBeRawPtr<Range> nextMatch = editor.findRangeOfString(target, previousMatch, options);
-    if (!nextMatch)
-        return nullptr;
+static EphemeralRangeTemplate<Strategy> findStringAndScrollToVisibleAlgorithm(Editor& editor, const String& target, const EphemeralRangeTemplate<Strategy>& previousMatch, FindOptions options)
+{
+    const EphemeralRangeTemplate<Strategy>& nextMatch = editor.findRangeOfString(target, previousMatch, options);
+    if (nextMatch.isCollapsed())
+        return EphemeralRangeTemplate<Strategy>();
+
+    if (auto* layoutObject = nextMatch.startPosition().nodeAsRangeFirstNode()->layoutObject()) {
+        layoutObject->scrollRectToVisible(
+            LayoutRect(nextMatch.textBoundingBox()),
+            ScrollAlignment::alignCenterIfNeeded,
+            ScrollAlignment::alignCenterIfNeeded,
+            UserScroll);
+    }
+
+    return nextMatch;
+}
 
-    nextMatch->firstNode()->layoutObject()->scrollRectToVisible(LayoutRect(nextMatch->boundingBox()),
-        ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded, UserScroll);
+EphemeralRange Editor::findStringAndScrollToVisible(const String& target, const EphemeralRange& range, FindOptions options)
+{
+    return findStringAndScrollToVisibleAlgorithm(*this, target, range, options);
+}
 
-    return nextMatch.release();
+EphemeralRangeInComposedTree Editor::findStringAndScrollToVisible(const String& target, const EphemeralRangeInComposedTree& range, FindOptions options)
+{
+    ASSERT(RuntimeEnabledFeatures::selectionForComposedTreeEnabled());
+    return findStringAndScrollToVisibleAlgorithm(*this, target, range, options);
 }
 
-PassRefPtrWillBeRawPtr<Range> Editor::findStringAndScrollToVisible(const String& target, Range* range, FindOptions options)
+// This function checks whether start and end position of the |range|
+// crossing shadow boundaries.
+// It should be equivalent to !Range::create(ephemeralRange)->collapsed().
+template <typename Strategy>
+static bool rangeBoundariesAreValid(const EphemeralRangeTemplate<Strategy>& range)
 {
-    if (RuntimeEnabledFeatures::selectionForComposedTreeEnabled())
-        return findStringAndScrollToVisibleAlgorithm<EditingInComposedTreeStrategy>(*this, target, EphemeralRangeInComposedTree(range), options);
-    return findStringAndScrollToVisibleAlgorithm<EditingStrategy>(*this, target, EphemeralRange(range), options);
+    const Node* endRootContainer = range.endPosition().computeContainerNode();
+    const Node* startRootContainer = range.startPosition().computeContainerNode();
+    if (!startRootContainer || !endRootContainer)
+        return false;
+
+    ASSERT(Strategy::commonAncestor(*startRootContainer, *endRootContainer));
+    return (range.startPosition().compareTo(range.endPosition()) < 0);

[yosin_UTC9, 2015/11/02 03:44:17]

nit: Could you omit parenthesis around return expression?

[Andrey Kraynov, 2015/11/02 14:06:57]

Done.

 }
 
-// TODO(yosin) We should return |EphemeralRange| rather than |Range|. We use
-// |Range| object for checking whether start and end position crossing shadow
-// boundaries, however we can do it without |Range| object.
 template <typename Strategy>
-static PassRefPtrWillBeRawPtr<Range> findStringBetweenPositions(const String& target, const EphemeralRangeTemplate<Strategy>& referenceRange, FindOptions options)
+static EphemeralRangeTemplate<Strategy> findStringBetweenPositions(const String& target, const EphemeralRangeTemplate<Strategy>& referenceRange, FindOptions options)
 {
     EphemeralRangeTemplate<Strategy> searchRange(referenceRange);
 
     bool forward = !(options & Backwards);
 
     while (true) {
-        EphemeralRangeTemplate<Strategy> resultRange = findPlainText(searchRange, target, options);
+        const EphemeralRangeTemplate<Strategy>& resultRange = findPlainText(searchRange, target, options);
         if (resultRange.isCollapsed())
-            return nullptr;
+            return EphemeralRangeTemplate<Strategy>();
 
-        RefPtrWillBeRawPtr<Range> rangeObject = Range::create(resultRange.document(), toPositionInDOMTree(resultRange.startPosition()), toPositionInDOMTree(resultRange.endPosition()));
-        if (!rangeObject->collapsed())
-            return rangeObject.release();
+        if (rangeBoundariesAreValid(resultRange))
+            return resultRange;
 
         // Found text spans over multiple TreeScopes. Since it's impossible to
         // return such section as a Range, we skip this match and seek for the
@@ -1214,14 +1234,14 @@ static PassRefPtrWillBeRawPtr<Range> findStringBetweenPositions(const String& ta
     }
 
     ASSERT_NOT_REACHED();
-    return nullptr;
+    return EphemeralRangeTemplate<Strategy>();
 }
 
 template <typename Strategy>
-static PassRefPtrWillBeRawPtr<Range> findRangeOfStringAlgorithm(Document& document, const String& target, const EphemeralRangeTemplate<Strategy>& referenceRange, FindOptions options)
+static EphemeralRangeTemplate<Strategy> findRangeOfStringAlgorithm(Document& document, const String& target, const EphemeralRangeTemplate<Strategy>& referenceRange, FindOptions options)
 {
     if (target.isEmpty())
-        return nullptr;
+        return EphemeralRangeTemplate<Strategy>();
 
     // Start from an edge of the reference range. Which edge is used depends on
     // whether we're searching forward or backward, and whether startInSelection
@@ -1243,32 +1263,32 @@ static PassRefPtrWillBeRawPtr<Range> findRangeOfStringAlgorithm(Document& docume
             searchRange = EphemeralRangeTemplate<Strategy>(documentRange.startPosition(), referenceRange.startPosition());
     }
 
-    RefPtrWillBeRawPtr<Range> resultRange = findStringBetweenPositions(target, searchRange, options);
+    EphemeralRangeTemplate<Strategy> resultRange = findStringBetweenPositions(target, searchRange, options);
 
     // If we started in the reference range and the found range exactly matches
     // the reference range, find again. Build a selection with the found range
     // to remove collapsed whitespace. Compare ranges instead of selection
     // objects to ignore the way that the current selection was made.
-    if (resultRange && startInReferenceRange && normalizeRange(EphemeralRangeTemplate<Strategy>(resultRange.get())) == referenceRange) {
+    if (!resultRange.isCollapsed() && startInReferenceRange && normalizeRange(resultRange) == referenceRange) {
         if (forward)
-            searchRange = EphemeralRangeTemplate<Strategy>(fromPositionInDOMTree<Strategy>(resultRange->endPosition()), searchRange.endPosition());
+            searchRange = EphemeralRangeTemplate<Strategy>(resultRange.endPosition(), searchRange.endPosition());
         else
-            searchRange = EphemeralRangeTemplate<Strategy>(searchRange.startPosition(), fromPositionInDOMTree<Strategy>(resultRange->startPosition()));
+            searchRange = EphemeralRangeTemplate<Strategy>(searchRange.startPosition(), resultRange.startPosition());
         resultRange = findStringBetweenPositions(target, searchRange, options);
     }
 
-    if (!resultRange && options & WrapAround)
+    if (resultRange.isCollapsed() && options & WrapAround)
         return findStringBetweenPositions(target, documentRange, options);
 
-    return resultRange.release();
+    return resultRange;
 }
 
-PassRefPtrWillBeRawPtr<Range> Editor::findRangeOfString(const String& target, const EphemeralRange& reference, FindOptions options)
+EphemeralRange Editor::findRangeOfString(const String& target, const EphemeralRange& reference, FindOptions options)
 {
     return findRangeOfStringAlgorithm<EditingStrategy>(*frame().document(), target, reference, options);
 }
 
-PassRefPtrWillBeRawPtr<Range> Editor::findRangeOfString(const String& target, const EphemeralRangeInComposedTree& reference, FindOptions options)
+EphemeralRangeInComposedTree Editor::findRangeOfString(const String& target, const EphemeralRangeInComposedTree& reference, FindOptions options)
 {
     return findRangeOfStringAlgorithm<EditingInComposedTreeStrategy>(*frame().document(), target, reference, options);
 }