Skip to content

GitLab

T
tangled
Commit 16fd012f authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

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

This is a reland of 4e246b2e
Original change's description:
> [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: Emil A Eklund <eae@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524039}

TBR=eae@chromium.org

Change-Id: I5aceb5c45c8fa60b20fa481318f05c609ca2b46b
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
Reviewed-on: https://chromium-review.googlesource.com/826985Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524074}
parent e0d46d4d
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 @ 16fd012f
......@@ -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 @ 16fd012f
<!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 @ 16fd012f
<!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 @ 16fd012f
{
"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 @ 16fd012f
{
"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 @ 16fd012f
<!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 @ 16fd012f
<!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 @ 16fd012f
......@@ -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 @ 16fd012f
......@@ -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 @ 16fd012f
......@@ -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