Commit 6dfcf903 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Ensure update of scrollbar and caret visual rects on ancestor clip change

When an ancestor clip changes, we set kSubtreeVisualRectUpdate to update
descendant visual rects which may be affected by the clip change.
Previously we early returned in PaintInvalidator::InvalidatePaint() for
descendants that had only kSubtreeVisualRectUpdate flag set. This caused
other visual rects (of e.g. scrollbars, carets) that are updated during
LayoutObject::InvalidatePaint() not updated.

Now move the early return from PaintInvalidator::InvalidatePaint() into
ObjectPaintInvalidator::ComputePaintInvalidationReason() to ensure the
visual rects are updated.

Bug: 727155
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I28f92281363c693b16121f48d21ab4f808990f14
Reviewed-on: https://chromium-review.googlesource.com/719716
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509648}
parent c7c9a1c6
......@@ -903,6 +903,7 @@ Bug(none) paint/invalidation/overflow-changed-on-child-of-composited-layer.html
Bug(none) paint/invalidation/overflow-hidden-yet-scrolled-with-custom-scrollbar.html [ Failure ]
Bug(none) paint/invalidation/overflow-hidden-yet-scrolled.html [ Failure ]
Bug(none) paint/invalidation/paint-invalidation-with-reparent-across-frame-boundaries.html [ Failure ]
Bug(none) paint/invalidation/scrollbar-ancestor-clip-change.html [ Failure ]
Bug(none) paint/invalidation/svg/resize-svg-invalidate-children-2.html [ Failure ]
Bug(none) paint/invalidation/svg/resize-svg-invalidate-children.html [ Failure ]
......
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"paintInvalidations": [
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 95, 112],
"reason": "paint property change"
},
{
"object": "LayoutTextControl INPUT id='target'",
"rect": [8, 8, 95, 112],
"reason": "paint property change"
},
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 95, 50],
"reason": "paint property change"
},
{
"object": "LayoutTextControl INPUT id='target'",
"rect": [8, 8, 95, 50],
"reason": "paint property change"
}
]
}
]
}
<!DOCTYPE html>
<script>
onload = function() { target.focus(); }
</script>
<div style="width: 200px; height: 300px; overflow: hidden">
<input id="target" style="font-size: 100px; width: 100px; margin-left: -10px; margin-top: -10px">
</div>
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"paintInvalidations": [
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 200, 300],
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 200, 300],
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV id='clip'",
"rect": [8, 58, 200, 250],
"reason": "incremental"
}
]
}
],
"objectPaintInvalidations": [
{
"object": "LayoutBlockFlow DIV id='clip'",
"reason": "incremental"
},
{
"object": "LayoutBlockFlow DIV",
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV",
"reason": "geometry"
},
{
"object": "RootInlineBox",
"reason": "geometry"
}
]
}
<!DOCTYPE html>
<script src="resources/text-based-repaint.js"></script>
<script>
function repaintTest() {
clip.style.height = '300px';
}
onload = function() {
target.focus();
runRepaintAndPixelTest();
}
</script>
<div id="clip" style="width: 200px; height: 50px; overflow: hidden">
<!-- These two divs isolate layout of |target| from |clip|, so that we won't
layout |target| on |clip|'s height change. -->
<div style="width: 300px; height: 300px; overflow: hidden">
<div style="width: 300px; height: 300px; overflow: hidden">
<!-- Use negative margins to avoid scroll on focus which would cause extra invalidations. -->
<input id="target" style="font-size: 100px; width: 100px; margin-left: -10px; margin-top: -10px">
</div>
</div>
</div>
<!DOCTYPE html>
<div style="width: 100px; height: 200px; overflow: scroll">
<div style="width: 400px; height: 400px"></div>
</div>
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"paintInvalidations": [
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 100, 300],
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV",
"rect": [8, 8, 100, 300],
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV id='clip'",
"rect": [8, 108, 100, 200],
"reason": "incremental"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"rect": [8, 193, 85, 15],
"reason": "scroll control"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"rect": [93, 8, 15, 185],
"reason": "scroll control"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"rect": [93, 8, 15, 100],
"reason": "scroll control"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"rect": [93, 193, 15, 15],
"reason": "scroll control"
}
]
}
],
"objectPaintInvalidations": [
{
"object": "LayoutBlockFlow DIV id='clip'",
"reason": "incremental"
},
{
"object": "LayoutBlockFlow DIV",
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV",
"reason": "geometry"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"reason": "geometry"
},
{
"object": "HorizontalScrollbar",
"reason": "scroll control"
},
{
"object": "LayoutBlockFlow DIV id='target'",
"reason": "geometry"
},
{
"object": "VerticalScrollbar",
"reason": "scroll control"
}
]
}
<!DOCTYPE html>
<script src="resources/text-based-repaint.js"></script>
<script>
function repaintTest() {
clip.style.height = '300px';
}
onload = runRepaintAndPixelTest;
</script>
<div id="clip" style="width: 100px; height: 100px; overflow: hidden">
<!-- These two divs isolate layout of |target| from |clip|, so that we won't
layout |target| on |clip|'s height change. -->
<div style="width: 300px; height: 300px; overflow: hidden">
<div style="width: 300px; height: 300px; overflow: hidden">
<div id="target" style="width: 100px; height: 200px; overflow: scroll">
<div style="width: 400px; height: 400px"></div>
</div>
</div>
</div>
</div>
......@@ -189,6 +189,7 @@ TEST_P(BoxPaintInvalidatorTest, ComputePaintInvalidationReasonBasic) {
target.setAttribute(HTMLNames::styleAttr, "background: blue");
GetDocument().View()->UpdateAllLifecyclePhases();
box.SetMayNeedPaintInvalidation();
LayoutRect visual_rect = box.VisualRect();
EXPECT_EQ(LayoutRect(0, 0, 50, 100), visual_rect);
......
......@@ -442,6 +442,16 @@ ObjectPaintInvalidatorWithContext::ComputePaintInvalidationReason() {
background_obscuration_changed = true;
}
if (!object_.ShouldCheckForPaintInvalidation() &&
(!context_.subtree_flags ||
context_.subtree_flags ==
PaintInvalidatorContext::kSubtreeVisualRectUpdate)) {
// No paint invalidation flag, or just kSubtreeVisualRectUpdate (which has
// been handled in PaintInvalidator). No paint invalidation is needed.
DCHECK(!background_obscuration_changed);
return PaintInvalidationReason::kNone;
}
if (context_.subtree_flags &
PaintInvalidatorContext::kSubtreeFullInvalidation)
return PaintInvalidationReason::kSubtree;
......
......@@ -504,14 +504,6 @@ void PaintInvalidator::InvalidatePaint(
UpdateEmptyVisualRectFlag(object, context);
UpdateVisualRectIfNeeded(object, tree_builder_context, context);
if (!object.ShouldCheckForPaintInvalidation() &&
!(context.subtree_flags &
~PaintInvalidatorContext::kSubtreeVisualRectUpdate)) {
// We are done updating anything needed. No other paint invalidation work to
// do for this object.
return;
}
PaintInvalidationReason reason = object.InvalidatePaint(context);
switch (reason) {
case PaintInvalidationReason::kDelayedFull:
......
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