Bug 19813 - [GTK] WebKit crashes on invalid settings notify callback
Summary: [GTK] WebKit crashes on invalid settings notify callback
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Major
Assignee: Christian Dywan
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2008-06-29 07:31 PDT by Christian Dywan
Modified: 2008-07-17 04:53 PDT (History)
2 users (show)

See Also:


Attachments
Disconnect notify callback on finalize (1.04 KB, patch)
2008-06-29 07:37 PDT, Christian Dywan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Dywan 2008-06-29 07:31:11 PDT
A bug reported for Midori [1] is in fact also a bug in WebKit. The WebView connects a notify callback for its WebSettings. But this callback isn't disconnected when the WebView is finalized which means that the callback is still emitted but the, by then destroyed, web view is invalid.

[1] http://software.twotoasts.de/bugs/index.php?do=details&task_id=15&project=0
Comment 1 Christian Dywan 2008-06-29 07:37:23 PDT
Created attachment 21996 [details]
Disconnect notify callback on finalize
Comment 2 Alp Toker 2008-06-30 23:12:28 PDT
(In reply to comment #1)
> Created an attachment (id=21996) [edit]
> Disconnect notify callback on finalize
> 

Would it be cleaner to re-use webkit_web_view_set_settings() for this purpose?

If we change webkit_web_view_set_settings() to accept a NULL WebKitWebSettings*, it already has the code to disconnect the signal, unref etc. This would allow developers using the API to disassociate the WebView from WebSettings too using the same code path.
Comment 3 Christian Dywan 2008-07-03 15:48:07 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=21996) [edit]
> > Disconnect notify callback on finalize
> > 
> 
> Would it be cleaner to re-use webkit_web_view_set_settings() for this purpose?
> 
> If we change webkit_web_view_set_settings() to accept a NULL
> WebKitWebSettings*, it already has the code to disconnect the signal, unref
> etc. This would allow developers using the API to disassociate the WebView from
> WebSettings too using the same code path.

I thought about this for a bit. I tend to think that's not really useful. Basically the view will always need some kind of WebSettings instance. That means if you could unset it via _set_settings (view, NULL), the view would have two options to cope with this:

a) Unref the old settings and disconnect notify. Then essentially enter an undefined state.

b) Unref the old settings, disconnect notify and immeditately create a new settings instance with default values. The state is 'back to defaults'

I don't find either option too attractive.

This is my estimation. If you, or anyone for that matter, have a use case I will probably not mind to add something in this direction.
Comment 4 Holger Freyther 2008-07-16 11:02:14 PDT
Comment on attachment 21996 [details]
Disconnect notify callback on finalize

Looks sensible.

I only wonder about the (gpointer) G_CALLBACK() mix in regard to handling this callback in the various methods but I have not checked if I we get compile warnings when always using G_CALLBACK...
Comment 5 Christian Dywan 2008-07-17 04:53:37 PDT
Comment on attachment 21996 [details]
Disconnect notify callback on finalize

(In reply to comment #4)
> (From update of attachment 21996 [details] [edit])
> Looks sensible.
> 
> I only wonder about the (gpointer) G_CALLBACK() mix in regard to handling this
> callback in the various methods but I have not checked if I we get compile
> warnings when always using G_CALLBACK...

We can't use G_CALLBACK() when disconnecting the handler, since that unfortunately breaks with the compiler saying that it can't cast »void (*)()« to »void*«.

Committed in revision 35219.