You need to
before you can comment on or make changes to this bug.
CRASH in DocumentLoader::removeSubresourceLoader due to null m_frame
This started appearing as a top-crash in a recent Chrome dev channel build:
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 an attachment (id=29406) [details]
testcase - frame.html
Created an attachment (id=29407) [details]
testcase - careful, this may crash your browser
Created an attachment (id=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.
(From update of attachment 29412 [details])
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 an attachment (id=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 an attachment (id=29430) [details]
patch v1 + layout test
Take 2. This time with commented out code snippets removed.
(From update of attachment 29430 [details])
Landed as: http://trac.webkit.org/changeset/42442
fwiw this is also rdar://6748719