Commit c20bb989 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Ensure change type for OverflowControlsClip is returned

This at least ensures that we will update the paint properites for the
composited overflow control layers in pre-CompositeAfterPaint to avoid
stale properties on the layers.

The test doesn't actually reproduce the bug because any test simpler
than the bug case couldn't reproduce the bug as the update would be
triggered in other code paths (any style change, layout change, etc.).

Anyway this CL does fix the bug case.

Bug: 1137603
Change-Id: I5cca970bcf8cda6085527f79a97f269c4e3e9986
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500264Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820986}
parent 99375ddd
......@@ -234,4 +234,56 @@ TEST_F(CompositingLayerPropertyUpdaterTest,
}
}
TEST_F(CompositingLayerPropertyUpdaterTest, OverflowControlsClip) {
SetBodyInnerHTML(R"HTML(
<style>
::-webkit-scrollbar { width: 20px; }
#container {
width: 5px;
height: 100px;
}
#target {
overflow: scroll;
will-change: transform;
width: 100%;
height: 100%;
}
</style>
<div id="container">
<div id="target"></div>
</div>
)HTML");
// Initially the vertical scrollbar overflows the narrow border box.
auto* container = GetDocument().getElementById("container");
auto* target = ToLayoutBox(GetLayoutObjectByElementId("target"));
auto* scrollbar_layer =
target->GetScrollableArea()->GraphicsLayerForVerticalScrollbar();
auto target_state = target->FirstFragment().LocalBorderBoxProperties();
auto scrollbar_state = target_state;
auto* overflow_controls_clip =
target->FirstFragment().PaintProperties()->OverflowControlsClip();
ASSERT_TRUE(overflow_controls_clip);
scrollbar_state.SetClip(*overflow_controls_clip);
EXPECT_EQ(scrollbar_state, scrollbar_layer->GetPropertyTreeState());
// Widen target to make the vertical scrollbar contained by the border box.
container->setAttribute(html_names::kStyleAttr, "width: 100px");
UpdateAllLifecyclePhasesForTest();
LOG(ERROR) << target->Size();
EXPECT_FALSE(
target->FirstFragment().PaintProperties()->OverflowControlsClip());
EXPECT_EQ(target_state, scrollbar_layer->GetPropertyTreeState());
// Narrow down target back.
container->removeAttribute(html_names::kStyleAttr);
UpdateAllLifecyclePhasesForTest();
scrollbar_state = target_state;
overflow_controls_clip =
target->FirstFragment().PaintProperties()->OverflowControlsClip();
ASSERT_TRUE(overflow_controls_clip);
scrollbar_state.SetClip(*overflow_controls_clip);
EXPECT_EQ(scrollbar_state, scrollbar_layer->GetPropertyTreeState());
}
} // namespace blink
......@@ -1664,21 +1664,21 @@ void FragmentPaintPropertyTreeBuilder::UpdateOverflowControlsClip() {
if (NeedsOverflowControlsClip()) {
// Clip overflow controls to the border box rect. Not wrapped with
// OnUpdateClip() because this clip doesn't affect descendants.
// OnUpdateClip() because this clip doesn't affect descendants. Wrap with
// OnUpdate() to let PrePaintTreeWalk see the change. This may cause
// unnecessary subtree update, but is not a big deal because it is rare.
const auto& clip_rect = PhysicalRect(context_.current.paint_offset,
ToLayoutBox(object_).Size());
properties_->UpdateOverflowControlsClip(
OnUpdate(properties_->UpdateOverflowControlsClip(
*context_.current.clip,
ClipPaintPropertyNode::State(context_.current.transform,
FloatRoundedRect(FloatRect(clip_rect)),
ToSnappedClipRect(clip_rect)));
ToSnappedClipRect(clip_rect))));
} else {
properties_->ClearOverflowControlsClip();
OnClear(properties_->ClearOverflowControlsClip());
}
// No need to set force_subtree_update_reasons and clip_changed because
// OverflowControlsClip applies to overflow controls only, not descendants.
// We also don't walk into custom scrollbars in PrePaintTreeWalk and
// We don't walk into custom scrollbars in PrePaintTreeWalk because
// LayoutObjects under custom scrollbars don't support paint properties.
}
......
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