Merge lp:~zsombi/ubuntu-ui-toolkit/alarmmodel-get-crash into lp:ubuntu-ui-toolkit/staging

[Zsombor Egri]

Fixing crashes when AlarmModel::get() returned valid stock alarm ends up to be NULL in QML.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1457
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1597/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1956/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/324
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/327
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/327/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/324
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1723/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1954
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1954/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19114

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1597/rebuild

review: Needs Fixing (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

Tested on vivid desktop where without this MP, the clock app crashes pretty much every time while using the clock app. With this MP after 15-20 times, not a single crash was reproducible. This MP seems to have done the trick.

review: Approve (testing)

[Cris Dywan (kalikiana)]

Looks sensible.

FTR I can't reproduce crashes but I get errors with staging
qml: [CLOCK] Error saving alarm, code: 2
file:///usr/share/ubuntu-clock-app/common/AnalogClockMarker.qml:40: TypeError: Cannot read property of null
And this by the looks of it is garbage collection at work.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp'
--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp	2015-02-26 15:09:23 +0000
+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp	2015-03-24 16:52:43 +0000
@@ -292,6 +292,12 @@
     }
 }
 
+void AlarmDataAdapter::copyAlarmData(const UCAlarm &other)
+{
+    AlarmDataAdapter *pOther = static_cast<AlarmDataAdapter*>(AlarmDataAdapter::get(&other));
+    setData(pOther->data());
+}
+
 void AlarmDataAdapter::adjustDowSettings(UCAlarm::AlarmType type, UCAlarm::DaysOfWeek days)
 {
     QOrganizerItemRecurrence old = event.detail(QOrganizerItemDetail::TypeRecurrence);
@@ -369,13 +375,6 @@
     }
 }
 
-void AlarmDataAdapter::copyData(const UCAlarm &other)
-{
-    AlarmDataAdapter *pOther = static_cast<AlarmDataAdapter*>(AlarmDataAdapter::get(&other));
-    setData(pOther->data());
-}
-
-
 /*-----------------------------------------------------------------------------
  * Adaptation layer for Alarms.
  */

=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h'
--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h	2015-02-26 15:09:23 +0000
+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h	2015-03-24 16:52:43 +0000
@@ -58,6 +58,7 @@
     bool wait(int msec);
     void completeSave();
     void completeCancel();
+    void copyAlarmData(const UCAlarm &other);
 
 // adaptation specific data
     void adjustDowSettings(UCAlarm::AlarmType type, UCAlarm::DaysOfWeek days);
@@ -69,7 +70,6 @@
         return event;
     }
     void setData(const QOrganizerTodo &data);
-    void copyData(const UCAlarm &other);
 
 protected:
     QOrganizerTodo event;
@@ -108,7 +108,7 @@
         UCAlarm *oldAlarm = takeAt(index);
         // copy the other alarm data
         AlarmDataAdapter *pAlarm = static_cast<AlarmDataAdapter*>(AlarmDataAdapter::get(oldAlarm));
-        pAlarm->copyData(alarm);
+        pAlarm->copyAlarmData(alarm);
         // and insert it back
         QDateTime dt = oldAlarm->date();
         QOrganizerItemId id = oldAlarm->cookie().value<QOrganizerItemId>();
@@ -123,7 +123,7 @@
         QOrganizerItemId id = alarm.cookie().value<QOrganizerItemId>();
         idHash.insert(id, dt);
         UCAlarm *newAlarm = new UCAlarm;
-        static_cast<AlarmDataAdapter*>(AlarmDataAdapter::get(newAlarm))->copyData(alarm);
+        UCAlarmPrivate::get(newAlarm)->copyAlarmData(alarm);
         data.insert(QPair<QDateTime, QOrganizerItemId>(dt, id), newAlarm);
         return indexOf(id);
     }
@@ -140,6 +140,8 @@
         UCAlarm *alarm = takeAt(index);
         delete alarm;
     }
+
+protected:
     // removes alarm data at index and returns the alarm pointer
     UCAlarm *takeAt(int index)
     {

=== modified file 'modules/Ubuntu/Components/plugin/ucalarm_p.h'
--- modules/Ubuntu/Components/plugin/ucalarm_p.h	2014-12-02 11:21:22 +0000
+++ modules/Ubuntu/Components/plugin/ucalarm_p.h	2015-03-24 16:52:43 +0000
@@ -59,6 +59,7 @@
     virtual void reset() = 0;
     virtual void completeSave() = 0;
     virtual void completeCancel() = 0;
+    virtual void copyAlarmData(const UCAlarm &other) = 0;
 
     // common privates
     UCAlarm *q_ptr;

=== modified file 'modules/Ubuntu/Components/plugin/ucalarmmodel.cpp'
--- modules/Ubuntu/Components/plugin/ucalarmmodel.cpp	2015-03-03 13:47:48 +0000
+++ modules/Ubuntu/Components/plugin/ucalarmmodel.cpp	2015-03-24 16:52:43 +0000
@@ -22,6 +22,7 @@
 #include "alarmmanager_p.h"
 #include <QtQml/QQmlPropertyMap>
 #include <QtQml/QQmlInfo>
+#include <QtQml/QQmlEngine>
 
 /*!
  * \qmltype AlarmModel
@@ -213,7 +214,14 @@
  */
 UCAlarm* UCAlarmModel::get(int index)
 {
-    return AlarmManager::instance().alarmAt(index);
+    UCAlarm *alarm = AlarmManager::instance().alarmAt(index);
+    if (alarm) {
+        UCAlarm *tempAlarm = new UCAlarm(this);
+        UCAlarmPrivate::get(tempAlarm)->copyAlarmData(*alarm);
+        alarm = tempAlarm;
+        QQmlEngine::setObjectOwnership(tempAlarm, QQmlEngine::JavaScriptOwnership);
+    }
+    return alarm;
 }
 
 /*!

=== modified file 'tests/unit_x11/tst_components/tst_components.pro'
--- tests/unit_x11/tst_components/tst_components.pro	2014-08-01 15:26:23 +0000
+++ tests/unit_x11/tst_components/tst_components.pro	2015-03-24 16:52:43 +0000
@@ -7,4 +7,5 @@
 SOURCES += tst_components.cpp tabsmodel.cpp
 HEADERS += tabsmodel.h
 
-OTHER_FILES += $$system(ls *.qml)
+OTHER_FILES += $$system(ls *.qml) \
+    tst_stress_alarmmodel.qml

=== added file 'tests/unit_x11/tst_components/tst_stress_alarmmodel.qml'
--- tests/unit_x11/tst_components/tst_stress_alarmmodel.qml	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_components/tst_stress_alarmmodel.qml	2015-03-24 16:52:43 +0000
@@ -0,0 +1,92 @@
+/*
+ * Copyright 2013 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/>.
+ */
+
+import QtQuick 2.0
+import QtTest 1.0
+import Ubuntu.Test 1.0
+import Ubuntu.Components 1.2
+
+MainView {
+    width: units.gu(40)
+    height: units.gu(71)
+
+    AlarmModel {
+        id: model
+    }
+
+    Alarm {
+        id: testAlarm
+    }
+
+    UbuntuTestCase {
+        name: "AlarmModelStressTest"
+        when: windowShown
+
+        SignalSpy {
+            id: modelSpy
+            signalName: "modelReset"
+            target: model
+        }
+
+        property string testAlarmMessage: "test_"
+
+        function createAlarm(pattern, idx) {
+            testAlarm.reset();
+            testAlarm.message = pattern + idx;
+            var dt = new Date();
+            dt.setMinutes(dt.getMinutes() + 10);
+            testAlarm.date = dt;
+            modelSpy.signalName = "rowsInserted";
+            testAlarm.save();
+            modelSpy.wait();
+        }
+
+        function clearTestAlarms(pattern) {
+            var i = 0;
+            modelSpy.signalName = "rowsRemoved";
+            while (i < model.count) {
+                var alarm = model.get(i);
+                // this will fail if get() returns NULL
+                if (alarm.message.indexOf(pattern) == 0) {
+                    alarm.cancel();
+                    modelSpy.wait();
+                    modelSpy.clear();
+                    i = 0;
+                } else {
+                    i++;
+                }
+            }
+        }
+
+        // need to create a huge amount of alarms then cleanone part of it
+        function initTestCase() {
+            for (var i = 0; i < 20; i++) {
+                createAlarm(testAlarmMessage, i)
+            }
+            for (var i = 0; i < 40; i++) {
+                createAlarm("testAlarm_", i)
+            }
+        }
+        function cleanupTestCase() {
+            clearTestAlarms(testAlarmMessage);
+        }
+
+        function test_remove_alarms()
+        {
+            clearTestAlarms("testAlarm_");
+        }
+    }
+}