Merge lp:~jpakkane/unity-notifications/lifecycle-fix into lp:unity-notifications

[Jussi Pakkanen]

Don't call into deleted objects in destructors.

Detach all pending notifications from the server on shutdown so their destructors don't call into already destructed objects.

Do not top approve yet. I'm not convinced that this is the best solution.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:181
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/21/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-amd64-ci/16/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/21/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-i386-ci/19

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity-notifications-ci/21/rebuild

review: Needs Fixing (continuous-integration)

[Jussi Pakkanen (jpakkane)]

Build failure is due to Jenkins, not the code:

ssh: connect to host bazaar.launchpad.net port 22: Connection refused

[Michał Sawicz (saviq)]

Yeah, this seems to make sure I'm not getting crashes in notifications any more on exit.

review: Approve (functional)

[lp:~jpakkane/unity-notifications/lifecycle-fix]

182. By Jussi Pakkanen on 2013-10-15

Documented lifecycle fix hackness.

[Jussi Pakkanen (jpakkane)]

A proper fix would take too much time. Instead documented what needs to be done once the time is right. Feel free to top approve if you feel this to be acceptable.

[Michał Sawicz (saviq)]

Yeah, let's not crash if possible :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:182
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-amd64-ci/17
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/22
        deb: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/22/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-i386-ci/20

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity-notifications-ci/22/rebuild

review: Approve (continuous-integration)

=== modified file 'include/Notification.h'
--- include/Notification.h	2013-10-01 08:49:54 +0000
+++ include/Notification.h	2013-10-15 07:33:12 +0000
@@ -109,6 +109,7 @@
     void setType(Type type);
     ActionModel* getActions() const;
     void setActions(QStringList actions);
+    void detachFromServer();
 
     QVariantMap getHints() const;
     void setHints(const QVariantMap& hints);

=== modified file 'src/Notification.cpp'
--- src/Notification.cpp	2013-10-02 23:05:05 +0000
+++ src/Notification.cpp	2013-10-15 07:33:12 +0000
@@ -192,6 +192,19 @@
     }
 }
 
+void Notification::detachFromServer() {
+    /*
+     * FIXME. This function should not exist. In fact
+     * a notification should not need to know the server
+     * it is connected to. Instead the way p->server
+     * is used needs to be converted into a proper
+     * Qt signal. Unfortunately, due to deadlines,
+     * we did not have time. The next time this is worked
+     * on, fix this properly.
+     */
+    p->server = nullptr;
+}
+
 QVariantMap Notification::getHints() const {
     return p->hints;
 }

=== modified file 'src/NotificationModel.cpp'
--- src/NotificationModel.cpp	2013-10-01 08:49:54 +0000
+++ src/NotificationModel.cpp	2013-10-15 07:33:12 +0000
@@ -45,6 +45,18 @@
 }
 
 NotificationModel::~NotificationModel() {
+    for(int i=0; i<p->ephemeralQueue.size(); i++) {
+        p->ephemeralQueue[i]->detachFromServer();
+    }
+    for(int i=0; i<p->interactiveQueue.size(); i++) {
+        p->interactiveQueue[i]->detachFromServer();
+    }
+    for(int i=0; i<p->snapQueue.size(); i++) {
+        p->snapQueue[i]->detachFromServer();
+    }
+    for(int i=0; i<p->displayedNotifications.size(); i++) {
+        p->displayedNotifications[i]->detachFromServer();
+    }
 }
 
 int NotificationModel::rowCount(const QModelIndex &parent) const {