Skip to content

GitLab

T
tangled
Commit 23a9f596 authored by mstensho@opera.com's avatar mstensho@opera.com
Browse files
Options

Update outer flow thread membership before changing multicolness. 

This change is about being more strict about applying style changes in tree
order: first adjust the relationship to the ancestry, THEN adjust the children.

This order is important if a multicol container has a child out-of-flow
multicol container with a spanner, and this child is changed to become in-flow
at the same time as it ceases to be a multicol container, and instead becomes a
spanner. If we change it from multicol to spanner first (instead of making it
part of the outer multicol container first), the outer multicol container is
going to believe that it contains the inner spanner, and we'd end up with a
spanner inside another spanner, which isn't allowed.

#a - multicol
  #b - abspos multicol, changing it to static spanner
    #c - spanner (but it should become a regular block once #b becomes a spanner)

The effect of this fix is that we swap the ordering of notifying the flow thread
about descendant style changes (flowThreadDescendantStyleWillChange(),
flowThreadDescendantStyleDidChange()), compared to when handling style changes
locally on the object (styleWillChange(), styleDidChange()) takes place. More
specifically, we need to get to flowThreadDescendantStyleDidChange() first
(which registers or unregisters descendants in the flow thread - i.e. updates
the LayoutMultiColumnSet / LayoutMultiColumnSpanner placeholder structure), and
THEN to evacuateAndDestroy() (via LayoutBlockFlow::styleDidChange() and 
createOrDestroyMultiColumnFlowThreadIfNeeded()), instead of the other way around.
This way we register #b (now a spanner) in #a first. That will prevent #a from
seeing anything inside #b (spanners are rather opaque).

Since we're now notifying the flow thread from LayoutBox instead of
LayoutObject, we can change the style change notification methods to take
LayoutBox instead of any kind of LayoutObject. The flow thread only cares about
LayoutBox or better here anyway. This allows for some cleanup in the
notification methods, since we no longer need to worry about computed style
weirdness on text layout objects.

BUG=516532
R=eae@chromium.org,jchaffraix@chromium.org,leviw@chromium.org

Review URL: https://codereview.chromium.org/1320843005

git-svn-id: svn://svn.chromium.org/blink/trunk@201993 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent c73ad41c
Hide whitespace changes
Inline Side-by-side
Showing with 42 additions and 32 deletions
third_party/WebKit/LayoutTests/fast/multicol/dynamic/abspos-multicol-with-spanner-becomes-spanner-expected.html 0 → 100644
View file @ 23a9f596
<!DOCTYPE html>
<p>We have a multicol with an absolutely positioned multicol child. That child has a spanner inside. The absolutely positioned multicol child isn't part of its parent multicol, since it's out of flow. The absolutely positioned multicol also has column-span:all, but it doesn't apply, since it's out of flow. Now make it statically positioned. That will turn it into a spanner in the parent multicol. At the same time, change it from multicol to regular block. It will still be a spanner, but since it's no longer multicol, its child spanner can no longer be one, since you cannot nest spanners in the same multicol context. So you end up with a multicol with a spanner with a regular (invalid spanner) block.</p>
<p>Below there should be four squares stacked vertically, in the following order: hotpink, yellow, papayawhip, olive.</p>
<div style="height:200px; width:50px; background:olive;">
<div style="height:50px; background:hotpink;"></div>
<div style="height:50px; background:yellow;"></div>
<div style="height:50px; background:papayawhip;"></div>
</div>
third_party/WebKit/LayoutTests/fast/multicol/dynamic/abspos-multicol-with-spanner-becomes-spanner.html 0 → 100644
View file @ 23a9f596
<!DOCTYPE html>
<p>We have a multicol with an absolutely positioned multicol child. That child has a spanner inside. The absolutely positioned multicol child isn't part of its parent multicol, since it's out of flow. The absolutely positioned multicol also has column-span:all, but it doesn't apply, since it's out of flow. Now make it statically positioned. That will turn it into a spanner in the parent multicol. At the same time, change it from multicol to regular block. It will still be a spanner, but since it's no longer multicol, its child spanner can no longer be one, since you cannot nest spanners in the same multicol context. So you end up with a multicol with a spanner with a regular (invalid spanner) block.</p>
<p>Below there should be four squares stacked vertically, in the following order: hotpink, yellow, papayawhip, olive.</p>
<div style="-webkit-column-count:3; -webkit-column-gap:0; width:50px; background:olive;">
<div style="height:150px; background:hotpink;"></div>
<div id="elm" style="position:absolute; -webkit-column-count:3; -webkit-column-span:all; padding-top:50px; background:yellow;">
<div style="-webkit-column-span:all; height:50px; background:papayawhip;"></div>
</div>
<div style="height:150px;"></div>
</div>
<script>
document.documentElement.offsetTop;
var elm = document.getElementById("elm");
elm.style.position = "static";
elm.style.webkitColumnCount = "auto";
</script>
third_party/WebKit/Source/core/layout/LayoutBox.cpp
View file @ 23a9f596
......@@ -157,6 +157,10 @@ void LayoutBox::styleWillChange(StyleDifference diff, const ComputedStyle& newSt
{
const ComputedStyle* oldStyle = style();
if (oldStyle) {
LayoutFlowThread* flowThread = flowThreadContainingBlock();
if (flowThread && flowThread != this)
flowThread->flowThreadDescendantStyleWillChange(this, diff, newStyle);
// The background of the root element or the body element could propagate up to
// the canvas. Just dirty the entire canvas when our style changes substantially.
if ((diff.needsPaintInvalidation() || diff.needsLayout()) && node()
......@@ -251,6 +255,12 @@ void LayoutBox::styleDidChange(StyleDifference diff, const ComputedStyle* oldSty
placeholder->layoutObjectInFlowThreadStyleDidChange(oldStyle);
updateSlowRepaintStatusAfterStyleChange();
if (oldStyle) {
LayoutFlowThread* flowThread = flowThreadContainingBlock();
if (flowThread && flowThread != this)
flowThread->flowThreadDescendantStyleDidChange(this, diff, *oldStyle);
}
}
void LayoutBox::updateSlowRepaintStatusAfterStyleChange()
......
third_party/WebKit/Source/core/layout/LayoutFlowThread.h
View file @ 23a9f596
......@@ -71,8 +71,8 @@ public:
virtual void flowThreadDescendantWasInserted(LayoutObject*) { }
virtual void flowThreadDescendantWillBeRemoved(LayoutObject*) { }
virtual void flowThreadDescendantStyleWillChange(LayoutObject*, StyleDifference, const ComputedStyle& newStyle) { }
virtual void flowThreadDescendantStyleDidChange(LayoutObject*, StyleDifference, const ComputedStyle& oldStyle) { }
virtual void flowThreadDescendantStyleWillChange(LayoutBox*, StyleDifference, const ComputedStyle& newStyle) { }
virtual void flowThreadDescendantStyleDidChange(LayoutBox*, StyleDifference, const ComputedStyle& oldStyle) { }
bool nodeAtPoint(HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) final;
......
third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
View file @ 23a9f596
......@@ -837,42 +837,28 @@ static inline bool needsToInsertIntoFlowThread(const ComputedStyle& oldStyle, co
return (!newStyle.hasOutOfFlowPosition() && oldStyle.hasOutOfFlowPosition()) || needsToReinsertIntoFlowThread(oldStyle, newStyle);
}
void LayoutMultiColumnFlowThread::flowThreadDescendantStyleWillChange(LayoutObject* descendant, StyleDifference diff, const ComputedStyle& newStyle)
void LayoutMultiColumnFlowThread::flowThreadDescendantStyleWillChange(LayoutBox* descendant, StyleDifference diff, const ComputedStyle& newStyle)
{
if (descendant->isText()) {
// Text nodes inherit all properties from the parent node (including non-inheritable
// ones). We don't care what its 'position' is. In fact, we _must_ ignore it, since the
// parent may be the multicol container, and having that accidentally leaked into children
// of the multicol is bad.
return;
}
if (needsToRemoveFromFlowThread(descendant->styleRef(), newStyle))
flowThreadDescendantWillBeRemoved(descendant);
}
void LayoutMultiColumnFlowThread::flowThreadDescendantStyleDidChange(LayoutObject* descendant, StyleDifference diff, const ComputedStyle& oldStyle)
void LayoutMultiColumnFlowThread::flowThreadDescendantStyleDidChange(LayoutBox* descendant, StyleDifference diff, const ComputedStyle& oldStyle)
{
if (descendant->isText()) {
// Text nodes inherit all properties from the parent node (including non-inheritable
// ones). We don't care what its 'position' is. In fact, we _must_ ignore it, since the
// parent may be the multicol container, and having that accidentally leaked into children
// of the multicol is bad.
return;
}
if (needsToInsertIntoFlowThread(oldStyle, descendant->styleRef())) {
flowThreadDescendantWasInserted(descendant);
return;
}
if (descendantIsValidColumnSpanner(descendant)) {
// We went from being regular column content to becoming a spanner.
ASSERT(!toLayoutBox(descendant)->spannerPlaceholder());
ASSERT(!descendant->spannerPlaceholder());
// First remove this as regular column content. Note that this will walk the entire subtree
// of |descendant|. There might be spanners there (which won't be spanners anymore, since
// we're not allowed to nest spanners), whose placeholders must die.
flowThreadDescendantWillBeRemoved(descendant);
createAndInsertSpannerPlaceholder(toLayoutBox(descendant), nextInPreOrderAfterChildrenSkippingOutOfFlow(this, descendant));
createAndInsertSpannerPlaceholder(descendant, nextInPreOrderAfterChildrenSkippingOutOfFlow(this, descendant));
}
}
......
third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.h
View file @ 23a9f596
......@@ -218,8 +218,8 @@ private:
void skipColumnSpanner(LayoutBox*, LayoutUnit logicalTopInFlowThread) override;
void flowThreadDescendantWasInserted(LayoutObject*) final;
void flowThreadDescendantWillBeRemoved(LayoutObject*) final;
void flowThreadDescendantStyleWillChange(LayoutObject*, StyleDifference, const ComputedStyle& newStyle) override;
void flowThreadDescendantStyleDidChange(LayoutObject*, StyleDifference, const ComputedStyle& oldStyle) override;
void flowThreadDescendantStyleWillChange(LayoutBox*, StyleDifference, const ComputedStyle& newStyle) override;
void flowThreadDescendantStyleDidChange(LayoutBox*, StyleDifference, const ComputedStyle& oldStyle) override;
void computePreferredLogicalWidths() override;
void computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logicalTop, LogicalExtentComputedValues&) const override;
void updateLogicalWidth() override;
......
third_party/WebKit/Source/core/layout/LayoutObject.cpp
View file @ 23a9f596
......@@ -1811,11 +1811,6 @@ void LayoutObject::setStyle(PassRefPtr<ComputedStyle> style)
diff = adjustStyleDifference(diff);
if (m_style) {
LayoutFlowThread* flowThread = flowThreadContainingBlock();
if (flowThread && flowThread != this)
flowThread->flowThreadDescendantStyleWillChange(this, diff, *style);
}
styleWillChange(diff, *style);
RefPtr<ComputedStyle> oldStyle = m_style.release();
......@@ -1832,11 +1827,6 @@ void LayoutObject::setStyle(PassRefPtr<ComputedStyle> style)
bool doesNotNeedLayoutOrPaintInvalidation = !m_parent;
styleDidChange(diff, oldStyle.get());
if (oldStyle.get()) {
LayoutFlowThread* flowThread = flowThreadContainingBlock();
if (flowThread && flowThread != this)
flowThread->flowThreadDescendantStyleDidChange(this, diff, *oldStyle.get());
}
// FIXME: |this| might be destroyed here. This can currently happen for a LayoutTextFragment when
// its first-letter block gets an update in LayoutTextFragment::styleDidChange. For LayoutTextFragment(s),
......
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