Commit a511c255 authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot

Make sure we recalc for re-attach for v0 distributed nodes.

Similarly to what we do for slot elements. When moving from a separate
re-attach where computing style as part of AttachLayoutTree to
RecalcStyle for re-attach we need to detect that we recalc style for
distributed nodes for re-attach to SetNonAttachedStyle(). Since we can't
really propagate the kReattach up from inside the shadow tree recalc to
the shadow host when recalculating light tree children, we recalculate
the distributed nodes from the insertion point when we are in a kReattach
change.

Bug: 873129, 873279
Change-Id: I2364c5f1dce3a79e725d3cb94750f1cfb3e98221
Reviewed-on: https://chromium-review.googlesource.com/1172424
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarAnders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583200}
parent 6aedc6ec
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<div id="host"><option></option></div>
<script>
test(() => {
const root = host.createShadowRoot();
root.innerHTML = '<span style="display:none"><content /></span>';
document.body.offsetTop;
root.firstChild.style.display = "block";
}, "This test passes if there is no crash or DCHECK failure.");
</script>
...@@ -2271,7 +2271,16 @@ bool Element::ShouldCallRecalcStyleForChildren(StyleRecalcChange change) { ...@@ -2271,7 +2271,16 @@ bool Element::ShouldCallRecalcStyleForChildren(StyleRecalcChange change) {
void Element::RecalcStyle(StyleRecalcChange change) { void Element::RecalcStyle(StyleRecalcChange change) {
DCHECK(GetDocument().InStyleRecalc()); DCHECK(GetDocument().InStyleRecalc());
DCHECK(!GetDocument().Lifecycle().InDetach()); DCHECK(!GetDocument().Lifecycle().InDetach());
DCHECK(!ParentOrShadowHostNode()->NeedsStyleRecalc()); // If we are re-attaching in a Shadow DOM v0 tree, we recalc down to the
// distributed nodes to propagate kReattach down the flat tree (See
// V0InsertionPoint::DidRecalcStyle). That means we may have a shadow-
// including parent (V0InsertionPoint) with dirty recalc bit in the case where
// fallback content has been redistributed to a different insertion point.
// This will not happen for Shadow DOM v1 because we walk assigned nodes and
// slots themselves are assigned and part of the flat tree.
DCHECK(
!ParentOrShadowHostNode()->NeedsStyleRecalc() ||
(ParentOrShadowHostNode()->IsV0InsertionPoint() && change == kReattach));
DCHECK(InActiveDocument()); DCHECK(InActiveDocument());
if (HasCustomStyleCallbacks()) if (HasCustomStyleCallbacks())
...@@ -2320,6 +2329,8 @@ void Element::RecalcStyle(StyleRecalcChange change) { ...@@ -2320,6 +2329,8 @@ void Element::RecalcStyle(StyleRecalcChange change) {
change = kReattach; change = kReattach;
if (change == kReattach) if (change == kReattach)
SetNeedsReattachLayoutTree(); SetNeedsReattachLayoutTree();
else if (GetStyleChangeType() == kSubtreeStyleChange)
change = kForce;
} }
// Needed because the RebuildLayoutTree code needs to see what the // Needed because the RebuildLayoutTree code needs to see what the
...@@ -2388,7 +2399,6 @@ scoped_refptr<ComputedStyle> Element::PropagateInheritedProperties( ...@@ -2388,7 +2399,6 @@ scoped_refptr<ComputedStyle> Element::PropagateInheritedProperties(
StyleRecalcChange Element::RecalcOwnStyle(StyleRecalcChange change) { StyleRecalcChange Element::RecalcOwnStyle(StyleRecalcChange change) {
DCHECK(GetDocument().InStyleRecalc()); DCHECK(GetDocument().InStyleRecalc());
DCHECK(!ParentOrShadowHostNode()->NeedsStyleRecalc());
DCHECK(change >= kIndependentInherit || NeedsStyleRecalc()); DCHECK(change >= kIndependentInherit || NeedsStyleRecalc());
DCHECK(ParentComputedStyle()); DCHECK(ParentComputedStyle());
DCHECK(!GetNonAttachedStyle()); DCHECK(!GetNonAttachedStyle());
......
...@@ -138,18 +138,25 @@ void V0InsertionPoint::RebuildDistributedChildrenLayoutTrees( ...@@ -138,18 +138,25 @@ void V0InsertionPoint::RebuildDistributedChildrenLayoutTrees(
} }
} }
void V0InsertionPoint::WillRecalcStyle(StyleRecalcChange change) { void V0InsertionPoint::DidRecalcStyle(StyleRecalcChange change) {
StyleChangeType style_change_type = kNoStyleChange; if (!HasDistribution() || DistributedNodeAt(0)->parentNode() == this) {
// We either do not have distributed children or the distributed children
if (change > kInherit || GetStyleChangeType() > kLocalStyleChange) // are the fallback children. Fallback children have already been
style_change_type = kSubtreeStyleChange; // recalculated in ContainerNode::RecalcDescendantStyles().
else if (change > kNoInherit)
style_change_type = kLocalStyleChange;
else
return; return;
}
StyleChangeType style_change_type =
change == kForce ? kSubtreeStyleChange : kLocalStyleChange;
for (size_t i = 0; i < distributed_nodes_.size(); ++i) { for (size_t i = 0; i < distributed_nodes_.size(); ++i) {
distributed_nodes_.at(i)->SetNeedsStyleRecalc( Node* node = distributed_nodes_.at(i);
if (change == kReattach && node->IsElementNode()) {
if (node->ShouldCallRecalcStyle(kReattach))
ToElement(node)->RecalcStyle(kReattach);
continue;
}
node->SetNeedsStyleRecalc(
style_change_type, style_change_type,
StyleChangeReasonForTracing::Create( StyleChangeReasonForTracing::Create(
StyleChangeReason::kPropagateInheritChangeToDistributedNodes)); StyleChangeReason::kPropagateInheritChangeToDistributedNodes));
......
...@@ -83,7 +83,7 @@ class CORE_EXPORT V0InsertionPoint : public HTMLElement { ...@@ -83,7 +83,7 @@ class CORE_EXPORT V0InsertionPoint : public HTMLElement {
void ChildrenChanged(const ChildrenChange&) override; void ChildrenChanged(const ChildrenChange&) override;
InsertionNotificationRequest InsertedInto(ContainerNode*) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override;
void RemovedFrom(ContainerNode*) override; void RemovedFrom(ContainerNode*) override;
void WillRecalcStyle(StyleRecalcChange) override; void DidRecalcStyle(StyleRecalcChange) override;
private: private:
bool IsV0InsertionPoint() const = bool IsV0InsertionPoint() const =
......
...@@ -502,6 +502,7 @@ void HTMLSlotElement::DidRecalcStyle(StyleRecalcChange change) { ...@@ -502,6 +502,7 @@ void HTMLSlotElement::DidRecalcStyle(StyleRecalcChange change) {
return; return;
for (auto& node : assigned_nodes_) { for (auto& node : assigned_nodes_) {
if (change == kReattach && node->IsElementNode()) { if (change == kReattach && node->IsElementNode()) {
DCHECK(node->ShouldCallRecalcStyle(kReattach));
ToElement(node)->RecalcStyle(kReattach); ToElement(node)->RecalcStyle(kReattach);
continue; continue;
} }
......
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