1118764 - Putting |overflow:scroll;| on a gaia-list webcombonent causes crashed in |nsCSSFrameConstructor::ProcessChildren||

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Etienne Segonzac (:etienne)]

Attachment: Test Case

The attached test case displays a list rendered by the gaia-list webcomponent [1]
If you open it (with |dom.webcomponents.enabled| set to true) and start clicking on list items it will cause the tab/window/app to crash.

Removing the |overflow:scroll| and putting the gaia-list inside a container with |overflow-scroll| fixes the issue.

[1] https://github.com/gaia-components/gaia-list/

Comment 1

[Etienne Segonzac (:etienne)]

Attachment: crash log

Comment 2

[Boris Zbarsky [:bzbarsky]]

In a debug build I fail a fatal assert way before getting to ProcessChildren.

Specifically, we're in ShadowRoot::AttributeChanged and we're doing:

  DistributeSingleNode(aElement);

aElement is a <xul:scrollbar> and aElement->IsRootOfAnonymousSubtree() is true (red flag!).

In DistributeSingleNode we find an insertionPoint, isIndexFound is false, so we assert that aContent is in the explicit kids of mPoolHost.  Which it's not, of course; it's anonymous content.

I expect that the frame constructor issues are due to this same invariant violation.

OK, so why are we ending up here at all?  Scrolling updates attributes on scrollbars.  Scrollable elements have anonymous scrollbar kids.  ShadowRoot is a mutation observe and is seeing these mutations.  All fine so far.

But shouldn't ShadowRoot::IsPooledNode return false if nsContentUtils::IsInSameAnonymousTree(aContainer, aContent) is false when aContainer == aHost or something along those lines?

Comment 3

[William Chen [:wchen]]

Attachment: Don't distribute anonymous root content in shadow DOM

Yeah, that was the bug. When nodes are distributed, it's assumed that distributed nodes are in the child list of the parent but this isn't true with anonymous content which may end up with a shadow host as the parent.

After fixing the issue above, the scroll bar was still not visible and I was running into another assertion saying that we had a parent style context when no style context was expected (https://dxr.mozilla.org/mozilla-central/source/layout/base/RestyleManager.cpp#1865). I updated nsPlaceholderFrame::GetParentStyleContext to use GetFlattenedTreeParent instead of GetParent to fix this problem.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8546264 [details] [diff] [review]
Don't distribute anonymous root content in shadow DOM

> nsIContent::GetFlattenedTreeParent() const
>+      nsContentUtils::IsInSameAnonymousTree(parent, this)) {

IsInSameAnonymousTree is not null-safe.  This happens to be safe because HasDistributedChildren returns false if !parent, but that's worth at least a comment here and another one before that false return in HasDistributedChildren.  Or better yet putting the null-check here explicitly before calling HasDistributedChildren?

>@@ -583,18 +583,22 @@ ShadowRoot::IsPooledNode(nsIContent* aCo
>+  if (aContainer == aHost &&
>+    nsContentUtils::IsInSameAnonymousTree(aContainer, aContent)) {

Weird indent.  Please fix.

r=me with that.  Thank you for fixing this!

Comment 5

[William Chen [:wchen]]

(In reply to Boris Zbarsky [:bz] from comment #4)
> Comment on attachment 8546264 [details] [diff] [review]
> Or better yet putting the null-check here
> explicitly before calling HasDistributedChildren?
Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb4ffa05e89c

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eb4ffa05e89c