Commit 1f8c10c5 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Chromium LUCI CQ

Revert "Enable LayoutNGFullPositionForPoint Blink runtime feature."

This reverts commit b2cf6bb4.

Reason for revert: We are worried about use-after-free bugs (long-lived HitTestResult keeping a NGPhysicalBoxFragment reference, which may point to a dead LayoutObject) with this solution; see crbug.com/1152696

Original change's description:
> Enable LayoutNGFullPositionForPoint Blink runtime feature.
>
> Had to change some tests because of this:
>
> WebFrameTest.SmartClipData and
> WebFrameTest.SmartClipDataWithPinchZoom:
> Force Android editing behavior, or else it would fail on Mac and UNIX.
> CL:2552591 copied behavior from LayoutBlock::PositionForPoint(), where
> we'd consider non-hit-test candidates if there were no actual hit-test
> candidates. We now do this consistently, while in legacy layout we'll
> only do it for block children. The test here has only floated children
> (which means that ChildrenInline() will return true). (Adding an empty
> regular block next to the floats would change the behavior completely
> (since we'd then get the "block children behavior"), which is bogus).
> Since the point is below the float, on Mac and UNIX the start offset
> will now be at the end position inside the float that's just above the
> rectangle (because the caret is moved to the element boundary), rather
> than including the entire (parent) container. On Android and Windows
> this doesn't happen (since the caret isn't moved to the element
> boundary, and the caret happens to be exactly at offset 0, because of
> the x position of the point).
>
> editing/selection/click-on-block-image.html:
> Since NG PositionForPoint() finds what actually is the closest box
> (rather than doing it inaccurately in some cases, like legacy), we'll
> hit the line boxes in the test more than before. Test a bit further down
> to keep the test passing. Note that the manual version of the test
> passes anyway (i.e. the caret is placed on the correct side of the
> image, even when we hit the line boxes instead of the image).
>
> editing/selection/continuations-with-move-caret-to-boundary.html:
> Again, we now find the box that's actually the closest one, so aim
> better.
>
> editing/selection/offset-from-point.html:
> Remove workaround for vertical writing modes, since it would make us
> fail now.
>
> fast/writing-mode/positionForPoint.html:
> We now hit what's actually the closest box. This test is part of
> https://bugs.webkit.org/show_bug.cgi?id=92202 , and there's nothing that
> suggests that we depend on the old behavior.
>
> Bug: 1150362, 829028
> Change-Id: I733b154ef473ab3d8064554213dde2e9b948e040
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2553547
> Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
> Reviewed-by: Koji Ishii <kojii@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#831049}

TBR=yosin@chromium.org,kojii@chromium.org,ikilpatrick@chromium.org,mstensho@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1150362
Bug: 829028
Change-Id: I6a6fc96c11acd379a01060f105f861e70f320ea2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584855Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836235}
parent bb97ab9b
...@@ -5841,8 +5841,6 @@ TEST_F(WebFrameTest, SmartClipData) { ...@@ -5841,8 +5841,6 @@ TEST_F(WebFrameTest, SmartClipData) {
WebLocalFrame* frame = web_view_helper.LocalMainFrame(); WebLocalFrame* frame = web_view_helper.LocalMainFrame();
web_view_helper.Resize(gfx::Size(500, 500)); web_view_helper.Resize(gfx::Size(500, 500));
UpdateAllLifecyclePhases(web_view_helper.GetWebView()); UpdateAllLifecyclePhases(web_view_helper.GetWebView());
web_view_helper.GetWebView()->GetSettings()->SetEditingBehavior(
mojom::EditingBehavior::kEditingAndroidBehavior);
WebRect crop_rect(300, 125, 152, 50); WebRect crop_rect(300, 125, 152, 50);
frame->ExtractSmartClipData(crop_rect, clip_text, clip_html, clip_rect); frame->ExtractSmartClipData(crop_rect, clip_text, clip_html, clip_rect);
EXPECT_EQ(kExpectedClipText, clip_text); EXPECT_EQ(kExpectedClipText, clip_text);
...@@ -5880,8 +5878,6 @@ TEST_F(WebFrameTest, SmartClipDataWithPinchZoom) { ...@@ -5880,8 +5878,6 @@ TEST_F(WebFrameTest, SmartClipDataWithPinchZoom) {
WebLocalFrame* frame = web_view_helper.LocalMainFrame(); WebLocalFrame* frame = web_view_helper.LocalMainFrame();
web_view_helper.Resize(gfx::Size(500, 500)); web_view_helper.Resize(gfx::Size(500, 500));
UpdateAllLifecyclePhases(web_view_helper.GetWebView()); UpdateAllLifecyclePhases(web_view_helper.GetWebView());
web_view_helper.GetWebView()->GetSettings()->SetEditingBehavior(
mojom::EditingBehavior::kEditingAndroidBehavior);
web_view_helper.GetWebView()->SetPageScaleFactor(1.5); web_view_helper.GetWebView()->SetPageScaleFactor(1.5);
web_view_helper.GetWebView()->SetVisualViewportOffset(gfx::PointF(167, 100)); web_view_helper.GetWebView()->SetVisualViewportOffset(gfx::PointF(167, 100));
WebRect crop_rect(200, 38, 228, 75); WebRect crop_rect(200, 38, 228, 75);
......
...@@ -1104,7 +1104,6 @@ ...@@ -1104,7 +1104,6 @@
// enabled, we'll typically fall back to legacy code for block children. // enabled, we'll typically fall back to legacy code for block children.
name: "LayoutNGFullPositionForPoint", name: "LayoutNGFullPositionForPoint",
implied_by: ["LayoutNGBlockFragmentation", "LayoutNGFragmentTraversal"], implied_by: ["LayoutNGBlockFragmentation", "LayoutNGFragmentTraversal"],
status: "stable",
}, },
{ {
name: "LayoutNGGrid", name: "LayoutNGGrid",
......
...@@ -41,8 +41,8 @@ function automaticTest() { ...@@ -41,8 +41,8 @@ function automaticTest() {
var elem; var elem;
elem = document.getElementById("myimage"); elem = document.getElementById("myimage");
runTest(elem.offsetLeft - 10, elem.offsetTop + 20, document.getElementById("test"), 1); runTest(elem.offsetLeft - 10, elem.offsetTop + 10, document.getElementById("test"), 1);
runTest(elem.offsetLeft + elem.offsetWidth + 10, elem.offsetTop + 20, document.getElementById("test"), 2); runTest(elem.offsetLeft + elem.offsetWidth + 10, elem.offsetTop + 10, document.getElementById("test"), 2);
} }
} }
......
...@@ -30,7 +30,7 @@ selection_test( ...@@ -30,7 +30,7 @@ selection_test(
selection_test( selection_test(
[kStyle, '<p>AAAAA</p><p>BBBBB</p>'], [kStyle, '<p>AAAAA</p><p>BBBBB</p>'],
selection => dragOverBlocks(selection, -5), selection => dragOverBlocks(selection, -15),
[kStyle, [kStyle,
isMac isMac
? '<p>^AAAAA</p><p>|BBBBB</p>' ? '<p>^AAAAA</p><p>|BBBBB</p>'
......
...@@ -28,6 +28,8 @@ function testOffsetFromPoint(element, name) { ...@@ -28,6 +28,8 @@ function testOffsetFromPoint(element, name) {
var isVertical = writingMode && writingMode[0] == "v"; var isVertical = writingMode && writingMode[0] == "v";
var results = [ name ]; var results = [ name ];
getOffsetFromPoint(element, isVertical, results); getOffsetFromPoint(element, isVertical, results);
if (isVertical) // The last character in vertical is flaky by win/mac/linux
results.splice(-1, 1);
for (var result of results) { for (var result of results) {
var div = document.createElement("div"); var div = document.createElement("div");
div.innerText = result; div.innerText = result;
......
...@@ -30,5 +30,5 @@ ...@@ -30,5 +30,5 @@
} }
test(1, 20, 50); test(1, 20, 50);
test(2, 40, 180); test(2, 20, 180);
</script> </script>
12 345 6789
12 345 6789
12 345 6789
12 345 6789
LTR
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
LTR (Complex Path)
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
RTL
11=-1 (-1)
8=11 (12)
9=31 (20)
10=51 (20)
7=71 (20)
6=91 (20)
4=111 (20)
5=131 (20)
3=151 (20)
2=171 (20)
1=191 (20)
0=211 (20)
RTL (Complex Path)
11=-1 (-1)
8=11 (12)
9=31 (20)
10=51 (20)
7=71 (20)
6=91 (20)
4=111 (20)
5=131 (20)
3=151 (20)
2=171 (20)
1=191 (20)
0=211 (20)
VERTICAL-LR
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
0=220 (9)
VERTICAL-LR (Complex Path)
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
0=220 (9)
VERTICAL-RL
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
1=220 (9)
VERTICAL-RL (Complex Path)
0=-1 (-1)
1=11 (12)
2=31 (20)
3=51 (20)
4=71 (20)
5=91 (20)
6=111 (20)
7=131 (20)
8=151 (20)
9=171 (20)
10=191 (20)
11=211 (20)
1=220 (9)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment