Commit ea4d26d0 authored by wangxianzhu's avatar wangxianzhu Committed by Commit bot

[SPInvalidation] Fix PrePaintTreeWalk for multicol spanner

We should not do anything for the spanner placeholder but just walk the
spanner directly.

Out-of-flow positioned descendants of a multicol spanner need to be
specially handled because their container may be not their ancestor in
the order of PrePaintTreeWalk.

This fixes the following layout tests for slimmingPaintInvalidation:
fast/multicol/dynamic/*spanner*.html
fast/multicol/span/*.html
(except for fast/multicol/dynamic/abspos-multicol-with-spanner-becomes-spanner.html
which is still failing because of some other reason.)

BUG=646176
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2388723004
Cr-Commit-Position: refs/heads/master@{#423713}
parent 9c3fd623
......@@ -666,6 +666,40 @@ void PaintPropertyTreeBuilder::updateOutOfFlowContext(
properties->clearCssClipFixedPosition();
}
// Override ContainingBlockContext based on the properties of a containing block
// that was previously walked in a subtree other than the current subtree being
// walked. Used for out-of-flow positioned descendants of multi-column spanner
// when the containing block is not in the normal tree walk order.
// For example:
// <div id="columns" style="columns: 2">
// <div id="relative" style="position: relative">
// <div id="spanner" style="column-span: all">
// <div id="absolute" style="position: absolute"></div>
// </div>
// </div>
// <div>
// The real containing block of "absolute" is "relative" which is not in the
// tree-walk order of "columns" -> spanner placeholder -> spanner -> absolute.
// Here we rebuild a ContainingBlockContext based on the properties of
// "relative" for "absolute".
static void overrideContaineringBlockContextFromRealContainingBlock(
const LayoutBlock& containingBlock,
PaintPropertyTreeBuilderContext::ContainingBlockContext& context) {
const auto* properties =
containingBlock.objectPaintProperties()->localBorderBoxProperties();
DCHECK(properties);
context.transform = properties->propertyTreeState.transform();
context.paintOffset = properties->paintOffset;
context.shouldFlattenInheritedTransform =
context.transform && context.transform->flattensInheritedTransform();
context.renderingContextID =
context.transform ? context.transform->renderingContextID() : 0;
context.clip = properties->propertyTreeState.clip();
context.scroll = const_cast<ScrollPaintPropertyNode*>(
properties->propertyTreeState.scroll());
}
static void deriveBorderBoxFromContainerContext(
const LayoutObject& object,
PaintPropertyTreeBuilderContext& context) {
......@@ -681,7 +715,25 @@ static void deriveBorderBoxFromContainerContext(
context.current.paintOffset += boxModelObject.offsetForInFlowPosition();
break;
case AbsolutePosition: {
context.current = context.absolutePosition;
if (context.isUnderMultiColumnSpanner) {
const LayoutObject* container = boxModelObject.container();
if (container != context.containerForAbsolutePosition) {
// The container of the absolute-position is not in the normal tree-
// walk order.
context.containerForAbsolutePosition =
toLayoutBoxModelObject(container);
// The container is never a LayoutInline. In the example above
// overrideContaineringBlockContextFromRealContainingBlock(), if we
// change the container to an inline, there will be an anonymous
// blocks created because the spanner is always a block.
overrideContaineringBlockContextFromRealContainingBlock(
toLayoutBlock(*container), context.current);
}
} else {
DCHECK(context.containerForAbsolutePosition ==
boxModelObject.container());
context.current = context.absolutePosition;
}
// Absolutely positioned content in an inline should be positioned
// relative to the inline.
......@@ -699,7 +751,14 @@ static void deriveBorderBoxFromContainerContext(
context.current.paintOffset += boxModelObject.offsetForInFlowPosition();
break;
case FixedPosition:
context.current = context.fixedPosition;
if (context.isUnderMultiColumnSpanner) {
// The container of the fixed-position object may or may not be in the
// normal tree-walk order.
overrideContaineringBlockContextFromRealContainingBlock(
toLayoutBlock(*boxModelObject.container()), context.current);
} else {
context.current = context.fixedPosition;
}
break;
default:
ASSERT_NOT_REACHED();
......
......@@ -71,6 +71,8 @@ struct PaintPropertyTreeBuilderContext {
// Therefore, we don't need extra bookkeeping for effect nodes and can
// generate the effect tree from a DOM-order traversal.
const EffectPaintPropertyNode* currentEffect = nullptr;
bool isUnderMultiColumnSpanner = false;
};
// Creates paint property tree nodes for special things in the layout tree.
......
......@@ -71,6 +71,12 @@ class PaintPropertyTreeBuilderTest
return frameView->scroll();
}
LayoutPoint paintOffset(const LayoutObject* object) {
return object->objectPaintProperties()
->localBorderBoxProperties()
->paintOffset;
}
private:
void SetUp() override {
Settings::setMockScrollbarsEnabled(true);
......@@ -2787,4 +2793,87 @@ TEST_P(PaintPropertyTreeBuilderTest,
MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
}
TEST_P(PaintPropertyTreeBuilderTest, PaintOffsetsUnderMultiColumn) {
setBodyInnerHTML(
"<style>"
" body { margin: 0; }"
" .space { height: 30px; }"
" .abs { position: absolute; width: 20px; height: 20px; }"
"</style>"
"<div style='columns:2; width: 200px; column-gap: 0'>"
" <div style='position: relative'>"
" <div id=space1 class=space></div>"
" <div id=space2 class=space></div>"
" <div id=spanner style='column-span: all'>"
" <div id=normal style='height: 50px'></div>"
" <div id=top-left class=abs style='top: 0; left: 0'></div>"
" <div id=bottom-right class=abs style='bottom: 0; right: 0'></div>"
" </div>"
" <div id=space3 class=space></div>"
" <div id=space4 class=space></div>"
" </div>"
"</div>");
// Above the spanner.
// Column 1.
EXPECT_EQ(LayoutPoint(), paintOffset(getLayoutObjectByElementId("space1")));
// Column 2. TODO(crbug.com/648274): This is incorrect. Should be (100, 0).
EXPECT_EQ(LayoutPoint(0, 30),
paintOffset(getLayoutObjectByElementId("space2")));
// The spanner's normal flow.
EXPECT_EQ(LayoutPoint(0, 30),
paintOffset(getLayoutObjectByElementId("spanner")));
EXPECT_EQ(LayoutPoint(0, 30),
paintOffset(getLayoutObjectByElementId("normal")));
// Below the spanner.
// Column 1. TODO(crbug.com/648274): This is incorrect. Should be (0, 80).
EXPECT_EQ(LayoutPoint(0, 60),
paintOffset(getLayoutObjectByElementId("space3")));
// Column 2. TODO(crbug.com/648274): This is incorrect. Should be (100, 80).
EXPECT_EQ(LayoutPoint(0, 90),
paintOffset(getLayoutObjectByElementId("space4")));
// Out-of-flow positioned descendants of the spanner. They are laid out in
// the relative-position container.
// "top-left" should be aligned to the top-left corner of space1.
EXPECT_EQ(LayoutPoint(0, 0),
paintOffset(getLayoutObjectByElementId("top-left")));
// "bottom-right" should be aligned to the bottom-right corner of space4.
// TODO(crbug.com/648274): This is incorrect. Should be (180, 90).
EXPECT_EQ(LayoutPoint(80, 100),
paintOffset(getLayoutObjectByElementId("bottom-right")));
}
// Ensures no crash with multi-column containing relative-position inline with
// spanner with absolute-position children.
TEST_P(PaintPropertyTreeBuilderTest,
MultiColumnInlineRelativeAndSpannerAndAbsPos) {
setBodyInnerHTML(
"<div style='columns:2; width: 200px; column-gap: 0'>"
" <span style='position: relative'>"
" <span id=spanner style='column-span: all'>"
" <div id=absolute style='position: absolute'>absolute</div>"
" </span>"
" </span>"
"</div>");
// The "spanner" isn't a real spanner because it's an inline.
EXPECT_FALSE(getLayoutObjectByElementId("spanner")->isColumnSpanAll());
setBodyInnerHTML(
"<div style='columns:2; width: 200px; column-gap: 0'>"
" <span style='position: relative'>"
" <div id=spanner style='column-span: all'>"
" <div id=absolute style='position: absolute'>absolute</div>"
" </div>"
" </span>"
"</div>");
// There should be anonymous block created containing the inline "relative",
// serving as the container of "absolute".
EXPECT_TRUE(
getLayoutObjectByElementId("absolute")->container()->isLayoutBlock());
}
} // namespace blink
......@@ -56,6 +56,18 @@ void PrePaintTreeWalk::walk(const LayoutObject& object,
const PrePaintTreeWalkContext& context) {
PrePaintTreeWalkContext localContext(context);
if (object.isLayoutMultiColumnSpannerPlaceholder()) {
// Walk multi-column spanner as if it replaces the placeholder.
// Set the flag so that the tree builder can specially handle out-of-flow
// positioned descendants if their containers are between the multi-column
// container and the spanner. See PaintPropertyTreeBuilder for details.
localContext.treeBuilderContext.isUnderMultiColumnSpanner = true;
walk(*toLayoutMultiColumnSpannerPlaceholder(object)
.layoutObjectInFlowThread(),
localContext);
return;
}
m_propertyTreeBuilder.buildTreeNodesForSelf(object,
localContext.treeBuilderContext);
if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
......@@ -66,17 +78,12 @@ void PrePaintTreeWalk::walk(const LayoutObject& object,
for (const LayoutObject* child = object.slowFirstChild(); child;
child = child->nextSibling()) {
// Column spanners are walked through their placeholders. See below.
// Column spanners are walked through their placeholders. See above.
if (child->isColumnSpanAll())
continue;
walk(*child, localContext);
}
if (object.isLayoutMultiColumnSpannerPlaceholder())
walk(*toLayoutMultiColumnSpannerPlaceholder(object)
.layoutObjectInFlowThread(),
localContext);
if (object.isLayoutPart()) {
const LayoutPart& layoutPart = toLayoutPart(object);
Widget* widget = layoutPart.widget();
......
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