Commit 6b253b35 authored by chaopeng's avatar chaopeng Committed by Commit Bot

[blink-gen-property-trees] Always add scroll node for layout view

The layout test crash because:

1. We always create scrollbar layers even the outer viewport does not scrollable
2. Outer viewport does not scrollable then we don't create scroll node in
   NeedsScrollNode.
3. cc gen property trees alway create scroll node because it is scrollable in
   cc side (RequiresCompositingForRootScroller).

In this patch, we always add scroll node for layout view. After this CL, some
tests in virtual/android/fullscreen still failure but not crash, devtools mobile
emulator still other crash.

Bug: 865560
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I21a54cf7739055832b4634e28b850bf63543d9ed
Reviewed-on: https://chromium-review.googlesource.com/1156002
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584169}
parent 9208ac4b
...@@ -4978,3 +4978,5 @@ crbug.com/869726 [ Mac ] virtual/scroll_customization/fast/events/touch/touch-la ...@@ -4978,3 +4978,5 @@ crbug.com/869726 [ Mac ] virtual/scroll_customization/fast/events/touch/touch-la
# Test frequently times out on Mac CQ bots. # Test frequently times out on Mac CQ bots.
crbug.com/874703 [ Mac ] http/tests/devtools/extensions/extensions-panel.js [ Timeout Pass ] crbug.com/874703 [ Mac ] http/tests/devtools/extensions/extensions-panel.js [ Timeout Pass ]
# This won't pass until the bug fixed.
crbug.com/875287 fast/frames/crash-frameset-CSS-content-property.html [ Crash ]
\ No newline at end of file
...@@ -256,7 +256,11 @@ bool CompositingReasonFinder::RequiresCompositingForRootScroller( ...@@ -256,7 +256,11 @@ bool CompositingReasonFinder::RequiresCompositingForRootScroller(
const PaintLayer& layer) { const PaintLayer& layer) {
// The root scroller needs composited scrolling layers even if it doesn't // The root scroller needs composited scrolling layers even if it doesn't
// actually have scrolling since CC has these assumptions baked in for the // actually have scrolling since CC has these assumptions baked in for the
// viewport. // viewport. Because this is only needed for CC, we can skip it if compositing
// is not enabled.
const auto& settings = *layer.GetLayoutObject().GetDocument().GetSettings();
if (!settings.GetAcceleratedCompositingEnabled())
return false;
return RootScrollerUtil::IsGlobal(layer); return RootScrollerUtil::IsGlobal(layer);
} }
......
...@@ -98,7 +98,12 @@ TEST_P(PaintControllerPaintTest, ChunkIdClientCacheFlag) { ...@@ -98,7 +98,12 @@ TEST_P(PaintControllerPaintTest, ChunkIdClientCacheFlag) {
// Verify that the background does not scroll. // Verify that the background does not scroll.
const PaintChunk& background_chunk = RootPaintController().PaintChunks()[0]; const PaintChunk& background_chunk = RootPaintController().PaintChunks()[0];
auto* transform = background_chunk.properties.Transform(); auto* transform = background_chunk.properties.Transform();
EXPECT_EQ(nullptr, transform->ScrollNode()); // TODO(crbug.com/732611): SPv2 invalidations are incorrect if there is
// scrolling.
if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
EXPECT_FALSE(transform->ScrollNode());
else
EXPECT_TRUE(transform->ScrollNode());
const EffectPaintPropertyNode* effect_node = const EffectPaintPropertyNode* effect_node =
div.FirstFragment().PaintProperties()->Effect(); div.FirstFragment().PaintProperties()->Effect();
......
...@@ -187,23 +187,39 @@ class FragmentPaintPropertyTreeBuilder { ...@@ -187,23 +187,39 @@ class FragmentPaintPropertyTreeBuilder {
bool property_added_or_removed_ = false; bool property_added_or_removed_ = false;
}; };
static bool IsRootScroller(const LayoutBox& box) {
auto* scrollable_area = box.GetScrollableArea();
DCHECK(scrollable_area);
auto* layer = scrollable_area->Layer();
return layer &&
CompositingReasonFinder::RequiresCompositingForRootScroller(*layer);
}
static bool HasScrollsOverflow(const LayoutBox& box) {
// TODO(crbug.com/839341): Remove ScrollTimeline check once we support
// main-thread AnimationWorklet and don't need to promote the scroll-source.
return box.GetScrollableArea()->ScrollsOverflow() ||
ScrollTimeline::HasActiveScrollTimeline(box.GetNode());
}
static bool NeedsScrollNode(const LayoutObject& object) { static bool NeedsScrollNode(const LayoutObject& object) {
if (!object.HasOverflowClip()) if (!object.HasOverflowClip())
return false; return false;
// TODO(crbug.com/839341): Remove ScrollTimeline check once we support const LayoutBox& box = ToLayoutBox(object);
// main-thread AnimationWorklet and don't need to promote the scroll-source. // TODO(pdr): SPV2 has invalidation issues (crbug.com/732611) as well as
return ToLayoutBox(object).GetScrollableArea()->ScrollsOverflow() || // subpixel issues (crbug.com/693741) which prevent us from compositing the
ScrollTimeline::HasActiveScrollTimeline(object.GetNode()); // root scroller.
if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
return HasScrollsOverflow(box);
return HasScrollsOverflow(box) || IsRootScroller(box);
} }
static CompositingReasons CompositingReasonsForScroll(const LayoutBox& box) { static CompositingReasons CompositingReasonsForScroll(const LayoutBox& box) {
CompositingReasons compositing_reasons = CompositingReason::kNone; CompositingReasons compositing_reasons = CompositingReason::kNone;
if (auto* scrollable_area = box.GetScrollableArea()) {
if (auto* layer = scrollable_area->Layer()) { if (IsRootScroller(box))
if (CompositingReasonFinder::RequiresCompositingForRootScroller(*layer))
compositing_reasons |= CompositingReason::kRootScroller; compositing_reasons |= CompositingReason::kRootScroller;
}
}
// TODO(pdr): Set other compositing reasons for scroll here, see: // TODO(pdr): Set other compositing reasons for scroll here, see:
// PaintLayerScrollableArea::ComputeNeedsCompositedScrolling. // PaintLayerScrollableArea::ComputeNeedsCompositedScrolling.
return compositing_reasons; return compositing_reasons;
...@@ -1449,10 +1465,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation() { ...@@ -1449,10 +1465,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation() {
if (properties_->Scroll()) if (properties_->Scroll())
context_.current.scroll = properties_->Scroll(); context_.current.scroll = properties_->Scroll();
if (properties_->ScrollTranslation()) { if (properties_->ScrollTranslation())
context_.current.transform = properties_->ScrollTranslation(); context_.current.transform = properties_->ScrollTranslation();
context_.current.should_flatten_inherited_transform = false;
}
} }
void FragmentPaintPropertyTreeBuilder::UpdateOutOfFlowContext() { void FragmentPaintPropertyTreeBuilder::UpdateOutOfFlowContext() {
......
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