From 25f69bffe60bdd9c3dbba46967d30eb73b4f03b3 Mon Sep 17 00:00:00 2001 From: Javier Date: Sun, 9 Feb 2014 01:31:36 +0100 Subject: fix some leaks and avoid exporting secondary menubars --- module/appmenu.cc | 1 + module/menuproxy.cc | 6 ++- module/topmenumenubarimpl.cc | 101 ++++++++++++++++++++++++++++++++++--------- module/topmenumenubarimpl.h | 12 ++--- 4 files changed, 94 insertions(+), 26 deletions(-) diff --git a/module/appmenu.cc b/module/appmenu.cc index 4902533..63407ec 100644 --- a/module/appmenu.cc +++ b/module/appmenu.cc @@ -15,6 +15,7 @@ AppMenu::~AppMenu() { if (m_menu) { gtk_widget_destroy(GTK_WIDGET(m_menu)); + g_object_unref(m_menu); } } diff --git a/module/menuproxy.cc b/module/menuproxy.cc index 5536bf5..a59a576 100644 --- a/module/menuproxy.cc +++ b/module/menuproxy.cc @@ -53,9 +53,11 @@ MenuProxy::~MenuProxy() { Q_FOREACH(GtkMenu *menu, m_menus) { gtk_widget_destroy(GTK_WIDGET(menu)); + g_object_unref(menu); } Q_FOREACH(GtkMenuItem *item, m_items) { gtk_widget_destroy(GTK_WIDGET(item)); + g_object_unref(item); } if (m_accel) { g_object_unref(m_accel); @@ -165,6 +167,7 @@ void MenuProxy::removeAction(QAction *action) } gtk_widget_destroy(GTK_WIDGET(item)); + g_object_unref(item); m_items.remove(action); } @@ -175,11 +178,12 @@ void MenuProxy::removeMenu(QMenu *menu) GtkWidget *g_item = gtk_menu_get_attach_widget(g_menu); Q_ASSERT(g_item); GtkMenu *g_parent = GTK_MENU(gtk_widget_get_parent(g_item)); - Q_ASSERT(g_parent), + Q_ASSERT(g_parent); menu->removeEventFilter(this); gtk_widget_destroy(GTK_WIDGET(g_menu)); + g_object_unref(g_menu); m_menus.remove(menu); } diff --git a/module/topmenumenubarimpl.cc b/module/topmenumenubarimpl.cc index 1753394..5ce2538 100644 --- a/module/topmenumenubarimpl.cc +++ b/module/topmenumenubarimpl.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "topmenumenubarimpl.h" @@ -43,17 +44,35 @@ static inline QAction * get_action_for_shortcut(QShortcut *shortcut) static inline void set_action_for_shortcut(QShortcut *shortcut, QAction *action) { - if (shortcut) { + if (action) { shortcut->setProperty("topmenu-action", QVariant::fromValue(action)); } else { shortcut->setProperty("topmenu-action", QVariant()); } } +static inline TopMenuMenuBarImpl * get_menubarimpl_for_widget(QWidget *widget) +{ + QVariant v = widget->property("topmenu-impl"); + if (v.isValid()) { + return static_cast(v.value()); + } + return 0; +} + +static inline void set_menubarimpl_for_widget(QWidget *widget, TopMenuMenuBarImpl *impl) +{ + if (impl) { + widget->setProperty("topmenu-impl", QVariant::fromValue(impl)); + } else { + widget->setProperty("topmenu-impl", QVariant()); + } +} + TopMenuMenuBarImpl::TopMenuMenuBarImpl(QObject *parent) : MenuProxy(parent), - m_window(0), m_menubar(0), m_appmenu(0), - m_disable(false), m_visible(true) + m_window(0), m_menubar(0), m_appmenubar(0), m_appmenu(0), + m_disable(false) { // A new instance of this class will be created for each window. if (!staticInitialized) { @@ -62,36 +81,55 @@ TopMenuMenuBarImpl::TopMenuMenuBarImpl(QObject *parent) : XSetErrorHandler(qt_x_errhandler); menuMonitor = topmenu_monitor_get_instance(); - // TODO g_signal_connect() staticInitialized = true; } + + m_monitor_connection_id = g_signal_connect(menuMonitor, "notify::available", + G_CALLBACK(handleMonitorAvailabilityChanged), this); + qDebug() << "Constructed" << this; } TopMenuMenuBarImpl::~TopMenuMenuBarImpl() { - if (m_menubar) { - g_object_unref(m_menubar); - m_menubar = 0; + if (m_appmenubar) { + gtk_widget_destroy(GTK_WIDGET(m_appmenubar)); + g_object_unref(m_appmenubar); + m_appmenubar = 0; + } + if (m_window) { + if (get_menubarimpl_for_widget(m_window) == this) { + set_menubarimpl_for_widget(m_window, 0); + } + } + if (m_monitor_connection_id) { + g_signal_handler_disconnect(menuMonitor, m_monitor_connection_id); } + qDebug() << "Destructed" << this; } void TopMenuMenuBarImpl::init(QMenuBar *menuBar) { Q_ASSERT(menuBar); + Q_ASSERT(!m_menubar); - m_menubar = topmenu_app_menu_bar_new(); - g_object_ref_sink(m_menubar); + m_menubar = menuBar; + + Q_ASSERT(!m_appmenubar); + m_appmenubar = topmenu_app_menu_bar_new(); + g_object_ref_sink(m_appmenubar); m_appmenu = new AppMenu(this); - topmenu_app_menu_bar_set_app_menu(m_menubar, + topmenu_app_menu_bar_set_app_menu(m_appmenubar, GTK_WIDGET(m_appmenu->getGtkMenu())); - gtk_widget_show(GTK_WIDGET(m_menubar)); - setTargetMenu(GTK_MENU_SHELL(m_menubar)); + gtk_widget_show(GTK_WIDGET(m_appmenubar)); + setTargetMenu(GTK_MENU_SHELL(m_appmenubar)); } void TopMenuMenuBarImpl::setVisible(bool visible) { - m_visible = visible; + if (m_menubar) { + m_menubar->QWidget::setVisible(visible); + } } void TopMenuMenuBarImpl::actionEvent(QActionEvent *e) @@ -116,8 +154,10 @@ void TopMenuMenuBarImpl::handleReparent(QWidget *oldParent, QWidget *newParent, Q_UNUSED(oldParent); Q_UNUSED(newParent); - if (oldWindow) { - GdkWindow *old_window = gdk_window_foreign_new(oldWindow->winId()); + Q_ASSERT(m_appmenubar); // init() must have been called + + if (m_window) { + GdkWindow *old_window = gdk_window_foreign_new(m_window->winId()); topmenu_client_disconnect_window(old_window); Q_FOREACH(QAction *action, m_rootitems) { @@ -127,19 +167,31 @@ void TopMenuMenuBarImpl::handleReparent(QWidget *oldParent, QWidget *newParent, delete shortcut; } } + + if (get_menubarimpl_for_widget(m_window) == this) { + set_menubarimpl_for_widget(m_window, 0); + } } if (newWindow) { - Q_ASSERT(m_menubar); + if (get_menubarimpl_for_widget(newWindow)) { + // The new window already has a menubar. Let's not override it. + m_window = 0; + return; + } + GdkWindow *new_window = gdk_window_foreign_new(newWindow->winId()); - topmenu_client_connect_window_widget(new_window, GTK_WIDGET(m_menubar)); + topmenu_client_connect_window_widget(new_window, GTK_WIDGET(m_appmenubar)); Q_FOREACH(QAction *action, m_rootitems) { createMnemonicShortcut(action, newWindow); } - } - m_window = newWindow; + m_window = newWindow; + set_menubarimpl_for_widget(m_window, this); + } else { + m_window = 0; + } } bool TopMenuMenuBarImpl::allowCornerWidgets() const @@ -159,7 +211,9 @@ void TopMenuMenuBarImpl::setNativeMenuBar(bool value) bool TopMenuMenuBarImpl::isNativeMenuBar() const { - if (m_disable || QCoreApplication::instance()->testAttribute(Qt::AA_DontUseNativeMenuBar)) + //return false; + if (m_disable || !m_window || + QCoreApplication::instance()->testAttribute(Qt::AA_DontUseNativeMenuBar)) return false; return menuMonitor->available; } @@ -243,6 +297,13 @@ QShortcut * TopMenuMenuBarImpl::createMnemonicShortcut(QAction *action, QWidget return shortcut; } +void TopMenuMenuBarImpl::handleMonitorAvailabilityChanged(TopMenuMenuBarImpl *self) +{ + if (self->m_menubar) { + self->m_menubar->updateGeometry(); + } +} + void TopMenuMenuBarImpl::handleShortcutActivated() { QShortcut *shortcut = static_cast(sender()); diff --git a/module/topmenumenubarimpl.h b/module/topmenumenubarimpl.h index 7573f76..49e8d83 100644 --- a/module/topmenumenubarimpl.h +++ b/module/topmenumenubarimpl.h @@ -49,6 +49,7 @@ protected: private: QShortcut * createMnemonicShortcut(QAction *action, QWidget *parent); + static void handleMonitorAvailabilityChanged(TopMenuMenuBarImpl *self); private slots: void handleShortcutActivated(); @@ -58,14 +59,15 @@ private: static TopMenuMonitor *menuMonitor; QWidget *m_window; - TopMenuAppMenuBar *m_menubar; + QMenuBar *m_menubar; + TopMenuAppMenuBar *m_appmenubar; + AppMenu *m_appmenu; - QList m_rootitems; + ulong m_monitor_connection_id; - AppMenu *m_appmenu; + QList m_rootitems; - bool m_disable : 1; - bool m_visible : 1; + bool m_disable; }; #endif // TOPMENUMENUBARIMPL_H -- cgit v1.2.3