683159 - stylesheet not found assertions at shutdown.

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The assertion at http://hg.mozilla.org/mozilla-central/file/31b79d4e90f4/content/base/src/nsDocument.cpp#l3556 is firing on shutdown on Mac mochitest-other runs on tinderbox.  I added some instrumentation and made this assert a hard abort and found the following:

That assertion fires with the following stack:
###!!! ABORT: stylesheet not found: file /builds/slave/try-osx64-dbg/build/content/base/src/nsDocument.cpp, line 3573
nsDocument::RemoveStyleSheet [content/base/src/nsDocument.cpp:3574]
nsStyleLinkElement::DoUpdateStyleSheet [content/base/src/nsStyleLinkElement.cpp:226]
nsStyleLinkElement::UpdateStyleSheetInternal [content/base/src/nsStyleLinkElement.cpp:207]
nsXMLStylesheetPI::UnbindFromTree [content/xml/content/src/nsXMLStylesheetPI.cpp:141]
nsDocument::cycleCollection::Unlink [content/base/src/nsDocument.cpp:1921]
nsXULDocument::cycleCollection::Unlink [content/xul/document/src/nsXULDocument.cpp:391]
nsCycleCollector::CollectWhite [xpcom/base/nsCycleCollector.cpp:2009]
nsCycleCollector::FinishCollection [xpcom/base/nsCycleCollector.cpp:2838]
nsCycleCollectorRunner::Collect [xpcom/base/nsCycleCollector.cpp:3526]
nsCycleCollector_collect [xpcom/base/nsCycleCollector.cpp:3592]
nsJSContext::CycleCollectNow [dom/base/nsJSEnvironment.cpp:3193]

The document is chrome://global/content/customizeToolbar.xul and the sheet in question is chrome://global/skin/customizeToolbar.css.

That sheet was previously removed in a call to nsDocument::RemoveStyleSheet with the following stack:
nsDocument::RemoveStyleSheet [content/base/src/nsDocument.cpp:3560]
nsDocument::UpdateStyleSheets [content/base/src/nsDocument.cpp:3616]
nsChromeRegistry::RefreshWindow [chrome/src/nsChromeRegistry.cpp:531]
nsChromeRegistry::RefreshWindow [chrome/src/nsChromeRegistry.cpp:439]
nsChromeRegistry::RefreshSkins [chrome/src/nsChromeRegistry.cpp:403]
nsChromeRegistryChrome::Observe [chrome/src/nsChromeRegistryChrome.cpp:384]
nsPrefBranch::NotifyObserver [modules/libpref/src/nsPrefBranch.cpp:674]
pref_DoCallback [modules/libpref/src/prefapi.cpp:944]
pref_HashPref [modules/libpref/src/prefapi.cpp:801]
PREF_SetCharPref [modules/libpref/src/prefapi.cpp:283]
nsPrefBranch::SetCharPref [modules/libpref/src/nsPrefBranch.cpp:196]
mozilla::Preferences::SetCharPref [Preferences.h:77]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:195]

This happens during browser_bug592338.js.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Actually, this fires on other platforms too, just with a useless stack (at least on Windows).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, the RefreshSkins thing is sorta broken....

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This is causing oranges now :/

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67060725ec8d to reduce the spatter across the following tests.  I still need to clean up the later SimpleTest.expectAssertions calls.

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Adjusted in https://hg.mozilla.org/integration/mozilla-inbound/rev/57fc2a37629f

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/67060725ec8d
https://hg.mozilla.org/mozilla-central/rev/57fc2a37629f

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Followup test cleanup in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba681ef61a3a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

With bug 850090 backed out, comment 6 should be safe to backout now (and actually was already partially done in another push as a bustage fix).
https://tbpl.mozilla.org/?tree=Try&rev=e0d9d310bcd0

Comment 10

[Boris Zbarsky [:bzbarsky]]

This got fixed in bug 850090.  We should be able to remove the expected assertions annotation now.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Remove now-irrelevant assertion annotation.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 756973 [details] [diff] [review]
Remove now-irrelevant assertion annotation.

Review of attachment 756973 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

Comment 14

[Ed Morley [:emorley]]

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