Stop creating VisiblePosition in VisiblePositionForContentsPoint()

This patch moves CreateVisiblePosition() out of
VisiblePositionForContentsPoint() to the callers.
Rename VisiblePositionForContentsPoint() to
PositionForContentsPointRespectingEditingBoundary().

Bug: 657237
Change-Id: I00c60a783c35d5e5a59b9163eb3e42b7e9fa2b38
Reviewed-on: https://chromium-review.googlesource.com/c/1307315
Commit-Queue: Zhuoyu Qian <zhuoyu.qian@samsung.com>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604500}

third_party/blink/renderer/core/editing/frame_selection.cc

@@ -165,8 +165,8 @@ void FrameSelection::MoveCaretSelection(const IntPoint& point) {
   if (!editable)
     return;
 
-  const VisiblePosition position =
-      VisiblePositionForContentsPoint(point, GetFrame());
+  const VisiblePosition position = CreateVisiblePosition(
+      PositionForContentsPointRespectingEditingBoundary(point, GetFrame()));
   SelectionInDOMTree::Builder builder;
   if (position.IsNotNull())
     builder.Collapse(position.ToPositionWithAffinity());
@@ -1145,9 +1145,11 @@ void FrameSelection::MoveRangeSelection(const IntPoint& base_point,
                                         const IntPoint& extent_point,
                                         TextGranularity granularity) {
   const VisiblePosition& base_position =
-      VisiblePositionForContentsPoint(base_point, GetFrame());
+      CreateVisiblePosition(PositionForContentsPointRespectingEditingBoundary(
+          base_point, GetFrame()));
   const VisiblePosition& extent_position =
-      VisiblePositionForContentsPoint(extent_point, GetFrame());
+      CreateVisiblePosition(PositionForContentsPointRespectingEditingBoundary(
+          extent_point, GetFrame()));
   MoveRangeSelectionInternal(
       SelectionInDOMTree::Builder()
           .SetBaseAndExtentDeprecated(base_position.DeepEquivalent(),

third_party/blink/renderer/core/editing/granularity_strategy.cc

@@ -72,8 +72,8 @@ void CharacterGranularityStrategy::Clear() {}
 SelectionInDOMTree CharacterGranularityStrategy::UpdateExtent(
     const IntPoint& extent_point,
     LocalFrame* frame) {
-  const VisiblePosition& extent_position =
-      VisiblePositionForContentsPoint(extent_point, frame);
+  const VisiblePosition& extent_position = CreateVisiblePosition(
+      PositionForContentsPointRespectingEditingBoundary(extent_point, frame));
   const VisibleSelection& selection =
       frame->Selection().ComputeVisibleSelectionInDOMTree();
   if (extent_position.IsNull() || selection.VisibleBase().DeepEquivalent() ==
@@ -133,7 +133,8 @@ SelectionInDOMTree DirectionGranularityStrategy::UpdateExtent(
   }
 
   VisiblePosition new_offset_extent_position =
-      VisiblePositionForContentsPoint(new_offset_extent_point, frame);
+      CreateVisiblePosition(PositionForContentsPointRespectingEditingBoundary(
+          new_offset_extent_point, frame));
   if (new_offset_extent_position.IsNull())
     return selection.AsSelection();
   IntPoint new_offset_location = PositionLocation(new_offset_extent_position);
@@ -145,8 +146,8 @@ SelectionInDOMTree DirectionGranularityStrategy::UpdateExtent(
     offset_ = 0;
     granularity_ = TextGranularity::kCharacter;
     new_offset_extent_point = extent_point;
-    new_offset_extent_position =
-        VisiblePositionForContentsPoint(extent_point, frame);
+    new_offset_extent_position = CreateVisiblePosition(
+        PositionForContentsPointRespectingEditingBoundary(extent_point, frame));
   }
 
   const VisiblePosition base = selection.VisibleBase();

third_party/blink/renderer/core/editing/granularity_strategy_test.cc

@@ -720,11 +720,13 @@ TEST_F(GranularityStrategyTest, UpdateExtentWithNullPositionForCharacter) {
                                .SetIsDirectional(true)
                                .Build());
 
-  // Since, it is not obvious that |visiblePositionForContentsPoint()| returns
-  // null position, we verify here.
-  ASSERT_EQ(Position(),
-            VisiblePositionForContentsPoint(IntPoint(0, 0), &GetFrame())
-                .DeepEquivalent())
+  // Since, it is not obvious that
+  // |PositionForContentsPointRespectingEditingBoundary()| returns null
+  // position, we verify here.
+  ASSERT_EQ(Position(), CreateVisiblePosition(
+                            PositionForContentsPointRespectingEditingBoundary(
+                                IntPoint(0, 0), &GetFrame()))
+                            .DeepEquivalent())
       << "This test requires null position.";
 
   // Point to RANGE inside shadow root to get null position from
@@ -756,11 +758,13 @@ TEST_F(GranularityStrategyTest, UpdateExtentWithNullPositionForDirectional) {
                                .SetIsDirectional(true)
                                .Build());
 
-  // Since, it is not obvious that |visiblePositionForContentsPoint()| returns
-  // null position, we verify here.
-  ASSERT_EQ(Position(),
-            VisiblePositionForContentsPoint(IntPoint(0, 0), &GetFrame())
-                .DeepEquivalent())
+  // Since, it is not obvious that
+  // |PositionForContentsPointRespectingEditingBoundary()| returns null
+  // position, we verify here.
+  ASSERT_EQ(Position(), CreateVisiblePosition(
+                            PositionForContentsPointRespectingEditingBoundary(
+                                IntPoint(0, 0), &GetFrame()))
+                            .DeepEquivalent())
       << "This test requires null position.";
 
   // Point to RANGE inside shadow root to get null position from

third_party/blink/renderer/core/editing/visible_units.cc

@@ -564,8 +564,9 @@ bool HasRenderedNonAnonymousDescendantsWithHeight(
   return false;
 }
 
-VisiblePosition VisiblePositionForContentsPoint(const IntPoint& contents_point,
-                                                LocalFrame* frame) {
+PositionWithAffinity PositionForContentsPointRespectingEditingBoundary(
+    const IntPoint& contents_point,
+    LocalFrame* frame) {
   HitTestRequest request = HitTestRequest::kMove | HitTestRequest::kReadOnly |
                            HitTestRequest::kActive |
                            HitTestRequest::kIgnoreClipping;
@@ -574,11 +575,11 @@ VisiblePosition VisiblePositionForContentsPoint(const IntPoint& contents_point,
   frame->GetDocument()->GetLayoutView()->HitTest(location, result);
 
   if (Node* node = result.InnerNode()) {
-    return CreateVisiblePosition(PositionRespectingEditingBoundary(
+    return PositionRespectingEditingBoundary(
         frame->Selection().ComputeVisibleSelectionInDOMTreeDeprecated().Start(),
-        result.LocalPoint(), node));
+        result.LocalPoint(), node);
   }
-  return VisiblePosition();
+  return PositionWithAffinity();
 }
 
 // TODO(yosin): We should use |AssociatedLayoutObjectOf()| in "visible_units.cc"

third_party/blink/renderer/core/editing/visible_units.h

@@ -245,10 +245,11 @@ CORE_EXPORT bool IsEndOfEditableOrNonEditableContent(
 
 bool HasRenderedNonAnonymousDescendantsWithHeight(const LayoutObject*);
 
-// Returns a hit-tested VisiblePosition for the given point in contents-space
-// coordinates.
-CORE_EXPORT VisiblePosition VisiblePositionForContentsPoint(const IntPoint&,
-                                                            LocalFrame*);
+// TODO(editing-dev): We should move this functions to somewhere else.
+// Returns a hit-tested PositionWithAffinity for the given point in
+// contents-space coordinates.
+CORE_EXPORT PositionWithAffinity
+PositionForContentsPointRespectingEditingBoundary(const IntPoint&, LocalFrame*);
 
 CORE_EXPORT bool RendersInDifferentPosition(const Position&, const Position&);
 

third_party/blink/renderer/core/frame/web_local_frame_impl.cc

@@ -2431,10 +2431,10 @@ void WebLocalFrameImpl::ExtractSmartClipData(WebRect rect_in_viewport,
 static String CreateMarkupInRect(LocalFrame* frame,
                                  const IntPoint& start_point,
                                  const IntPoint& end_point) {
-  VisiblePosition start_visible_position =
-      VisiblePositionForContentsPoint(start_point, frame);
-  VisiblePosition end_visible_position =
-      VisiblePositionForContentsPoint(end_point, frame);
+  VisiblePosition start_visible_position = CreateVisiblePosition(
+      PositionForContentsPointRespectingEditingBoundary(start_point, frame));
+  VisiblePosition end_visible_position = CreateVisiblePosition(
+      PositionForContentsPointRespectingEditingBoundary(end_point, frame));
 
   Position start_position = start_visible_position.DeepEquivalent();
   Position end_position = end_visible_position.DeepEquivalent();