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

RESOLVED FIXED in mozilla37

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: etienne, Assigned: wchen)

Tracking

(Blocks: 1 bug)

unspecified
mozilla37
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
Created attachment 8545260 [details]
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/
(Reporter)

Comment 1

3 years ago
Created attachment 8545261 [details]
crash log
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?
Blocks: 811542
Component: Layout → DOM
Flags: needinfo?(wchen)
(Assignee)

Comment 3

3 years ago
Created attachment 8546264 [details] [diff] [review]
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.
Assignee: nobody → wchen
Flags: needinfo?(wchen)
Attachment #8546264 - Flags: review?(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!
Attachment #8546264 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

3 years ago
(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
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/eb4ffa05e89c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.