From e29f198b2a91b30678ebeea0d4908634e7c2b5be Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Thu, 6 Oct 2011 16:29:52 +1000 Subject: [PATCH] Fix v8 handle management in QSGLoader Previously, setting the source to a remote url and specified some initial property values while active was false, and then setting active to true, would cause undefined behaviour. This commit ensures that the handles are managed correctly, and that an appropriate v8 context exists during object creation with initial property values specified. Task-number: QTBUG-21868 Change-Id: I101c6546537aa05aaeb420195aca670bc71f31e1 Reviewed-on: http://codereview.qt-project.org/6109 Reviewed-by: Qt Sanity Bot Reviewed-by: Aaron Kennedy --- src/declarative/items/qsgloader.cpp | 5 ++++- src/declarative/qml/qdeclarativecomponent.cpp | 3 +++ .../qsgloader/data/initialPropertyValues.8.qml | 20 ++++++++++++++++++++ tests/auto/declarative/qsgloader/tst_qsgloader.cpp | 11 +++++++++++ 4 files changed, 38 insertions(+), 1 deletions(-) create mode 100644 tests/auto/declarative/qsgloader/data/initialPropertyValues.8.qml diff --git a/src/declarative/items/qsgloader.cpp b/src/declarative/items/qsgloader.cpp index 91dce3b..93f32b8 100644 --- a/src/declarative/items/qsgloader.cpp +++ b/src/declarative/items/qsgloader.cpp @@ -266,7 +266,6 @@ void QSGLoader::setActive(bool newVal) } else { loadFromSourceComponent(); } - d->disposeInitialPropertyValues(); // release persistent handles } else { if (d->item) { QSGItemPrivate *p = QSGItemPrivate::get(d->item); @@ -487,6 +486,7 @@ void QSGLoader::setSource(QDeclarativeV8Function *args) d->clear(); QUrl sourceUrl = d->resolveSourceUrl(args); if (!ipv.IsEmpty()) { + d->disposeInitialPropertyValues(); d->initialPropertyValues = qPersistentNew(ipv); d->qmlGlobalForIpv = qPersistentNew(args->qmlGlobal()); } @@ -539,6 +539,7 @@ void QSGLoaderPrivate::_q_sourceLoaded() emit q->sourceComponentChanged(); emit q->statusChanged(); emit q->progressChanged(); + disposeInitialPropertyValues(); // cleanup return; } @@ -557,6 +558,7 @@ void QSGLoaderPrivate::_q_sourceLoaded() completeCreateWithInitialPropertyValues(c, obj, initialPropertyValues, qmlGlobalForIpv); delete obj; delete ctxt; + disposeInitialPropertyValues(); // cleanup return; } if (obj) { @@ -589,6 +591,7 @@ void QSGLoaderPrivate::_q_sourceLoaded() emit q->itemChanged(); emit q->loaded(); } + disposeInitialPropertyValues(); // cleanup } /*! diff --git a/src/declarative/qml/qdeclarativecomponent.cpp b/src/declarative/qml/qdeclarativecomponent.cpp index 0bcfd5a..e4c3003 100644 --- a/src/declarative/qml/qdeclarativecomponent.cpp +++ b/src/declarative/qml/qdeclarativecomponent.cpp @@ -1134,6 +1134,9 @@ QObject *QDeclarativeComponentPrivate::completeCreateObjectWithInitialProperties { QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(engine); QV8Engine *v8engine = ep->v8engine(); + + v8::HandleScope handle_scope; + v8::Context::Scope scope(v8engine->context()); v8::Handle ov = v8engine->newQObject(toCreate); Q_ASSERT(ov->IsObject()); v8::Handle object = v8::Handle::Cast(ov); diff --git a/tests/auto/declarative/qsgloader/data/initialPropertyValues.8.qml b/tests/auto/declarative/qsgloader/data/initialPropertyValues.8.qml new file mode 100644 index 0000000..79e9264 --- /dev/null +++ b/tests/auto/declarative/qsgloader/data/initialPropertyValues.8.qml @@ -0,0 +1,20 @@ +import QtQuick 2.0 + +Item { + id: root + property int initialValue: 0 + + Loader { + id: loader + objectName: "loader" + active: false + onLoaded: { + root.initialValue = loader.item.canary; // should be six + } + } + + Component.onCompleted: { + loader.setSource("http://127.0.0.1:14450/InitialPropertyValuesComponent.qml", {"canary": 6}); + loader.active = true; + } +} diff --git a/tests/auto/declarative/qsgloader/tst_qsgloader.cpp b/tests/auto/declarative/qsgloader/tst_qsgloader.cpp index ba8d5e0..a073e1f 100644 --- a/tests/auto/declarative/qsgloader/tst_qsgloader.cpp +++ b/tests/auto/declarative/qsgloader/tst_qsgloader.cpp @@ -651,6 +651,11 @@ void tst_QSGLoader::initialPropertyValues_data() << QStringList() << (QStringList() << "loaderValue" << "createObjectValue") << (QVariantList() << 1 << 1); + + QTest::newRow("ensure initial property values aren't disposed prior to component completion") << TEST_FILE("initialPropertyValues.8.qml") + << QStringList() + << (QStringList() << "initialValue") + << (QVariantList() << 6); } void tst_QSGLoader::initialPropertyValues() @@ -660,12 +665,18 @@ void tst_QSGLoader::initialPropertyValues() QFETCH(QStringList, propertyNames); QFETCH(QVariantList, propertyValues); + TestHTTPServer server(SERVER_PORT); + QVERIFY(server.isValid()); + server.serveDirectory(SRCDIR "/data"); + foreach (const QString &warning, expectedWarnings) QTest::ignoreMessage(QtWarningMsg, warning.toAscii().constData()); QDeclarativeComponent component(&engine, qmlFile); QObject *object = component.create(); QVERIFY(object != 0); + qApp->processEvents(); + QTest::qWait(50); for (int i = 0; i < propertyNames.size(); ++i) QCOMPARE(object->property(propertyNames.at(i).toAscii().constData()), propertyValues.at(i)); -- 1.7.2.5