RESOLVED DUPLICATE of bug 15707 18803
CRASH: ContainerNode::willRemove() called on deleted node
https://bugs.webkit.org/show_bug.cgi?id=18803
Summary CRASH: ContainerNode::willRemove() called on deleted node
Eric Seidel (no email)
Reported 2008-04-29 13:56:45 PDT
I haven't looked into detail as to what's causing the crash, but I have created a reduction (probably could be even smaller). This is a reduced copy of HTMLFrameElement09.html which started crashing for me when opened it after leaving a debugging alert in selfhtml.js (an overly complicated support file provided by the w3c). That alert caused my copy of Safari to crash, and I reduced it to this...
Attachments
multi-file reduced test case (will crash Safari!) (10.82 KB, application/x-compressed)
2008-04-29 13:58 PDT, Eric Seidel (no email)
no flags
First pass fix (5.19 KB, patch)
2008-04-30 12:37 PDT, Eric Seidel (no email)
darin: review-
Fix updated to apply cleanly to ToT (5.39 KB, patch)
2008-11-18 10:20 PST, Eric Seidel (no email)
no flags
Eric Seidel (no email)
Comment 1 2008-04-29 13:58:26 PDT
Created attachment 20892 [details] multi-file reduced test case (will crash Safari!)
Eric Seidel (no email)
Comment 2 2008-04-29 14:00:38 PDT
function loadComplete() { // loadComplete is called by each subframe as it's loaded (as well as the main document) // I think this was an error on the author's part. alert("Geronimo!"); // CAUSES CRASH IN SAFARI var testNode = document.getElementById("Frame1"); document.open(); }
Eric Seidel (no email)
Comment 3 2008-04-29 15:13:19 PDT
I don't see this on my mac system, but happens ever time on my windows box. I haven't stopped it in the debugger. It's probably because I still have heap checking on for Safari from long ago.
Eric Seidel (no email)
Comment 4 2008-04-29 16:46:57 PDT
If you run Safari under MallocScribble (on the mac), it's very easy to reproduce this given the test case. Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000055555555 Crashed Thread: 0 Thread 0 Crashed: 0 com.apple.WebCore 0x025fb4da WebCore::ContainerNode::willRemove() + 20 (ContainerNode.cpp:347) 1 com.apple.WebCore 0x02775ee7 WebCore::HTMLFrameOwnerElement::willRemove() + 67 (HTMLFrameOwnerElement.cpp:50) 2 com.apple.WebCore 0x025fb4eb WebCore::ContainerNode::willRemove() + 37 (ContainerNode.cpp:346) 3 com.apple.WebCore 0x025fb4eb WebCore::ContainerNode::willRemove() + 37 (ContainerNode.cpp:346) 4 com.apple.WebCore 0x025fcc4f WebCore::willRemoveChild(WebCore::Node*) + 77 (ContainerNode.cpp:363) 5 com.apple.WebCore 0x025fcc8c WebCore::ContainerNode::removeChildren() + 48 (ContainerNode.cpp:464) 6 com.apple.WebCore 0x026896ea WebCore::Document::clear() + 70 (Document.cpp:1702) 7 com.apple.WebCore 0x0268f246 WebCore::Document::implicitOpen() + 28 (Document.cpp:1438) 8 com.apple.WebCore 0x0269251f WebCore::Document::open() + 379 (Document.cpp:1416) 9 com.apple.WebCore 0x02858355 WebCore::JSHTMLDocument::open(KJS::ExecState*, KJS::List const&) + 301 (JSHTMLDocumentCustom.cpp:116) 10 com.apple.WebCore 0x02856c3c WebCore::jsHTMLDocumentPrototypeFunctionOpen(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 96 (JSHTMLDocument.cpp:276) 11 com.apple.JavaScriptCore 0x0044dd5c KJS::PrototypeFunction::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 34 (function.cpp:906) 12 com.apple.JavaScriptCore 0x0046fffa KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 222 (object.cpp:99) 13 com.apple.JavaScriptCore 0x004d2ad6 KJS::FunctionCallDotNode::inlineEvaluate(KJS::ExecState*) + 802 (nodes.cpp:1495) 14 com.apple.JavaScriptCore 0x00486a4a KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) + 30 (nodes.cpp:1501) 15 com.apple.JavaScriptCore 0x00477559 KJS::ExprStatementNode::execute(KJS::ExecState*) + 43 (nodes.cpp:3993) 16 com.apple.JavaScriptCore 0x00459a5b KJS::statementListExecute(WTF::Vector<WTF::RefPtr<KJS::StatementNode>, 0ul>&, KJS::ExecState*) + 85 (nodes.cpp:3946) 17 com.apple.JavaScriptCore 0x00459ae8 KJS::BlockNode::execute(KJS::ExecState*) + 26 (nodes.cpp:3972) 18 com.apple.JavaScriptCore 0x00467cb2 KJS::FunctionBodyNode::execute(KJS::ExecState*) + 34 (nodes.cpp:4891) 19 com.apple.JavaScriptCore 0x004684bc KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 134 (function.cpp:78) 20 com.apple.JavaScriptCore 0x0046fffa KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 222 (object.cpp:99) 21 com.apple.JavaScriptCore 0x004d2ad6 KJS::FunctionCallDotNode::inlineEvaluate(KJS::ExecState*) + 802 (nodes.cpp:1495) 22 com.apple.JavaScriptCore 0x00486a4a KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) + 30 (nodes.cpp:1501) 23 com.apple.JavaScriptCore 0x00477559 KJS::ExprStatementNode::execute(KJS::ExecState*) + 43 (nodes.cpp:3993) 24 com.apple.JavaScriptCore 0x00459a5b KJS::statementListExecute(WTF::Vector<WTF::RefPtr<KJS::StatementNode>, 0ul>&, KJS::ExecState*) + 85 (nodes.cpp:3946) 25 com.apple.JavaScriptCore 0x00459ae8 KJS::BlockNode::execute(KJS::ExecState*) + 26 (nodes.cpp:3972) 26 com.apple.JavaScriptCore 0x00467cb2 KJS::FunctionBodyNode::execute(KJS::ExecState*) + 34 (nodes.cpp:4891) 27 com.apple.JavaScriptCore 0x004684bc KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 134 (function.cpp:78) 28 com.apple.JavaScriptCore 0x0046fffa KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 222 (object.cpp:99) 29 com.apple.WebCore 0x02bd922e WebCore::JSAbstractEventListener::handleEvent(WebCore::Event*, bool) + 670 (kjs_events.cpp:100) 30 com.apple.WebCore 0x02689c09 WebCore::Document::handleWindowEvent(WebCore::Event*, bool) + 281 (Document.cpp:2614) 31 com.apple.WebCore 0x026e9964 WebCore::EventTargetNode::dispatchWindowEvent(WebCore::AtomicString const&, bool, bool) + 332 (EventTargetNode.cpp:149) 32 com.apple.WebCore 0x0268ef43 WebCore::Document::implicitClose() + 675 (Document.cpp:1550) 33 com.apple.WebCore 0x02721b5c WebCore::FrameLoader::checkCallImplicitClose() + 226 (FrameLoader.cpp:1330) 34 com.apple.WebCore 0x0272da2a WebCore::FrameLoader::checkCompleted() + 268 (FrameLoader.cpp:1285) 35 com.apple.WebCore 0x0272d330 WebCore::FrameLoader::completed() + 148 (FrameLoader.cpp:1996) 36 com.apple.WebCore 0x0272da80 WebCore::FrameLoader::checkCompleted() + 354 (FrameLoader.cpp:1289) 37 com.apple.WebCore 0x0273049a WebCore::FrameLoader::finishedParsing() + 90 (FrameLoader.cpp:1233) 38 com.apple.WebCore 0x0268cf64 WebCore::Document::finishedParsing() + 204 (Document.cpp:3699) 39 com.apple.WebCore 0x027e882f WebCore::ImageTokenizer::finish() + 577 (ImageDocument.cpp:129) 40 com.apple.WebCore 0x02687122 WebCore::Document::finishParsing() + 40 (Document.cpp:1693) 41 com.apple.WebCore 0x0272dbee WebCore::FrameLoader::endIfNotLoadingMainResource() + 118 (FrameLoader.cpp:1060) 42 com.apple.WebCore 0x0272dc23 WebCore::FrameLoader::end() + 27 (FrameLoader.cpp:1045) 43 com.apple.WebCore 0x026b3488 WebCore::DocumentLoader::finishedLoading() + 76 (DocumentLoader.cpp:337) 44 com.apple.WebCore 0x02728d56 WebCore::FrameLoader::finishedLoading() + 72 (FrameLoader.cpp:2893) 45 com.apple.WebCore 0x0291aedb WebCore::MainResourceLoader::didFinishLoading() + 207 (MainResourceLoader.cpp:320) 46 com.apple.WebCore 0x02a2f290 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*) + 24 (ResourceLoader.cpp:390) 47 com.apple.WebCore 0x02a2c9f5 -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 101 (ResourceHandleMac.mm:521) 48 com.apple.Foundation 0x94c8e8b7 -[NSURLConnection(NSURLConnectionReallyInternal) sendDidFinishLoading] + 87 49 com.apple.Foundation 0x94c8e844 _NSURLConnectionDidFinishLoading + 68 50 com.apple.CFNetwork 0x9059a7f3 sendDidFinishLoadingCallback + 148 51 com.apple.CFNetwork 0x90597920 _CFURLConnectionSendCallbacks + 1994 52 com.apple.CFNetwork 0x905970d9 muxerSourcePerform + 283 53 com.apple.CoreFoundation 0x90b2562e CFRunLoopRunSpecific + 3166 54 com.apple.CoreFoundation 0x90b25d18 CFRunLoopRunInMode + 88 55 com.apple.HIToolbox 0x926296a0 RunCurrentEventLoopInMode + 283 56 com.apple.HIToolbox 0x926294b9 ReceiveNextEventCommon + 374 57 com.apple.HIToolbox 0x9262932d BlockUntilNextEventMatchingListInMode + 106 58 com.apple.AppKit 0x90c3f7d9 _DPSNextEvent + 657 59 com.apple.AppKit 0x90c3f08e -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 60 com.apple.Safari 0x00007f2e 0x1000 + 28462 61 com.apple.AppKit 0x90c380c5 -[NSApplication run] + 795 62 com.apple.AppKit 0x90c0530a NSApplicationMain + 574 63 com.apple.Safari 0x000b9906 0x1000 + 755974
Eric Seidel (no email)
Comment 5 2008-04-30 11:04:03 PDT
Ok, I stopped this in the debugger for DRT. And have a better stack trace. We're re-entering ContainerNode::removeChildren(), this is similar to a bug we fixed a year ago in ContainerNode::removeAllChildren(). In this case, we're re-entering JS and running a second on-load handler while we're running the first. I doubt that's what we intended... Probably we should have been in a forbid event dispatch block? I'd be curious to hear other's thoughts. #0 0x025fcc55 in WebCore::ContainerNode::removeChildren at ContainerNode.cpp:466 #1 0x026896ca in WebCore::Document::clear at Document.cpp:1700 #2 0x0268f226 in WebCore::Document::implicitOpen at Document.cpp:1437 #3 0x026924ff in WebCore::Document::open at Document.cpp:1414 #4 0x02858335 in WebCore::JSHTMLDocument::open at JSHTMLDocumentCustom.cpp:115 #5 0x02856c1c in WebCore::jsHTMLDocumentPrototypeFunctionOpen at JSHTMLDocument.cpp:276 #6 0x00271d5c in KJS::PrototypeFunction::callAsFunction at function.cpp:905 #7 0x00293ffa in KJS::JSObject::call at object.cpp:99 #8 0x002f6ad6 in KJS::FunctionCallDotNode::inlineEvaluate at nodes.cpp:1495 #9 0x002aaa4a in KJS::FunctionCallDotNode::evaluate at nodes.cpp:1500 #10 0x0029b559 in KJS::ExprStatementNode::execute at nodes.cpp:3993 #11 0x0027da5b in statementListExecute at nodes.cpp:3946 #12 0x0027dae8 in KJS::BlockNode::execute at nodes.cpp:3971 #13 0x0028bcb2 in KJS::FunctionBodyNode::execute at nodes.cpp:4890 #14 0x0028c4bc in KJS::FunctionImp::callAsFunction at function.cpp:78 #15 0x00293ffa in KJS::JSObject::call at object.cpp:99 #16 0x002f6ad6 in KJS::FunctionCallDotNode::inlineEvaluate at nodes.cpp:1495 #17 0x002aaa4a in KJS::FunctionCallDotNode::evaluate at nodes.cpp:1500 #18 0x0029b559 in KJS::ExprStatementNode::execute at nodes.cpp:3993 #19 0x0027da5b in statementListExecute at nodes.cpp:3946 #20 0x0027dae8 in KJS::BlockNode::execute at nodes.cpp:3971 #21 0x0028bcb2 in KJS::FunctionBodyNode::execute at nodes.cpp:4890 #22 0x0028c4bc in KJS::FunctionImp::callAsFunction at function.cpp:78 #23 0x00293ffa in KJS::JSObject::call at object.cpp:99 #24 0x02bd920e in WebCore::JSAbstractEventListener::handleEvent at kjs_events.cpp:100 #25 0x02689be9 in WebCore::Document::handleWindowEvent at Document.cpp:2616 #26 0x026e9944 in WebCore::EventTargetNode::dispatchWindowEvent at EventTargetNode.cpp:147 #27 0x0268ef23 in WebCore::Document::implicitClose at Document.cpp:1549 #28 0x02721b3c in WebCore::FrameLoader::checkCallImplicitClose at FrameLoader.cpp:1329 #29 0x0272da0a in WebCore::FrameLoader::checkCompleted at FrameLoader.cpp:1281 #30 0x0272d310 in WebCore::FrameLoader::completed at FrameLoader.cpp:1995 #31 0x0272da60 in WebCore::FrameLoader::checkCompleted at FrameLoader.cpp:1288 // NOW WE'RE IN TROUBLE... #32 0x0272dade in WebCore::FrameLoader::mainReceivedCompleteError at FrameLoader.cpp:4583 #33 0x026b2e21 in WebCore::DocumentLoader::mainReceivedError at DocumentLoader.cpp:261 #34 0x0272e095 in WebCore::FrameLoader::receivedMainResourceError at FrameLoader.cpp:3506 #35 0x0291ab96 in WebCore::MainResourceLoader::didCancel at MainResourceLoader.cpp:102 #36 0x02a2f6e6 in WebCore::ResourceLoader::cancel at ResourceLoader.cpp:349 #37 0x02a2f721 in WebCore::ResourceLoader::cancel at ResourceLoader.cpp:339 #38 0x026b2f28 in WebCore::DocumentLoader::stopLoading at DocumentLoader.cpp:299 #39 0x027281dc in WebCore::FrameLoader::stopAllLoaders at FrameLoader.cpp:2488 #40 0x0272860d in WebCore::FrameLoader::stopLoadingSubframes at FrameLoader.cpp:2473 #41 0x027281ae in WebCore::FrameLoader::stopAllLoaders at FrameLoader.cpp:2486 #42 0x02730e49 in WebCore::FrameLoader::frameDetached at FrameLoader.cpp:3294 #43 0x02775ebc in WebCore::HTMLFrameOwnerElement::willRemove at HTMLFrameOwnerElement.cpp:46 // HERE WE GO.... #44 0x025fb4ab in WebCore::ContainerNode::willRemove at ContainerNode.cpp:347 #45 0x025fb4ab in WebCore::ContainerNode::willRemove at ContainerNode.cpp:347 #46 0x025fcc0f in willRemoveChild at ContainerNode.cpp:361 #47 0x025fcc68 in WebCore::ContainerNode::removeChildren at ContainerNode.cpp:467 #48 0x026896ca in WebCore::Document::clear at Document.cpp:1700 #49 0x0268f226 in WebCore::Document::implicitOpen at Document.cpp:1437 #50 0x026924ff in WebCore::Document::open at Document.cpp:1414 #51 0x02858335 in WebCore::JSHTMLDocument::open at JSHTMLDocumentCustom.cpp:115 #52 0x02856c1c in WebCore::jsHTMLDocumentPrototypeFunctionOpen at JSHTMLDocument.cpp:276 #53 0x00271d5c in KJS::PrototypeFunction::callAsFunction at function.cpp:905 #54 0x00293ffa in KJS::JSObject::call at object.cpp:99 #55 0x002f6ad6 in KJS::FunctionCallDotNode::inlineEvaluate at nodes.cpp:1495 #56 0x002aaa4a in KJS::FunctionCallDotNode::evaluate at nodes.cpp:1500 #57 0x0029b559 in KJS::ExprStatementNode::execute at nodes.cpp:3993 #58 0x0027da5b in statementListExecute at nodes.cpp:3946 #59 0x0027dae8 in KJS::BlockNode::execute at nodes.cpp:3971 #60 0x0028bcb2 in KJS::FunctionBodyNode::execute at nodes.cpp:4890 #61 0x0028c4bc in KJS::FunctionImp::callAsFunction at function.cpp:78 #62 0x00293ffa in KJS::JSObject::call at object.cpp:99 #63 0x002f6ad6 in KJS::FunctionCallDotNode::inlineEvaluate at nodes.cpp:1495 #64 0x002aaa4a in KJS::FunctionCallDotNode::evaluate at nodes.cpp:1500 #65 0x0029b559 in KJS::ExprStatementNode::execute at nodes.cpp:3993 #66 0x0027da5b in statementListExecute at nodes.cpp:3946 #67 0x0027dae8 in KJS::BlockNode::execute at nodes.cpp:3971 #68 0x0028bcb2 in KJS::FunctionBodyNode::execute at nodes.cpp:4890 #69 0x0028c4bc in KJS::FunctionImp::callAsFunction at function.cpp:78 #70 0x00293ffa in KJS::JSObject::call at object.cpp:99 #71 0x02bd920e in WebCore::JSAbstractEventListener::handleEvent at kjs_events.cpp:100 #72 0x02689be9 in WebCore::Document::handleWindowEvent at Document.cpp:2616 #73 0x026e9944 in WebCore::EventTargetNode::dispatchWindowEvent at EventTargetNode.cpp:147 #74 0x0268ef23 in WebCore::Document::implicitClose at Document.cpp:1549 #75 0x02721b3c in WebCore::FrameLoader::checkCallImplicitClose at FrameLoader.cpp:1329 #76 0x0272da0a in WebCore::FrameLoader::checkCompleted at FrameLoader.cpp:1281 #77 0x0272d310 in WebCore::FrameLoader::completed at FrameLoader.cpp:1995 #78 0x0272da60 in WebCore::FrameLoader::checkCompleted at FrameLoader.cpp:1288 #79 0x0273047a in WebCore::FrameLoader::finishedParsing at FrameLoader.cpp:1231 #80 0x0268cf44 in WebCore::Document::finishedParsing at Document.cpp:3698 #81 0x027e880f in WebCore::ImageTokenizer::finish at ImageDocument.cpp:128 #82 0x02687102 in WebCore::Document::finishParsing at Document.cpp:1692 #83 0x0272dbce in WebCore::FrameLoader::endIfNotLoadingMainResource at FrameLoader.cpp:1060 #84 0x0272dc03 in WebCore::FrameLoader::end at FrameLoader.cpp:1044 #85 0x026b3468 in WebCore::DocumentLoader::finishedLoading at DocumentLoader.cpp:335 #86 0x02728d36 in WebCore::FrameLoader::finishedLoading at FrameLoader.cpp:2892 #87 0x0291aebb in WebCore::MainResourceLoader::didFinishLoading at MainResourceLoader.cpp:319 #88 0x02a2f270 in WebCore::ResourceLoader::didFinishLoading at ResourceLoader.cpp:389 #89 0x02a2c9d5 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] at ResourceHandleMac.mm:521 #90 0x94c8e8b7 in -[NSURLConnection(NSURLConnectionReallyInternal) sendDidFinishLoading] #91 0x94c8e844 in _NSURLConnectionDidFinishLoading #92 0x9059a7f3 in sendDidFinishLoadingCallback #93 0x90597920 in _CFURLConnectionSendCallbacks #94 0x905970d9 in muxerSourcePerform #95 0x90b2562e in CFRunLoopRunSpecific #96 0x90b25d18 in CFRunLoopRunInMode #97 0x94c5db15 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] #98 0x0000598a in runTest at DumpRenderTree.mm:896 #99 0x0000604c in dumpRenderTree (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) (100 of 101 frames) at DumpRenderTree.mm:424 #100 0x000061d4 in main at DumpRenderTree.mm:464
Eric Seidel (no email)
Comment 6 2008-04-30 11:10:16 PDT
Ah! I was wrong. The problem is that willRemoveChild(n) could have side-effects of causing n to be deleted. for (n = m_firstChild; n; n = n->nextSibling()) willRemoveChild(n); If we end up re-entering this method below that call, n will have been deleted (along with all of the siblings). One solution would be so simply hold a Ref to n during traversal. Not sure if that's the right fix.
Eric Seidel (no email)
Comment 7 2008-04-30 12:37:38 PDT
Created attachment 20904 [details] First pass fix LayoutTests/ChangeLog | 13 +++++++++++++ LayoutTests/fast/dom/removeChildren-crash.html | 14 ++++++++++++++ .../fast/dom/resources/removeChildren-crash.html | 3 +++ .../fast/dom/resources/removeChildren-crash.png | Bin 0 -> 144 bytes .../fast/dom/resources/removeChildren-crash2.html | 6 ++++++ .../fast/dom/resources/removeChildren-crash2.png | Bin 0 -> 10292 bytes WebCore/ChangeLog | 12 ++++++++++++ WebCore/dom/ContainerNode.cpp | 20 ++++++++------------ 8 files changed, 56 insertions(+), 12 deletions(-)
Darin Adler
Comment 8 2008-05-01 09:56:44 PDT
Comment on attachment 20904 [details] First pass fix This looks good. It's great that it fixes the crash. I worry that the willRemoveChild could still be used to make an infinite loop. + for (RefPtr<Node> n = m_firstChild; m_firstChild; n = m_firstChild) { I can't see any reason to not write this as: while (RefPtr<Node> n = m_firstChild) + // children), so we keep n in a RefPtr, lest n->nextSibling() crash (18803) I don't think the bug number is really all that valuable, not the specific "lest n->nextSibling() crash". Maybe the png files should have more generic names. Do we really need two separate images for this? Is there a way to make the timing right for the crash without having images?
Darin Adler
Comment 9 2008-06-08 12:00:20 PDT
Comment on attachment 20904 [details] First pass fix I tried to land this patch and: 1) Couldn't apply the patch because svn-apply didn't seem to understand the copied .png files. 2) Noticed the patch didn't include results for the test. 3) Couldn't get the test case to crash without the fix applied. So I need to change this to review-. Lets get a working regression test and then find a way to land this fix.
Eric Seidel (no email)
Comment 10 2008-06-11 00:58:31 PDT
(In reply to comment #9) > (From update of attachment 20904 [details] [edit]) > I tried to land this patch and: > > 1) Couldn't apply the patch because svn-apply didn't seem to understand the > copied .png files. > 2) Noticed the patch didn't include results for the test. > 3) Couldn't get the test case to crash without the fix applied. > > So I need to change this to review-. Lets get a working regression test and > then find a way to land this fix. The regression test only crashes if you run under MallocScribble. I have not re-verified that today, but that was the case when I wrote the test (see earlier comments in the bug). I've rebased that git branch to TOT, and can re-post tomorrow (including test results).
Alexey Proskuryakov
Comment 11 2008-06-21 11:57:45 PDT
See also: bug 15707.
Eric Seidel (no email)
Comment 12 2008-11-11 11:00:22 PST
I guess I should finally update and land this. Assigning to myself. Now I just have to find the git branch...
Deirdre Saoirse Moen
Comment 13 2008-11-14 16:19:49 PST
Eric Seidel (no email)
Comment 14 2008-11-18 09:41:35 PST
Odd. I can't get the test case to crash either anymore. :( I seems that we're no longer firing the main body onload like we were before. This code still looks wrong to me, but I don't have a test case for it. So I guess maybe we'll just wait and see if it crashes again... :(
Eric Seidel (no email)
Comment 15 2008-11-18 10:08:09 PST
I still believe the code can crash. Applying my ninja debugging skillz to see if I can't make a test case crash.
Eric Seidel (no email)
Comment 16 2008-11-18 10:20:35 PST
Created attachment 25240 [details] Fix updated to apply cleanly to ToT LayoutTests/ChangeLog | 13 +++++++++++++ LayoutTests/fast/dom/removeChildren-crash.html | 18 ++++++++++++++++++ .../fast/dom/resources/removeChildren-crash.html | 3 +++ .../fast/dom/resources/removeChildren-crash.png | Bin 0 -> 144 bytes .../fast/dom/resources/removeChildren-crash2.html | 6 ++++++ .../fast/dom/resources/removeChildren-crash2.png | Bin 0 -> 10292 bytes WebCore/ChangeLog | 12 ++++++++++++ WebCore/dom/ContainerNode.cpp | 20 ++++++++------------ 8 files changed, 60 insertions(+), 12 deletions(-)
Eric Seidel (no email)
Comment 17 2008-11-18 12:27:15 PST
Ok, this test almost works: <body> <script> function gc() { if (window.GCController) return GCController.collect(); } var outerDiv = document.createElement("div"); outerDiv.appendChild(document.createElement("div")); outerDiv.appendChild(document.createElement("div")); outerDiv.appendChild(document.createElement("div")); document.body.appendChild(outerDiv); // This function should be called right before innerDiv is removed // by clearing the outerDiv, innerDiv->nextSibling() call should // then crash once the event listener returns. This crash was fixed // by https://bugs.webkit.org/show_bug.cgi?id=18803 function kidRemoved(event) { outerDiv.removeEventListener("DOMNodeRemoved", kidRemoved, false); gc(); // Remove any remaining kids to hopefully hit a crash. outerDiv.innerHTML = ""; } outerDiv.addEventListener("DOMNodeRemoved", kidRemoved, false); // Remove all children to kick off the event sending outerDiv.innerHTML = ""; </script> <div>PASS</div> Unfortunately the MutationEvent is still holding onto the node at the time it should crash. I still think it's possible to crash this code.
Eric Seidel (no email)
Comment 18 2008-11-18 14:33:04 PST
I think it's no longer crashing due to a change to the HTMLParser where we reject <frameset> elements found inside a body element. I wonder if that's a tested change? ;) Attempting to re-work the original test case now.
Eric Seidel (no email)
Comment 19 2008-11-18 17:01:27 PST
Ok, giving up. I think this was partially caused by another bug. Since now we seem to properly stop onload events if document.open was called between when the onload was registered and when it fired.
Alexey Proskuryakov
Comment 20 2009-02-18 10:06:31 PST
Bug 15707 has both a test and a fix, so marking as a duplicate. *** This bug has been marked as a duplicate of 15707 ***
Lucas Forschler
Comment 21 2019-02-06 09:04:02 PST
Mass moving XML DOM bugs to the "DOM" Component.
Note You need to log in before you can comment on or make changes to this bug.