Merge lp:~larsu/gsettings-qt/lp1503693 into lp:gsettings-qt

[Lars Karlitski]

qgsettings-qml: explicitly emit changed events

We're working around QTBUG-32859 by not dispatching changed signals
directly. This leads to two issues:

1. Changed signals are not emitted by GSettingsQml, because it only
emits those when the value is different from its cached one, which is
already updated by the time the queued signal arrives.

2. Calling schema.reset() from qml and immediately fetching the value
after returns the old value, because the value was only updated when the
signal arrives.

Fix these by explicitly emitting the changed signal when setting the
value from qml (the later one is then ignored) and setting the cached
value when calling reset().

Also, remove changesSpy from test_reset(), because that now counts
changes correctly and includes the ones from previous tests.

This patch can be reverted once the Qt bug is fixed.

qgsettings-qml: explicitly emit changed events

[Albert Astals Cid (aacid)]

any reason you removed the signalspy? i think it's good checking that reset does actually trigger the signal too, proves that qml will re-evaluate things that depend on it, something like

    changesSpy.clear();
    settings.schema.reset('testInteger');
    tryCompare(changesSpy, "count", 1);

What do you think?

review: Needs Information

[lp:~larsu/gsettings-qt/lp1503693]

75. By Lars Karlitski on 2015-11-04

qml tests: put changesSpy back

To check that the signal gets emitted when a key is reset.

[Lars Karlitski (larsu)]

> any reason you removed the signalspy? i think it's good checking that reset
> does actually trigger the signal too, proves that qml will re-evaluate things
> that depend on it, something like
>
> changesSpy.clear();
> settings.schema.reset('testInteger');
> tryCompare(changesSpy, "count", 1);
>
> What do you think?

Good point. I removed it because it was counting the emitted signals for all tests. Didn't think about looking at the docs to find something like "clear". Thanks!

[Albert Astals Cid (aacid)]

It's non ideal but since fixing that bug in qt is much harder than this, let's approve this and hope they fix the bug in qt at some point.

review: Approve

=== modified file 'GSettings/gsettings-qml.cpp'
--- GSettings/gsettings-qml.cpp	2015-06-02 10:10:03 +0000
+++ GSettings/gsettings-qml.cpp	2015-11-04 15:29:45 +0000
@@ -104,8 +104,13 @@
 {
     GSettingsQml *parent = (GSettingsQml *) this->parent();
 
-    if (parent->priv->settings != NULL)
+    if (parent->priv->settings != NULL) {
         parent->priv->settings->reset(key);
+
+        // make sure this object gets the new value immediately and not on the
+        // next main loop iteration (see updateValue)
+        parent->settingChanged(key);
+    }
 }
 
 GSettingsQml::GSettingsQml(QObject *parent): QQmlPropertyMap(this, parent)
@@ -161,6 +166,13 @@
         return QVariant();
 
     if (priv->settings->trySet(key, value)) {
+        // due to QTBUG-32859, QGSettings doesn't dispatch its changed signal
+        // directly, but on a new main loop iteration. At that point, this
+        // object already has the new value set and doesn't emit its own
+        // changed signal (see ::settingChanged). Emit it here so that it is
+        // sent even when the setting is changed from qml.
+        Q_EMIT(changed(key, value));
+
         return value;
     }
     else {

=== modified file 'tests/tst_GSettings.qml'
--- tests/tst_GSettings.qml	2015-06-02 10:10:03 +0000
+++ tests/tst_GSettings.qml	2015-11-04 15:29:45 +0000
@@ -95,9 +95,11 @@
 
   function test_reset() {
     settings.testInteger = 4;
+
+    changesSpy.clear();
     settings.schema.reset('testInteger');
-    tryCompare(changesSpy, "count", 1); // this merely exists only to connect to the "changed" signal; resetting changes is async in gsettings
     compare(settings.testInteger, 42);
+    tryCompare(changesSpy, "count", 1);
   }
 
   function test_invalid_schema() {