|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Xianzhu Wang <wangxianzhu@chromium.org> |
| 3 | +Date: Mon, 16 Nov 2020 17:26:33 +0000 |
| 4 | +Subject: Ensure change type for OverflowControlsClip is returned |
| 5 | + |
| 6 | +This at least ensures that we will update the paint properites for the |
| 7 | +composited overflow control layers in pre-CompositeAfterPaint to avoid |
| 8 | +stale properties on the layers. |
| 9 | + |
| 10 | +The test doesn't actually reproduce the bug because any test simpler |
| 11 | +than the bug case couldn't reproduce the bug as the update would be |
| 12 | +triggered in other code paths (any style change, layout change, etc.). |
| 13 | + |
| 14 | +Anyway this CL does fix the bug case. |
| 15 | + |
| 16 | +TBR=wangxianzhu@chromium.org |
| 17 | + |
| 18 | +(cherry picked from commit c20bb9897ef6d26a46391a4dc1658c5d33e0c100) |
| 19 | + |
| 20 | +(cherry picked from commit cfb81e677a508871f56d8bec958d0b585298ae0c) |
| 21 | + |
| 22 | +Bug: 1137603 |
| 23 | +Change-Id: I5cca970bcf8cda6085527f79a97f269c4e3e9986 |
| 24 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500264 |
| 25 | +Reviewed-by: Stefan Zager <szager@chromium.org> |
| 26 | +Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> |
| 27 | +Cr-Original-Original-Commit-Position: refs/heads/master@{#820986} |
| 28 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536910 |
| 29 | +Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> |
| 30 | +Cr-Original-Commit-Position: refs/branch-heads/4240@{#1446} |
| 31 | +Cr-Original-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218} |
| 32 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2540592 |
| 33 | +Reviewed-by: Victor-Gabriel Savu <vsavu@google.com> |
| 34 | +Commit-Queue: Jana Grill <janagrill@chromium.org> |
| 35 | +Cr-Commit-Position: refs/branch-heads/4240_112@{#26} |
| 36 | +Cr-Branched-From: 427c00d3874b6abcf4c4c2719768835fc3ef26d6-refs/branch-heads/4240@{#1291} |
| 37 | +Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218} |
| 38 | + |
| 39 | +diff --git a/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater_test.cc b/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater_test.cc |
| 40 | +index b7946eb567463938066b54ecd54ca710f649380e..f7ff1d68e36840c8647b863943f8c9134233c8ee 100644 |
| 41 | +--- a/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater_test.cc |
| 42 | ++++ b/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater_test.cc |
| 43 | +@@ -174,4 +174,56 @@ TEST_F(CompositingLayerPropertyUpdaterTest, |
| 44 | + } |
| 45 | + } |
| 46 | + |
| 47 | ++TEST_F(CompositingLayerPropertyUpdaterTest, OverflowControlsClip) { |
| 48 | ++ SetBodyInnerHTML(R"HTML( |
| 49 | ++ <style> |
| 50 | ++ ::-webkit-scrollbar { width: 20px; } |
| 51 | ++ #container { |
| 52 | ++ width: 5px; |
| 53 | ++ height: 100px; |
| 54 | ++ } |
| 55 | ++ #target { |
| 56 | ++ overflow: scroll; |
| 57 | ++ will-change: transform; |
| 58 | ++ width: 100%; |
| 59 | ++ height: 100%; |
| 60 | ++ } |
| 61 | ++ </style> |
| 62 | ++ <div id="container"> |
| 63 | ++ <div id="target"></div> |
| 64 | ++ </div> |
| 65 | ++ )HTML"); |
| 66 | ++ |
| 67 | ++ // Initially the vertical scrollbar overflows the narrow border box. |
| 68 | ++ auto* container = GetDocument().getElementById("container"); |
| 69 | ++ auto* target = ToLayoutBox(GetLayoutObjectByElementId("target")); |
| 70 | ++ auto* scrollbar_layer = |
| 71 | ++ target->GetScrollableArea()->GraphicsLayerForVerticalScrollbar(); |
| 72 | ++ auto target_state = target->FirstFragment().LocalBorderBoxProperties(); |
| 73 | ++ auto scrollbar_state = target_state; |
| 74 | ++ auto* overflow_controls_clip = |
| 75 | ++ target->FirstFragment().PaintProperties()->OverflowControlsClip(); |
| 76 | ++ ASSERT_TRUE(overflow_controls_clip); |
| 77 | ++ scrollbar_state.SetClip(*overflow_controls_clip); |
| 78 | ++ EXPECT_EQ(scrollbar_state, scrollbar_layer->GetPropertyTreeState()); |
| 79 | ++ |
| 80 | ++ // Widen target to make the vertical scrollbar contained by the border box. |
| 81 | ++ container->setAttribute(html_names::kStyleAttr, "width: 100px"); |
| 82 | ++ UpdateAllLifecyclePhasesForTest(); |
| 83 | ++ LOG(ERROR) << target->Size(); |
| 84 | ++ EXPECT_FALSE( |
| 85 | ++ target->FirstFragment().PaintProperties()->OverflowControlsClip()); |
| 86 | ++ EXPECT_EQ(target_state, scrollbar_layer->GetPropertyTreeState()); |
| 87 | ++ |
| 88 | ++ // Narrow down target back. |
| 89 | ++ container->removeAttribute(html_names::kStyleAttr); |
| 90 | ++ UpdateAllLifecyclePhasesForTest(); |
| 91 | ++ scrollbar_state = target_state; |
| 92 | ++ overflow_controls_clip = |
| 93 | ++ target->FirstFragment().PaintProperties()->OverflowControlsClip(); |
| 94 | ++ ASSERT_TRUE(overflow_controls_clip); |
| 95 | ++ scrollbar_state.SetClip(*overflow_controls_clip); |
| 96 | ++ EXPECT_EQ(scrollbar_state, scrollbar_layer->GetPropertyTreeState()); |
| 97 | ++} |
| 98 | ++ |
| 99 | + } // namespace blink |
| 100 | +diff --git a/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc b/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc |
| 101 | +index 6810fb3e4f7ba1c994812c3fa983009792e00cc4..7d391839432a7d11102db78ef84b6369357eb77f 100644 |
| 102 | +--- a/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc |
| 103 | ++++ b/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc |
| 104 | +@@ -1525,21 +1525,21 @@ void FragmentPaintPropertyTreeBuilder::UpdateOverflowControlsClip() { |
| 105 | + |
| 106 | + if (NeedsOverflowControlsClip()) { |
| 107 | + // Clip overflow controls to the border box rect. Not wrapped with |
| 108 | +- // OnUpdateClip() because this clip doesn't affect descendants. |
| 109 | ++ // OnUpdateClip() because this clip doesn't affect descendants. Wrap with |
| 110 | ++ // OnUpdate() to let PrePaintTreeWalk see the change. This may cause |
| 111 | ++ // unnecessary subtree update, but is not a big deal because it is rare. |
| 112 | + const auto& clip_rect = PhysicalRect(context_.current.paint_offset, |
| 113 | + ToLayoutBox(object_).Size()); |
| 114 | +- properties_->UpdateOverflowControlsClip( |
| 115 | ++ OnUpdate(properties_->UpdateOverflowControlsClip( |
| 116 | + *context_.current.clip, |
| 117 | + ClipPaintPropertyNode::State(context_.current.transform, |
| 118 | + FloatRoundedRect(FloatRect(clip_rect)), |
| 119 | +- ToSnappedClipRect(clip_rect))); |
| 120 | ++ ToSnappedClipRect(clip_rect)))); |
| 121 | + } else { |
| 122 | +- properties_->ClearOverflowControlsClip(); |
| 123 | ++ OnClear(properties_->ClearOverflowControlsClip()); |
| 124 | + } |
| 125 | + |
| 126 | +- // No need to set force_subtree_update_reasons and clip_changed because |
| 127 | +- // OverflowControlsClip applies to overflow controls only, not descendants. |
| 128 | +- // We also don't walk into custom scrollbars in PrePaintTreeWalk and |
| 129 | ++ // We don't walk into custom scrollbars in PrePaintTreeWalk because |
| 130 | + // LayoutObjects under custom scrollbars don't support paint properties. |
| 131 | + } |
| 132 | + |
0 commit comments