Bug 1409088: Fix destination insertion point removal algorithm. r=bz
When an insertion point (a) is added to the document before another insertion
point (b), and that insertion point matches nodes that used to match (b), the
following happens in RedistributeAllNodes:
* Loop through (a), and clear the existing insertion points on nodes
distributed into it (none, since it was just inserted).
* Go through the node pool and add the matched nodes. That makes the node
(which already had (b) in the insertion point array) have [(b), (a)] as the
insertion points.
* Go through (b), and clear the existing insertion points on the nodes
distributed to it. That used to do IndexOf() + SetLength(), but since (b) was
the first node by then in the insertion point array, we'll leave the
insertion point array empty, while (a) would still think that the node is
distributed to it.
This causes the bloom filter code, which loops through the flattened tree
parents, to not insert any (because the node doesn't know about where it's
inserted).
Also, add a debug phase to verify the flat tree before restyling that would've
caught this more clearly (happy to remove it if you don't think it's worth).
We still can't assert that the insertion point is properly referenced due to the
hacky way mInsertionPoints is cleared in
HTMLContentElement::UpdateFallbackDistribution, but we'll still clear the
insertion points either there, or on the rest of insertion point removal code in
ShadowRoot::DistributeAllNodes.
MozReview-Commit-ID: 9k2gnsAKMEe
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -238,19 +238,20 @@ ShadowRoot::RemoveDestInsertionPoint(nsI
nsTArray<nsIContent*>& aDestInsertionPoints)
{
// Remove the insertion point from the destination insertion points.
// Also remove all succeeding insertion points because it is no longer
// possible for the content to be distributed into deeper node trees.
int32_t index = aDestInsertionPoints.IndexOf(aInsertionPoint);
// It's possible that we already removed the insertion point while processing
- // other insertion point removals.
+ // other insertion point removals / fallback content redistribution (which
+ // does DestInsertionPoints().Clear()).
if (index >= 0) {
- aDestInsertionPoints.SetLength(index);
+ aDestInsertionPoints.RemoveElementAt(index);
}
}
void
ShadowRoot::DistributionChanged()
{
// FIXME(emilio): We could be more granular in a bunch of cases.
auto* host = GetHost();
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -1202,19 +1202,40 @@ ServoRestyleManager::DoProcessPendingRes
styleSet->MaybeGCRuleTree();
// Note: We are in the scope of |animationsWithDestroyedFrame|, so
// |mAnimationsWithDestroyedFrame| is still valid.
MOZ_ASSERT(mAnimationsWithDestroyedFrame);
mAnimationsWithDestroyedFrame->StopAnimationsForElementsWithoutFrames();
}
+#ifdef DEBUG
+static void
+VerifyFlatTree(const nsIContent& aContent)
+{
+ StyleChildrenIterator iter(&aContent);
+
+ for (auto* content = iter.GetNextChild();
+ content;
+ content = iter.GetNextChild()) {
+ MOZ_ASSERT(content->GetFlattenedTreeParentNodeForStyle() == &aContent);
+ VerifyFlatTree(*content);
+ }
+}
+#endif
+
void
ServoRestyleManager::ProcessPendingRestyles()
{
+#ifdef DEBUG
+ if (auto* root = mPresContext->Document()->GetRootElement()) {
+ VerifyFlatTree(*root);
+ }
+#endif
+
DoProcessPendingRestyles(ServoTraversalFlags::Empty);
}
void
ServoRestyleManager::ProcessAllPendingAttributeAndStateInvalidations()
{
if (mSnapshots.IsEmpty()) {
return;
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1409088.html
@@ -0,0 +1,16 @@
+<!doctype html>
+<div></div>
+<script>
+let host = document.querySelector('div');
+let shadow = host.createShadowRoot();
+
+// Append two divs and three spans into host.
+host.innerHTML = '<div></div><span></span><div></div><span></span><span></span>';
+shadow.innerHTML = '<content select="div" id="divpoint"></content><content select="div, span" id="allpoint"></content>';
+
+let divPoint = shadow.getElementById("divpoint");
+let allPoint = shadow.getElementById("allpoint");
+
+shadow.removeChild(allPoint);
+shadow.insertBefore(allPoint, divPoint);
+</script>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -505,9 +505,10 @@ load 1398500.html
load 1400438-1.html
load 1400599-1.html
load 1401739.html
load 1401840.html
load 1402476.html
load 1404789-1.html
load 1404789-2.html
load 1406562.html
+load 1409088.html
load 1409147.html
