Merge lp:~agateau/unity-2d/dont-disconnect-from-dead-windows into lp:unity-2d/3.0

[Aurélien Gâteau]

[libunity-2d] Introduce GConnector, a class to track connections to GObject signals.

Use GConnector in WindowHelper, avoiding some GLib critical messages when the active window is closed. Those messages were caused by us trying to disconnect from a dead wnck window handle.

This branch has been extracted from the unity-core branch. It does two things:

- Introduces GConnector, a class to track connections to GObject signals.

- Uses GConnector in WindowHelper, avoiding some GLib critical messages when the active window is closed. Those messages were caused by us trying to disconnect from a dead wnck window handle.

[Florian Boucault (fboucault)]

Works as advertised, code is clean. Thank you.

review: Approve

=== modified file 'libunity-2d-private/src/CMakeLists.txt'
--- libunity-2d-private/src/CMakeLists.txt	2011-06-22 08:22:14 +0000
+++ libunity-2d-private/src/CMakeLists.txt	2011-07-12 13:50:20 +0000
@@ -4,6 +4,7 @@
 
 # Sources
 set(libunity-2d-private_SRCS
+    gconnector.cpp
     gnomesessionclient.cpp
     keyboardmodifiersmonitor.cpp
     hotkeymonitor.cpp

=== added file 'libunity-2d-private/src/gconnector.cpp'
--- libunity-2d-private/src/gconnector.cpp	1970-01-01 00:00:00 +0000
+++ libunity-2d-private/src/gconnector.cpp	2011-07-12 13:50:20 +0000
@@ -0,0 +1,81 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * Authors:
+ * - Aurélien Gâteau <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+// Self
+#include "gconnector.h"
+
+// Local
+#include <debug_p.h>
+
+// Qt
+
+GConnector::GConnector()
+{
+}
+
+GConnector::~GConnector()
+{
+    disconnectAll();
+}
+
+void GConnector::connect(gpointer instance, const char* signal, GCallback handler, gpointer data)
+{
+    gulong id = g_signal_connect(instance, signal, handler, data);
+    auto it = m_connections.find(instance);
+    if (it == m_connections.end()) {
+        g_object_weak_ref(G_OBJECT(instance),
+            GWeakNotify(weakNotifyCB),
+            reinterpret_cast<gpointer>(this));
+        m_connections.insert(instance, ConnectionIdList() << id);
+    } else {
+        it.value() << id;
+    }
+}
+
+void GConnector::disconnect(gpointer instance)
+{
+    auto it = m_connections.find(instance);
+    UQ_RETURN_IF_FAIL(it != m_connections.end());
+    disconnect(it);
+}
+
+void GConnector::disconnectAll()
+{
+    while (!m_connections.isEmpty()) {
+        disconnect(m_connections.begin());
+    }
+}
+
+void GConnector::disconnect(Connections::Iterator it)
+{
+    gpointer instance = it.key();
+    Q_FOREACH(gulong id, it.value()) {
+        g_signal_handler_disconnect(instance, id);
+    }
+    g_object_weak_unref(G_OBJECT(instance),
+        GWeakNotify(weakNotifyCB),
+        reinterpret_cast<gpointer>(this));
+    m_connections.erase(it);
+}
+
+void GConnector::weakNotifyCB(GConnector* that, GObject* instance)
+{
+    that->m_connections.remove(instance);
+}

=== added file 'libunity-2d-private/src/gconnector.h'
--- libunity-2d-private/src/gconnector.h	1970-01-01 00:00:00 +0000
+++ libunity-2d-private/src/gconnector.h	2011-07-12 13:50:20 +0000
@@ -0,0 +1,67 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * Authors:
+ * - Aurélien Gâteau <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef GCONNECTOR_H
+#define GCONNECTOR_H
+
+// Local
+
+// Qt
+#include <QHash>
+#include <QList>
+
+// GLib
+#include <glib-object.h>
+
+/**
+ * An helper class to ensure GObject signals are correctly disconnected.
+ */
+class GConnector
+{
+public:
+    GConnector();
+    ~GConnector();
+
+    /**
+     * Connect a signal to a callback
+     */
+    void connect(gpointer instance, const char* signal, GCallback handler, gpointer data);
+
+    /**
+     * Disconnect from all signals of instance
+     */
+    void disconnect(gpointer instance);
+
+    /**
+     * Disconnect from all signals we connected to
+     */
+    void disconnectAll();
+
+private:
+    typedef QList<gulong> ConnectionIdList;
+    typedef QHash<gpointer, ConnectionIdList> Connections;
+    Connections m_connections;
+
+    void disconnect(Connections::Iterator);
+
+    static void weakNotifyCB(GConnector*, GObject*);
+};
+
+#endif /* GCONNECTOR_H */

=== modified file 'libunity-2d-private/tests/CMakeLists.txt'
--- libunity-2d-private/tests/CMakeLists.txt	2011-06-13 09:32:04 +0000
+++ libunity-2d-private/tests/CMakeLists.txt	2011-07-12 13:50:20 +0000
@@ -5,6 +5,7 @@
     ${libunity-2d-private_SOURCE_DIR}/src
     ${libunity-2d-private_SOURCE_DIR}/Unity2d
     ${CMAKE_CURRENT_BINARY_DIR}
+    ${GLIB_INCLUDE_DIRS}
     ${QT_QTTEST_INCLUDE_DIR}
     )
 
@@ -33,6 +34,7 @@
 endmacro(libunity_2d_tests)
 
 libunity_2d_tests(
+    gconnectortest
     keyboardmodifiersmonitortest
     paneltest
     unity2dtrtest

=== added file 'libunity-2d-private/tests/gconnectortest.cpp'
--- libunity-2d-private/tests/gconnectortest.cpp	1970-01-01 00:00:00 +0000
+++ libunity-2d-private/tests/gconnectortest.cpp	2011-07-12 13:50:20 +0000
@@ -0,0 +1,96 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * Authors:
+ * - Aurélien Gâteau <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+// Local
+#include <debug_p.h>
+#include <gconnector.h>
+
+// Qt
+#include <QtTest>
+
+// GLib
+#include <gio/gio.h>
+
+class CallbackSpy {
+public:
+    CallbackSpy()
+    : count(0)
+    {}
+
+    static void increase(GCancellable*, gpointer data)
+    {
+        CallbackSpy* spy = reinterpret_cast<CallbackSpy*>(data);
+        ++spy->count;
+    }
+
+    int count;
+};
+
+class GConnectorTest : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void init()
+    {
+        g_type_init();
+    }
+
+    void testDisconnectAll()
+    {
+        // Create a cancellable object as it is easy to get it to emit a
+        // signal.
+        GCancellable* object = g_cancellable_new();
+
+        CallbackSpy spy;
+        GConnector connector;
+        connector.connect(object, "cancelled", G_CALLBACK(CallbackSpy::increase), &spy);
+
+        // Emit signal, check callback gets called
+        g_cancellable_cancel(object);
+        QCOMPARE(spy.count, 1);
+
+        // Disconnect and emit signal, check callback does *not* get called
+        connector.disconnectAll();
+        g_cancellable_reset(object);
+        g_cancellable_cancel(object);
+        QCOMPARE(spy.count, 1);
+
+        g_object_unref(object);
+    }
+
+    void testDeletedInstance()
+    {
+        GCancellable* object = g_cancellable_new();
+
+        // Create a dummy connection
+        CallbackSpy spy;
+        GConnector connector;
+        connector.connect(object, "cancelled", G_CALLBACK(CallbackSpy::increase), &spy);
+        g_object_unref(object);
+
+        // Should not crash because connection should have been removed
+        connector.disconnectAll();
+    }
+};
+
+QTEST_MAIN(GConnectorTest)
+
+#include "gconnectortest.moc"

=== modified file 'panel/applets/appname/windowhelper.cpp'
--- panel/applets/appname/windowhelper.cpp	2011-06-22 14:49:34 +0000
+++ panel/applets/appname/windowhelper.cpp	2011-07-12 13:50:20 +0000
@@ -26,6 +26,7 @@
 
 // unity-2d
 #include <debug_p.h>
+#include <gconnector.h>
 
 // Bamf
 #include <bamf-matcher.h>
@@ -45,6 +46,7 @@
 struct WindowHelperPrivate
 {
     WnckWindow* m_window;
+    GConnector m_connector;
 };
 
 WindowHelper::WindowHelper(QObject* parent)
@@ -96,14 +98,13 @@
     uint xid = bamfWindow ? bamfWindow->xid() : 0;
 
     if (d->m_window) {
-        g_signal_handlers_disconnect_by_func(d->m_window, gpointer(stateChangedCB), this);
-        g_signal_handlers_disconnect_by_func(d->m_window, gpointer(nameChangedCB), this);
+        d->m_connector.disconnectAll();
         d->m_window = 0;
     }
     if (xid != 0) {
         d->m_window = wnck_window_get(xid);
-        g_signal_connect(G_OBJECT(d->m_window), "name-changed", G_CALLBACK(nameChangedCB), this);
-        g_signal_connect(G_OBJECT(d->m_window), "state-changed", G_CALLBACK(stateChangedCB), this);
+        d->m_connector.connect(G_OBJECT(d->m_window), "name-changed", G_CALLBACK(nameChangedCB), this);
+        d->m_connector.connect(G_OBJECT(d->m_window), "state-changed", G_CALLBACK(stateChangedCB), this);
     }
     stateChanged();
     nameChanged();