Bug 6944 – REGRESSION: crash when loading page w/ <link> that has a DOMSubtreeModified event listener

• Component: DOM
• Status: RESOLVED FIXED

[Adele Peterson]

When you load the attached test page, you get this crash:

Thread 0 Crashed:
0   <<00000000>> 	0xa6481890 0 + -1505224560
1   com.apple.WebCore        	0x01f56170 -[DOMNamedNodeMap getNamedItem:] + 200 (DOM.mm:676)
2   com.apple.Syndication    	0x99c764f4 -[DocumentLinks _findLinksInDOM:] + 248
3   com.apple.Syndication    	0x99c763e4 -[DocumentLinks initWithURL:DOM:] + 120
4   com.apple.SyndicationUI  	0x99cbe3b8 -[SafariSyndication feedURLFromDOM:baseURL:] + 80
5   com.apple.Safari         	0x0015a900 -[SyndicationController feedURLFromDOM:baseURL:] + 88 (SyndicationController.m:198)
6   com.apple.Safari         	0x00179e4c -[BrowserWebView updateCounterpartURLForRSS] + 712 (BrowserWebView.m:2317)
7   com.apple.Safari         	0x000cb11c -[LocationChangeHandler webView:locationChangeDone:forDataSource:] + 908 (LocationChangeHandler.m:682)
8   com.apple.Safari         	0x000cb498 -[LocationChangeHandler webView:didFinishLoadForFrame:] + 116 (LocationChangeHandler.m:717)
9   libobjc.A.dylib          	0x909c5214 objc_msgSendv + 180
10  com.apple.Foundation     	0x928d02a8 -[NSInvocation invoke] + 944
11  com.apple.Foundation     	0x928d0858 -[NSInvocation invokeWithTarget:] + 64
12  com.apple.WebKit         	0x012b0aa8 -[_WebSafeForwarder forwardInvocation:] + 624 (WebView.m:1489)
13  com.apple.Foundation     	0x928c8654 -[NSObject(NSForwardInvocation) forward::] + 408
14  libobjc.A.dylib          	0x909c50d0 _objc_msgForward + 176
15  com.apple.WebKit         	0x01262db4 -[WebFrame(WebPrivate) _checkLoadCompleteForThisFrame] + 2476 (WebFrame.m:1189)
16  com.apple.WebKit         	0x01263028 -[WebFrame(WebPrivate) _checkLoadComplete] + 164 (WebFrame.m:1223)
17  com.apple.WebKit         	0x012ab3f4 -[WebView(WebPrivate) _mainReceivedBytesSoFar:fromDataSource:complete:] + 236 (WebView.m:607)
18  com.apple.WebKit         	0x01293c74 -[WebMainResourceLoader didFinishLoading] + 656 (WebMainResourceLoader.m:398)
19  com.apple.WebKit         	0x01251bf4 -[WebLoader connectionDidFinishLoading:] + 184 (WebLoader.m:663)
20  com.apple.Foundation     	0x92906dbc -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback] + 188
21  com.apple.Foundation     	0x92905028 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 556
22  com.apple.Foundation     	0x92904d80 _sendCallbacks + 156
23  com.apple.CoreFoundation 	0x9075ea68 __CFRunLoopDoSources0 + 384
24  com.apple.CoreFoundation 	0x9075df98 __CFRunLoopRun + 452
25  com.apple.CoreFoundation 	0x9075da18 CFRunLoopRunSpecific + 268
26  com.apple.HIToolbox      	0x9317c380 RunCurrentEventLoopInMode + 264
27  com.apple.HIToolbox      	0x9317b98c ReceiveNextEventCommon + 244
28  com.apple.HIToolbox      	0x9317b880 BlockUntilNextEventMatchingListInMode + 96
29  com.apple.AppKit         	0x9367a104 _DPSNextEvent + 384
30  com.apple.AppKit         	0x93679dc8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
31  com.apple.Safari         	0x00031048 -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 296 (BrowserApplication.m:152)
32  com.apple.AppKit         	0x9367630c -[NSApplication run] + 472
33  com.apple.AppKit         	0x93766e68 NSApplicationMain + 452
34  com.apple.Safari         	0x000f9878 main + 160 (main.m:23)
35  com.apple.Safari         	0x00002eb0 _start + 340 (crt.c:272)
36  com.apple.Safari         	0x00002d58 start + 60

[Adele Peterson]

Created attachment 6120 [details]
crashing test case

this does not crash with Safari-417.8

[Adele Peterson]

I'm working on a patch.

[Adele Peterson]

Created attachment 6121 [details]
patch

While debugging, I saw the event get destroyed, and then my node, which was an AttrImpl, getting destroyed.  This patch fixes my test case.

[Adele Peterson]

Comment on attachment 6121 [details]
patch

uh oh.  I think this causes some leaks.  I'll have to rethink it.

[Darin Adler]

Comment on attachment 6121 [details]
patch

I think this patch is correct, so if it causes a leak, I would expect it would be a bug elsewhere.

[Adele Peterson]

Last patch doesn't work because AttrImpl has a reference to it's AttributeImpl

[Adele Peterson]

I'm trying to find some way to ref the AttrImpl in NamedAttrMapImpl::getNamedItem so that the AttrImpl stays around long enough, but I haven't been able to get anything to work.

[Darin Adler]

The trick here is that the AttributeImpl must be retained when appendChildNode is called on it. I have a patch in the works.

[Adele Peterson]

Created attachment 6311 [details]
patch

new and improved patch.

[Maciej Stachowiak]

Comment on attachment 6311 [details]
patch

r=me

Nice work

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.