Bug 19813 – [GTK] WebKit crashes on invalid settings notify callback

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Christian Dywan]

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

[Christian Dywan]

Created attachment 21996 [details]
Disconnect notify callback on finalize

[Alp Toker]

(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.

[Christian Dywan]

(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.

[Holger Freyther]

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...

[Christian Dywan]

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.