[webkit-reviews] review denied: [Bug 23194] [GTK] fix crashers : [Attachment 26531] suggested fix

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has denied Benjamin Otte <otte at gnome.org>'s
request for review:
Bug 23194: [GTK] fix crashers
https://bugs.webkit.org/show_bug.cgi?id=23194

Attachment 26531: suggested fix
https://bugs.webkit.org/attachment.cgi?id=26531&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>
Please split this patch into two parts. There is no relationship between the
first crash and the second one.



>      webkit_web_policy_decision_ignore(decision);
>  
> +#if 0
>      if (!priv->isCancelled)
>	  
(core(priv->frame)->loader()->*(priv->framePolicyFunction))(WebCore::PolicyDown
load);
> +#endif
>  }

what happens if you just remove the call to webkit_web_policy_decision_ignore? 






>  
>  void webkit_web_policy_decision_cancel(WebKitWebPolicyDecision* decision)
> diff --git a/WebKit/gtk/webkit/webkitwebview.cpp
b/WebKit/gtk/webkit/webkitwebview.cpp
> index 91f3b80..d3f7cd7 100644
> --- a/WebKit/gtk/webkit/webkitwebview.cpp
> +++ b/WebKit/gtk/webkit/webkitwebview.cpp
> @@ -807,31 +807,55 @@ static void
webkit_web_view_real_paste_clipboard(WebKitWebView* webView)
>      frame->editor()->command("Paste").execute();
>  }
>  
> -static void webkit_web_view_finalize(GObject* object)
> +static void webkit_web_view_dispose(GObject* object)

okay.


> -    webkit_web_view_stop_loading(WEBKIT_WEB_VIEW(object));
> +    if (priv->corePage) {
> +	   webkit_web_view_stop_loading(WEBKIT_WEB_VIEW(object));
>  
> -    core(priv->mainFrame)->loader()->detachChildren();
> -    delete priv->corePage;
> +	   core(priv->mainFrame)->loader()->detachChildren();
> +	   delete priv->corePage;
> +	   priv->corePage = 0;
> +    }


alternatively you can write this as
if (priv->mainFrame)
    core(priv->mainFrame)->loader()->detachChildren()
delete priv->corePage;
priv->corePage;


>  
> -    if (priv->horizontalAdjustment)
> +    if (priv->horizontalAdjustment) {
>	   g_object_unref(priv->horizontalAdjustment);
> -    if (priv->verticalAdjustment)
> +	   priv->horizontalAdjustment = NULL;

Do not use NULL here and at the other occasions.