Merge lp:~nick-dedekind/qmenumodel/unitymenuaction.lp1334203 into lp:qmenumodel

[Nick Dedekind]

Unset the UnityMenuActions::model when destroying UnityMenuModel

Unset the UnityMenuActions::model when destroying UnityMenuModel

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:106
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/49/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-amd64-ci/2/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-armhf-ci/2/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/49/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:107
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/50/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-utopic-armhf-ci/3/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/50/rebuild

review: Approve (continuous-integration)

[Lars Karlitski (larsu)]

Looks good to me, thanks.

review: Approve

[Andrea Cimitan (cimi)]

Fixes for me, thx

review: Approve

=== modified file 'libqmenumodel/src/unitymenuaction.cpp'
--- libqmenumodel/src/unitymenuaction.cpp	2013-09-03 10:02:47 +0000
+++ libqmenumodel/src/unitymenuaction.cpp	2014-07-01 10:17:47 +0000
@@ -33,9 +33,7 @@
 
 UnityMenuAction::~UnityMenuAction()
 {
-    if (m_model) {
-        m_model->unregisterAction(this);
-    }
+    unregisterAction();
 }
 
 QString UnityMenuAction::name() const
@@ -59,9 +57,7 @@
 void UnityMenuAction::setModel(UnityMenuModel* model)
 {
     if (m_model != model) {
-        if (!model) {
-            unregisterAction();
-        }
+        unregisterAction();
         m_model = model;
         registerAction();
         Q_EMIT modelChanged(model);
@@ -107,7 +103,7 @@
     }
 }
 
-int UnityMenuAction::index() const 
+int UnityMenuAction::index() const
 {
     return m_index;
 }

=== modified file 'libqmenumodel/src/unitymenumodel.cpp'
--- libqmenumodel/src/unitymenumodel.cpp	2014-04-02 10:19:02 +0000
+++ libqmenumodel/src/unitymenumodel.cpp	2014-07-01 10:17:47 +0000
@@ -79,6 +79,7 @@
     QHash<QByteArray, int> roles;
     ActionStateParser* actionStateParser;
     QHash<UnityMenuAction*, GtkSimpleActionObserver*> registeredActions;
+    bool destructorGuard;
 
     static void nameAppeared(GDBusConnection *connection, const gchar *name, const gchar *owner, gpointer user_data);
     static void nameVanished(GDBusConnection *connection, const gchar *name, gpointer user_data);
@@ -113,6 +114,7 @@
     this->connection = NULL;
     this->nameWatchId = 0;
     this->actionStateParser = new ActionStateParser(model);
+    this->destructorGuard = false;
 
     this->muxer = gtk_action_muxer_new ();
 
@@ -126,6 +128,7 @@
     this->connection = NULL;
     this->nameWatchId = 0;
     this->actionStateParser = new ActionStateParser(model);
+    this->destructorGuard = false;
 
     this->muxer = GTK_ACTION_MUXER( g_object_ref(other.muxer));
 
@@ -134,6 +137,7 @@
 
 UnityMenuModelPrivate::~UnityMenuModelPrivate()
 {
+    this->destructorGuard = true;
     this->clearItems(false);
 
     g_sequence_free(this->items);
@@ -144,6 +148,7 @@
     QHash<UnityMenuAction*, GtkSimpleActionObserver*>::const_iterator it = this->registeredActions.constBegin();
     for (; it != this->registeredActions.constEnd(); ++it) {
         g_object_unref(it.value());
+        it.key()->setModel(NULL);
     }
     this->registeredActions.clear();
 
@@ -787,6 +792,9 @@
 
 void UnityMenuModel::registerAction(UnityMenuAction* action)
 {
+    if (priv->destructorGuard)
+        return;
+
     if (!priv->registeredActions.contains(action)) {
         GtkSimpleActionObserver* observer_item;
         observer_item = gtk_simple_action_observer_new(GTK_ACTION_OBSERVABLE (priv->muxer),
@@ -808,6 +816,9 @@
 
 void UnityMenuModel::unregisterAction(UnityMenuAction* action)
 {
+    if (priv->destructorGuard)
+        return;
+
     if (priv->registeredActions.contains(action)) {
         GtkSimpleActionObserver* observer_item;
         observer_item = priv->registeredActions[action];

=== modified file 'tests/client/CMakeLists.txt'
--- tests/client/CMakeLists.txt	2013-08-27 16:20:22 +0000
+++ tests/client/CMakeLists.txt	2014-07-01 10:17:47 +0000
@@ -60,6 +60,7 @@
 declare_test(convertertest)
 declare_test(modelsignalstest)
 declare_test(treetest)
+declare_test(unitymenuactiontest)
 
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/qmlfiles.h.in
                ${CMAKE_CURRENT_BINARY_DIR}/qmlfiles.h)

=== added file 'tests/client/unitymenuactiontest.cpp'
--- tests/client/unitymenuactiontest.cpp	1970-01-01 00:00:00 +0000
+++ tests/client/unitymenuactiontest.cpp	2014-07-01 10:17:47 +0000
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Nick Dedekind <[e-mail]>
+ */
+
+#include "unitymenumodel.h"
+#include "unitymenuaction.h"
+
+#include <QObject>
+#include <QtTest>
+
+class UnityMenuActionTest : public QObject
+{
+    Q_OBJECT
+private:
+
+private Q_SLOTS:
+
+    /*
+     * Test if the propety busType handle correct integer values
+     */
+    void testDestroyAfterModel()
+    {
+        UnityMenuModel* model = new UnityMenuModel;
+        UnityMenuAction* action = new UnityMenuAction;
+        action->setModel(model);
+
+        delete model;
+        delete action;
+    }
+
+    /*
+     * Test if the propety busType handle correct integer values
+     */
+    void testDestroyBeforeModel()
+    {
+        UnityMenuModel* model = new UnityMenuModel;
+        UnityMenuAction* action = new UnityMenuAction;
+        action->setModel(model);
+
+        delete action;
+        delete model;
+    }
+};
+
+QTEST_MAIN(UnityMenuActionTest)
+
+#include "unitymenuactiontest.moc"