This is only an issue in debug builds because in release builds there is a null check for document immediately after the assert. A probable fix is to just remove the ASSERT() although it would be interesting to find out why document is null only on GTK. Backtrace: #0 0x00007fc0ce043ed1 in WebCore::FrameLoader::saveDocumentState (this=0xb2f930) at WebCore/loader/FrameLoader.cpp:4001 #1 0x00007fc0ce05f029 in WebCore::FrameLoader::closeURL (this=0x7fffd7019640) at WebCore/loader/FrameLoader.cpp:641 #2 0x00007fc0ce05f07d in WebCore::FrameLoader::detachFromParent (this=0xb2f930) at WebCore/loader/FrameLoader.cpp:3194 #3 0x00007fc0cdfaf105 in WebCore::HTMLFrameOwnerElement::willRemove (this=0xa068c0) at WebCore/html/HTMLFrameOwnerElement.cpp:46 #4 0x00007fc0cddd679e in WebCore::ContainerNode::willRemove (this=0xb07860) at WebCore/dom/ContainerNode.cpp:347 #5 0x00007fc0cddd679e in WebCore::ContainerNode::willRemove (this=0xaf6e30) at WebCore/dom/ContainerNode.cpp:347 #6 0x00007fc0cddd679e in WebCore::ContainerNode::willRemove (this=0x82a580) at WebCore/dom/ContainerNode.cpp:347 #7 0x00007fc0cddd679e in WebCore::ContainerNode::willRemove (this=0x851100) at WebCore/dom/ContainerNode.cpp:347 #8 0x00007fc0ce04ef0f in WebCore::FrameLoader::clear (this=0x810fe0, clearWindowProperties=true, clearScriptObjects=true) at WebCore/loader/FrameLoader.cpp:803 #9 0x00007fc0ce057eef in WebCore::FrameLoader::begin (this=0x810fe0, url=@0x811190, dispatch=false, origin=0x0) at WebCore/loader/FrameLoader.cpp:898 #10 0x00007fc0ce05bda2 in WebCore::FrameLoader::receivedFirstData (this=0x7fffd7019640) at WebCore/loader/FrameLoader.cpp:848 #11 0x00007fc0ce05c7c8 in WebCore::FrameLoader::setEncoding (this=0x810fe0, name=@0x80e048, userChosen=false) at WebCore/loader/FrameLoader.cpp:1749 #12 0x00007fc0cdb4f335 in WebKit::FrameLoaderClient::committedLoad (this=<value optimized out>, loader=<value optimized out>, data=0xfded60 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.0//EN\">\n<html>\n <title>The Acid3 Test (Reference Rendering)</title>\n <style type=\"text/css\">\n html { margin: 0; padding: 0; }\n body { background: #c0c0c0 ur"..., length=1256) at WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:165 #13 0x00007fc0ce03c683 in WebCore::DocumentLoader::commitLoad (this=0xfcfd80, data=0xfded60 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.0//EN\">\n<html>\n <title>The Acid3 Test (Reference Rendering)</title>\n <style type=\"text/css\">\n html { margin: 0; padding: 0; }\n body { background: #c0c0c0 ur"..., length=1256) at WebCore/loader/DocumentLoader.cpp:325 #14 0x00007fc0ce089ee2 in WebCore::ResourceLoader::didReceiveData (this=0xfcec10, data=0xfded60 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.0//EN\">\n<html>\n <title>The Acid3 Test (Reference Rendering)</title>\n <style type=\"text/css\">\n html { margin: 0; padding: 0; }\n body { background: #c0c0c0 ur"..., length=1256, lengthReceived=0, allAtOnce=false) at WebCore/loader/ResourceLoader.cpp:234 #15 0x00007fc0ce07f4e3 in WebCore::MainResourceLoader::didReceiveData (this=0xfcec10, data=0xfded60 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.0//EN\">\n<html>\n <title>The Acid3 Test (Reference Rendering)</title>\n <style type=\"text/css\">\n html { margin: 0; padding: 0; }\n body { background: #c0c0c0 ur"..., length=1256, lengthReceived=0, allAtOnce=160) at WebCore/loader/MainResourceLoader.cpp:296 #16 0x00007fc0ce271057 in writeCallback (ptr=0xfded60, size=<value optimized out>, nmemb=<value optimized out>, data=<value optimized out>) at WebCore/platform/network/curl/ResourceHandleManager.cpp:126 #17 0x00007fc0c844b838 in ?? () from /usr/lib/libcurl-gnutls.so.4 #18 0x00007fc0c8464b79 in ?? () from /usr/lib/libcurl-gnutls.so.4 #19 0x00007fc0c8464d2f in ?? () from /usr/lib/libcurl-gnutls.so.4 #20 0x00007fc0c845e16a in ?? () from /usr/lib/libcurl-gnutls.so.4 #21 0x00007fc0c8462e9c in ?? () from /usr/lib/libcurl-gnutls.so.4 #22 0x00007fc0c846380b in curl_multi_perform () from /usr/lib/libcurl-gnutls.so.4 #23 0x00007fc0ce2759bc in WebCore::ResourceHandleManager::downloadTimerCallback (this=0x897320, timer=<value optimized out>) at WebCore/platform/network/curl/ResourceHandleManager.cpp:308 #24 0x00007fc0ce15e350 in WebCore::TimerBase::fireTimers (fireTime=1206441151.5012569, firingTimers=@0x7fffd701a880) at WebCore/platform/Timer.cpp:347 #25 0x00007fc0ce15e870 in WebCore::TimerBase::sharedTimerFired () at WebCore/platform/Timer.cpp:368 #26 0x00007fc0cdb86492 in timeout_cb () at WebCore/platform/gtk/SharedTimerGtk.cpp:48 #27 0x00007fc0cccd294b in ?? () from /usr/lib/libglib-2.0.so.0 #28 0x00007fc0cccd2222 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0 #29 0x00007fc0cccd54d6 in ?? () from /usr/lib/libglib-2.0.so.0 #30 0x00007fc0cccd5797 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0 #31 0x00007fc0cd2e3ee3 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #32 0x00000000004040b8 in main (argc=1, argv=0x7fffd701abc8) at juniper.c:46
Created attachment 20080 [details] remove bogus assertion I'm pretty sure the ASSERT is bogus, because a frame can contain no document under certain circumstances, and because there's an if() test immediately after that protects against the case that the ASSERT is checking for. Also because although the ASSERT trips on the debug build (for example when clicking the reference rendering link on http://acid3.acidtests.org/), the release build works fine in the same circumstances. Thus this patch removes the ASSERT.
(In reply to comment #1) > Created an attachment (id=20080) [edit] > remove bogus assertion > > I'm pretty sure the ASSERT is bogus, because a frame can contain no document > under certain circumstances, and because there's an if() test immediately after > that protects against the case that the ASSERT is checking for. I would be more cautious about removing the ASSERT because first it is never triggered on the Mac port only on Qt/Gtk (as mentionned in http://bugs.webkit.org/show_bug.cgi?id=17984). You should also be carefull as ASSERTs are not enforced on release build so the null check is a necessity for those cases. Finally in bug17984, I showed another way of resolving the bug that does not involve removing the ASSERT (but may lead to memory leaks). > > Also because although the ASSERT trips on the debug build (for example when > clicking the reference rendering link on http://acid3.acidtests.org/), the > release build works fine in the same circumstances. > > Thus this patch removes the ASSERT. > Your patch lacks a test case (it is pretty straightforward as the assertion can be triggered by a page with an iframe and just reloading it).
*** Bug 17984 has been marked as a duplicate of this bug. ***
Comment on attachment 20080 [details] remove bogus assertion r=me
Comment on attachment 20080 [details] remove bogus assertion Clearing review flag because I think Julien is right. The assertion is correct and in fact it's the null check after the assertion that's wrong We should fix the bug instead.
Jasper, the saveDocumentState does try to save frame's information but it is pointless to save frame's information when they are not connected to a Document. The core of the issue is how the frames are released. That's why the gtk port is the only one concerned with this bug. I have said that the Qt port would also fail but it is not longer the case. I could not find exactly when it was fixed. If you want to attack the real issue, I would advise you to have a look at http://trac.webkit.org/projects/webkit/changeset/31521. This is the changeset where Zecke has moved the portion of the code I had hold responsible for the assertion failure (the code in FrameLoaderClient::detachedFromParent4()). The other Qt commits from the same day should give an answer to how and when the frame should be released to avoid both the assertion and a memory leak.
*** Bug 18334 has been marked as a duplicate of this bug. ***
I thought Frames were always created with "Empty Documents"? Maciej went through significant pains to accomplish that for us, and we spent time cleaning up all the loose ends, but are better off for it. Maybe that empty document creation has pieces reaching into WebKit code and thats why Win/Mac are fine but not Qt/GTK?
The patch at bug #18411 fixes this issue so i'll mark this as dependent on that bug. Feel free to remove the dependency if you think otherwise.
(In reply to comment #9) > The patch at bug #18411 fixes this issue so i'll mark this as dependent on that > bug. Feel free to remove the dependency if you think otherwise. > This bug seems to have been fixed by the patch at #18411. Closing as FIXED.