From 5dc7649f5ad7dca6e13707e827d001c3f118a6ef Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Tue, 28 Jan 2014 14:04:58 +0100 Subject: [Regression] Fix lazy binding evaluation Commit 04774bb14c81688f86a2b31b8624bde8ebf59062 long time ago introduced the concept of fixed order binding initialization with lazy evaluation, where a bit is reserved for each binding that indicates whether it's been initialized the first time or not. When reading a property on a QObject, we'd check if the corresponding binding for the property has been initialized or not and flush (i.e. execute) the binding if necessary. As part of the V4/V8 clean-up, commit 1eb41200948ab414f1c47d93123b41c547a993df removed the StoreV8Binding instruction, which made the call for setting the this-binding-is-not-evaluated-yet bit. Nowadays we only use StoreBinding, for which this optimization was never implemented (and not needed really). Now that we have a unified JS code path, we need to set the pending binding bit and also make sure that we call flushPendingBinding for any JS side property access (accelerated or not). Also flushPendingBindingImpl had two bugs: * In an attempt of trying to find the binding to flush, it could happen that we'd try to flush a previously destroyed binding (m_mePtr is null), so the b variable would remain the first binding in the object and we'd flush the wrong one (instead of none). Added a missing check to verify that the property index matches. * Also resetting the mePtr must be done through clear(), to ensure that the pointer in bindValues in the VME is also cleared, to avoid re-enabling the same binding again in complete(); Task-number: QTBUG-36441 Change-Id: Icdb0c8fb036051fd5d6c4d33b10cd0c0ed9a9d5c Reviewed-by: Lars Knoll --- src/qml/jsruntime/qv4qobjectwrapper.cpp | 3 ++- src/qml/qml/qqmlengine.cpp | 4 ++-- src/qml/qml/qqmlvme.cpp | 6 ++++++ .../auto/qml/qqmlecmascript/data/LazyBindingComponent.qml | 13 +++++++++++++ .../qml/qqmlecmascript/data/lazyBindingEvaluation.qml | 6 ++++++ tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 15 ++++++++++++++- 6 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 tests/auto/qml/qqmlecmascript/data/LazyBindingComponent.qml create mode 100644 tests/auto/qml/qqmlecmascript/data/lazyBindingEvaluation.qml diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 61f92a0f5c..cd0c81846c 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -317,7 +317,6 @@ ReturnedValue QObjectWrapper::getQmlProperty(ExecutionContext *ctx, QQmlContextD return QV4::Object::get(this, name, hasProperty); } - QQmlData::flushPendingBinding(m_object, result->coreIndex); QQmlData *ddata = QQmlData::get(m_object, false); if (revisionMode == QV4::QObjectWrapper::CheckRevision && result->hasRevision()) { @@ -338,6 +337,8 @@ ReturnedValue QObjectWrapper::getProperty(QObject *object, ExecutionContext *ctx { QV4::Scope scope(ctx); + QQmlData::flushPendingBinding(object, property->coreIndex); + if (property->isFunction() && !property->isVarProperty()) { if (property->isVMEFunction()) { QQmlVMEMetaObject *vmemo = QQmlVMEMetaObject::get(object); diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index f8e5ad5874..acc66c8ce5 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -777,8 +777,8 @@ void QQmlData::flushPendingBindingImpl(int coreIndex) while (b && *b->m_mePtr && b->propertyIndex() != coreIndex) b = b->nextBinding(); - if (b) { - b->m_mePtr = 0; + if (b && b->propertyIndex() == coreIndex) { + b->clear(); b->setEnabled(true, QQmlPropertyPrivate::BypassInterceptor | QQmlPropertyPrivate::DontRemoveBinding); } diff --git a/src/qml/qml/qqmlvme.cpp b/src/qml/qml/qqmlvme.cpp index 1881b554ed..dfc184b9f7 100644 --- a/src/qml/qml/qqmlvme.cpp +++ b/src/qml/qml/qqmlvme.cpp @@ -881,6 +881,12 @@ QObject *QQmlVME::run(QList *errors, CLEAN_PROPERTY(target, QDPP::bindingIndex(instr.property)); bind->addToObject(); + + if (!instr.property.isValueTypeVirtual()) { + QQmlData *data = QQmlData::get(target); + Q_ASSERT(data); + data->setPendingBindingBit(target, instr.property.coreIndex); + } } QML_END_INSTR(StoreBinding) diff --git a/tests/auto/qml/qqmlecmascript/data/LazyBindingComponent.qml b/tests/auto/qml/qqmlecmascript/data/LazyBindingComponent.qml new file mode 100644 index 0000000000..81cb56f0e5 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/LazyBindingComponent.qml @@ -0,0 +1,13 @@ +import QtQuick 2.0 + +Item +{ + property int someInt: 4 + property var variantArray: [1, 2] + property int arrayLength: 0 + + onSomeIntChanged: + { + arrayLength = variantArray.length + } +} diff --git a/tests/auto/qml/qqmlecmascript/data/lazyBindingEvaluation.qml b/tests/auto/qml/qqmlecmascript/data/lazyBindingEvaluation.qml new file mode 100644 index 0000000000..2f55ff0709 --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/lazyBindingEvaluation.qml @@ -0,0 +1,6 @@ +import QtQuick 2.1 + +LazyBindingComponent +{ + someInt: 5 +} diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index c45750caac..3f412ee6d4 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -321,6 +321,7 @@ private slots: void qtbug_34792(); void noCaptureWhenWritingProperty(); void singletonWithEnum(); + void lazyBindingEvaluation(); private: // static void propertyVarWeakRefCallback(v8::Persistent object, void* parameter); @@ -667,7 +668,7 @@ void tst_qqmlecmascript::methods() void tst_qqmlecmascript::bindingLoop() { QQmlComponent component(&engine, testFileUrl("bindingLoop.qml")); - QString warning = component.url().toString() + ":5:9: QML MyQmlObject: Binding loop detected for property \"stringProperty\""; + QString warning = component.url().toString() + ":9:9: QML MyQmlObject: Binding loop detected for property \"stringProperty\""; QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData()); QObject *object = component.create(); QVERIFY(object != 0); @@ -7520,6 +7521,18 @@ void tst_qqmlecmascript::singletonWithEnum() QCOMPARE(prop.toInt(), int(SingletonWithEnum::TestValue)); } +void tst_qqmlecmascript::lazyBindingEvaluation() +{ + QQmlComponent component(&engine, testFileUrl("lazyBindingEvaluation.qml")); + QScopedPointer obj(component.create()); + if (obj.isNull()) + qDebug() << component.errors().first().toString(); + QVERIFY(!obj.isNull()); + QVariant prop = obj->property("arrayLength"); + QVERIFY(prop.type() == QVariant::Int); + QCOMPARE(prop.toInt(), 2); +} + QTEST_MAIN(tst_qqmlecmascript) #include "tst_qqmlecmascript.moc" -- cgit v1.2.3