[Webkit-unassigned] [Bug 21837] critical error in Gtk FrameLoaderClient::frameLoaderDestroyed()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21837





------- Comment #9 from zecke at selfish.org  2008-11-25 12:29 PDT -------
(From update of attachment 25422)
> diff --git a/WebKit/gtk/ChangeLog b/WebKit/gtk/ChangeLog
> index 8d05ebc..50c8605 100644
> --- a/WebKit/gtk/ChangeLog
> +++ b/WebKit/gtk/ChangeLog
> @@ -1,3 +1,21 @@
> +2008-11-24  Jan Michael Alonzo  <jmalonzo at webkit.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Original patch by Alp Toker. Updated by Jan Alonzo.
> +
> +        critical error in Gtk FrameLoaderClient::frameLoaderDestroyed()
> +        http://bugs.webkit.org/show_bug.cgi?id=21837


^^^ be a bit more verbose?

IIRC:
   WebCore::Frame and WebCore::FrameLoaderClient have the same lifetime.
Initially we wanted to have WebKitWebFrame have the same lifetime as well
otherwise you have to add plenty of NULL checks to WebKitWebFrame... Think of
someone doing a g_object_ref on the WebKitWebFrame but the page the user
navigated to was left, the WebCore::Frame destroyed....if WebCore::Frame is
destroyed before the WebKitWebFrame we will have to add null checks to
WebKitWebFrame. So what we have instead is something like a circular dependency
and fail to break the circle. (or alternatively the lifetime of WebKitWebFrame
should determine the ifetime of WebCore::Frame).



>  bool FrameLoaderClient::hasWebView() const
>  {
> -    notImplemented();
> -    return true;
> +    return getViewFromFrame(m_frame) != NULL;

No NULL in C++ code, this should never be false?




> diff --git a/WebKit/gtk/webkit/webkitwebframe.cpp b/WebKit/gtk/webkit/webkitwebframe.cpp
> index 6a4c4d5..3d8548f 100644
> --- a/WebKit/gtk/webkit/webkitwebframe.cpp
> +++ b/WebKit/gtk/webkit/webkitwebframe.cpp
> @@ -97,9 +97,11 @@ static void webkit_web_frame_finalize(GObject* object)
>      WebKitWebFrame* frame = WEBKIT_WEB_FRAME(object);
>      WebKitWebFramePrivate* priv = frame->priv;
>  
> -    priv->coreFrame->loader()->cancelAndClear();

FrameLoader is already gone?

>      priv->coreFrame = 0;
>  
> +    // priv->client will be deleted when the WebCore::Frame is actually
> +    // deleted, which may happen slightly later than WebFrame finalization.
> +

this is where the fun starts... A g_object_ref(frame) will keep the
WebCore::Frame
alive as well?


>      g_free(priv->name);
>      g_free(priv->title);
>      g_free(priv->uri);
> @@ -239,7 +241,7 @@ WebKitWebFrame* webkit_web_frame_init_with_web_view(WebKitWebView* webView, HTML
>  
>      priv->webView = webView;
>      priv->client = new WebKit::FrameLoaderClient(frame);
> -    priv->coreFrame = Frame::create(viewPriv->corePage, element, priv->client).releaseRef();
> +    priv->coreFrame = Frame::create(viewPriv->corePage, element, priv->client);


okay, we start with refcount one and the above should be case?


>  
> -    webkit_web_view_stop_loading(WEBKIT_WEB_VIEW(object));
> +    if (Frame* mainFrame = priv->corePage->mainFrame())
> +        mainFrame->loader()->detachFromParent();

hmm... 




> +//    priv->children = 0;

NO!


> -    g_object_unref(priv->backForwardList);
> -    g_signal_handlers_disconnect_by_func(priv->webSettings, (gpointer)webkit_web_view_settings_notify, webView);
> +    g_signal_handlers_disconnect_by_func(priv->webSettings,
> +                                         (gpointer)webkit_web_view_settings_notify, webView);
>      g_object_unref(priv->webSettings);
> +    g_object_unref(priv->backForwardList);

unrelated changes? separate patch then?


>      g_object_unref(priv->webInspector);
> -    g_object_unref(priv->mainFrame);

I have to check this when I'm awake. But we should write something regarding
the lifetime and ownership of the various components.


>      g_object_unref(priv->imContext);
>      gtk_target_list_unref(priv->copy_target_list);
>      gtk_target_list_unref(priv->paste_target_list);
> @@ -1513,7 +1516,11 @@ static void webkit_web_view_init(WebKitWebView* webView)
>  #endif
>  
>      GTK_WIDGET_SET_FLAGS(webView, GTK_CAN_FOCUS);
> -    priv->mainFrame = WEBKIT_WEB_FRAME(webkit_web_frame_new(webView));
> +
> +    // This unintuitively creates a new WebFrame and sets it as the main frame
> +    // of the WebView when the owner element argument is 0.
> +    webkit_web_frame_init_with_web_view(webView, 0);
> +
>      priv->lastPopupXPosition = priv->lastPopupYPosition = -1;
>      priv->editable = false;
>  
> @@ -1878,8 +1885,7 @@ WebKitWebFrame* webkit_web_view_get_main_frame(WebKitWebView* webView)
>  {
>      g_return_val_if_fail(WEBKIT_IS_WEB_VIEW(webView), NULL);
>  
> -    WebKitWebViewPrivate* priv = webView->priv;
> -    return priv->mainFrame;
> +    return kit(core(webView)->mainFrame());


this can only make sense if someone is calling get_main_frame from within the
_finalize of the WebKitWebView? Do we want to support that?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.