Make Behaviors work correctly with value types.

When a value type is referenced by sub-component a grouped
property is built. This code path did not handle behaviors being
declared on the entire value type. Add support for value interceptors
in this code path. Also issue an additional write to the value
type property before calling the interceptor on components
that are not being intercepted, so that the other
sub-components don't get stale values during write back.

Task-number: QTBUG-22625
Change-Id: I3365f422dfa1ab2e536e19575efcceceffb85e10
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

2 files changed, 73 insertions, 7 deletions

diff --git a/src/qml/qml/qqmlcompiler.cpp b/src/qml/qml/qqmlcompiler.cpp
index fd42ab8382..e17163528c 100644
--- a/src/qml/qml/qqmlcompiler.cpp
+++ b/src/qml/qml/qqmlcompiler.cpp
@@ -1413,6 +1413,8 @@ void QQmlCompiler::genValueTypeProperty(QQmlScript::Object *obj,QQmlScript::Prop
     pop.type = prop->type;
     pop.bindingSkipList = 0;
     output->addInstruction(pop);
+
+    genPropertyAssignment(prop, obj);
 }
 
 void QQmlCompiler::genComponent(QQmlScript::Object *obj)
@@ -2106,10 +2108,15 @@ bool QQmlCompiler::buildGroupedProperty(QQmlScript::Property *prop,
         if (prop->type >= 0 && enginePrivate->valueTypes[prop->type]) {
 
             if (!prop->values.isEmpty()) {
-                if (prop->values.first()->location < prop->value->location) {
-                    COMPILE_EXCEPTION(prop->value, tr( "Property has already been assigned a value"));
-                } else {
-                    COMPILE_EXCEPTION(prop->values.first(), tr( "Property has already been assigned a value"));
+                // Only error if we are assigning values, and not e.g. a property interceptor
+                for (Property *dotProp = prop->value->properties.first(); dotProp; dotProp = prop->value->properties.next(dotProp)) {
+                    if (!dotProp->values.isEmpty()) {
+                        if (prop->values.first()->location < prop->value->location) {
+                            COMPILE_EXCEPTION(prop->value, tr( "Property has already been assigned a value"));
+                        } else {
+                            COMPILE_EXCEPTION(prop->values.first(), tr( "Property has already been assigned a value"));
+                        }
+                    }
                 }
             }
 
@@ -2117,7 +2124,6 @@ bool QQmlCompiler::buildGroupedProperty(QQmlScript::Property *prop,
                 COMPILE_EXCEPTION(prop, tr( "Invalid property assignment: \"%1\" is a read-only property").arg(prop->name().toString()));
             }
 
-
             if (prop->isAlias) {
                 for (Property *vtProp = prop->value->properties.first(); vtProp; vtProp = prop->value->properties.next(vtProp)) {
                     vtProp->isAlias = true;
@@ -2126,7 +2132,26 @@ bool QQmlCompiler::buildGroupedProperty(QQmlScript::Property *prop,
 
             COMPILE_CHECK(buildValueTypeProperty(enginePrivate->valueTypes[prop->type],
                                                  prop->value, obj, ctxt.incr()));
+
+            // When building a value type where sub components are declared, this
+            // code path is followed from buildProperty, even if there is a previous
+            // assignment to the value type as a whole. Therefore we need to look
+            // for (and build) assignments to the entire value type before looking
+            // for any onValue assignments.
+            for (Value *v = prop->values.first(); v; v = Property::ValueList::next(v)) {
+                if (v->object) {
+                    COMPILE_EXCEPTION(v->object, tr("Objects cannot be assigned to value types"));
+                }
+                COMPILE_CHECK(buildPropertyLiteralAssignment(prop, obj, v, ctxt));
+            }
+
+            for (Value *v = prop->onValues.first(); v; v = Property::ValueList::next(v)) {
+                Q_ASSERT(v->object);
+                COMPILE_CHECK(buildPropertyOnAssignment(prop, obj, obj, v, ctxt));
+            }
+
             obj->addValueTypeProperty(prop);
+
         } else {
             COMPILE_EXCEPTION(prop, tr("Invalid grouped property access"));
         }
diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index cc2a158b0a..1c07dd676e 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -577,9 +577,50 @@ int QQmlVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a)
                     else valueType = QQmlValueTypeFactory::valueType(type);
                     Q_ASSERT(valueType);
 
-                    valueType->setValue(QVariant(type, a[0]));
+                    //
+                    // Consider the following case:
+                    //  color c = { 0.1, 0.2, 0.3 }
+                    //  interceptor exists on c.r
+                    //  write { 0.2, 0.4, 0.6 }
+                    //
+                    // The interceptor may choose not to update the r component at this
+                    // point (for example, a behavior that creates an animation). But we
+                    // need to ensure that the g and b components are updated correctly.
+                    //
+                    // So we need to perform a full write where the value type is:
+                    //    r = old value, g = new value, b = new value
+                    //
+                    // And then call the interceptor which may or may not write the
+                    // new value to the r component.
+                    //
+                    // This will ensure that the other components don't contain stale data
+                    // and any relevant signals are emitted.
+                    //
+                    // To achieve this:
+                    //   (1) Store the new value type as a whole (needed due to
+                    //       aliasing between a[0] and static storage in value type).
+                    //   (2) Read the entire existing value type from object -> valueType temp.
+                    //   (3) Read the previous value of the component being changed
+                    //       from the valueType temp.
+                    //   (4) Write the entire new value type into the temp.
+                    //   (5) Overwrite the component being changed with the old value.
+                    //   (6) Perform a full write to the value type (which may emit signals etc).
+                    //   (7) Issue the interceptor call with the new component value.
+                    //
+
                     QMetaProperty valueProp = valueType->metaObject()->property(valueIndex);
-                    vi->write(valueProp.read(valueType));
+                    QVariant newValue(type, a[0]);
+
+                    valueType->read(object, id);
+                    QVariant prevComponentValue = valueProp.read(valueType);
+
+                    valueType->setValue(newValue);
+                    QVariant newComponentValue = valueProp.read(valueType);
+
+                    valueProp.write(valueType, prevComponentValue);
+                    valueType->write(object, id, QQmlPropertyPrivate::DontRemoveBinding | QQmlPropertyPrivate::BypassInterceptor);
+
+                    vi->write(newComponentValue);
 
                     if (!ep) delete valueType;
                     return -1;