Merge lp:~fboucault/bamf-qt/bamf-qt-crashfix702566 into lp:bamf-qt

[Florian Boucault]

Added support for indicators (new BamfIndicator class).
Added safeguards for when BamfFactory::view_for_path returns NULL.
Replaced static_cast with qobject_cast for values returned from BamfFactory::view_for_path thus avoiding enforcing wrong casts.

[Florian Boucault (fboucault)]

From Ugo Riboni's previous review:

"""
Functionally it does its job, the crash disappears.

However I don't exactly understand *why* this MR fixes the issue, so I'm not sure I'm the most qualified to review this. Or perhaps some more comments in the code are in order ?

Anyway, please fix the (mostly style) issues below:

* The methods Address and Path in BamfIndicator should return QString() when failing, instead of NULL.
* In the BamfIndicator constructor, please align the arguments on the same line. The same for OnActiveApplicationChanged and OnActiveWindowChanged in BamfMatcher
"""

[Florian Boucault (fboucault)]

The why this MR fixes the issue is two-folds:
1) using static cast is wrong; a dynamic cast needs to be used so that if the cast fails a NULL pointer is returned. qobject_cast does the same but is smarter about other things.
2) the MR adds NULL pointer checks in relevant places

[lp:~fboucault/bamf-qt/bamf-qt-crashfix702566]

364. By Florian Boucault on 2011-01-17

Methods Address and Path in BamfIndicator now return QString() when failing, instead of NULL.

365. By Florian Boucault on 2011-01-17

Fixed indentation.

[Florian Boucault (fboucault)]

The 2 stylistic issues raised are now fixed.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2011-01-13 17:57:36 +0000
+++ CMakeLists.txt	2011-01-17 04:32:10 +0000
@@ -13,6 +13,8 @@
     bamf-control.cpp
     bamf-control-proxy.cpp
     bamf-factory.cpp
+    bamf-indicator.cpp
+    bamf-indicator-proxy.cpp
     bamf-matcher.cpp
     bamf-matcher-proxy.cpp
     bamf-view.cpp
@@ -27,6 +29,8 @@
     bamf-control.h
     bamf-control-proxy.h
     bamf-factory.h
+    bamf-indicator.h
+    bamf-indicator-proxy.h
     bamf-list.h
     bamf-matcher.h
     bamf-matcher-proxy.h

=== modified file 'bamf-application.cpp'
--- bamf-application.cpp	2010-12-21 09:56:42 +0000
+++ bamf-application.cpp	2011-01-17 04:32:10 +0000
@@ -85,7 +85,10 @@
     BamfViewList *children = this->children();
     for(int i = 0; i < children->size(); ++i)
     {
-        result.prepend(static_cast<BamfWindow *>(children->at(i)));
+        BamfWindow* window = qobject_cast<BamfWindow *>(children->at(i));
+        if (window != NULL) {
+            result.prepend(window);
+        }
     }
     delete children;
     return new BamfWindowList(result);
@@ -95,13 +98,19 @@
 BamfApplication::OnWindowAdded(const QString &path)
 {
     BamfView *view = BamfFactory::get_default().view_for_path(path);
-    WindowAdded(static_cast<BamfWindow *>(view));
+    BamfWindow* window = qobject_cast<BamfWindow *>(view);
+    if (window != NULL) {
+        WindowAdded(window);
+    }
 }
 
 void
 BamfApplication::OnWindowRemoved(const QString &path)
 {
     BamfView *view = BamfFactory::get_default().view_for_path(path);
-    WindowRemoved(static_cast<BamfWindow *>(view));
+    BamfWindow* window = qobject_cast<BamfWindow *>(view);
+    if (window != NULL) {
+        WindowRemoved(window);
+    }
 }
 

=== modified file 'bamf-factory.cpp'
--- bamf-factory.cpp	2010-12-21 09:56:42 +0000
+++ bamf-factory.cpp	2011-01-17 04:32:10 +0000
@@ -22,7 +22,7 @@
 #include "bamf-view.h"
 #include "bamf-application.h"
 #include "bamf-window.h"
-//#include "bamf-indicator.h"
+#include "bamf-indicator.h"
 
 BamfFactory::BamfFactory() :
     m_views(NULL)
@@ -48,8 +48,8 @@
         view = static_cast<BamfView *>(new BamfApplication(path));
     else if (type == "window")
         view = static_cast<BamfView *>(new BamfWindow(path));
-    //else if (type == "indicator")
-    //    view = static_cast<BamfView *>(new BamfIndicator(path));
+    else if (type == "indicator")
+        view = static_cast<BamfView *>(new BamfIndicator(path));
 
     if (view != NULL)
     {

=== added file 'bamf-indicator-proxy.cpp'
--- bamf-indicator-proxy.cpp	1970-01-01 00:00:00 +0000
+++ bamf-indicator-proxy.cpp	2011-01-17 04:32:10 +0000
@@ -0,0 +1,26 @@
+/*
+ * This file was generated by qdbusxml2cpp version 0.7
+ * Command line was: qdbusxml2cpp bamf-indicator-glue.xml -p bamf-indicator-proxy
+ *
+ * qdbusxml2cpp is Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * This is an auto-generated file.
+ * This file may have been hand-edited. Look for HAND-EDIT comments
+ * before re-generating it.
+ */
+
+#include "bamf-indicator-proxy.h"
+
+/*
+ * Implementation of interface class OrgAyatanaBamfIndicatorInterface
+ */
+
+OrgAyatanaBamfIndicatorInterface::OrgAyatanaBamfIndicatorInterface(const QString &service, const QString &path, const QDBusConnection &connection, QObject *parent)
+    : QDBusAbstractInterface(service, path, staticInterfaceName(), connection, parent)
+{
+}
+
+OrgAyatanaBamfIndicatorInterface::~OrgAyatanaBamfIndicatorInterface()
+{
+}
+

=== added file 'bamf-indicator-proxy.h'
--- bamf-indicator-proxy.h	1970-01-01 00:00:00 +0000
+++ bamf-indicator-proxy.h	2011-01-17 04:32:10 +0000
@@ -0,0 +1,61 @@
+/*
+ * This file was generated by qdbusxml2cpp version 0.7
+ * Command line was: qdbusxml2cpp bamf-indicator-glue.xml -p bamf-indicator-proxy
+ *
+ * qdbusxml2cpp is Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * This is an auto-generated file.
+ * Do not edit! All changes made to it will be lost.
+ */
+
+#ifndef BAMF-INDICATOR-PROXY_H_1294953124
+#define BAMF-INDICATOR-PROXY_H_1294953124
+
+#include <QtCore/QObject>
+#include <QtCore/QByteArray>
+#include <QtCore/QList>
+#include <QtCore/QMap>
+#include <QtCore/QString>
+#include <QtCore/QStringList>
+#include <QtCore/QVariant>
+#include <QtDBus/QtDBus>
+
+/*
+ * Proxy class for interface org.ayatana.bamf.indicator
+ */
+class OrgAyatanaBamfIndicatorInterface: public QDBusAbstractInterface
+{
+    Q_OBJECT
+public:
+    static inline const char *staticInterfaceName()
+    { return "org.ayatana.bamf.indicator"; }
+
+public:
+    OrgAyatanaBamfIndicatorInterface(const QString &service, const QString &path, const QDBusConnection &connection, QObject *parent = 0);
+
+    ~OrgAyatanaBamfIndicatorInterface();
+
+public Q_SLOTS: // METHODS
+    inline QDBusPendingReply<QString> Address()
+    {
+        QList<QVariant> argumentList;
+        return asyncCallWithArgumentList(QLatin1String("Address"), argumentList);
+    }
+
+    inline QDBusPendingReply<QString> Path()
+    {
+        QList<QVariant> argumentList;
+        return asyncCallWithArgumentList(QLatin1String("Path"), argumentList);
+    }
+
+Q_SIGNALS: // SIGNALS
+};
+
+namespace org {
+  namespace ayatana {
+    namespace bamf {
+      typedef ::OrgAyatanaBamfIndicatorInterface indicator;
+    }
+  }
+}
+#endif

=== added file 'bamf-indicator.cpp'
--- bamf-indicator.cpp	1970-01-01 00:00:00 +0000
+++ bamf-indicator.cpp	2011-01-17 04:32:10 +0000
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2010 Canonical, Ltd.
+ *
+ * Authors:
+ *  Olivier Tilloy <[e-mail]>
+ *
+ * This library 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 library 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/>.
+ */
+
+#include "bamf-indicator.h"
+#include "bamf-indicator-proxy.h"
+#include "bamf-factory.h"
+
+BamfIndicator::BamfIndicator(QString path) :
+    BamfView(path), m_indicator_proxy(NULL)
+{
+    m_indicator_proxy = new OrgAyatanaBamfIndicatorInterface("org.ayatana.bamf", path,
+                                                             QDBusConnection::sessionBus(),
+                                                             static_cast<QObject*>(this));
+}
+
+BamfIndicator::~BamfIndicator()
+{
+    delete m_indicator_proxy;
+}
+
+const QString
+BamfIndicator::address() const
+{
+    QDBusPendingReply<QString> reply = m_indicator_proxy->Address();
+    reply.waitForFinished();
+    if (reply.isError()) {
+        qWarning() << reply.error();
+        return QString();
+    } else {
+        return reply.value();
+    }
+}
+
+const QString
+BamfIndicator::path() const
+{
+    QDBusPendingReply<QString> reply = m_indicator_proxy->Path();
+    reply.waitForFinished();
+    if (reply.isError()) {
+        qWarning() << reply.error();
+        return QString();
+    } else {
+        return reply.value();
+    }
+}

=== added file 'bamf-indicator.h'
--- bamf-indicator.h	1970-01-01 00:00:00 +0000
+++ bamf-indicator.h	2011-01-17 04:32:10 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2010 Canonical, Ltd.
+ *
+ * Authors:
+ *  Florian Boucault <[e-mail]>
+ *
+ * This library 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 library 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/>.
+ */
+
+#ifndef QT_BAMF_INDICATOR_H
+#define QT_BAMF_INDICATOR_H
+
+#include <QtCore/QObject>
+
+#include "bamf-view.h"
+
+class OrgAyatanaBamfIndicatorInterface;
+
+class BamfIndicator: public BamfView
+{
+    Q_OBJECT
+
+    Q_PROPERTY(QString address READ address);
+    Q_PROPERTY(QString path READ path);
+
+friend class BamfFactory;
+
+private:
+    BamfIndicator(QString path);
+    ~BamfIndicator();
+    BamfIndicator(BamfIndicator const&);
+    void operator=(BamfIndicator const&);
+
+public:
+    // getters
+    const QString address() const;
+    const QString path() const;
+
+private:
+    OrgAyatanaBamfIndicatorInterface *m_indicator_proxy;
+};
+
+#endif
+

=== modified file 'bamf-matcher.cpp'
--- bamf-matcher.cpp	2010-12-21 09:56:42 +0000
+++ bamf-matcher.cpp	2011-01-17 04:32:10 +0000
@@ -62,7 +62,7 @@
             return NULL;
 
         BamfView *view = BamfFactory::get_default().view_for_path(path);
-        return static_cast<BamfApplication *>(view);
+        return qobject_cast<BamfApplication *>(view);
     }
 }
 
@@ -80,7 +80,7 @@
             return NULL;
 
         BamfView *view = BamfFactory::get_default().view_for_path(path);
-        return static_cast<BamfWindow *>(view);
+        return qobject_cast<BamfWindow *>(view);
     }
 }
 
@@ -98,7 +98,7 @@
             return NULL;
 
         BamfView *view = BamfFactory::get_default().view_for_path(path);
-        return static_cast<BamfApplication *>(view);
+        return qobject_cast<BamfApplication *>(view);
     }
 }
 
@@ -127,7 +127,10 @@
         QStringList list = reply.value();
         for (int i = 0; i < list.size(); ++i) {
             BamfView *view = BamfFactory::get_default().view_for_path(list.at(i));
-            result.prepend(static_cast<BamfApplication *>(view));
+            BamfApplication* application = qobject_cast<BamfApplication *>(view);
+            if (application != NULL) {
+                result.prepend(application);
+            }
         }
     }
     return new BamfApplicationList(result);
@@ -145,7 +148,10 @@
         QStringList list = reply.value();
         for (int i = 0; i < list.size(); ++i) {
             BamfView *view = BamfFactory::get_default().view_for_path(list.at(i));
-            result.prepend(static_cast<BamfApplication *>(view));
+            BamfApplication* application = qobject_cast<BamfApplication *>(view);
+            if (application != NULL) {
+                result.prepend(application);
+            }
         }
     }
     return new BamfApplicationList(result);
@@ -156,8 +162,8 @@
 {
     BamfView *former = BamfFactory::get_default().view_for_path(old_active);
     BamfView *current = BamfFactory::get_default().view_for_path(new_active);
-    ActiveApplicationChanged(static_cast<BamfApplication *>(former),
-                                  static_cast<BamfApplication *>(current));
+    ActiveApplicationChanged(qobject_cast<BamfApplication *>(former),
+                             qobject_cast<BamfApplication *>(current));
 }
 
 void
@@ -165,8 +171,8 @@
 {
     BamfView *former = BamfFactory::get_default().view_for_path(old_active);
     BamfView *current = BamfFactory::get_default().view_for_path(new_active);
-    ActiveWindowChanged(static_cast<BamfWindow *>(former),
-                             static_cast<BamfWindow *>(current));
+    ActiveWindowChanged(qobject_cast<BamfWindow *>(former),
+                        qobject_cast<BamfWindow *>(current));
 }
 
 void

=== modified file 'bamf-plugin.cpp'
--- bamf-plugin.cpp	2011-01-13 17:57:36 +0000
+++ bamf-plugin.cpp	2011-01-17 04:32:10 +0000
@@ -27,6 +27,7 @@
 #include "bamf-view.h"
 #include "bamf-application.h"
 #include "bamf-window.h"
+#include "bamf-indicator.h"
 #include "bamf-control.h"
 #include "bamf-matcher.h"
 
@@ -42,6 +43,7 @@
     qmlRegisterUncreatableType<BamfApplication>(uri, 0, 2, "BamfApplication", message);
     qmlRegisterUncreatableType<BamfApplicationList>(uri, 0, 2, "BamfApplicationList", message);
     qmlRegisterUncreatableType<BamfWindow>(uri, 0, 2, "BamfWindow", message);
+    qmlRegisterUncreatableType<BamfIndicator>(uri, 0, 2, "BamfIndicator", message);
     qmlRegisterUncreatableType<BamfWindowList>(uri, 0, 2, "BamfWindowList", message);
     qmlRegisterUncreatableType<BamfControl>(uri, 0, 2, "BamfControl", message);
     qmlRegisterUncreatableType<BamfMatcher>(uri, 0, 2, "BamfMatcher", message);

=== modified file 'bamf-window.cpp'
--- bamf-window.cpp	2010-12-21 09:56:42 +0000
+++ bamf-window.cpp	2011-01-17 04:32:10 +0000
@@ -47,7 +47,7 @@
         return NULL;
     } else {
         BamfView *view = BamfFactory::get_default().view_for_path(reply.value());
-        return static_cast<BamfWindow *>(view);
+        return qobject_cast<BamfWindow *>(view);
     }
 }