Commit db323146 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

Account for layout overflow when omitting overflow clip

When deciding whether an overflow clip paint property node is needed, we
previously checked if the visual overflow extended beyond the overflow clip
rect. This would miss cases when the visual overflow was empty (e.g.,
descendants with width: 0) but the content was still scrollable. This patch adds
a check for layout overflow.

While in the area, the condition for visual overflow has been refactored to make
the self-painting PaintLayer case a little clearer. The early-out for PaintLayer
descendants is required for correctness.

Bug: 918675
Change-Id: I41fe5fd71a957f196b5b8095965d4e361e535196
Reviewed-on: https://chromium-review.googlesource.com/c/1393885Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619749}
parent 031d7fa5
...@@ -1416,12 +1416,6 @@ static bool CanOmitOverflowClip(const LayoutObject& object) { ...@@ -1416,12 +1416,6 @@ static bool CanOmitOverflowClip(const LayoutObject& object) {
return false; return false;
const auto& block = ToLayoutBlock(object); const auto& block = ToLayoutBlock(object);
// This is a heuristic to avoid costly paint property subtree rebuild on
// CanOmitOverflowClip() changes, e.g. on selection. This also avoids omitting
// overflow clip when there is any self-painting descendant which is not
// covered by ContentsVisualOverflowRect().
if (block.HasLayer() && block.Layer()->FirstChild())
return false;
// Selection may overflow. // Selection may overflow.
if (block.IsSelected()) if (block.IsSelected())
return false; return false;
...@@ -1446,7 +1440,19 @@ static bool CanOmitOverflowClip(const LayoutObject& object) { ...@@ -1446,7 +1440,19 @@ static bool CanOmitOverflowClip(const LayoutObject& object) {
LayoutPoint(), kExcludeOverlayScrollbarSizeForHitTesting); LayoutPoint(), kExcludeOverlayScrollbarSizeForHitTesting);
if (clip_rect != clip_rect_excluding_overlay_scrollbars) if (clip_rect != clip_rect_excluding_overlay_scrollbars)
return false; return false;
return clip_rect.Contains(block.ContentsVisualOverflowRect());
// Visual overflow extending beyond the clip rect must be clipped.
// ContentsVisualOverflowRect() does not include self-painting descendants
// (see comment above |BoxOverflowModel|) so, as a simplification, do not
// omit the clip if there are any PaintLayer descendants.
if (block.HasLayer() && block.Layer()->FirstChild())
return false;
if (!clip_rect.Contains(block.ContentsVisualOverflowRect()))
return false;
// Content can scroll, and needs to be clipped, if the layout overflow extends
// beyond the clip rect.
return clip_rect.Contains(block.LayoutOverflowRect());
} }
void FragmentPaintPropertyTreeBuilder::UpdateOverflowClip() { void FragmentPaintPropertyTreeBuilder::UpdateOverflowClip() {
......
...@@ -1537,6 +1537,26 @@ TEST_P(PaintPropertyTreeBuilderTest, SVGForeignObjectOverflowClip) { ...@@ -1537,6 +1537,26 @@ TEST_P(PaintPropertyTreeBuilderTest, SVGForeignObjectOverflowClip) {
EXPECT_EQ(nullptr, properties2); EXPECT_EQ(nullptr, properties2);
} }
TEST_P(PaintPropertyTreeBuilderTest, OverflowClipWithEmptyVisualOverflow) {
SetBodyInnerHTML(R"HTML(
<style>
body { margin: 0 }
::-webkit-scrollbar {
width: 10px;
height: 10px;
}
</style>
<div id='container' style='width: 100px; height: 100px;
will-change: transform; overflow: scroll; background: lightblue;'>
<div id='forcescroll' style='width: 0; height: 400px;'></div>
</div>
)HTML");
const auto* clip = PaintPropertiesForElement("container")->OverflowClip();
EXPECT_NE(nullptr, clip);
EXPECT_EQ(FloatRect(0, 0, 90, 90), clip->ClipRect().Rect());
}
TEST_P(PaintPropertyTreeBuilderTest, TEST_P(PaintPropertyTreeBuilderTest,
PaintOffsetTranslationSVGHTMLBoundaryMulticol) { PaintOffsetTranslationSVGHTMLBoundaryMulticol) {
SetBodyInnerHTML(R"HTML( SetBodyInnerHTML(R"HTML(
......
...@@ -21,11 +21,6 @@ ...@@ -21,11 +21,6 @@
"object": "NGPhysicalTextFragment 'Should have green background'", "object": "NGPhysicalTextFragment 'Should have green background'",
"rect": [8, 8, 197, 20], "rect": [8, 8, 197, 20],
"reason": "selection" "reason": "selection"
},
{
"object": "LayoutNGBlockFlow DIV id='t'",
"rect": [8, 27, 197, 1],
"reason": "incremental"
} }
] ]
} }
......
...@@ -18,14 +18,9 @@ ...@@ -18,14 +18,9 @@
"backgroundColor": "#FFFFFF", "backgroundColor": "#FFFFFF",
"paintInvalidations": [ "paintInvalidations": [
{ {
"object": "InlineTextBox '\n '", "object": "LayoutBlockFlow DIV id='dv'",
"rect": [8, 74, 80, 19], "rect": [8, 74, 80, 19],
"reason": "disappeared" "reason": "chunk disappeared"
},
{
"object": "InlineTextBox 'Lorem ipsum'",
"rect": [8, 74, 80, 19],
"reason": "disappeared"
}, },
{ {
"object": "InlineTextBox 'Lorem ipsu'", "object": "InlineTextBox 'Lorem ipsu'",
......
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