1484773 - Crash in mozilla::dom::DocumentOrShadowRoot::RemoveSheet

Status: RESOLVED FIXED

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

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is
report bp-769d4303-40c2-4435-a994-b59cf0180819.
=============================================================

Top 10 frames of crashing thread:

0 XUL mozilla::dom::DocumentOrShadowRoot::RemoveSheet xpcom/ds/nsTArray.h:509
1 XUL mozilla::dom::ShadowRoot::RemoveSheet dom/base/ShadowRoot.cpp:395
2 XUL nsStyleLinkElement::DoUpdateStyleSheet dom/base/nsStyleLinkElement.cpp:351
3 XUL mozilla::dom::HTMLStyleElement::UnbindFromTree dom/html/HTMLStyleElement.cpp:131
4 XUL mozilla::dom::FragmentOrElement::cycleCollection::Unlink dom/base/FragmentOrElement.cpp:1471
5 XUL nsCycleCollector::CollectWhite xpcom/base/nsCycleCollector.cpp:3437
6 XUL nsCycleCollector::Collect xpcom/base/nsCycleCollector.cpp:3808
7 XUL nsCycleCollector_collectSlice xpcom/base/nsCycleCollector.cpp:4394
8 XUL nsJSContext::RunCycleCollectorSlice dom/base/nsJSEnvironment.cpp:1563
9 XUL ICCRunnerFired dom/base/nsJSEnvironment.cpp:1622

=============================================================

There are 9 crashes in nightly 63 starting with buildid 20180818100051.
:emilio, could you investigate please ?

Comment 1

[Emilio Cobos Álvarez [:emilio]]

This is technically a regression from bug 1483882. But the code was making the wrong assumption before.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Containing shadow could be null after unlink.

Bug 1483882 removed the line that used to remove the SHADOW_TREE bit during
unbind. I missed this caller which still assumed that `IsInShadowTree() ->
GetContainingShadow()`.

That means that before that patch it was going through
nsIDocument::RemoveStyleSheet, which looks pretty wrong to me.

Fix it so that we don't remove the stylesheet (we weren't doing it before
anyway), but we don't crash.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9002589 [details]
Containing shadow could be null after unlink.

Olli Pettay [:smaug] has approved the revision.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1598dc35d16a
Containing shadow could be null after unlink. r=smaug

Comment 5

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/1598dc35d16a