Partial fix for deferred properties when combined with components

Current deferred properties implementation did not store context
or compiled data pointers correctly. Those pointers are now stored
when the defer is reached, so as to avoid confusion (confusion leads
to asserts or crashes).

Does not extend the deferred property support to allow multiple
deferred blocks per item. This now prints and error and the side
effect is only that one of the deferred blocks is lost. This use
case is sufficiently rare that it may not be worth the cost.

Task-number: QTBUG-30325
Change-Id: I80cb074ed4452e95020208a0142a91e721bced7d
Reviewed-by: Matthew Vogt <matthew.vogt@qinetic.com.au>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/qml/qml/qqmlcompileddata.cpp b/src/qml/qml/qqmlcompileddata.cpp
index 7279762..cd527f6 100644 (file)
--- a/src/qml/qml/qqmlcompileddata.cpp
+++ b/src/qml/qml/qqmlcompileddata.cpp
@@ -223,7 +223,6 @@ QQmlInstruction::Type QQmlCompiledData::instructionType(const QQmlInstruction *i
         return QQmlInstruction::I;
 
     FOR_EACH_QML_INSTR(QML_CHECK_INSTR_CODE)
-    Q_UNREACHABLE();
     Q_ASSERT_X(false, Q_FUNC_INFO, "Invalid instruction address");
     return static_cast<QQmlInstruction::Type>(0);
 #  undef QML_CHECK_INSTR_CODE

diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 14f1fef..cedb2d6 100644 (file)
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -82,7 +82,7 @@ public:
           hasTaintedV8Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
           hasVMEMetaObject(false), parentFrozen(false), notifyList(0), context(0), outerContext(0),
           bindings(0), signalHandlers(0), nextContextObject(0), prevContextObject(0), bindingBitsSize(0), bindingBits(0),
-          lineNumber(0), columnNumber(0), compiledData(0), deferredIdx(0), v8objectid(0),
+          lineNumber(0), columnNumber(0), compiledData(0), deferredData(0), v8objectid(0),
           propertyCache(0), guards(0), extendedData(0) {
         init();
     }
@@ -173,8 +173,13 @@ public:
     quint16 lineNumber;
     quint16 columnNumber;
 
+    struct DeferredData {
+        unsigned int deferredIdx;
+        QQmlCompiledData *compiledData;//Not always the same as the other compiledData
+        QQmlContextData *context;//Could be either context or outerContext
+    };
     QQmlCompiledData *compiledData;
-    unsigned int deferredIdx;
+    DeferredData *deferredData;
 
     quint32 v8objectid;
     v8::Persistent<v8::Object> v8object;

diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index c7e69a7..6466213 100644 (file)
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1285,7 +1285,7 @@ void qmlExecuteDeferred(QObject *object)
 {
     QQmlData *data = QQmlData::get(object);
 
-    if (data && data->compiledData && data->deferredIdx) {
+    if (data && data->deferredData) {
         QQmlObjectCreatingProfiler prof;
         if (prof.enabled) {
             QQmlType *type = QQmlMetaType::qmlType(object->metaObject());
@@ -1300,8 +1300,9 @@ void qmlExecuteDeferred(QObject *object)
         QQmlComponentPrivate::beginDeferred(ep, object, &state);
 
         // Release the reference for the deferral action (we still have one from construction)
-        data->compiledData->release();
-        data->compiledData = 0;
+        data->deferredData->compiledData->release();
+        delete data->deferredData;
+        data->deferredData = 0;
 
         QQmlComponentPrivate::complete(ep, &state);
     }
@@ -1542,6 +1543,12 @@ void QQmlData::destroyed(QObject *object)
         compiledData = 0;
     }
 
+    if (deferredData) {
+        deferredData->compiledData->release();
+        delete deferredData;
+        deferredData = 0;
+    }
+
     QQmlAbstractBoundSignal *signalHandler = signalHandlers;
     while (signalHandler) {
         if (signalHandler->isEvaluating()) {

diff --git a/src/qml/qml/qqmlinstruction.cpp b/src/qml/qml/qqmlinstruction.cpp
index c2eba72..d58971b 100644 (file)
--- a/src/qml/qml/qqmlinstruction.cpp
+++ b/src/qml/qml/qqmlinstruction.cpp
@@ -58,7 +58,7 @@ void QQmlCompiledData::dump(QQmlInstruction *instr, int idx)
         qWarning().nospace() << idx << "\t\t" << "INIT\t\t\t" << instr->init.bindingsSize << "\t" << instr->init.parserStatusSize << "\t" << instr->init.contextCache << "\t" << instr->init.compiledBinding;
         break;
     case QQmlInstruction::DeferInit:
-        qWarning().nospace() << idx << "\t\t" << "DEFER_INIT\t\t" << instr->deferInit.bindingsSize << "\t" << instr->deferInit.parserStatusSize;
+        qWarning().nospace() << idx << "\t\t" << "DEFER_INIT\t\t" << instr->deferInit.bindingsSize << "\t" << instr->deferInit.parserStatusSize << "\t" << instr->deferInit.objectStackSize << "\t" << instr->deferInit.listStackSize;
         break;
     case QQmlInstruction::Done:
         qWarning().nospace() << idx << "\t\t" << "DONE";

diff --git a/src/qml/qml/qqmlvme.cpp b/src/qml/qml/qqmlvme.cpp
index 8c2902a..736ed5f 100644 (file)
--- a/src/qml/qml/qqmlvme.cpp
+++ b/src/qml/qml/qqmlvme.cpp
@@ -47,6 +47,7 @@
 #include <private/qmetaobjectbuilder_p.h>
 #include "qqmldata_p.h"
 #include "qqml.h"
+#include "qqmlinfo.h"
 #include "qqmlcustomparser_p.h"
 #include "qqmlengine.h"
 #include "qqmlcontext.h"
@@ -134,12 +135,12 @@ bool QQmlVME::initDeferred(QObject *object)
 {
     QQmlData *data = QQmlData::get(object);
 
-    if (!data || !data->context || !data->compiledData)
+    if (!data || !data->deferredData)
         return false;
 
-    QQmlContextData *ctxt = data->context;
-    QQmlCompiledData *comp = data->compiledData;
-    int start = data->deferredIdx;
+    QQmlContextData *ctxt = data->deferredData->context;
+    QQmlCompiledData *comp = data->deferredData->compiledData;
+    int start = data->deferredData->deferredIdx;
 
     State initState;
     initState.flags = State::Deferred;
@@ -1045,10 +1046,19 @@ QObject *QQmlVME::run(QList<QQmlError> *errors,
             if (instr.deferCount) {
                 QObject *target = objects.top();
                 QQmlData *data = QQmlData::get(target, true);
-                data->compiledData = COMP;
-                data->compiledData->addref(); // Keep this data referenced until we're initialized
-                data->deferredIdx = INSTRUCTIONSTREAM - COMP->bytecode.constData();
-                Q_ASSERT(data->deferredIdx != 0);
+                if (data->deferredData) {
+                    //This rare case still won't always work right
+                    qmlInfo(target) << "Setting deferred property across multiple components may not work";
+                    delete data->deferredData;
+                }
+                data->deferredData = new QQmlData::DeferredData;
+                //If we're in a CreateQML here, data->compiledData could be reset later
+                data->deferredData->compiledData = COMP;
+                data->deferredData->context = CTXT;
+                // Keep this data referenced until we're initialized
+                data->deferredData->compiledData->addref();
+                data->deferredData->deferredIdx = INSTRUCTIONSTREAM - COMP->bytecode.constData();
+                Q_ASSERT(data->deferredData->deferredIdx != 0);
                 INSTRUCTIONSTREAM += instr.deferCount;
             }
         QML_END_INSTR(Defer)

diff --git a/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent.qml b/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent.qml
new file mode 100644 (file)
index 0000000..1432e7d
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent.qml
@@ -0,0 +1,10 @@
+import Qt.test 1.0
+import QtQml 2.0
+
+MyDeferredObject {
+    id: root
+    property QtObject target: null
+    objectProperty: MyQmlObject {
+        value: target.value
+    }
+}

diff --git a/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent2.qml b/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent2.qml
new file mode 100644 (file)
index 0000000..de73629
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/MyDeferredComponent2.qml
@@ -0,0 +1,3 @@
+import Qt.test 1.0
+
+MyDeferredObject {}

diff --git a/tests/auto/qml/qqmlecmascript/data/deferredPropertiesInComponents.qml b/tests/auto/qml/qqmlecmascript/data/deferredPropertiesInComponents.qml
new file mode 100644 (file)
index 0000000..868b7b1
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/deferredPropertiesInComponents.qml
@@ -0,0 +1,15 @@
+import Qt.test 1.0
+import QtQml 2.0
+
+QtObject {
+    id: root
+    property int value: 10
+    property QtObject deferredInside: MyDeferredComponent {
+                                          target: root
+                                      }
+    property QtObject deferredOutside: MyDeferredComponent2 {
+                                           objectProperty: MyQmlObject {
+                                               value: root.value
+                                           }
+                                       }
+}

diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 06590f0..c417879 100644 (file)
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -88,6 +88,7 @@ private slots:
     void objectPropertiesTriggerReeval();
     void deferredProperties();
     void deferredPropertiesErrors();
+    void deferredPropertiesInComponents();
     void extensionObjects();
     void overrideExtensionProperties();
     void attachedProperties();
@@ -865,6 +866,37 @@ void tst_qqmlecmascript::deferredPropertiesErrors()
     delete object;
 }
 
+void tst_qqmlecmascript::deferredPropertiesInComponents()
+{
+    // Test that it works when the property is set inside and outside component
+    QQmlComponent component(&engine, testFileUrl("deferredPropertiesInComponents.qml"));
+    QObject *object = component.create();
+    if (!object)
+        qDebug() << component.errorString();
+    QVERIFY(object != 0);
+    QCOMPARE(object->property("value").value<int>(), 10);
+
+    MyDeferredObject *defObjectA =
+        qobject_cast<MyDeferredObject *>(object->property("deferredInside").value<QObject*>());
+    QVERIFY(defObjectA != 0);
+    QVERIFY(defObjectA->objectProperty() == 0);
+
+    qmlExecuteDeferred(defObjectA);
+    QVERIFY(defObjectA->objectProperty() != 0);
+    QCOMPARE(defObjectA->objectProperty()->property("value").value<int>(), 10);
+
+    MyDeferredObject *defObjectB =
+        qobject_cast<MyDeferredObject *>(object->property("deferredOutside").value<QObject*>());
+    QVERIFY(defObjectB != 0);
+    QVERIFY(defObjectB->objectProperty() == 0);
+
+    qmlExecuteDeferred(defObjectB);
+    QVERIFY(defObjectB->objectProperty() != 0);
+    QCOMPARE(defObjectB->objectProperty()->property("value").value<int>(), 10);
+
+    delete object;
+}
+
 void tst_qqmlecmascript::extensionObjects()
 {
     QQmlComponent component(&engine, testFileUrl("extensionObjects.qml"));