[LayoutNG] Fix selection issue when ending in legacy object

Previously HitTestResult::LocalPoint() is in pure physical coordinates
if the result is produced by LayoutNG, while is in flipped physical
coordinates produced by Legacy. We tried to use it as pure physical
in LayoutNG and flipped in Legacy in PositionForPoint, but failed when
it is passed between Legacy and LayoutNG, especially in the case when
it's passed through LayoutObject hierarchy where is unclear about the
Legacy and LayoutNG boundary.

This CL makes HitTestResult::LocalPoint() always in flipped physical
coordinates, and let LayoutObject::PositionForPoint() always accept
flipped physical coordinates (including LayoutNG objects), and flip the
point when needed.

This CL is temporary because future CLs will change
HitTestResult::LocalPoint() and LayoutObject::PositionForPoint() always
physica, but those CLs are too big to fix the bug in M76.

Bug: 966731
Change-Id: I19e0cd392d2a92bd0f5467c6fc816dfa86f94933
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1643911
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666428}

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

@@ -1313,28 +1313,30 @@ PositionWithAffinity PositionRespectingEditingBoundary(
     const Position& position,
     const LayoutPoint& local_point,
     Node* target_node) {
-  if (!target_node->GetLayoutObject())
+  const LayoutObject* target_object = target_node->GetLayoutObject();
+  if (!target_object)
     return PositionWithAffinity();
 
+  // Note that local_point is in flipped blocks direction.
   LayoutPoint selection_end_point = local_point;
   Element* editable_element = RootEditableElementOf(position);
 
   if (editable_element && !editable_element->contains(target_node)) {
-    if (!editable_element->GetLayoutObject())
+    const LayoutObject* editable_object = editable_element->GetLayoutObject();
+    if (!editable_object)
       return PositionWithAffinity();
 
     // TODO(yosin): This kIgnoreTransforms correct here?
-    PhysicalOffset absolute_point =
-        target_node->GetLayoutObject()->LocalToAbsolutePoint(
-            PhysicalOffsetToBeNoop(selection_end_point), kIgnoreTransforms);
-    selection_end_point =
-        editable_element->GetLayoutObject()
-            ->AbsoluteToLocalPoint(absolute_point, kIgnoreTransforms)
-            .ToLayoutPoint();
-    target_node = editable_element;
+    PhysicalOffset absolute_point = target_object->LocalToAbsolutePoint(
+        target_object->FlipForWritingMode(selection_end_point),
+        kIgnoreTransforms);
+    selection_end_point = editable_object->FlipForWritingMode(
+        editable_object->AbsoluteToLocalPoint(absolute_point,
+                                              kIgnoreTransforms));
+    target_object = editable_object;
   }
 
-  return target_node->GetLayoutObject()->PositionForPoint(selection_end_point);
+  return target_object->PositionForPoint(selection_end_point);
 }
 
 Position ComputePositionForNodeRemoval(const Position& position,

third_party/blink/renderer/core/layout/hit_test_result.cc

@@ -502,4 +502,10 @@ Node* HitTestResult::InnerNodeOrImageMapImage() const {
   return image_map_image_element;
 }
 
+void HitTestResult::SetNodeAndPosition(Node* node, const PhysicalOffset& p) {
+  SetNodeAndPosition(node, node && node->GetLayoutObject()
+                               ? node->GetLayoutObject()->FlipForWritingMode(p)
+                               : p.ToLayoutPoint());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/layout/hit_test_result.h

@@ -47,6 +47,7 @@ class Node;
 class LayoutObject;
 class Region;
 class Scrollbar;
+struct PhysicalOffset;
 
 // List-based hit test testing can continue even after a hit has been found.
 // This is used to support fuzzy matching with rect-based hit tests as well as
@@ -108,12 +109,14 @@ class CORE_EXPORT HitTestResult {
   }
   LocalFrame* InnerNodeFrame() const;
 
-  // The hit-tested point in the coordinates of the inner node.
+  // The hit-tested point in the coordinates of the inner node, in physical
+  // coordinates with flipped blocks direction.
   const LayoutPoint& LocalPoint() const { return local_point_; }
   void SetNodeAndPosition(Node* node, const LayoutPoint& p) {
     local_point_ = p;
     SetInnerNode(node);
   }
+  void SetNodeAndPosition(Node* node, const PhysicalOffset& p);
 
   PositionWithAffinity GetPosition() const;
   LayoutObject* GetLayoutObject() const;

third_party/blink/renderer/core/layout/layout_box.h

@@ -1320,10 +1320,6 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
     DCHECK(!IsHorizontalWritingMode());
     return frame_rect_.Width() - (position + width);
   }
-  WARN_UNUSED_RESULT LayoutPoint
-  FlipForWritingMode(const PhysicalOffset& offset) const {
-    return LayoutPoint(FlipForWritingMode(offset.left), offset.top);
-  }
   // Inherit other flipping methods from LayoutObject.
   using LayoutObject::FlipForWritingMode;
 

third_party/blink/renderer/core/layout/layout_view_test.cc

@@ -337,7 +337,7 @@ TEST_P(LayoutViewTest, HitTestVerticalRL) {
   // Top-left corner of div.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(51, 101)), result);
   EXPECT_EQ(div, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 1 : 199, 1), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(199, 1), result.LocalPoint());
   // TODO(crbug.com/966731): Verify if the difference reflects any issue.
   EXPECT_EQ(
       LayoutNGOrAndroidOrWindows()
@@ -359,21 +359,21 @@ TEST_P(LayoutViewTest, HitTestVerticalRL) {
   // Top-right corner (inside) of div and span1.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(249, 101)), result);
   EXPECT_EQ(text1, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 199 : 1, 1), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(1, 1), result.LocalPoint());
   EXPECT_EQ(PositionWithAffinity(Position(text1, 0), TextAffinity::kDownstream),
             result.GetPosition());
 
   // Bottom-right corner (inside) of span1.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(249, 149)), result);
   EXPECT_EQ(text1, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 199 : 1, 49), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(1, 49), result.LocalPoint());
   EXPECT_EQ(PositionWithAffinity(Position(text1, 5), TextAffinity::kUpstream),
             result.GetPosition());
 
   // Bottom-right corner (outside) of span1 but inside of div.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(249, 151)), result);
   EXPECT_EQ(div, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 199 : 1, 51), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(1, 51), result.LocalPoint());
   EXPECT_EQ(PositionWithAffinity(Position(text2, 0), TextAffinity::kDownstream),
             result.GetPosition());
 
@@ -387,7 +387,7 @@ TEST_P(LayoutViewTest, HitTestVerticalRL) {
   // Bottom-left corner (inside) of div.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(51, 179)), result);
   EXPECT_EQ(div, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 1 : 199, 79), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(199, 79), result.LocalPoint());
   // TODO(crbug.com/966731): Verify if the difference reflects any issue.
   EXPECT_EQ(
       LayoutNGOrAndroidOrWindows()
@@ -398,14 +398,14 @@ TEST_P(LayoutViewTest, HitTestVerticalRL) {
   // Bottom-left corner (outside) of span1.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(241, 151)), result);
   EXPECT_EQ(div, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 191 : 9, 51), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(9, 51), result.LocalPoint());
   EXPECT_EQ(PositionWithAffinity(Position(text2, 0), TextAffinity::kDownstream),
             result.GetPosition());
 
   // Top-right corner (inside) of span2.
   GetLayoutView().HitTest(HitTestLocation(LayoutPoint(219, 151)), result);
   EXPECT_EQ(text2, result.InnerNode());
-  EXPECT_EQ(LayoutPoint(LayoutNG() ? 199 : 1, 51), result.LocalPoint());
+  EXPECT_EQ(LayoutPoint(1, 51), result.LocalPoint());
   EXPECT_EQ(PositionWithAffinity(Position(text2, 0), TextAffinity::kDownstream),
             result.GetPosition());
 }

third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc

@@ -327,8 +327,10 @@ PositionWithAffinity LayoutNGMixin<Base>::PositionForPoint(
   if (!PaintFragment())
     return Base::CreatePositionWithAffinity(0);
 
+  // Flip because |point| is in flipped physical coordinates while
+  // NGPaintFragment::PositionForPoint() requires pure physical coordinates.
   const PositionWithAffinity ng_position =
-      PaintFragment()->PositionForPoint(PhysicalOffset(point));
+      PaintFragment()->PositionForPoint(Base::FlipForWritingMode(point));
   if (ng_position.IsNotNull())
     return ng_position;
   return Base::CreatePositionWithAffinity(0);

third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc

@@ -1049,7 +1049,7 @@ bool NGBoxFragmentPainter::NodeAtPoint(
       if (!result.InnerNode() && node) {
         LayoutPoint point =
             location_in_container.Point() - ToLayoutSize(physical_offset);
-        result.SetNodeAndPosition(node, point);
+        result.SetNodeAndPosition(node, PhysicalOffsetToBeNoop(point));
       }
       if (result.AddNodeToListBasedTestResult(node, location_in_container,
                                               bounds_rect) == kStopHitTesting) {
@@ -1096,7 +1096,7 @@ bool NGBoxFragmentPainter::HitTestTextFragment(
       LayoutPoint point =
           location_in_container.Point() - ToLayoutSize(physical_offset) +
           text_paint_fragment.InlineOffsetToContainerBox().ToLayoutPoint();
-      result.SetNodeAndPosition(node, point);
+      result.SetNodeAndPosition(node, PhysicalOffsetToBeNoop(point));
     }
 
     if (result.AddNodeToListBasedTestResult(node, location_in_container,
@@ -1148,7 +1148,7 @@ bool NGBoxFragmentPainter::HitTestLineBoxFragment(
     const LayoutPoint point =
         location_in_container.Point() - ToLayoutSize(physical_offset) +
         fragment.InlineOffsetToContainerBox().ToLayoutPoint();
-    result.SetNodeAndPosition(node, point);
+    result.SetNodeAndPosition(node, PhysicalOffsetToBeNoop(point));
   }
   return result.AddNodeToListBasedTestResult(node, location_in_container,
                                              bounds_rect) == kStopHitTesting;

third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc

@@ -133,9 +133,12 @@ base::Optional<PositionWithAffinity> PositionForPointInChild(
   const bool should_fallback = child.PhysicalFragment().IsBlockFlow() ||
                                child.PhysicalFragment().IsLegacyLayoutRoot();
   const PositionWithAffinity result =
-      should_fallback ? child.GetLayoutObject()->PositionForPoint(
-                            child_point.ToLayoutPoint())
-                      : child.PositionForPoint(child_point);
+      should_fallback
+          ? child.GetLayoutObject()->PositionForPoint(
+                // Flip because LayoutObject::PositionForPoint() requires
+                // flipped physical coordinates.
+                child.GetLayoutObject()->FlipForWritingMode(child_point))
+          : child.PositionForPoint(child_point);
   if (result.IsNotNull())
     return result;
   return base::nullopt;

third_party/blink/web_tests/editing/selection/mouse/mouse-selection-horizontal.html

+<!DOCTYPE html>
+<script src="../../../resources/ahem.js"></script>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../resources/mouse-selection.js"></script>
+<style>
+html {
+  writing-mode: vertical-rl;
+  font: 20px/20px Ahem;
+  word-wrap: break-word;
+}
+body {
+  margin: 0;
+}
+div {
+  height: 200px;
+  margin: 200px;
+}
+</style>
+<!-- 40 Xs, 4 lines -->
+<div id="div">XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX</div>
+<script>
+var node = div.firstChild;
+testMouseSelection(590, 310, 530, 310, node, 5, node, 35,
+                   'Middle of the first line -> Middle of the last line');
+testMouseSelection(590, 310, 550, 500, node, 5, node, 30,
+                   'Middle of the first line -> Outside below the third line');
+testMouseSelection(590, 310, 550, 100, node, 5, node, 20,
+                   'Middel of the first line -> Outside above the third line');
+testMouseSelection(590, 310, 300, 310, node, 5, node, 35,
+                   'Middle of the first line -> Outside left, vertical middle');
+testMouseSelection(590, 310, 300, 500, node, 5, node, 40,
+                   'Middle of the first line -> Outside left, below');
+testMouseSelection(530, 310, 700, 310, node, 5, node, 35,
+                   'Middle of the last line -> Outside right, vertical middle');
+testMouseSelection(530, 310, 570, 100, node, 10, node, 35,
+                   'Middle of the last line -> Outside above the second line');
+testMouseSelection(530, 310, 570, 500, node, 20, node, 35,
+                   'Middle of the last line -> Outside below the second line');
+</script>

third_party/blink/web_tests/editing/selection/mouse/mouse-selection-vertical-lr.html

+<!DOCTYPE html>
+<script src="../../../resources/ahem.js"></script>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../resources/mouse-selection.js"></script>
+<style>
+html {
+  writing-mode: vertical-lr;
+  font: 20px/20px Ahem;
+  word-wrap: break-word;
+}
+body {
+  margin: 0;
+}
+div {
+  height: 200px;
+  margin: 200px;
+}
+</style>
+<!-- 40 Xs, 4 lines -->
+<div id="div">XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX</div>
+<script>
+var node = div.firstChild;
+testMouseSelection(210, 310, 270, 310, node, 5, node, 35,
+                   'Middle of the first line -> Middle of the last line');
+testMouseSelection(210, 310, 250, 500, node, 5, node, 30,
+                   'Middle of the first line -> Outside below the third line');
+testMouseSelection(210, 310, 250, 100, node, 5, node, 20,
+                   'Middel of the first line -> Outside above the third line');
+testMouseSelection(210, 310, 600, 310, node, 5, node, 35,
+                   'Middle of the first line -> Outside right, vertical middle');
+testMouseSelection(210, 310, 600, 500, node, 5, node, 40,
+                   'Middle of the first line -> Outside right, below');
+testMouseSelection(270, 310, 100, 310, node, 5, node, 35,
+                   'Middle of the last line -> Outside left, vertical middle');
+testMouseSelection(270, 310, 230, 100, node, 10, node, 35,
+                   'Middle of the last line -> Outside above the second line');
+testMouseSelection(270, 310, 230, 500, node, 20, node, 35,
+                   'Middle of the last line -> Outside below the second line');
+</script>

third_party/blink/web_tests/editing/selection/mouse/mouse-selection-vertical-rl.html

+<!DOCTYPE html>
+<script src="../../../resources/ahem.js"></script>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../resources/mouse-selection.js"></script>
+<style>
+html {
+  writing-mode: vertical-rl;
+  font: 20px/20px Ahem;
+  word-wrap: break-word;
+}
+body {
+  margin: 0;
+}
+div {
+  height: 200px;
+  margin: 200px;
+}
+</style>
+<!-- 40 Xs, 4 lines -->
+<div id="div">XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX</div>
+<script>
+var node = div.firstChild;
+testMouseSelection(590, 310, 530, 310, node, 5, node, 35,
+                   'Middle of the first line -> Middle of the last line');
+testMouseSelection(590, 310, 550, 500, node, 5, node, 30,
+                   'Middle of the first line -> Outside below the third line');
+testMouseSelection(590, 310, 550, 100, node, 5, node, 20,
+                   'Middel of the first line -> Outside above the third line');
+testMouseSelection(590, 310, 300, 310, node, 5, node, 35,
+                   'Middle of the first line -> Outside left, vertical middle');
+testMouseSelection(590, 310, 300, 500, node, 5, node, 40,
+                   'Middle of the first line -> Outside left, below');
+testMouseSelection(530, 310, 700, 310, node, 5, node, 35,
+                   'Middle of the last line -> Outside right, vertical middle');
+testMouseSelection(530, 310, 570, 100, node, 10, node, 35,
+                   'Middle of the last line -> Outside above the second line');
+testMouseSelection(530, 310, 570, 500, node, 20, node, 35,
+                   'Middle of the last line -> Outside below the second line');
+</script>

third_party/blink/web_tests/editing/selection/resources/mouse-selection.js

+function testMouseSelectionOneDirection(
+    start_x, start_y, end_x, end_y,
+    expected_start_container, expected_start_offset,
+    expected_end_container, expected_end_offset, name) {
+  promise_test(() => {
+    return new Promise((resolve, reject) => {
+      if (!window.chrome || !chrome.gpuBenchmarking)
+        return reject();
+      window.getSelection().removeAllRanges();
+      chrome.gpuBenchmarking.pointerActionSequence(
+        [{source: "mouse",
+          actions: [
+            {name: "pointerDown", x: start_x, y: start_y},
+            {name: "pointerMove", x: end_x, y: end_y},
+            {name: "pointerUp"},
+          ],
+        }], resolve);
+    }).then(() => {
+      var selection = window.getSelection();
+      assert_equals(selection.rangeCount, 1, 'rangeCount');
+      var range = selection.getRangeAt(0);
+      assert_equals(range.startContainer, expected_start_container, 'startContainer');
+      assert_equals(range.startOffset, expected_start_offset, 'startOffset');
+      assert_equals(range.endContainer, expected_end_container, 'endContainer');
+      assert_equals(range.endOffset, expected_end_offset, 'endOffset');
+    });
+  }, name);
+}
+
+function testMouseSelection(
+    start_x, start_y, end_x, end_y,
+    expected_start_container, expected_start_offset,
+    expected_end_container, expected_end_offset, name) {
+  testMouseSelectionOneDirection(
+      start_x, start_y, end_x, end_y,
+      expected_start_container, expected_start_offset,
+      expected_end_container, expected_end_offset, name);
+  testMouseSelectionOneDirection(
+      end_x, end_y, start_x, start_y,
+      expected_start_container, expected_start_offset,
+      expected_end_container, expected_end_offset,
+      name + ' Reversed');
+}