Get rid of flat tree version of createVisibleSelection() taking two VisiblePosition

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

Index: third_party/WebKit/Source/core/editing/FrameSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.cpp b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
index 87da464c3e58dd84af4fe4492893e167581a7fb0..b434bbc19e2b0b8567bfda2209f37cfbee0c032b 100644
--- a/third_party/WebKit/Source/core/editing/FrameSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
@@ -945,10 +945,15 @@ void FrameSelection::selectFrameElementInParentIfFullySelected() {
       Position(ownerElementParent, ownerElementNodeIndex + 1),
       VP_UPSTREAM_IF_POSSIBLE);
 
+  SelectionInDOMTree::Builder builder;
+  builder
+      .setBaseAndExtentDeprecated(beforeOwnerElement.deepEquivalent(),
+                                  afterOwnerElement.deepEquivalent())
+      .setAffinity(beforeOwnerElement.affinity());
+
   // Focus on the parent frame, and then select from before this element to
   // after.
-  VisibleSelection newSelection =
-      createVisibleSelection(beforeOwnerElement, afterOwnerElement);
+  VisibleSelection newSelection = createVisibleSelection(builder.build());
   page->focusController().setFocusedFrame(parent);
   // setFocusedFrame can dispatch synchronous focus/blur events.  The document
   // tree might be modified.
@@ -1387,9 +1392,13 @@ bool FrameSelection::selectWordAroundPosition(const VisiblePosition& position) {
     String text =
         plainText(EphemeralRange(start.deepEquivalent(), end.deepEquivalent()));
     if (!text.isEmpty() && !isSeparator(text.characterStartingAt(0))) {
-      setSelection(createVisibleSelection(start, end),
-                   CloseTyping | ClearTypingStyle,
-                   CursorAlignOnScroll::IfNeeded, WordGranularity);
+      setSelection(
+          createVisibleSelection(SelectionInDOMTree::Builder()
+                                     .collapse(start.toPositionWithAffinity())
+                                     .extend(end.deepEquivalent())
+                                     .build()),
+          CloseTyping | ClearTypingStyle, CursorAlignOnScroll::IfNeeded,
+          WordGranularity);
       return true;
     }
   }
@@ -1428,11 +1437,18 @@ void FrameSelection::moveRangeSelectionExtent(const IntPoint& contentsPoint) {
                CursorAlignOnScroll::IfNeeded, CharacterGranularity);
 }
 
+// TODO(yosin): We should make |FrameSelection::moveRangeSelection()| to take
+// two |IntPoint| instead of two |VisiblePosition| like
+// |moveRangeSelectionExtent()|.
 void FrameSelection::moveRangeSelection(const VisiblePosition& basePosition,
                                         const VisiblePosition& extentPosition,
                                         TextGranularity granularity) {
-  VisibleSelection newSelection =
-      createVisibleSelection(basePosition, extentPosition);
+  SelectionInDOMTree::Builder builder;
+  builder
+      .setBaseAndExtent(basePosition.deepEquivalent(),

[Xiaocheng, 2016/10/24 06:58:49]

Is it guaranteed to have non-null basePosition and extentPosition here?

[yosin_UTC9, 2016/10/24 07:40:17]

Oops, No. 
We should use setBaseAndExtentDeprecated().

+                        extentPosition.deepEquivalent())
+      .setAffinity(basePosition.affinity());
+  VisibleSelection newSelection = createVisibleSelection(builder.build());
   newSelection.expandUsingGranularity(granularity);
 
   if (newSelection.isNone())