Merge lp:~fboucault/ubuntu-ui-toolkit/state_saving_store_type into lp:ubuntu-ui-toolkit/staging

[Florian Boucault]

StateSaver: also save the type of each property along with its value so that we can convert them back to the right type during state restoration.
This makes some problematic cases such as enumeration to work.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1163
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/668/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2582
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2065
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/500
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/500
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/500/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/500
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2720
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3825
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3825/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10534
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1727
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2318
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2318/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Cris Dywan (kalikiana)]

See comments inline.

Question: can this ultimately be addressed in QSettings and could a bug be filed/ added as a comment? Provided it's not a fundamental design problem.

review: Needs Fixing

[Florian Boucault (fboucault)]

> See comments inline.
>
> Question: can this ultimately be addressed in QSettings and could a bug be
> filed/ added as a comment? Provided it's not a fundamental design problem.

It does sound like QSettings should do a better job. Not sure how to describe it.

[Cris Dywan (kalikiana)]

Nice. Thanks a lot!

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1164
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/673/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2599
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2082
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/505
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/505
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/505/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/505
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2733
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3842
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3842/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10553
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1740
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2335
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2335/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-autolanding/284/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2614/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2097
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-autolanding/228
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-autolanding/228
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-autolanding/228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-autolanding/228
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2746/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3857
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3857/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10567
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1751
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2350
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2350/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1166
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/676/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2617/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/508
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/508
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/508/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/508
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2749/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3860
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3860/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10571
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1754
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2353
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2353/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Cris Dywan (kalikiana)]

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/unity8/process_helpers.py", line 123, in restart_unity_with_testability
    return restart_unity(*args)
  File "/usr/lib/python3/dist-packages/unity8/process_helpers.py", line 155, in restart_unity
    pid = _get_unity_pid()
  File "/usr/lib/python3/dist-packages/unity8/process_helpers.py", line 178, in _get_unity_pid
    return int(status.split()[-1])
ValueError: invalid literal for int() with base 10: 'start/running'

[lp:~fboucault/ubuntu-ui-toolkit/state_saving_store_type]

1167. By Florian Boucault on 2014-07-29

Empty commit

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1167
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/677/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2641
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2111
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/509
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/509
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/509/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/509
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2769
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3884
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3884/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10602
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1765
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2368
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2368/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Cris Dywan (kalikiana)]

review: Approve

=== modified file 'modules/Ubuntu/Components/plugin/statesaverbackend_p.cpp'
--- modules/Ubuntu/Components/plugin/statesaverbackend_p.cpp	2014-03-17 11:57:53 +0000
+++ modules/Ubuntu/Components/plugin/statesaverbackend_p.cpp	2014-07-29 09:48:52 +0000
@@ -145,8 +145,20 @@
         }
         QQmlProperty qmlProperty(item, propertyName.toLocal8Bit().constData(), qmlContext(item));
         if (qmlProperty.isValid() && qmlProperty.isWritable()) {
-            qmlProperty.write(value);
-            result++;
+            QVariant type = m_archive.data()->value(propertyName + "_TYPE");
+            value.convert(type.toInt());
+            bool writeSuccess = qmlProperty.write(value);
+            if (writeSuccess) {
+                result++;
+            } else {
+                qmlInfo(item) << UbuntuI18n::instance().tr("property \"%1\" of "
+                    "object %2 has type %3 and cannot be set to value \"%4\" of"
+                    " type %5").arg(propertyName)
+                               .arg(qmlContext(item)->nameForObject(item))
+                               .arg(qmlProperty.propertyTypeName())
+                               .arg(value.toString())
+                               .arg(value.typeName());
+            }
         } else {
             qmlInfo(item) << UbuntuI18n::instance().tr("property \"%1\" does not exist or is not writable for object %2")
                              .arg(propertyName).arg(qmlContext(item)->nameForObject(item));
@@ -173,6 +185,15 @@
             QVariant value = qmlProperty.read();
             if (static_cast<QMetaType::Type>(value.type()) != QMetaType::QObjectStar) {
                 m_archive.data()->setValue(propertyName, value);
+                /* Save the type of the property along with its value.
+                 * This is important because QSettings deserializes values as QString.
+                 * Setting these strings to QML properties usually works because the
+                 * implicit type conversion from string to the type of the QML property
+                 * usually works. In some cases cases however (e.g. enum) it fails.
+                 *
+                 * See Qt Bug: https://bugreports.qt-project.org/browse/QTBUG-40474
+                 */
+                m_archive.data()->setValue(propertyName + "_TYPE", QVariant::fromValue((int)value.type()));
                 result++;
             }
         }

=== added file 'tests/unit_x11/tst_statesaver/SaveEnum.qml'
--- tests/unit_x11/tst_statesaver/SaveEnum.qml	1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_statesaver/SaveEnum.qml	2014-07-29 09:48:52 +0000
@@ -0,0 +1,23 @@
+/*
+ * 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/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 1.1
+
+Text {
+    id: testItem
+    StateSaver.properties: "horizontalAlignment"
+}

=== modified file 'tests/unit_x11/tst_statesaver/tst_statesaver.cpp'
--- tests/unit_x11/tst_statesaver/tst_statesaver.cpp	2014-04-10 09:36:36 +0000
+++ tests/unit_x11/tst_statesaver/tst_statesaver.cpp	2014-07-29 09:48:52 +0000
@@ -36,8 +36,10 @@
 #include <signal.h>
 
 #define protected public
+#define private public
 #include "ucstatesaver.h"
 #include "statesaverbackend_p.h"
+#undef private
 #undef protected
 
 class tst_StateSaverTest : public QObject
@@ -59,6 +61,8 @@
     {
         Q_EMIT StateSaverBackend::instance().initiateStateSaving();
         view.reset();
+        // Make sure that the state is reloaded from file
+        StateSaverBackend::instance().m_archive.data()->sync();
         view.reset(new UbuntuTestCase(file));
     }
 
@@ -66,6 +70,8 @@
     {
         Q_EMIT StateSaverBackend::instance().initiateStateSaving();
         view.reset();
+        // Make sure that the state is reloaded from file
+        StateSaverBackend::instance().m_archive.data()->sync();
         view.reset(createView(file));
     }
 
@@ -171,6 +177,31 @@
         }
     }
 
+    void test_SaveEnum()
+    {
+        /* This test is important because when saving the value of an enum
+         * property to QSettings, it is deserialized as a string. Setting that
+         * string value to an enum property fails and therefore the state
+         * restoration does not work.
+         * In most cases implicit type conversion from string to the appropriate
+         * type works.
+         */
+        QScopedPointer<QQuickView> view(createView("SaveEnum.qml"));
+        QVERIFY(view);
+        QObject *testItem = view->rootObject();
+        QVERIFY(testItem);
+
+        testItem->setProperty("horizontalAlignment", Qt::AlignRight);
+
+        resetView(view, "SaveEnum.qml");
+        QVERIFY(view);
+        testItem = view->rootObject();
+        QVERIFY(testItem);
+
+
+        QVERIFY(testItem->property("horizontalAlignment") == Qt::AlignRight);
+    }
+
     void test_SavePropertyGroup()
     {
         QScopedPointer<QQuickView> view(createView("SavePropertyGroups.qml"));

=== modified file 'tests/unit_x11/tst_statesaver/tst_statesaver.pro'
--- tests/unit_x11/tst_statesaver/tst_statesaver.pro	2014-02-06 19:16:34 +0000
+++ tests/unit_x11/tst_statesaver/tst_statesaver.pro	2014-07-29 09:48:52 +0000
@@ -26,4 +26,5 @@
     RepeaterStates.qml \
     ListViewItems.qml \
     GridViewItems.qml \
-    NormalAppClose.qml
+    NormalAppClose.qml \
+    SaveEnum.qml