Merge lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-segfault into lp:ubuntu-ui-toolkit

[Zsombor Egri]

Fix AlamrModel segfault when updating elements.

[Tim Peeters (tpeeters)]

Seems good.

Including a regression test would be even better, but may be difficult to write.

review: Approve

[Tim Peeters (tpeeters)]

39 +private:
40 + bool m_moved:1;

why not initialize it with 'false'?

[lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-segfault]

1197. By Zsombor Egri on 2015-06-16

make sure nested moves are not handled

[Zsombor Egri (zsombi)]

> 39 +private:
> 40 + bool m_moved:1;
>
> why not initialize it with 'false'?

Uhm... it is initialised with false...

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1196
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/2213/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3208
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-amd64-ci/37
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-armhf-ci/37
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-armhf-ci/37/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2692
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3206
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3206/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21212

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2213/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1197
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/2214/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3212
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-amd64-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-armhf-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-vivid-armhf-ci/38/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3210
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3210/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21220

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/2214/rebuild

review: Needs Fixing (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

On testing on Nexus 4 ubuntu-touch/rc-proposed/ubuntu r162, I am unable to reproduce the crash after applying this patch. This might just be it :). That said, this crash is pretty rare and difficult to reproduce and so I have been trying hard to ensure that it is gone for good. But there could be a small chance that it may still exist (purely from testing).

With that I'm giving my approval.

review: Approve

[Tim Peeters (tpeeters)]

Good to land, but it needs to be manually included in a landing or retargeted to staging.

review: Approve

Unmerged revisions

=== modified file 'modules/Ubuntu/Components/plugin/ucalarmmodel.cpp'
--- modules/Ubuntu/Components/plugin/ucalarmmodel.cpp	2015-03-24 16:51:39 +0000
+++ modules/Ubuntu/Components/plugin/ucalarmmodel.cpp	2015-06-16 13:15:48 +0000
@@ -125,6 +125,7 @@
 
 UCAlarmModel::UCAlarmModel(QObject *parent)
     : QAbstractListModel(parent)
+    , m_moved(false)
 {
     // keep in sync with alarms collection changes
     // some of the connections can be asynchronous, others synchronous
@@ -318,7 +319,10 @@
  */
 void UCAlarmModel::moveStarted(int from, int to)
 {
-    beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
+    if (m_moved) {
+        return;
+    }
+    m_moved = beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
 }
 
 /*!
@@ -327,5 +331,8 @@
  */
 void UCAlarmModel::moveFinished()
 {
-    endMoveRows();
+    if (m_moved) {
+        endMoveRows();
+    }
+    m_moved = false;
 }

=== modified file 'modules/Ubuntu/Components/plugin/ucalarmmodel.h'
--- modules/Ubuntu/Components/plugin/ucalarmmodel.h	2014-10-20 12:49:10 +0000
+++ modules/Ubuntu/Components/plugin/ucalarmmodel.h	2015-06-16 13:15:48 +0000
@@ -63,6 +63,9 @@
     void insertFinished();
     void moveStarted(int from, int to);
     void moveFinished();
+
+private:
+    bool m_moved:1;
 };
 
 #endif // UCALARMSMODEL_H