Bug 50173 – corrupted pointer in webkit_web_view_get_resource

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKitGTK

[Kevin Fenzi]

We are seeing a crash here in webkit_web_view_get resource. 
This is webkitgtk-1.3.6. 

The downstream report is: 

https://bugzilla.redhat.com/show_bug.cgi?id=657683

The stack trace is available at: 

https://bugzilla.redhat.com/attachment.cgi?id=463209

Created attachment 96876 [details]
proposed fix

[Xan Lopez]

Comment on attachment 96876 [details]
proposed fix

When we remove the main resource we set its identifier to the empty string "", so unless there's a good reason not to I'd rather set it to "" initially too. That would make this method just fail gracefully and return NULL without the need for a g_return_val_if_fail.

[Martin Robinson]

Comment on attachment 96876 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=96876&action=review

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4882
> +    g_return_val_if_fail(priv->mainResourceIdentifier.data(), 0);
> +

Won't this also print a critical warning?

[Naiem]

Created attachment 96944 [details]
Incorporating review comments

Created attachment 96955 [details]
update ref counts

Patch as suggested by Martin Robinson on irc.
Works nice. Does not crash anymore.

Created attachment 96960 [details]
correct version of previous patch

Sorry. I edited previous patch wrongly by mistake. This one is a proper version.

[Martin Robinson]

Comment on attachment 96960 [details]
correct version of previous patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96960&action=review

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4837
> +    g_object_ref(webView);

Please leave a comment here explaining that we want to keep the WebKitWebView alive until we have finished loading the main resource. You should also mention that the same approach is taken in the Mac port.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4855
>          priv->mainResource = 0;
>      } else
>        g_hash_table_remove(priv->subResources.get(), identifier);
> +
> +    g_object_unref(webView);

Here you should only unref the WebView if the main resource is done loading.

[Martin Robinson]

WebKit1 is no longer around, so I'm going to close this one. Sorry that we weren't able to incorporate it.