228124 - Crash nsXMLPrettyPrinter::EndUpdate removing bindings for null root

Status: RESOLVED FIXED

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

Description

[Bob Clary [:bc] (inactive)]

Run the above URL to reproduce the crash. I have reproduced with recent 1.6b
builds on WinXP and Linux (SuSE).

This crash occurs running the full DOM 3 Core Test Suite. It does not appear to
happen if you run each test individually. To run the tests individually, go to
<http://dom-ts.bclary.com/dist-dom3-core/ecmascript/level3/core/alltests.html>,
choose XML content type, choose either all available tests or any single test,
then click Load JSUnit, then click Run in the JsUnit window.

Note that JsUnit loads each test into an IFRAME and each test loads an XML
document into another IFRAME. Since the XML is unstyled, it is pretty printed.

jst thinks there is a bug in nsXMLPrettyPrinter::EndUpdate when it tries to
remove the bindings for a document with null root.

Stack from VC++6

CallQueryInterface(nsIDOMNode * 0x00000000, nsIDocument * * 0x0012d098) line 225
+ 13 bytes
nsContentUtils::GetDocumentAndPrincipal(nsIDOMNode * 0x00000000, nsIDocument * *
0x0012d098, nsIPrincipal * * 0x0012d094) line 447 + 13 bytes
nsContentUtils::CheckSameOrigin(nsIDOMNode * 0x03ce2a40, nsIDOMNode *
0x00000000) line 576 + 57 bytes
nsDocument::RemoveBinding(nsDocument * const 0x03ce2a60, nsIDOMElement *
0x00000000, const nsAString & {...}) line 2344 + 43 bytes
nsXMLPrettyPrinter::EndUpdate(nsXMLPrettyPrinter * const 0x03de77c8, nsIDocument
* 0x03ce29d8, unsigned int 2) line 239 + 49 bytes
nsDocument::EndUpdate(unsigned int 2) line 1664
nsStyleLinkElement::UpdateStyleSheet(nsStyleLinkElement * const 0x03f9374c,
nsIDocument * 0x03ce29d8, nsICSSLoaderObserver * 0x00000000) line 180
nsHTMLLinkElement::SetDocument(nsHTMLLinkElement * const 0x03f93720, nsIDocument
* 0x00000000, int 1, int 1) line 108
nsGenericElement::SetDocumentInChildrenOf(nsIContent * 0x03ef40b0, nsIDocument *
0x00000000, int 1) line 1676
nsGenericElement::SetDocument(nsGenericElement * const 0x03ef40b0, nsIDocument *
0x00000000, int 1, int 1) line 1729 + 17 bytes
nsGenericHTMLElement::SetDocument(nsGenericHTMLElement * const 0x03ef40b0,
nsIDocument * 0x00000000, int 1, int 1) line 1338 + 21 bytes
nsGenericElement::SetDocumentInChildrenOf(nsIContent * 0x03f14958, nsIDocument *
0x00000000, int 1) line 1676
nsGenericElement::SetDocument(nsGenericElement * const 0x03f14958, nsIDocument *
0x00000000, int 1, int 1) line 1729 + 17 bytes
nsXBLBinding::ChangeDocument(nsXBLBinding * const 0x03f107c0, nsIDocument *
0x03ce29d8, nsIDocument * 0x00000000) line 1018
nsBindingManager::ChangeDocumentFor(nsBindingManager * const 0x03e02a80,
nsIContent * 0x03de9238, nsIDocument * 0x03ce29d8, nsIDocument * 0x00000000)
line 554
nsGenericElement::SetDocument(nsGenericElement * const 0x03de9238, nsIDocument *
0x00000000, int 1, int 1) line 1697
nsDocument::RemoveChild(nsDocument * const 0x03ce2a40, nsIDOMNode * 0x03de925c,
nsIDOMNode * * 0x0012d990) line 3120
nsGenericElement::doInsertBefore(nsIDOMNode * 0x03de925c, nsIDOMNode *
0x00000000, nsIDOMNode * * 0x0012db88) line 2825 + 63 bytes
nsGenericContainerElement::AppendChild(nsGenericContainerElement * const
0x03dc7a18, nsIDOMNode * 0x03de925c, nsIDOMNode * * 0x0012db88) line 782
nsDocumentFragment::AppendChild(nsDocumentFragment * const 0x03dc7a18,
nsIDOMNode * 0x03de925c, nsIDOMNode * * 0x0012db88) line 105 + 20 bytes
XPTC_InvokeByIndex(nsISupports * 0x03dc7a3c, unsigned int 18, unsigned int 2,
nsXPTCVariant * 0x0012db78) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2022 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x03aeaf30, JSObject * 0x03d46fa0, unsigned int 1,
long * 0x0405441c, long * 0x0012de48) line 1272 + 14 bytes
js_Invoke(JSContext * 0x03aeaf30, unsigned int 1, unsigned int 0) line 943 + 23
bytes
js_Interpret(JSContext * 0x03aeaf30, long * 0x0012e838) line 2964 + 15 bytes
js_Execute(JSContext * 0x03aeaf30, JSObject * 0x03d460f0, JSScript * 0x03d641c8,
JSStackFrame * 0x0012f100, unsigned int 32, long * 0x0012e838) line 1157 + 13 bytes
obj_eval(JSContext * 0x03aeaf30, JSObject * 0x03ad3ef8, unsigned int 1, long *
0x04054368, long * 0x0012e838) line 1069 + 27 bytes
js_Invoke(JSContext * 0x03aeaf30, unsigned int 1, unsigned int 0) line 943 + 23
bytes
js_Interpret(JSContext * 0x03aeaf30, long * 0x0012f170) line 2964 + 15 bytes
js_Invoke(JSContext * 0x03aeaf30, unsigned int 1, unsigned int 0) line 960 + 13
bytes
js_Interpret(JSContext * 0x03aeaf30, long * 0x0012fac4) line 2964 + 15 bytes
js_Execute(JSContext * 0x03aeaf30, JSObject * 0x03ad3ef8, JSScript * 0x03e86ec0,
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012fac4) line 1157 + 13 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x03aeaf30, JSObject * 0x03ad3ef8,
JSPrincipals * 0x030c4d3c, const unsigned short * 0x02f39798, unsigned int 26,
const char * 0x03e5b478, unsigned int 177, long * 0x0012fac4) line 3523 + 25 bytes
nsJSContext::EvaluateString(nsJSContext * const 0x03aeaec8, const nsAString &
{...}, void * 0x03ad3ef8, nsIPrincipal * 0x030c4d38, const char * 0x03e5b478,
unsigned int 177, const char * 0x00c95430, nsAString & {...}, int * 0x0012fbe0)
line 894 + 85 bytes
GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x03e5b400) line 4906 + 115 bytes
GlobalWindowImpl::TimerCallback(nsITimer * 0x03ce0860, void * 0x03e5b400) line 5282
nsTimerImpl::Fire() line 382 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x02207248) line 616
nsAppShell::Run(nsAppShell * const 0x00a54b40) line 142
nsAppShellService::Run(nsAppShellService * const 0x00a54890) line 484
main1(int 1, char * * 0x002e2410, nsISupports * 0x009af0b0) line 1291 + 32 bytes
main(int 1, char * * 0x002e2410) line 1678 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e814c7(

Comment 1

[Johnny Stenback (:jst)]

Attachment: Only remove bindings from the root element if there actually is a root element.

Comment 2

[Johnny Stenback (:jst)]

This stops us dead in the middle of running these DOM tests, with this patch
we're able to run the whole way through. We want this fixed in 1.6 and 1.4.2, IMO.

Comment 3

[Bob Clary [:bc] (inactive)]

Confirming the tests run to completion with this patch applied.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 137258 [details] [diff] [review]
Only remove bindings from the root element if there actually is a root element.

sr=bzbarsky

Comment 5

[chris hofmann]

Comment on attachment 137258 [details] [diff] [review]
Only remove bindings from the root element if there actually is a root element.

a=chofmann for 1.6

Comment 6

[Asa Dotzler [:asa]]

Has this landed yet? It's going to need to be landed quickly to make the 1.6 branch.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

i'll do it

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in. (forgot to mention a= in checkin comment, but the bug was properly
approved before checkin by chofmann)

Comment 9

[Simon Paquet [:sipaq]]

Comment on attachment 137258 [details] [diff] [review]
Only remove bindings from the root element if there actually is a root element.

Asking for approval 1.4.2
As jst pointed out in comment 2, this stops us dead in the middle of running
these DOM tests, and with this patch
we're able to run the whole way through. This has already been fixed on the
trunk and 1.6-branch for over a month.

Comment 10

[Mike Kaply [:mkaply]]

Comment on attachment 137258 [details] [diff] [review]
Only remove bindings from the root element if there actually is a root element.

please check this in quickly and mark fixed1.4.2 in the keywords field

Comment 11

[Johnny Stenback (:jst)]

Fix checked in on the MOZILLA_1_4_BRANCH branch.