Commit 27644ac6 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Handle text objects correctly for scroll anchoring.

Use LayoutText::LinesBoundingBox() (which knows how to deal with both NG
and legacy layout), rather than reading out from the legacy line box
tree manually.

Added a crash unit test that would have crashed with this CL, had it not been
for https://chromium-review.googlesource.com/1193868 - we used to stumble
into dead layout objects.
This CL also fixes one existing unit test.

Bug: 889449

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Icb3f75038e1504968badaedd992565c8c1cc8419
Reviewed-on: https://chromium-review.googlesource.com/1158688Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594616}
parent 5d0b572f
...@@ -77,7 +77,6 @@ crbug.com/714962 external/wpt/css/css-fonts/font-features-across-space-1.html [ ...@@ -77,7 +77,6 @@ crbug.com/714962 external/wpt/css/css-fonts/font-features-across-space-1.html [
crbug.com/714962 external/wpt/css/css-fonts/font-features-across-space-3.html [ Pass ] crbug.com/714962 external/wpt/css/css-fonts/font-features-across-space-3.html [ Pass ]
crbug.com/591099 external/wpt/css/css-position/position-sticky-writing-modes.html [ Failure ] crbug.com/591099 external/wpt/css/css-position/position-sticky-writing-modes.html [ Failure ]
crbug.com/591099 external/wpt/css/css-rhythm/ [ Skip ] crbug.com/591099 external/wpt/css/css-rhythm/ [ Skip ]
crbug.com/714962 external/wpt/css/css-scroll-anchoring/wrapped-text.html [ Failure ]
crbug.com/591099 external/wpt/css/css-shapes/shape-outside/supported-shapes/polygon/shape-outside-polygon-017.html [ Pass ] crbug.com/591099 external/wpt/css/css-shapes/shape-outside/supported-shapes/polygon/shape-outside-polygon-017.html [ Pass ]
crbug.com/845902 external/wpt/css/css-sizing/whitespace-and-break.html [ Pass ] crbug.com/845902 external/wpt/css/css-sizing/whitespace-and-break.html [ Pass ]
crbug.com/591099 external/wpt/css/css-text/line-breaking/line-breaking-009.html [ Pass ] crbug.com/591099 external/wpt/css/css-text/line-breaking/line-breaking-009.html [ Pass ]
......
...@@ -99,9 +99,7 @@ static LayoutRect RelativeBounds(const LayoutObject* layout_object, ...@@ -99,9 +99,7 @@ static LayoutRect RelativeBounds(const LayoutObject* layout_object,
local_bounds.ShiftMaxYEdgeTo(max_y); local_bounds.ShiftMaxYEdgeTo(max_y);
} }
} else if (layout_object->IsText()) { } else if (layout_object->IsText()) {
// TODO(skobes): Use first and last InlineTextBox only? local_bounds.Unite(ToLayoutText(layout_object)->LinesBoundingBox());
for (InlineTextBox* box : ToLayoutText(layout_object)->TextBoxes())
local_bounds.Unite(box->FrameRect());
} else { } else {
// Only LayoutBox and LayoutText are supported. // Only LayoutBox and LayoutText are supported.
NOTREACHED(); NOTREACHED();
......
...@@ -854,10 +854,6 @@ TEST_P(ScrollAnchorTest, RestoreAnchorFailsForInvalidSelectors) { ...@@ -854,10 +854,6 @@ TEST_P(ScrollAnchorTest, RestoreAnchorFailsForInvalidSelectors) {
// element(meaning its corresponding LayoutObject can't be the anchor object) // element(meaning its corresponding LayoutObject can't be the anchor object)
// that restoration will still succeed. // that restoration will still succeed.
TEST_P(ScrollAnchorTest, RestoreAnchorSucceedsForNonBoxNonTextElement) { TEST_P(ScrollAnchorTest, RestoreAnchorSucceedsForNonBoxNonTextElement) {
// TODO(crbug.com/889449): The test fails in LayoutNG mode.
if (RuntimeEnabledFeatures::LayoutNGEnabled())
return;
SetBodyInnerHTML( SetBodyInnerHTML(
"<style> body { height: 1000px; margin: 0; } div { height: 100px } " "<style> body { height: 1000px; margin: 0; } div { height: 100px } "
"</style>" "</style>"
...@@ -919,4 +915,21 @@ TEST_P(ScrollAnchorTest, RestoreAnchorSucceedsWithExistingAnchorObject) { ...@@ -919,4 +915,21 @@ TEST_P(ScrollAnchorTest, RestoreAnchorSucceedsWithExistingAnchorObject) {
EXPECT_TRUE(GetScrollAnchor(LayoutViewport()).AnchorObject()); EXPECT_TRUE(GetScrollAnchor(LayoutViewport()).AnchorObject());
EXPECT_EQ(LayoutViewport()->ScrollOffsetInt().Height(), 0); EXPECT_EQ(LayoutViewport()->ScrollOffsetInt().Height(), 0);
} }
TEST_P(ScrollAnchorTest, DeleteAnonymousBlockCrash) {
SetBodyInnerHTML(R"HTML(
<div>
<div id="deleteMe" style="height:20000px;"></div>
torsk
</div>
)HTML");
// Removing #deleteMe will also remove the anonymous block around the text
// node. This would cause NG to point to dead layout objects, prior to
// https://chromium-review.googlesource.com/1193868 and therefore crash.
ScrollLayoutViewport(ScrollOffset(0, 20000));
GetDocument().getElementById("deleteMe")->remove();
Update();
}
} }
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