CRASH in DocumentLoader::removeSubresourceLoader due to null m_frame This started appearing as a top-crash in a recent Chrome dev channel build: http://code.google.com/p/chromium/issues/detail?id=9944 Sample crash stack: [chrome.dll - frameloader.cpp:2670] WebCore::FrameLoader::activeDocumentLoader() [chrome.dll - frameloader.cpp:2677] WebCore::FrameLoader::isLoading() [chrome.dll - documentloader.cpp:768] WebCore::DocumentLoader::removeSubresourceLoader(WebCore::ResourceLoader *) [chrome.dll - subresourceloader.cpp:225] WebCore::SubresourceLoader::didCancel(WebCore::ResourceError const &) [chrome.dll - resourceloader.cpp:359] WebCore::ResourceLoader::cancel(WebCore::ResourceError const &) [chrome.dll - resourceloader.cpp:349] WebCore::ResourceLoader::cancel() [chrome.dll - documentthreadableloader.cpp:109] WebCore::DocumentThreadableLoader::cancel() [chrome.dll - xmlhttprequest.cpp:709] WebCore::XMLHttpRequest::internalAbort() [chrome.dll - xmlhttprequest.cpp:1182] WebCore::XMLHttpRequest::stop() [chrome.dll - scriptexecutioncontext.cpp:156] WebCore::ScriptExecutionContext::stopActiveDOMObjects() [chrome.dll - document.cpp:1310] WebCore::Document::detach() [chrome.dll - frame.cpp:232] WebCore::Frame::setView(WebCore::FrameView *) [chrome.dll - frameloader.cpp:3434] WebCore::FrameLoader::closeAndRemoveChild(WebCore::Frame *) [chrome.dll - frameloader.cpp:3516] WebCore::FrameLoader::detachFromParent() [chrome.dll - frameloader.cpp:3426] WebCore::FrameLoader::detachChildren() It appears that this crash is only possible if a ResourceLoader starts after FrameLoader::stopAllLoaders has been called. I found a way to do that via an XMLHttpRequest onabort handler. Test case coming up...
Created attachment 29406 [details] testcase - frame.html
Created attachment 29407 [details] testcase - careful, this may crash your browser
Created attachment 29412 [details] patch v1 - simplistic null check This is a very simple minded solution to the crash. Other parts of DocumentLoader are fairly careful to null test m_frame before using it, and indeed removeSubresourceLoader null checks m_frame before using it (except in the case here where it calls updateLoading). I have been struggling to get a good layout test for this. For some reason my layout test appears to be rather flakey so I don't want to submit it just yet. I'll keep working on it. I just wanted to share this patch for feedback.
Comment on attachment 29412 [details] patch v1 - simplistic null check r=me If you can figure out how to do a regression test, that would be a great addition. Also, it's much better when there are per-function comments, even for a simple function like this one.
(In reply to comment #4) > Also, it's much better when there are per-function comments, even for > a simple function like this one. Per-function comment in the ChangeLog I mean.
> Per-function comment in the ChangeLog I mean. Will do. I think I figured out how to write a stable test :)
Created attachment 29429 [details] patch v1 + layout test This is the same patch as before but now with a layout test. The test requires subframes, so there are a couple resource files included.
Created attachment 29430 [details] patch v1 + layout test Take 2. This time with commented out code snippets removed.
Comment on attachment 29430 [details] patch v1 + layout test r=me
Landed as: http://trac.webkit.org/changeset/42442
fwiw this is also rdar://6748719