Summary: | [GTK] Closing inspector window crashes wk | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Claudio Saavedra <csaavedra> | ||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | berto, cgarcia, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Claudio Saavedra
2013-02-26 03:23:56 PST
Created attachment 205995 [details]
Patch
This happens because when we destroy priv->corePage in
webkit_web_view_dispose() we're triggering the deletion of
InspectorFrontendClient before it can clean up itself.
This can be fixed by waiting until the dispose method finishes before
actually destroying the corePage object. This way we ensure that the
webView's "destroy" signal is emitted first.
Comment on attachment 205995 [details]
Patch
I think it would be cleaner to delete the page in finalize instead of dispose. Since WebKitWebView uses the placement new syntax, you can make page a OwnPtr and it will be automatically deleted in finalize.
(In reply to comment #2) > I think it would be cleaner to delete the page in finalize instead > of dispose. Since WebKitWebView uses the placement new syntax, you > can make page a OwnPtr and it will be automatically deleted in > finalize. The problem is that the corePage pointer will still be != 0 in the meantime, and that will produce a crash during the disposal of the parent class: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff3d2f5e8 in WebCore::AXObjectCache::rootObject (this=0x0) at ../../Source/WebCore/accessibility/AXObjectCache.cpp:415 415 return getOrCreate(m_document->view()); #0 0x00007ffff3d2f5e8 in WebCore::AXObjectCache::rootObject (this=0x0) at ../../Source/WebCore/accessibility/AXObjectCache.cpp:415 #1 0x00007ffff3b8f2df in webkit_web_view_get_accessible (widget=0x5da2a0) at ../../Source/WebKit/gtk/webkit/webkitwebview.cpp:1416 #2 0x00007ffff1c34ee9 in gtk_container_accessible_real_remove_gtk (container=0x449320, widget=0x5da2a0, data=0x1a8a0e0) at gtkcontaineraccessible.c:137 [...] #7 0x00007ffff1a2abb4 in gtk_container_remove (container=<optimized out>, widget=widget@entry=0x5da2a0) at gtkcontainer.c:1546 #8 0x00007ffff1bf6cc2 in gtk_widget_dispose (object=0x5da2a0) at gtkwidget.c:10254 A different alternative would be to run parent->dispose() first. That seems to work fine it doesn't look like the rest of the code in that method would be affected by that. (In reply to comment #3) > (In reply to comment #2) > > I think it would be cleaner to delete the page in finalize instead > > of dispose. Since WebKitWebView uses the placement new syntax, you > > can make page a OwnPtr and it will be automatically deleted in > > finalize. > > The problem is that the corePage pointer will still be != 0 in the > meantime, and that will produce a crash during the disposal of the > parent class: > > Program received signal SIGSEGV, Segmentation fault. > 0x00007ffff3d2f5e8 in WebCore::AXObjectCache::rootObject (this=0x0) > at ../../Source/WebCore/accessibility/AXObjectCache.cpp:415 > 415 return getOrCreate(m_document->view()); > #0 0x00007ffff3d2f5e8 in WebCore::AXObjectCache::rootObject (this=0x0) > at ../../Source/WebCore/accessibility/AXObjectCache.cpp:415 > #1 0x00007ffff3b8f2df in webkit_web_view_get_accessible (widget=0x5da2a0) > at ../../Source/WebKit/gtk/webkit/webkitwebview.cpp:1416 > #2 0x00007ffff1c34ee9 in gtk_container_accessible_real_remove_gtk (container=0x449320, widget=0x5da2a0, data=0x1a8a0e0) > at gtkcontaineraccessible.c:137 > [...] > #7 0x00007ffff1a2abb4 in gtk_container_remove (container=<optimized out>, widget=widget@entry=0x5da2a0) > at gtkcontainer.c:1546 > #8 0x00007ffff1bf6cc2 in gtk_widget_dispose (object=0x5da2a0) at gtkwidget.c:10254 hmm, unfortunate. > A different alternative would be to run parent->dispose() first. That > seems to work fine it doesn't look like the rest of the code in that > method would be affected by that. It could be easier, yes, adding a comment explaining why parent dispose is called first. Created attachment 208556 [details] Patch (In reply to comment #4) > It could be easier, yes, adding a comment explaining why parent > dispose is called first. Here it is. Comment on attachment 208556 [details]
Patch
Thanks
Fixed in <http://trac.webkit.org/changeset/153991>. Comment on attachment 208556 [details]
Patch
This appears to have caused a few API tests to fail:
(./Tools/gtk/../Scripts/../../WebKitBuild/Release/Programs/unittests/testloading:9117): GLib-CRITICAL **: g_hash_table_lookup_extended: assertion `hash_table != NULL' failed
(./Tools/gtk/../Scripts/../../WebKitBuild/Release/Programs/unittests/testhttpbackend:9159): GLib-CRITICAL **: g_hash_table_lookup_extended: assertion `hash_table != NULL' failed
(In reply to comment #8) > (./Tools/gtk/../Scripts/../../WebKitBuild/Release/Programs/unittests/testloading:9117): GLib-CRITICAL **: g_hash_table_lookup_extended: assertion `hash_table != NULL' failed > (./Tools/gtk/../Scripts/../../WebKitBuild/Release/Programs/unittests/testhttpbackend:9159): GLib-CRITICAL **: g_hash_table_lookup_extended: assertion `hash_table != NULL' failed I think this is because of putting the code after the parent's dispose call. Going back to my original solution should fix this. I'll upload a new patch. Created attachment 208630 [details]
Patch
Here's the patch with the original fix.
I guess we need to roll out the patch first (In reply to comment #11) > I guess we need to roll out the patch first This one goes on top of the other already. It doesn't seem to apply in EWS bots reopening Created attachment 208658 [details]
Patch
Resubmitting patch for EWS processing.
Fixed in <http://trac.webkit.org/changeset/154014>. *** Bug 48203 has been marked as a duplicate of this bug. *** |