fix some leaks and avoid exporting secondary menubars

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 <QtCore/QDebug>
 #include <QtGui/QActionEvent>
 #include <QtGui/QMenu>
+#include <QtGui/QMenuBar>
 #include <QtGui/QX11Info>
 
 #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<QObject*>(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<TopMenuMenuBarImpl*>(v.value<QObject*>());
+	}
+	return 0;
+}
+
+static inline void set_menubarimpl_for_widget(QWidget *widget, TopMenuMenuBarImpl *impl)
+{
+	if (impl) {
+		widget->setProperty("topmenu-impl", QVariant::fromValue<QObject*>(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<QShortcut*>(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<QAction*> m_rootitems;
+	ulong m_monitor_connection_id;
 
-	AppMenu *m_appmenu;
+	QList<QAction*> m_rootitems;
 
-	bool m_disable : 1;
-	bool m_visible : 1;
+	bool m_disable;
 };
 
 #endif // TOPMENUMENUBARIMPL_H