2011-01-11 Carlos Garcia Campos <cgarcia@igalia.com>
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jan 2011 11:18:54 +0000 (11:18 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jan 2011 11:18:54 +0000 (11:18 +0000)
        Reviewed by Martin Robinson.

        [GTK] GRefPtr should not be used with Gtk widgets
        https://bugs.webkit.org/show_bug.cgi?id=51241

        GRefPtr breaks the widget life-cycle, the main problem is
        that GRefPtr calls g_object_unref() when it's destroyed,
        which is undesirable for widgets. In gtk+ widgets are created with
        a floating reference and when added to a container, the container
        takes the ownership of the widget consuming the floating
        reference. So you don't usually need to call g_object_ref/unref on
        widgets (only for some operations like reparent a widget) and
        toplevel widgets are destroyed with gtk_widget_destroy().

        * platform/ContextMenuItem.h:
        * platform/gtk/ContextMenuGtk.cpp:
        (WebCore::ContextMenu::ContextMenu):
        (WebCore::ContextMenu::~ContextMenu):
        (WebCore::ContextMenu::setPlatformDescription):
        * platform/gtk/ContextMenuItemGtk.cpp:
        (WebCore::ContextMenuItem::ContextMenuItem):
        (WebCore::ContextMenuItem::releasePlatformDescription):
        (WebCore::ContextMenuItem::type):
        (WebCore::ContextMenuItem::action):
        (WebCore::ContextMenuItem::setAction):
        (WebCore::ContextMenuItem::title):
        (WebCore::ContextMenuItem::setTitle):
        (WebCore::ContextMenuItem::platformSubMenu):
        (WebCore::ContextMenuItem::setSubMenu):
        (WebCore::ContextMenuItem::setChecked):
        (WebCore::ContextMenuItem::setEnabled):
2011-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>

        Reviewed by Martin Robinson.

        [GTK] GRefPtr should not be used with Gtk widgets
        https://bugs.webkit.org/show_bug.cgi?id=51241

        GRefPtr breaks the widget life-cycle, the main problem is
        that GRefPtr calls g_object_unref() when it's destroyed,
        which is undesirable for widgets. In gtk+ widgets are created with
        a floating reference and when added to a container, the container
        takes the ownership of the widget consuming the floating
        reference. So you don't usually need to call g_object_ref/unref on
        widgets (only for some operations like reparent a widget) and
        toplevel widgets are destroyed with gtk_widget_destroy().

        * WebCoreSupport/DragClientGtk.cpp:
        (WebKit::DragClient::DragClient):
        (WebKit::DragClient::~DragClient):
        (WebKit::DragClient::startDrag):
        * WebCoreSupport/DragClientGtk.h:
        * WebCoreSupport/FrameLoaderClientGtk.cpp:
        (WebKit::postCommitFrameViewSetup):
        * webkit/webkitwebview.cpp:
        (webkit_web_view_dispose):
        * webkit/webkitwebviewprivate.h:

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

Source/WebCore/ChangeLog
Source/WebCore/platform/ContextMenuItem.h
Source/WebCore/platform/gtk/ContextMenuGtk.cpp
Source/WebCore/platform/gtk/ContextMenuItemGtk.cpp
WebKit/gtk/ChangeLog
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp
WebKit/gtk/WebCoreSupport/DragClientGtk.h
WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
WebKit/gtk/webkit/webkitwebview.cpp
WebKit/gtk/webkit/webkitwebviewprivate.h

index 1e53505..6aac04a 100644 (file)
@@ -1,3 +1,37 @@
+2011-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Reviewed by Martin Robinson.
+
+        [GTK] GRefPtr should not be used with Gtk widgets
+        https://bugs.webkit.org/show_bug.cgi?id=51241
+
+        GRefPtr breaks the widget life-cycle, the main problem is
+        that GRefPtr calls g_object_unref() when it's destroyed,
+        which is undesirable for widgets. In gtk+ widgets are created with
+        a floating reference and when added to a container, the container
+        takes the ownership of the widget consuming the floating
+        reference. So you don't usually need to call g_object_ref/unref on
+        widgets (only for some operations like reparent a widget) and
+        toplevel widgets are destroyed with gtk_widget_destroy().
+
+        * platform/ContextMenuItem.h:
+        * platform/gtk/ContextMenuGtk.cpp:
+        (WebCore::ContextMenu::ContextMenu):
+        (WebCore::ContextMenu::~ContextMenu):
+        (WebCore::ContextMenu::setPlatformDescription):
+        * platform/gtk/ContextMenuItemGtk.cpp:
+        (WebCore::ContextMenuItem::ContextMenuItem):
+        (WebCore::ContextMenuItem::releasePlatformDescription):
+        (WebCore::ContextMenuItem::type):
+        (WebCore::ContextMenuItem::action):
+        (WebCore::ContextMenuItem::setAction):
+        (WebCore::ContextMenuItem::title):
+        (WebCore::ContextMenuItem::setTitle):
+        (WebCore::ContextMenuItem::platformSubMenu):
+        (WebCore::ContextMenuItem::setSubMenu):
+        (WebCore::ContextMenuItem::setChecked):
+        (WebCore::ContextMenuItem::setEnabled):
+
 2010-10-10  David Hyatt  <hyatt@apple.com>
 
         Reviewed by Simon Fraser.
index 441829f..145d795 100644 (file)
@@ -42,7 +42,6 @@ class NSMenuItem;
 #elif PLATFORM(WIN)
 typedef struct tagMENUITEMINFOW MENUITEMINFO;
 #elif PLATFORM(GTK)
-#include <GRefPtr.h>
 typedef struct _GtkMenuItem GtkMenuItem;
 #elif PLATFORM(QT)
 #include <QAction>
@@ -288,8 +287,6 @@ namespace WebCore {
 #else
 #if PLATFORM(MAC)
         RetainPtr<NSMenuItem> m_platformDescription;
-#elif PLATFORM(GTK)
-        GRefPtr<GtkMenuItem> m_platformDescription;
 #else
         PlatformMenuItemDescription m_platformDescription;
 #endif
index ad50a2d..b34631d 100644 (file)
@@ -28,14 +28,12 @@ namespace WebCore {
 ContextMenu::ContextMenu()
 {
     m_platformDescription = GTK_MENU(gtk_menu_new());
-
-    g_object_ref_sink(G_OBJECT(m_platformDescription));
 }
 
 ContextMenu::~ContextMenu()
 {
     if (m_platformDescription)
-        g_object_unref(m_platformDescription);
+        gtk_widget_destroy(GTK_WIDGET(m_platformDescription));
 }
 
 void ContextMenu::appendItem(ContextMenuItem& item)
@@ -52,10 +50,9 @@ void ContextMenu::setPlatformDescription(PlatformMenuDescription menu)
 {
     ASSERT(menu);
     if (m_platformDescription)
-        g_object_unref(m_platformDescription);
+        gtk_widget_destroy(GTK_WIDGET(m_platformDescription));
 
     m_platformDescription = menu;
-    g_object_ref(m_platformDescription);
 }
 
 PlatformMenuDescription ContextMenu::platformDescription() const
index 4b2b150..fee7a14 100644 (file)
@@ -145,7 +145,7 @@ ContextMenuItem::ContextMenuItem(ContextMenuItemType type, ContextMenuAction act
     m_platformDescription = GTK_MENU_ITEM(gtk_action_create_menu_item(platformAction));
     g_object_unref(platformAction);
 
-    g_object_set_data(G_OBJECT(m_platformDescription.get()), WEBKIT_CONTEXT_MENU_ACTION, GINT_TO_POINTER(action));
+    g_object_set_data(G_OBJECT(m_platformDescription), WEBKIT_CONTEXT_MENU_ACTION, GINT_TO_POINTER(action));
 
     if (subMenu)
         setSubMenu(subMenu);
@@ -169,16 +169,18 @@ ContextMenuItem::~ContextMenuItem()
 
 PlatformMenuItemDescription ContextMenuItem::releasePlatformDescription()
 {
-    return m_platformDescription.leakRef();
+    PlatformMenuItemDescription platformDescription = m_platformDescription;
+    m_platformDescription = 0;
+    return platformDescription;
 }
 
 ContextMenuItemType ContextMenuItem::type() const
 {
-    if (GTK_IS_SEPARATOR_MENU_ITEM(m_platformDescription.get()))
+    if (GTK_IS_SEPARATOR_MENU_ITEM(m_platformDescription))
         return SeparatorType;
-    if (GTK_IS_CHECK_MENU_ITEM(m_platformDescription.get()))
+    if (GTK_IS_CHECK_MENU_ITEM(m_platformDescription))
         return CheckableActionType;
-    if (gtk_menu_item_get_submenu(m_platformDescription.get()))
+    if (gtk_menu_item_get_submenu(m_platformDescription))
         return SubmenuType;
     return ActionType;
 }
@@ -191,41 +193,41 @@ void ContextMenuItem::setType(ContextMenuItemType type)
 
 ContextMenuAction ContextMenuItem::action() const
 {
-    return static_cast<ContextMenuAction>(GPOINTER_TO_INT(g_object_get_data(G_OBJECT(m_platformDescription.get()), WEBKIT_CONTEXT_MENU_ACTION)));
+    return static_cast<ContextMenuAction>(GPOINTER_TO_INT(g_object_get_data(G_OBJECT(m_platformDescription), WEBKIT_CONTEXT_MENU_ACTION)));
 }
 
 void ContextMenuItem::setAction(ContextMenuAction action)
 {
-    g_object_set_data(G_OBJECT(m_platformDescription.get()), WEBKIT_CONTEXT_MENU_ACTION, GINT_TO_POINTER(action));
+    g_object_set_data(G_OBJECT(m_platformDescription), WEBKIT_CONTEXT_MENU_ACTION, GINT_TO_POINTER(action));
 }
 
 String ContextMenuItem::title() const
 {
-    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription.get()));
+    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription));
     return action ? String(gtk_action_get_label(action)) : String();
 }
 
 void ContextMenuItem::setTitle(const String& title)
 {
-    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription.get()));
+    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription));
     if (action)
         gtk_action_set_label(action, title.utf8().data());
 }
 
 PlatformMenuDescription ContextMenuItem::platformSubMenu() const
 {
-    GtkWidget* subMenu = gtk_menu_item_get_submenu(m_platformDescription.get());
+    GtkWidget* subMenu = gtk_menu_item_get_submenu(m_platformDescription);
     return subMenu ? GTK_MENU(subMenu) : 0;
 }
 
 void ContextMenuItem::setSubMenu(ContextMenu* menu)
 {
-    gtk_menu_item_set_submenu(m_platformDescription.get(), GTK_WIDGET(menu->platformDescription()));
+    gtk_menu_item_set_submenu(m_platformDescription, GTK_WIDGET(menu->platformDescription()));
 }
 
 void ContextMenuItem::setChecked(bool shouldCheck)
 {
-    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription.get()));
+    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription));
     if (action && GTK_IS_TOGGLE_ACTION(action))
         gtk_toggle_action_set_active(GTK_TOGGLE_ACTION(action), shouldCheck);
 }
@@ -246,7 +248,7 @@ bool ContextMenuItem::enabled() const
 
 void ContextMenuItem::setEnabled(bool shouldEnable)
 {
-    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription.get()));
+    GtkAction* action = gtk_activatable_get_related_action(GTK_ACTIVATABLE(m_platformDescription));
     if (action)
         gtk_action_set_sensitive(action, shouldEnable);
 }
index 085edd4..5cae072 100644 (file)
@@ -1,3 +1,30 @@
+2011-01-11  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Reviewed by Martin Robinson.
+
+        [GTK] GRefPtr should not be used with Gtk widgets
+        https://bugs.webkit.org/show_bug.cgi?id=51241
+
+        GRefPtr breaks the widget life-cycle, the main problem is
+        that GRefPtr calls g_object_unref() when it's destroyed,
+        which is undesirable for widgets. In gtk+ widgets are created with
+        a floating reference and when added to a container, the container
+        takes the ownership of the widget consuming the floating
+        reference. So you don't usually need to call g_object_ref/unref on
+        widgets (only for some operations like reparent a widget) and
+        toplevel widgets are destroyed with gtk_widget_destroy().
+
+        * WebCoreSupport/DragClientGtk.cpp:
+        (WebKit::DragClient::DragClient):
+        (WebKit::DragClient::~DragClient):
+        (WebKit::DragClient::startDrag):
+        * WebCoreSupport/DragClientGtk.h:
+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
+        (WebKit::postCommitFrameViewSetup):
+        * webkit/webkitwebview.cpp:
+        (webkit_web_view_dispose):
+        * webkit/webkitwebviewprivate.h:
+
 2011-01-10  Martin Robinson  <mrobinson@igalia.com>
 
         Reviewed by Xan Lopez.
index c92c083..836ec28 100644 (file)
@@ -65,15 +65,15 @@ DragClient::DragClient(WebKitWebView* webView)
     , m_dragIconWindow(gtk_window_new(GTK_WINDOW_POPUP))
 {
 #ifdef GTK_API_VERSION_2
-    g_signal_connect(m_dragIconWindow.get(), "expose-event", G_CALLBACK(dragIconWindowDrawEventCallback), this);
+    g_signal_connect(m_dragIconWindow, "expose-event", G_CALLBACK(dragIconWindowDrawEventCallback), this);
 #else
-    g_signal_connect(m_dragIconWindow.get(), "draw", G_CALLBACK(dragIconWindowDrawEventCallback), this);
+    g_signal_connect(m_dragIconWindow, "draw", G_CALLBACK(dragIconWindowDrawEventCallback), this);
 #endif
 }
 
 DragClient::~DragClient()
 {
-    g_signal_handlers_disconnect_by_func(m_dragIconWindow.get(), (gpointer) dragIconWindowDrawEventCallback, this);
+    gtk_widget_destroy(m_dragIconWindow);
 }
 
 void DragClient::willPerformDragDestinationAction(DragDestinationAction, DragData*)
@@ -118,24 +118,24 @@ void DragClient::startDrag(DragImageRef image, const IntPoint& dragImageOrigin,
     if (image) {
         m_dragImage = image;
         IntSize imageSize(cairo_image_surface_get_width(image), cairo_image_surface_get_height(image));
-        gtk_window_resize(GTK_WINDOW(m_dragIconWindow.get()), imageSize.width(), imageSize.height());
+        gtk_window_resize(GTK_WINDOW(m_dragIconWindow), imageSize.width(), imageSize.height());
 
-        if (!gtk_widget_get_realized(m_dragIconWindow.get())) {
-            GdkScreen* screen = gtk_widget_get_screen(m_dragIconWindow.get());
+        if (!gtk_widget_get_realized(m_dragIconWindow)) {
+            GdkScreen* screen = gtk_widget_get_screen(m_dragIconWindow);
 #ifdef GTK_API_VERSION_2
             GdkColormap* rgba = gdk_screen_get_rgba_colormap(screen);
             if (rgba)
-                gtk_widget_set_colormap(m_dragIconWindow.get(), rgba);
+                gtk_widget_set_colormap(m_dragIconWindow, rgba);
 #else
             GdkVisual* visual = gdk_screen_get_rgba_visual(screen);
             if (!visual)
                 visual = gdk_screen_get_system_visual(screen);
-            gtk_widget_set_visual(m_dragIconWindow.get(), visual);
+            gtk_widget_set_visual(m_dragIconWindow, visual);
 #endif // GTK_API_VERSION_2
         }
 
         IntSize origin = eventPos - dragImageOrigin;
-        gtk_drag_set_icon_widget(context, m_dragIconWindow.get(),
+        gtk_drag_set_icon_widget(context, m_dragIconWindow,
                                  origin.width(), origin.height());
     } else
         gtk_drag_set_icon_default(context);
index 9c6d948..3a16ae5 100644 (file)
@@ -59,7 +59,7 @@ namespace WebKit {
     private:
         WebKitWebView* m_webView;
         WebCore::IntPoint m_startPos;
-        GRefPtr<GtkWidget> m_dragIconWindow;
+        GtkWidget* m_dragIconWindow;
         RefPtr<cairo_surface_t> m_dragImage;
     };
 }
index 13cbc59..8f1a819 100644 (file)
@@ -1300,9 +1300,8 @@ static void postCommitFrameViewSetup(WebKitWebFrame *frame, FrameView *view, boo
     g_object_notify(G_OBJECT(containingWindow->priv->viewportAttributes.get()), "valid");
 
     if (priv->currentMenu) {
-        GRefPtr<GtkMenu> menu(priv->currentMenu);
-        priv->currentMenu.clear();
-        gtk_menu_popdown(menu.get());
+        gtk_widget_destroy(GTK_WIDGET(priv->currentMenu));
+        priv->currentMenu = 0;
     }
 
     // Do not allow click counting between main frame loads.
index bd023f8..684fa5b 100644 (file)
@@ -1398,6 +1398,11 @@ static void webkit_web_view_dispose(GObject* object)
         priv->webSettings.clear();
     }
 
+    if (priv->currentMenu) {
+        gtk_widget_destroy(GTK_WIDGET(priv->currentMenu));
+        priv->currentMenu = 0;
+    }
+
     priv->webInspector.clear();
     priv->viewportAttributes.clear();
     priv->webWindowFeatures.clear();
index 29da1fe..03e632a 100644 (file)
@@ -60,7 +60,7 @@ struct _WebKitWebViewPrivate {
     WebKitWebFrame* mainFrame;
     GRefPtr<WebKitWebBackForwardList> backForwardList;
 
-    GRefPtr<GtkMenu> currentMenu;
+    GtkMenu* currentMenu;
     gint lastPopupXPosition;
     gint lastPopupYPosition;