2009-05-27 Xan Lopez <xlopez@igalia.com>
authorxan@webkit.org <xan@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 May 2009 14:52:53 +0000 (14:52 +0000)
committerxan@webkit.org <xan@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 May 2009 14:52:53 +0000 (14:52 +0000)
        Reviewed by Gustavo Noronha.

        https://bugs.webkit.org/show_bug.cgi?id=26039
        [GTK] WebKitWebHistoryItem lifetime fixes

        Modify the management of history items by the BackForward list.

        Having the objects add themselves to a hash table with an extra
        reference made impossible for the cleanup code in the dispose
        method to be ever called in normal conditions, since dispose is
        called before getting rid of the last reference, which the objects
        were making to themselves. Get rid of this extra reference and
        move the responsibility of the cleanup to the BackForward list
        itself, which effectively owns the WebKitWebHistoryItems now.

        Also, update the tests to reflect this change.

        * tests/testwebbackforwardlist.c:
        (test_webkit_web_history_item_lifetime):
        (test_webkit_web_back_forward_list_order):
        (test_webkit_web_back_forward_list_add_item):
        * tests/testwebhistoryitem.c:
        (web_history_item_fixture_setup):
        (web_history_item_fixture_teardown):
        * webkit/webkitprivate.h:
        * webkit/webkitwebbackforwardlist.cpp:
        (webkit_web_back_forward_list_dispose):
        (webkit_web_back_forward_list_class_init):
        * webkit/webkitwebhistoryitem.cpp:
        (webkit_history_items):
        (webkit_history_item_add):
        (webkit_web_history_item_dispose):
        (WebKit::kit):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@44228 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKit/gtk/ChangeLog
WebKit/gtk/tests/testwebbackforwardlist.c
WebKit/gtk/tests/testwebhistoryitem.c
WebKit/gtk/webkit/webkitprivate.h
WebKit/gtk/webkit/webkitwebbackforwardlist.cpp
WebKit/gtk/webkit/webkitwebhistoryitem.cpp

index f61f13760a484eceb33dfe65865f7c5fd48616e6..45829a7d9846bdf8693a9ae24ca4e1efbb9f4f31 100644 (file)
@@ -1,3 +1,39 @@
+2009-05-27  Xan Lopez  <xlopez@igalia.com>
+
+        Reviewed by Gustavo Noronha.
+
+        https://bugs.webkit.org/show_bug.cgi?id=26039
+        [GTK] WebKitWebHistoryItem lifetime fixes
+
+        Modify the management of history items by the BackForward list.
+
+        Having the objects add themselves to a hash table with an extra
+        reference made impossible for the cleanup code in the dispose
+        method to be ever called in normal conditions, since dispose is
+        called before getting rid of the last reference, which the objects
+        were making to themselves. Get rid of this extra reference and
+        move the responsibility of the cleanup to the BackForward list
+        itself, which effectively owns the WebKitWebHistoryItems now.
+
+        Also, update the tests to reflect this change.
+
+        * tests/testwebbackforwardlist.c:
+        (test_webkit_web_history_item_lifetime):
+        (test_webkit_web_back_forward_list_order):
+        (test_webkit_web_back_forward_list_add_item):
+        * tests/testwebhistoryitem.c:
+        (web_history_item_fixture_setup):
+        (web_history_item_fixture_teardown):
+        * webkit/webkitprivate.h:
+        * webkit/webkitwebbackforwardlist.cpp:
+        (webkit_web_back_forward_list_dispose):
+        (webkit_web_back_forward_list_class_init):
+        * webkit/webkitwebhistoryitem.cpp:
+        (webkit_history_items):
+        (webkit_history_item_add):
+        (webkit_web_history_item_dispose):
+        (WebKit::kit):
+
 2009-05-26  Xan Lopez  <xlopez@igalia.com>
 
         Reviewed by Jan Alonzo.
index 8d53c3e8e2ed75ad8e39758c2879d6bb2c219e71..095dac7a231f21ab3cf4d803bc82bfd9c6e8959d 100644 (file)
@@ -93,16 +93,12 @@ static void test_webkit_web_history_item_lifetime(void)
 
     g_list_free(forwardList);
     g_list_free(backList);
-    g_object_unref(item1);
-    g_object_unref(item2);
-    g_object_unref(item3);
-    g_object_unref(item4);
-
-    g_object_ref(backForwardList);
-    g_object_unref(webView);
-
+    g_assert_cmpint(G_OBJECT(item1)->ref_count, ==, 1);
+    g_assert_cmpint(G_OBJECT(item2)->ref_count, ==, 1);
+    g_assert_cmpint(G_OBJECT(item3)->ref_count, ==, 1);
+    g_assert_cmpint(G_OBJECT(item4)->ref_count, ==, 1);
     g_assert_cmpint(G_OBJECT(backForwardList)->ref_count, ==, 1);
-    g_object_unref(backForwardList);
+    g_object_unref(webView);
 }
 
 static void test_webkit_web_back_forward_list_order(void)
@@ -187,10 +183,6 @@ static void test_webkit_web_back_forward_list_order(void)
     g_assert_cmpstr(webkit_web_history_item_get_uri(currentItem), ==, "http://example.com/2/");
     g_assert_cmpstr(webkit_web_history_item_get_title(currentItem), ==, "Site 2");
 
-    g_object_unref(item1);
-    g_object_unref(item2);
-    g_object_unref(item3);
-    g_object_unref(item4);
     g_list_free(forwardList);
     g_object_unref(webView);
 }
@@ -238,7 +230,6 @@ static void test_webkit_web_back_forward_list_add_item(void)
     addItem2 = webkit_web_history_item_new_with_data("http://example.com/2/", "Added site 2");
     webkit_web_back_forward_list_add_item(webBackForwardList, addItem2);
     g_assert(webkit_web_back_forward_list_contains_item(webBackForwardList, addItem2));
-    g_object_unref(addItem2);
 
     // Check that the added item is new current item.
     currentItem = webkit_web_back_forward_list_get_current_item(webBackForwardList);
@@ -262,7 +253,6 @@ static void test_webkit_web_back_forward_list_add_item(void)
     g_assert(webkit_web_view_can_go_forward(webView));
     g_assert(!webkit_web_view_can_go_back(webView));
 
-    g_object_unref(addItem1);
     g_object_unref(webView);
 }
 
index 6038c64703e66482e40e3b3816b402dc7c8de57a..b940afb804023a5f7f849317ff8448fe54b64a2c 100644 (file)
@@ -31,7 +31,7 @@ static void web_history_item_fixture_setup(WebHistoryItemFixture* fixture,
                                            gconstpointer data)
 {
     fixture->item = webkit_web_history_item_new_with_data("http://example.com/", "Example1");
-    g_assert_cmpint(G_OBJECT(fixture->item)->ref_count, == , 2);
+    g_assert_cmpint(G_OBJECT(fixture->item)->ref_count, == , 1);
     g_assert(fixture->item != NULL);
 }
 
@@ -39,8 +39,6 @@ static void web_history_item_fixture_teardown(WebHistoryItemFixture* fixture,
                                               gconstpointer data)
 {
     g_assert(fixture->item != NULL);
-    g_assert_cmpint(G_OBJECT(fixture->item)->ref_count, ==, 2);
-    g_object_unref(fixture->item);
     g_assert_cmpint(G_OBJECT(fixture->item)->ref_count, ==, 1);
 }
 
index 93bb8204c6edd68f3bf01cd8e71f6661997f683f..26924dc7be49afece46be62c80d7a85380641686 100644 (file)
@@ -215,6 +215,9 @@ extern "C" {
 
     bool
     webkit_web_view_use_primary_for_paste(WebKitWebView* web_view);
+
+    GHashTable*
+    webkit_history_items(void);
 }
 
 #endif
index 42da8ac051b5201cee3c2c8b17e2184b4f0daf84..6d38130f236ffc53bc8994e5aae668e2ddfcb7d9 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008 Jan Michael C. Alonzo
+ * Copyright (C) 2009 Igalia S.L.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -53,14 +54,37 @@ using namespace WebKit;
 
 struct _WebKitWebBackForwardListPrivate {
     WebCore::BackForwardList* backForwardList;
+    gboolean disposed;
 };
 
 #define WEBKIT_WEB_BACK_FORWARD_LIST_GET_PRIVATE(obj)    (G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_WEB_BACK_FORWARD_LIST, WebKitWebBackForwardListPrivate))
 
 G_DEFINE_TYPE(WebKitWebBackForwardList, webkit_web_back_forward_list, G_TYPE_OBJECT);
 
+static void webkit_web_back_forward_list_dispose(GObject* object)
+{
+    WebKitWebBackForwardList* list = WEBKIT_WEB_BACK_FORWARD_LIST(object);
+    WebCore::BackForwardList* backForwardList = core(list);
+    WebKitWebBackForwardListPrivate* priv = list->priv;
+
+    if (!priv->disposed) {
+        priv->disposed = true;
+
+        WebCore::HistoryItemVector items = backForwardList->entries();
+        GHashTable* table = webkit_history_items();
+        for (unsigned i = 0; i < items.size(); i++)
+            g_hash_table_remove(table, items[i].get());
+    }
+
+    G_OBJECT_CLASS(webkit_web_back_forward_list_parent_class)->dispose(object);
+}
+
 static void webkit_web_back_forward_list_class_init(WebKitWebBackForwardListClass* klass)
 {
+    GObjectClass* object_class = G_OBJECT_CLASS(klass);
+
+    object_class->dispose = webkit_web_back_forward_list_dispose;
+
     g_type_class_add_private(klass, sizeof(WebKitWebBackForwardListPrivate));
 }
 
@@ -395,6 +419,10 @@ void webkit_web_back_forward_list_set_limit(WebKitWebBackForwardList* webBackFor
  *
  * Adds the item to the #WebKitWebBackForwardList.
  *
+ * The @webBackForwardList will steal the reference of the
+ * @webHistoryItem, so you don't need to unref it after adding it to
+ * the list.
+ *
  * Since: 1.1.1
  */
 void webkit_web_back_forward_list_add_item(WebKitWebBackForwardList *webBackForwardList, WebKitWebHistoryItem *webHistoryItem)
index e2da55bf53bd5966b8c1f785b80bff5341d77c5e..c70b64f9d288f4a81e3d523f52c130d808773ab0 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008, 2009 Jan Michael C. Alonzo
+ * Copyright (C) 2009 Igalia S.L.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -79,37 +80,28 @@ static void webkit_web_history_item_set_property(GObject* object, guint prop_id,
 
 static void webkit_web_history_item_get_property(GObject* object, guint prop_id, GValue* value, GParamSpec* pspec);
 
-static GHashTable* webkit_history_items()
+GHashTable* webkit_history_items()
 {
     static GHashTable* historyItems = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_object_unref);
     return historyItems;
 }
 
-static void webkit_history_item_add(WebKitWebHistoryItem* webHistoryItem, WebCore::HistoryItem* historyItem)
+void webkit_history_item_add(WebKitWebHistoryItem* webHistoryItem, WebCore::HistoryItem* historyItem)
 {
     g_return_if_fail(WEBKIT_IS_WEB_HISTORY_ITEM(webHistoryItem));
 
     GHashTable* table = webkit_history_items();
-
-    g_hash_table_insert(table, historyItem, g_object_ref(webHistoryItem));
+    g_hash_table_insert(table, historyItem, webHistoryItem);
 }
 
 static void webkit_web_history_item_dispose(GObject* object)
 {
     WebKitWebHistoryItem* webHistoryItem = WEBKIT_WEB_HISTORY_ITEM(object);
     WebKitWebHistoryItemPrivate* priv = webHistoryItem->priv;
-    WebCore::HistoryItem* item = core(webHistoryItem);
 
     if (!priv->disposed) {
-        GHashTable* table = webkit_history_items();
-
-        g_hash_table_remove(table, item);
+        WebCore::HistoryItem* item = core(webHistoryItem);
         item->deref();
-
-        /* destroy table if empty */
-        if (!g_hash_table_size(table))
-            g_hash_table_destroy(table);
-
         priv->disposed = true;
     }
 
@@ -496,11 +488,8 @@ WebKitWebHistoryItem* WebKit::kit(PassRefPtr<WebCore::HistoryItem> historyItem)
     g_return_val_if_fail(historyItem, NULL);
 
     RefPtr<WebCore::HistoryItem> item = historyItem;
-
-    WebKitWebHistoryItem* webHistoryItem;
     GHashTable* table = webkit_history_items();
-
-    webHistoryItem = (WebKitWebHistoryItem*) g_hash_table_lookup(table, item.get());
+    WebKitWebHistoryItem* webHistoryItem = (WebKitWebHistoryItem*) g_hash_table_lookup(table, item.get());
 
     if (!webHistoryItem) {
         webHistoryItem = WEBKIT_WEB_HISTORY_ITEM(g_object_new(WEBKIT_TYPE_WEB_HISTORY_ITEM, NULL));