1551621 - Assertion error in debug builds possibly related to shadow DOM

Status: NEW

(Core :: DOM: Events, defect, P2)

Description

[Mark Striemer [:mstriemer]]

In bug 1547835 there's a patch that has a debug test failure.

0:23.98 GECKO(43230) Assertion failure: nsContentUtils::ContentIsFlattenedTreeDescendantOf( leaf, aContentRemoved) || leaf->SubtreeRoot()->HasFlag(NODE_IS_ANONYMOUS_ROOT), at /builds/worker/workspace/build/src/dom/events/EventStateManager.cpp:5413

I haven't been able to isolate a smaller test case but the error happens when running toolkit/mozapps/extensions/test/browser/browser_html_updates.js. There are other patches that follow similar steps but do not cause the bug.

One thing I did notice is that the error happened when calling notesBtn.click(), but switching that line to EventUtils.synthesizeMouseAtCenter(notesBtn, {}, win) avoids the bug. I'm going to update my patch to use that to unblock it.

I will try and find a smaller test case in the future, but don't have much more time to spend right now.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Hover over the red box, and wait a bit.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1551621 - Treat an element changing its slot assignment like a content removal from ESM's point of view. r=smaug!,edgar

I think this makes sense. The ESM keeps a bunch of state in terms of the flat
tree after all, and when an element moves around on the flat tree we shouldn't
leave stale state around, which is what happens now.

May need a bit more care regarding the IME call from ESM, but I think this
should otherwise do. Let me know if you agree and I'll write a test for this :-)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Could you do a try push with my patch on top of the patch that triggered the assertion on automation, and confirm that this fixes it?

Thanks!

Comment 4

[Mark Striemer [:mstriemer]]

Sorry for the delay, I confirmed I can still generate the failure on central (after reverting the test change that avoids it), and this patch fixes the assertion error.

Without patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f12189f681d0db2f34691c3962386a030374625&selectedJob=253161719
With patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=75e2c0bad0a2f55ce4db9350e72d7d85162d3589

Comment 5

[Mark Striemer [:mstriemer]]

Any updates here? We're hitting this again in another test.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I will make the assertion non-fatal for now, since there's disagreement on what the right behavior is. I filed https://github.com/whatwg/html/issues/4795.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1551621 - Downgrade an assertion for now so as to not block people. r=edgar,bzbarsky

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1604c99e3ad1
Downgrade an assertion for now so as to not block people. r=bzbarsky

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/1604c99e3ad1

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 14

[Emilio Cobos Álvarez (:emilio)]

No

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.