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
+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.
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)
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);
}
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);
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);
}
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);
}
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);
}
bool
webkit_web_view_use_primary_for_paste(WebKitWebView* web_view);
+
+ GHashTable*
+ webkit_history_items(void);
}
#endif
/*
* 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
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));
}
*
* 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)
/*
* 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
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;
}
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));