Skip to content

GitLab

T
tangled
Commit 4e246b2e authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

[LayoutNG] Don't let legacy layout re-enter layout of a scrollable block node. 

We'd end up re-entering NGBlockNode::CopyFragmentDataToLayoutBox() for
the same node.

After layout of a fragment, NGBlockNode::CopyFragmentDataToLayoutBox() would
call LayoutBlock::UpdateAfterLayout(), which would call
scrollbar was toggled, it'd go ahead and call UpdateLayout() on the object,
which would take us back into the NG layout machinery, on the same node, via
Finally we'd re-enter NGBlockNode::CopyFragmentDataToLayoutBox(), causing misc
kinds of misery, such as not being able to paint the block at all. For
multicol, there'd even be a DCHECK failure "Variable fragment inline size not
supported", because the code got confused as to whether a fragment was a first
fragment or not.

LayoutBox: :UpdateAfterLayout(), which would call
PaintLayer: :UpdateSizeAndScrollingAfterLayout(), which would call
PaintLayerScrollableArea: :UpdateAfterLayout(). If this code detected that a
LayoutNGBlockFlow: :UpdateBlockLayout(), further to NGBlockNode::Layout().
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I832372223e324682bb4e38b3177dce30093df2e1
Reviewed-on: https://chromium-review.googlesource.com/824265
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524039}
parent 809aae3c
Hide whitespace changes
Inline Side-by-side
Showing with 308 additions and 6 deletions
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
View file @ 4e246b2e
......@@ -409,7 +409,6 @@ crbug.com/591099 compositing/rtl/rtl-iframe-fixed.html [ Failure ]
crbug.com/591099 compositing/rtl/rtl-iframe-relative.html [ Failure ]
crbug.com/591099 compositing/rtl/rtl-overflow-scrolling.html [ Failure ]
crbug.com/591099 compositing/rtl/rtl-relative.html [ Failure ]
crbug.com/591099 compositing/scrollbars/nested-overlay-scrollbars.html [ Crash ]
crbug.com/591099 compositing/self-painting-layers.html [ Failure ]
crbug.com/591099 compositing/shadows/shadow-drawing.html [ Failure ]
crbug.com/591099 compositing/sibling-positioning.html [ Failure ]
......@@ -10613,7 +10612,6 @@ crbug.com/591099 virtual/spv175/compositing/rtl/rtl-iframe-fixed.html [ Failure
crbug.com/591099 virtual/spv175/compositing/rtl/rtl-iframe-relative.html [ Failure ]
crbug.com/591099 virtual/spv175/compositing/rtl/rtl-overflow-scrolling.html [ Failure ]
crbug.com/591099 virtual/spv175/compositing/rtl/rtl-relative.html [ Failure ]
crbug.com/591099 virtual/spv175/compositing/scrollbars/nested-overlay-scrollbars.html [ Crash ]
crbug.com/591099 virtual/spv175/compositing/self-painting-layers.html [ Failure ]
crbug.com/591099 virtual/spv175/compositing/shadows/shadow-drawing.html [ Failure ]
crbug.com/591099 virtual/spv175/compositing/sibling-positioning.html [ Failure ]
......
third_party/WebKit/LayoutTests/fast/overflow/auto-with-block-overflow-expected.html 0 → 100644
View file @ 4e246b2e
<!DOCTYPE html>
<style>
/* Prevent auto viewport overflow, because that would trigger another
layout pass, hiding the bug. */
body { overflow:hidden; }
</style>
<p>There should be a green square below. No red, orange or yellow.</p>
<div style="width:50px; height:50px; background:green;"></div>
third_party/WebKit/LayoutTests/fast/overflow/auto-with-block-overflow.html 0 → 100644
View file @ 4e246b2e
<!DOCTYPE html>
<style>
/* Prevent auto viewport overflow, because that would trigger another
layout pass, hiding the bug. */
body { overflow:hidden; }
</style>
<p>There should be a green square below. No red, orange or yellow.</p>
<!-- The outer container is there to clip painting of the scrollbar,
to make it easier to write a reftest. -->
<div style="overflow:hidden; width:50px; height:50px; background:orange;">
<!-- The overflow:auto block needs auto width to be able to
reproduce the bug. Use a wide wrapper, so that the scrollbar
gets clipped. -->
<div style="width:100px; background:red;">
<div style="overflow:auto; height:50px; background:yellow;">
<div style="height:51px; background:green;"></div>
</div>
</div>
</div>
third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/compositing/scrollbars/nested-overlay-scrollbars-expected.txt 0 → 100644
View file @ 4e246b2e
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF"
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='outer'",
"position": [8, 8],
"bounds": [404, 404]
},
{
"name": "Scrolling Layer",
"position": [10, 10],
"bounds": [400, 400],
"drawsContent": false
},
{
"name": "Scrolling Contents Layer",
"position": [10, 10],
"bounds": [400, 704],
"transform": 1
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='inner'",
"position": [10, 510],
"bounds": [204, 204],
"transform": 1
},
{
"name": "Scrolling Layer",
"position": [12, 512],
"bounds": [200, 200],
"drawsContent": false,
"transform": 1
},
{
"name": "Scrolling Contents Layer",
"position": [12, 512],
"bounds": [5000, 9000],
"transform": 1
},
{
"name": "Squashing Containment Layer",
"position": [10, 10],
"drawsContent": false,
"transform": 1
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='grey'",
"position": [12, 512],
"bounds": [100, 800],
"contentsOpaque": true,
"backgroundColor": "#808080",
"transform": 1
},
{
"name": "Squashing Layer (first squashed layer: LayoutNGBlockFlow (positioned) DIV id='spacer')",
"position": [12, 2512],
"bounds": [5000, 1000],
"transform": 1
},
{
"name": "Overflow Controls Host Layer",
"position": [12, 512],
"bounds": [204, 204],
"drawsContent": false,
"transform": 1
},
{
"name": "Horizontal Scrollbar Layer",
"position": [14, 707],
"bounds": [193, 7],
"drawsContent": false,
"transform": 1
},
{
"name": "Vertical Scrollbar Layer",
"position": [207, 514],
"bounds": [7, 193],
"drawsContent": false,
"transform": 1
},
{
"name": "Scroll Corner Layer",
"position": [207, 707],
"bounds": [7, 7],
"transform": 1
},
{
"name": "Overflow Controls Host Layer",
"position": [8, 8],
"bounds": [404, 404],
"drawsContent": false
},
{
"name": "Vertical Scrollbar Layer",
"position": [403, 10],
"bounds": [7, 400],
"drawsContent": false
}
],
"transforms": [
{
"id": 1,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[0, -304, 0, 1]
],
"flattenInheritedTransform": false
}
]
}
third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/virtual/spv175/compositing/scrollbars/nested-overlay-scrollbars-expected.txt 0 → 100644
View file @ 4e246b2e
{
"layers": [
{
"name": "LayoutView #document",
"bounds": [800, 600],
"backgroundColor": "#FFFFFF"
},
{
"name": "Scrolling Layer",
"bounds": [800, 600],
"drawsContent": false
},
{
"name": "Scrolling Contents Layer",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF"
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='outer'",
"position": [8, 8],
"bounds": [404, 404]
},
{
"name": "Scrolling Layer",
"position": [10, 10],
"bounds": [400, 400],
"drawsContent": false
},
{
"name": "Scrolling Contents Layer",
"position": [10, 10],
"bounds": [400, 704],
"transform": 1
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='inner'",
"position": [10, 510],
"bounds": [204, 204],
"transform": 1
},
{
"name": "Scrolling Layer",
"position": [12, 512],
"bounds": [200, 200],
"drawsContent": false,
"transform": 1
},
{
"name": "Scrolling Contents Layer",
"position": [12, 512],
"bounds": [5000, 9000],
"transform": 1
},
{
"name": "Squashing Containment Layer",
"position": [10, 10],
"drawsContent": false,
"transform": 1
},
{
"name": "LayoutNGBlockFlow (positioned) DIV id='grey'",
"position": [12, 512],
"bounds": [100, 800],
"contentsOpaque": true,
"backgroundColor": "#808080",
"transform": 1
},
{
"name": "Squashing Layer (first squashed layer: LayoutNGBlockFlow (positioned) DIV id='spacer')",
"position": [12, 2512],
"bounds": [5000, 1000],
"transform": 1
},
{
"name": "Overflow Controls Host Layer",
"position": [12, 512],
"bounds": [204, 204],
"drawsContent": false,
"transform": 1
},
{
"name": "Horizontal Scrollbar Layer",
"position": [14, 707],
"bounds": [193, 7],
"drawsContent": false,
"transform": 1
},
{
"name": "Vertical Scrollbar Layer",
"position": [207, 514],
"bounds": [7, 193],
"drawsContent": false,
"transform": 1
},
{
"name": "Scroll Corner Layer",
"position": [207, 707],
"bounds": [7, 7],
"transform": 1
},
{
"name": "Overflow Controls Host Layer",
"position": [8, 8],
"bounds": [404, 404],
"drawsContent": false
},
{
"name": "Vertical Scrollbar Layer",
"position": [403, 10],
"bounds": [7, 400],
"drawsContent": false
}
],
"transforms": [
{
"id": 1,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[0, -304, 0, 1]
],
"flattenInheritedTransform": false
}
]
}
third_party/WebKit/LayoutTests/fragmentation/auto-overflow-expected.html 0 → 100644
View file @ 4e246b2e
<!DOCTYPE html>
<p>There should be a hotpink square below.</p>
<div style="width:50px; height:50px; background:hotpink;"></div>
third_party/WebKit/LayoutTests/fragmentation/auto-overflow.html 0 → 100644
View file @ 4e246b2e
<!DOCTYPE html>
<p>There should be a hotpink square below.</p>
<div id="multicol" style="columns:3; column-fill:auto; column-gap:0; height:50px;">
<div style="overflow:auto; height:100px; background:hotpink;"></div>
</div>
<script>
document.body.offsetTop;
document.getElementById("multicol").style.width = "75px";
</script>
third_party/WebKit/Source/core/layout/ng/legacy_layout_tree_walking.cc
View file @ 4e246b2e
......@@ -48,4 +48,15 @@ bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow* block) {
return false;
}
bool IsManagedByLayoutNG(const LayoutObject& object) {
if (!object.IsLayoutNGMixin())
return false;
const auto* containing_block = object.ContainingBlock();
if (!containing_block)
return false;
if (containing_block->IsLayoutFlowThread())
containing_block = containing_block->ContainingBlock();
return containing_block && containing_block->IsLayoutNGMixin();
}
} // namespace blink
third_party/WebKit/Source/core/layout/ng/legacy_layout_tree_walking.h
View file @ 4e246b2e
......@@ -24,6 +24,10 @@ LayoutObject* GetLayoutObjectForParentNode(LayoutObject*);
// established by |block| will be inline; see LayoutObject::ChildrenInline().
bool AreNGBlockFlowChildrenInline(const LayoutBlockFlow*);
// Return true if the layout object is a LayoutNG object that is managed by the
// LayoutNG engine (i.e. its containing block is a LayoutNG object as well).
bool IsManagedByLayoutNG(const LayoutObject&);
} // namespace blink
#endif // LegacyLayoutTreeWalking_h
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
View file @ 4e246b2e
......@@ -69,6 +69,7 @@
#include "core/layout/LayoutTheme.h"
#include "core/layout/LayoutView.h"
#include "core/layout/api/LayoutBoxItem.h"
#include "core/layout/ng/legacy_layout_tree_walking.h"
#include "core/loader/DocumentLoader.h"
#include "core/page/ChromeClient.h"
#include "core/page/FocusController.h"
......@@ -876,10 +877,13 @@ void PaintLayerScrollableArea::UpdateAfterLayout() {
Box().GetDocument().SetAnnotatedRegionsDirty(true);
// Our proprietary overflow: overlay value doesn't trigger a layout.
if ((horizontal_scrollbar_should_change &&
Box().Style()->OverflowX() != EOverflow::kOverlay) ||
(vertical_scrollbar_should_change &&
Box().Style()->OverflowY() != EOverflow::kOverlay)) {
// If the box is managed by LayoutNG, don't go here. We don't want to
// re-enter the NG layout algorithm for this box from here.
if (((horizontal_scrollbar_should_change &&
Box().Style()->OverflowX() != EOverflow::kOverlay) ||
(vertical_scrollbar_should_change &&
Box().Style()->OverflowY() != EOverflow::kOverlay)) &&
!IsManagedByLayoutNG(Box())) {
if ((vertical_scrollbar_should_change &&
Box().IsHorizontalWritingMode()) ||
(horizontal_scrollbar_should_change &&
......
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