Changeset 39575 in webkit


Ignore:
Timestamp:
Jan 3, 2009 11:52:56 AM (15 years ago)
Author:
zecke@webkit.org
Message:

[GTK] Fix the reference counting of WebKitWebFrames

The ownership is the following: WebKitWebView owns a WebCore::Page.
WebKitWebView is creating one WebKitWebFrame which will be the
mainFrame of the WebCore::Page (having the reference on the Frame).

The FrameLoaderClient has the reference of the WebKitWebFrame for
the main frame and also any other frame. This means when the
WebCore::Frame goes away the FrameLoaderClient will go away which
will normally remove the last reference of the WebKitWebFrame. Because
an API user might have g_object_ref'ed the WebKitWebFrame null
checks had to be added to WebKitWebFrame.

For WebCore::Frames created by the FrameLoaderClient the ownership
will be passed down to the FrameTree, the WebKitWebFrame is not holding
a reference to the WebCore::Frame.

Do not g_object_unref the mainFrame in the destructor of the
WebKitWebFrame as this will happen from within the WebCore::Page
destruction. Do not hold a reference to the WebCore::Frame (circle) in
WebKitWebFrame, add null checks as the WebCore::Frame might have gone
away. Do not keep track of the FrameLoaderClient in the private
structures as it was mostly unusued.

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

Location:
trunk/WebKit/gtk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKit/gtk/ChangeLog

    r39570 r39575  
     12009-01-03  Holger Hans Peter Freyther  <zecke@selfish.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        [GTK] Fix the reference counting of WebKitWebFrames
     6       
     7        The ownership is the following: WebKitWebView owns a WebCore::Page.
     8        WebKitWebView is creating one WebKitWebFrame which will be the
     9        mainFrame of the WebCore::Page (having the reference on the Frame).
     10       
     11        The FrameLoaderClient has the reference of the WebKitWebFrame for
     12        the main frame and also any other frame. This means when the
     13        WebCore::Frame goes away the FrameLoaderClient will go away which
     14        will normally remove the last reference of the WebKitWebFrame. Because
     15        an API user might have g_object_ref'ed the WebKitWebFrame null
     16        checks had to be added to WebKitWebFrame.
     17       
     18        For WebCore::Frames created by the FrameLoaderClient the ownership
     19        will be passed down to the FrameTree, the WebKitWebFrame is not holding
     20        a reference to the WebCore::Frame.
     21       
     22        Do not g_object_unref the mainFrame in the destructor of the
     23        WebKitWebFrame as this will happen from within the WebCore::Page
     24        destruction. Do not hold a reference to the WebCore::Frame (circle) in
     25        WebKitWebFrame, add null checks as the WebCore::Frame might have gone
     26        away. Do not keep track of the FrameLoaderClient in the private
     27        structures as it was mostly unusued.
     28       
     29        https://bugs.webkit.org/show_bug.cgi?id=21837
     30
     31        * WebCoreSupport/FrameLoaderClientGtk.cpp:
     32        (WebKit::FrameLoaderClient::frameLoaderDestroyed):
     33        (WebKit::FrameLoaderClient::createFrame):
     34        * tests/main.c: Add test case.
     35        (test_webkit_web_frame_create_destroy):
     36        (test_webkit_web_frame_lifetime):
     37        (main):
     38        * webkit/webkitprivate.cpp:
     39        (WebKit::core):
     40        * webkit/webkitprivate.h:
     41        * webkit/webkitwebframe.cpp:
     42        * webkit/webkitwebview.cpp:
     43
    1442009-01-02  Holger Hans Peter Freyther  <zecke@selfish.org>
    245

  • trunk/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp

    r39570 r39575  
    252252void FrameLoaderClient::frameLoaderDestroyed()
    253253{
     254    webkit_web_frame_core_frame_gone(m_frame);
    254255    g_object_unref(m_frame);
    255256    m_frame = 0;
    … …  
    386387
    387388    ASSERT(core(getViewFromFrame(webFrame())) == coreFrame->page());
    388     WebKitWebFrame* gtkFrame = WEBKIT_WEB_FRAME(webkit_web_frame_init_with_web_view(getViewFromFrame(webFrame()), ownerElement));
    389     RefPtr<Frame> childFrame(adoptRef(core(gtkFrame)));
     389
     390    RefPtr<Frame> childFrame = webkit_web_frame_init_with_web_view(getViewFromFrame(webFrame()), ownerElement);
    390391
    391392    coreFrame->tree()->appendChild(childFrame);

  • trunk/WebKit/gtk/tests/main.c

    r39275 r39575  
    2424#if GLIB_CHECK_VERSION(2, 16, 0) && GTK_CHECK_VERSION(2, 14, 0)
    2525
     26static void test_webkit_web_frame_create_destroy(void)
     27{
     28    WebKitWebView* webView;
     29    g_test_bug("21837");
     30
     31    webView = WEBKIT_WEB_VIEW(webkit_web_view_new());
     32    g_object_ref_sink(webView);
     33    g_assert_cmpint(G_OBJECT(webView)->ref_count, ==, 1);
     34
     35    // This crashed with the original version
     36    g_object_unref(webView);
     37}
     38
     39static void test_webkit_web_frame_lifetime(void)
     40{
     41    WebKitWebView* webView;
     42    WebKitWebFrame* webFrame;
     43    g_test_bug("21837");
     44
     45    webView = WEBKIT_WEB_VIEW(webkit_web_view_new());
     46    g_object_ref_sink(webView);
     47    g_assert_cmpint(G_OBJECT(webView)->ref_count, ==, 1);
     48    webFrame = webkit_web_view_get_main_frame(webView);
     49    g_assert_cmpint(G_OBJECT(webFrame)->ref_count, ==, 1);
     50
     51    // Add dummy reference on the WebKitWebFrame to keep it alive
     52    g_object_ref(webFrame);
     53    g_assert_cmpint(G_OBJECT(webFrame)->ref_count, ==, 2);
     54
     55    // This crashed with the original version
     56    g_object_unref(webView);
     57
     58    // Make sure that the frame got deleted as well. We did this
     59    // by adding an extra ref on the WebKitWebFrame and we should
     60    // be the one holding the last reference.
     61    g_assert_cmpint(G_OBJECT(webFrame)->ref_count, ==, 1);
     62    g_object_unref(webFrame);
     63}
     64
    2665int main(int argc, char** argv)
    2766{
    … …  
    3069
    3170    g_test_bug_base("https://bugs.webkit.org/");
     71    g_test_add_func("/webkit/webview/create_destroy", test_webkit_web_frame_create_destroy);
     72    g_test_add_func("/webkit/webframe/lifetime", test_webkit_web_frame_lifetime);
    3273
    3374    return g_test_run ();

  • trunk/WebKit/gtk/webkit/webkitprivate.cpp

    r39421 r39575  
    5353
    5454    WebKitWebFramePrivate* priv = frame->priv;
    55     return priv ? priv->coreFrame.get() : 0;
     55    return priv ? priv->coreFrame : 0;
    5656}
    5757

  • trunk/WebKit/gtk/webkit/webkitprivate.h

    r39421 r39575  
    107107    typedef struct _WebKitWebFramePrivate WebKitWebFramePrivate;
    108108    struct _WebKitWebFramePrivate {
    109         WTF::RefPtr<WebCore::Frame> coreFrame;
    110         WebCore::FrameLoaderClient* client;
     109        WebCore::Frame* coreFrame;
    111110        WebKitWebView* webView;
    112111
    … …  
    116115    };
    117116
    118     WebKitWebFrame*
     117    PassRefPtr<WebCore::Frame>
    119118    webkit_web_frame_init_with_web_view(WebKitWebView*, WebCore::HTMLFrameOwnerElement*);
     119
     120    void
     121    webkit_web_frame_core_frame_gone(WebKitWebFrame*);
    120122
    121123    WebKitWebHistoryItem*

  • trunk/WebKit/gtk/webkit/webkitwebframe.cpp

    r39532 r39575  
    112112}
    113113
     114// Called from the FrameLoaderClient when it is destroyed. Normally
     115// the unref in the FrameLoaderClient is destroying this object as
     116// well but due reference counting a user might have added a reference...
     117void webkit_web_frame_core_frame_gone(WebKitWebFrame* frame)
     118{
     119    ASSERT(WEBKIT_IS_WEB_FRAME(frame));
     120    frame->priv->coreFrame = 0;
     121}
     122
    114123static void webkit_web_frame_finalize(GObject* object)
    115124{
    … …  
    117126    WebKitWebFramePrivate* priv = frame->priv;
    118127
    119     priv->coreFrame->loader()->cancelAndClear();
    120     priv->coreFrame = 0;
     128    if (priv->coreFrame) {
     129        priv->coreFrame->loader()->cancelAndClear();
     130        priv->coreFrame = 0;
     131    }
    121132
    122133    g_free(priv->name);
    … …  
    244255
    245256    priv->webView = webView;
    246     priv->client = new WebKit::FrameLoaderClient(frame);
    247     priv->coreFrame = Frame::create(viewPriv->corePage, 0, priv->client).get();
     257    WebKit::FrameLoaderClient* client = new WebKit::FrameLoaderClient(frame);
     258    priv->coreFrame = Frame::create(viewPriv->corePage, 0, client).get();
    248259    priv->coreFrame->init();
    249260
    … …  
    251262}
    252263
    253 WebKitWebFrame* webkit_web_frame_init_with_web_view(WebKitWebView* webView, HTMLFrameOwnerElement* element)
     264PassRefPtr<Frame> webkit_web_frame_init_with_web_view(WebKitWebView* webView, HTMLFrameOwnerElement* element)
    254265{
    255266    WebKitWebFrame* frame = WEBKIT_WEB_FRAME(g_object_new(WEBKIT_TYPE_WEB_FRAME, NULL));
    … …  
    258269
    259270    priv->webView = webView;
    260     priv->client = new WebKit::FrameLoaderClient(frame);
    261     priv->coreFrame = Frame::create(viewPriv->corePage, element, priv->client).releaseRef();
    262 
    263     return frame;
     271    WebKit::FrameLoaderClient* client = new WebKit::FrameLoaderClient(frame);
     272
     273    RefPtr<Frame> coreFrame = Frame::create(viewPriv->corePage, element, client);
     274    priv->coreFrame = coreFrame.get();
     275
     276    return coreFrame.release();
    264277}
    265278
    … …  
    333346
    334347    Frame* coreFrame = core(frame);
    335     ASSERT(coreFrame);
     348    if (!coreFrame)
     349        return "";
    336350
    337351    String string = coreFrame->tree()->name();
    … …  
    353367
    354368    Frame* coreFrame = core(frame);
    355     ASSERT(coreFrame);
     369    if (!coreFrame)
     370        return NULL;
    356371
    357372    return kit(coreFrame->tree()->parent());
    … …  
    375390
    376391    Frame* coreFrame = core(frame);
    377     ASSERT(coreFrame);
     392    if (!coreFrame)
     393        return;
    378394
    379395    // TODO: Use the ResourceRequest carried by WebKitNetworkRequest when it is implemented.
    … …  
    393409
    394410    Frame* coreFrame = core(frame);
    395     ASSERT(coreFrame);
     411    if (!coreFrame)
     412        return;
    396413
    397414    coreFrame->loader()->stopAllLoaders();
    … …  
    409426
    410427    Frame* coreFrame = core(frame);
    411     ASSERT(coreFrame);
     428    if (!coreFrame)
     429        return;
    412430
    413431    coreFrame->loader()->reload();
    … …  
    437455
    438456    Frame* coreFrame = core(frame);
    439     ASSERT(coreFrame);
     457    if (!coreFrame)
     458        return NULL;
    440459
    441460    String nameString = String::fromUTF8(name);
    … …  
    457476
    458477    Frame* coreFrame = core(frame);
    459     ASSERT(coreFrame);
     478    if (!coreFrame)
     479        return NULL;
    460480
    461481    return toGlobalRef(coreFrame->script()->globalObject()->globalExec());
    … …  
    473493
    474494    Frame* coreFrame = core(frame);
    475     ASSERT(coreFrame);
     495    if (!coreFrame)
     496        return NULL;
    476497
    477498    GSList* children = NULL;
    … …  
    497518
    498519    Frame* coreFrame = core(frame);
    499     ASSERT(coreFrame);
     520    if (!coreFrame)
     521        return g_strdup("");
    500522
    501523    FrameView* view = coreFrame->view();
    … …  
    520542
    521543    Frame* coreFrame = core(frame);
    522     ASSERT(coreFrame);
     544    if (!coreFrame)
     545        return g_strdup("");
    523546
    524547    FrameView* view = coreFrame->view();
    … …  
    574597
    575598    Frame* coreFrame = core(frame);
    576     ASSERT(coreFrame);
     599    if (!coreFrame)
     600        return;
    577601
    578602    PrintContext printContext(coreFrame);
    … …  
    610634bool webkit_web_frame_pause_animation(WebKitWebFrame* frame, const gchar* name, double time, const gchar* element)
    611635{
     636    ASSERT(core(frame));
    612637    Element* coreElement = core(frame)->document()->getElementById(AtomicString(element));
    613638    if (!coreElement || !coreElement->renderer())
    … …  
    618643bool webkit_web_frame_pause_transition(WebKitWebFrame* frame, const gchar* name, double time, const gchar* element)
    619644{
     645    ASSERT(core(frame));
    620646    Element* coreElement = core(frame)->document()->getElementById(AtomicString(element));
    621647    if (!coreElement || !coreElement->renderer())

  • trunk/WebKit/gtk/webkit/webkitwebview.cpp

    r39532 r39575  
    827827    g_object_unref(priv->webInspector);
    828828    g_object_unref(priv->webWindowFeatures);
    829     g_object_unref(priv->mainFrame);
    830829    g_object_unref(priv->imContext);
    831830    gtk_target_list_unref(priv->copy_target_list);
Note: See TracChangeset for help on using the changeset viewer.