Chromium Code Reviews Chromium Code Reviews
Issue
1068193002:
Make Node::needsDistributionRecalc always check the root node of the tree of trees, instead of the … (Closed)
DescriptionMake Node::needsDistributionRecalc always check the root node of the tree of trees, instead of the node.
This is a follow-up patch for https://codereview.chromium.org/1058763002.
The previous patch's assertion is a weak assertion because it can't assert the following case:
var shadowHost = document.createElement("div");
var shadowRoot = shadowHost.createShadowRoot();
var content = document.createElement("content");
shadowRoot.appendChild(content);
// Note that |shadowHost| is *NOT* in a document.
// To clear distribution-recalc flag on all nodes in this detached subtree, call getDestinationInsertionPoints() at an arbitrary node.
content.getDestinationInsertionPoint();
// Append a child to the shadow host.
var shadowHostChild = document.createElement("div");
shadowHostChild.appendChild(shadowHostChild);
// Here, |shadowHostChild| and its ancestors (in a tree of trees), including |shadowHost|,
// have distribution-recalc flag be set. However, |content|'s flag is *NOT* set.
var distributedNodes = content.getDistributedNodes();
// Suppose that Blink forget to call |Node::updateDistribution()| before processing InsertionPoint::getDistributedNodes(),
// we want to detect this error by the assertion, but calling InsertionPoint.getDitributedNodes() wouldn't hit the assertion
// in the previous CL, unfortunately, because |content|'s flag is not set even when we need to re-calculate distributions in this case.
As a result, |content.getDistributedNodes| would return an empty NodeList wrongly, instead of hitting an assertion.
To hit the assertion, we should check the root node of the tree of trees.
Note that, strictly speaking, this use case doesn't apply to the previous CL exactly.
However, this use case might be enough to explain why the assertion in the previous CL is a weak condition.
BUG=473000
TEST=distribution-for-detached-subtree.html. As long as Blink calls updateDistribution() properly, it wouldn't hit an assertion in either case.
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193529
Patch Set 1 #Patch Set 2 : Add a test #
Messages
Total messages: 9 (4 generated)
|
|||||||||||||||||||||||||||||||||||||
