From d5c7229339a916b2f1f004ec4ea9de89995c003d Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 16 Mar 2018 17:27:38 +0100 Subject: Restore the QV4_WRITE_PERF_MAP feature We want to be able to generate perf map files for JITed code. Task-number: QTBUG-67056 Change-Id: I56899e1dbf184083d94efe926d21fca4f9ea1e18 Reviewed-by: Simon Hausmann --- src/qml/jit/qv4assembler.cpp | 47 ++++++++++++++++++--- tests/auto/qml/qjsengine/tst_qjsengine.cpp | 68 ++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/qml/jit/qv4assembler.cpp b/src/qml/jit/qv4assembler.cpp index 186e5952da..72b057b2bc 100644 --- a/src/qml/jit/qv4assembler.cpp +++ b/src/qml/jit/qv4assembler.cpp @@ -38,6 +38,7 @@ ****************************************************************************/ #include +#include #include "qv4engine_p.h" #include "qv4assembler_p.h" @@ -1367,6 +1368,17 @@ static void printDisassembledOutputWithCalls(QByteArray processedOutput, qDebug("%s", processedOutput.constData()); } +static QByteArray functionName(Function *function) +{ + QByteArray name = function->name()->toQString().toUtf8(); + if (name.isEmpty()) { + name = QByteArray::number(reinterpret_cast(function), 16); + name.prepend("QV4::Function(0x"); + name.append(')'); + } + return name; +} + void Assembler::link(Function *function) { for (const auto &jumpTarget : pasm()->patches) @@ -1388,12 +1400,7 @@ void Assembler::link(Function *function) buf.open(QIODevice::WriteOnly); WTF::setDataFile(new QIODevicePrintStream(&buf)); - QByteArray name = function->name()->toQString().toUtf8(); - if (name.isEmpty()) { - name = QByteArray::number(quintptr(function), 16); - name.prepend("QV4::Function(0x"); - name.append(')'); - } + QByteArray name = functionName(function); codeRef = linkBuffer.finalizeCodeWithDisassembly("%s", name.data()); WTF::setDataFile(stderr); @@ -1404,6 +1411,34 @@ void Assembler::link(Function *function) function->codeRef = new JSC::MacroAssemblerCodeRef(codeRef); function->jittedCode = reinterpret_cast(function->codeRef->code().executableAddress()); + +#if defined(Q_OS_LINUX) + // This implements writing of JIT'd addresses so that perf can find the + // symbol names. + // + // Perf expects the mapping to be in a certain place and have certain + // content, for more information, see: + // https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt + static bool doProfile = !qEnvironmentVariableIsEmpty("QV4_PROFILE_WRITE_PERF_MAP"); + if (doProfile) { + static QFile perfMapFile(QString::fromLatin1("/tmp/perf-%1.map") + .arg(QCoreApplication::applicationPid())); + static const bool isOpen = perfMapFile.open(QIODevice::WriteOnly); + if (!isOpen) { + qWarning("QV4::JIT::Assembler: Cannot write perf map file."); + doProfile = false; + } else { + perfMapFile.write(QByteArray::number(reinterpret_cast( + codeRef.code().executableAddress()), 16)); + perfMapFile.putChar(' '); + perfMapFile.write(QByteArray::number(static_cast(codeRef.size()), 16)); + perfMapFile.putChar(' '); + perfMapFile.write(functionName(function)); + perfMapFile.putChar('\n'); + perfMapFile.flush(); + } + } +#endif } void Assembler::addLabel(int offset) diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp index 8f3011001b..8ffaa96569 100644 --- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp +++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp @@ -202,6 +202,7 @@ private slots: void malformedExpression(); void scriptScopes(); + void perfMapFile(); signals: void testSignal(); @@ -4130,6 +4131,73 @@ void tst_QJSEngine::scriptScopes() QCOMPARE(use.toInt(), 42); } +static const char *perfMapKey = "QV4_PROFILE_WRITE_PERF_MAP"; +static const char *jitCallKey = "QV4_JIT_CALL_THRESHOLD"; + +struct EnvironmentModifier { + const bool hasPerfMap = false; + const bool hasJitCall = false; + const QByteArray perfMap; + const QByteArray jitCall; + + EnvironmentModifier() : + hasPerfMap(qEnvironmentVariableIsSet(perfMapKey)), + hasJitCall(qEnvironmentVariableIsSet(jitCallKey)), + perfMap(qgetenv(perfMapKey)), + jitCall(qgetenv(jitCallKey)) + { + qputenv(perfMapKey, "1"); + qputenv(jitCallKey, "0"); + } + + ~EnvironmentModifier() + { + if (hasPerfMap) + qputenv(perfMapKey, perfMap); + else + qunsetenv(perfMapKey); + + if (hasJitCall) + qputenv(jitCallKey, jitCall); + else + qunsetenv(jitCallKey); + } +}; + +void tst_QJSEngine::perfMapFile() +{ +#if !defined(Q_OS_LINUX) + QSKIP("perf map files are only generated on linux"); +#else + EnvironmentModifier modifier; + Q_UNUSED(modifier); + QJSEngine engine; + QJSValue def = engine.evaluate("'use strict'; function foo() { return 42 }"); + QVERIFY(!def.isError()); + QJSValue use = engine.evaluate("'use strict'; foo()"); + QVERIFY(use.isNumber()); + QFile file(QString::fromLatin1("/tmp/perf-%1.map").arg(QCoreApplication::applicationPid())); + QVERIFY(file.exists()); + QVERIFY(file.open(QIODevice::ReadOnly)); + QList functions; + while (!file.atEnd()) { + const QByteArray contents = file.readLine(); + QVERIFY(contents.endsWith('\n')); + QList fields = contents.split(' '); + QCOMPARE(fields.length(), 3); + bool ok = false; + const qulonglong address = fields[0].toULongLong(&ok, 16); + QVERIFY(ok); + QVERIFY(address > 0); + const ulong size = fields[1].toULong(&ok, 16); + QVERIFY(ok); + QVERIFY(size > 0); + functions.append(fields[2]); + } + QVERIFY(functions.contains("foo\n")); +#endif +} + QTEST_MAIN(tst_QJSEngine) #include "tst_qjsengine.moc" -- cgit v1.2.3 From a3ad52526f79c1528f170c8affe5af00b68ca61d Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 15 Mar 2018 17:08:21 +0100 Subject: Fix crash when calling QQmlEngine::clearComponentCache() We must protect various resources in the type loader with our existing lock. The QQmlTypeLoaderQmldirContent is now value based, so that we can release the lock on the shared cache early. Copying it involves adjusting the refcount of the QHash and QString instances in the QQmlDirParser. The safety of this was verified with a TSAN build and the example supplied in the task. It crashed reliably with TASN errors first and with this patch it runs without errors. Task-number: QTBUG-41465 Change-Id: I616843c4b8bdfd65d1277d4faa8cb884d8e77df8 Reviewed-by: Lars Knoll --- src/qml/qml/qqmldirparser_p.h | 2 -- src/qml/qml/qqmlengine.cpp | 2 ++ src/qml/qml/qqmlimport.cpp | 62 +++++++++++++++++++++--------------------- src/qml/qml/qqmlimport_p.h | 2 +- src/qml/qml/qqmltypeloader.cpp | 55 ++++++++++++++++++++----------------- src/qml/qml/qqmltypeloader_p.h | 9 ++++-- 6 files changed, 71 insertions(+), 61 deletions(-) diff --git a/src/qml/qml/qqmldirparser_p.h b/src/qml/qml/qqmldirparser_p.h index 95370398ad..820c40238d 100644 --- a/src/qml/qml/qqmldirparser_p.h +++ b/src/qml/qml/qqmldirparser_p.h @@ -63,8 +63,6 @@ class QQmlError; class QQmlEngine; class Q_QML_PRIVATE_EXPORT QQmlDirParser { - Q_DISABLE_COPY(QQmlDirParser) - public: QQmlDirParser(); ~QQmlDirParser(); diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 4054d2f0be..49f25e89fe 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1104,7 +1104,9 @@ QQmlEngine::~QQmlEngine() void QQmlEngine::clearComponentCache() { Q_D(QQmlEngine); + d->typeLoader.lock(); d->typeLoader.clearCache(); + d->typeLoader.unlock(); } /*! diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp index 8e6cbcbd7e..005db4248e 100644 --- a/src/qml/qml/qqmlimport.cpp +++ b/src/qml/qml/qqmlimport.cpp @@ -318,17 +318,17 @@ public: QQmlImportDatabase *database, QString *outQmldirFilePath, QString *outUrl); - static bool validateQmldirVersion(const QQmlTypeLoaderQmldirContent *qmldir, const QString &uri, int vmaj, int vmin, + static bool validateQmldirVersion(const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri, int vmaj, int vmin, QList *errors); bool importExtension(const QString &absoluteFilePath, const QString &uri, int vmaj, int vmin, QQmlImportDatabase *database, - const QQmlTypeLoaderQmldirContent *qmldir, + const QQmlTypeLoaderQmldirContent &qmldir, QList *errors); bool getQmldirContent(const QString &qmldirIdentifier, const QString &uri, - const QQmlTypeLoaderQmldirContent **qmldir, QList *errors); + QQmlTypeLoaderQmldirContent *qmldir, QList *errors); QString resolvedUri(const QString &dir_arg, QQmlImportDatabase *database); @@ -668,14 +668,14 @@ bool QQmlImports::resolveType(const QHashedStringRef &type, return false; } -bool QQmlImportInstance::setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent *qmldir, QQmlImportNamespace *nameSpace, QList *errors) +bool QQmlImportInstance::setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent &qmldir, QQmlImportNamespace *nameSpace, QList *errors) { Q_ASSERT(resolvedUrl.endsWith(Slash)); url = resolvedUrl; - qmlDirComponents = qmldir->components(); + qmlDirComponents = qmldir.components(); - const QQmlDirScripts &scripts = qmldir->scripts(); + const QQmlDirScripts &scripts = qmldir.scripts(); if (!scripts.isEmpty()) { // Verify that we haven't imported these scripts already for (QList::const_iterator it = nameSpace->imports.constBegin(); @@ -1068,26 +1068,26 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, const QString &uri, int vmaj, int vmin, QQmlImportDatabase *database, - const QQmlTypeLoaderQmldirContent *qmldir, + const QQmlTypeLoaderQmldirContent &qmldir, QList *errors) { - Q_ASSERT(qmldir); + Q_ASSERT(qmldir.hasContent()); if (qmlImportTrace()) qDebug().nospace() << "QQmlImports(" << qPrintable(base) << ")::importExtension: " << "loaded " << qmldirFilePath; - if (designerSupportRequired && !qmldir->designerSupported()) { + if (designerSupportRequired && !qmldir.designerSupported()) { if (errors) { QQmlError error; - error.setDescription(QQmlImportDatabase::tr("module does not support the designer \"%1\"").arg(qmldir->typeNamespace())); + error.setDescription(QQmlImportDatabase::tr("module does not support the designer \"%1\"").arg(qmldir.typeNamespace())); error.setUrl(QUrl::fromLocalFile(qmldirFilePath)); errors->prepend(error); } return false; } - int qmldirPluginCount = qmldir->plugins().count(); + int qmldirPluginCount = qmldir.plugins().count(); if (qmldirPluginCount == 0) return true; @@ -1098,7 +1098,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, // listed plugin inside qmldir. And for this reason, mixing dynamic and static plugins inside a // single module is not recommended. - QString typeNamespace = qmldir->typeNamespace(); + QString typeNamespace = qmldir.typeNamespace(); QString qmldirPath = qmldirFilePath; int slash = qmldirPath.lastIndexOf(Slash); if (slash > 0) @@ -1108,7 +1108,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, int staticPluginsFound = 0; #if defined(QT_SHARED) - const auto qmldirPlugins = qmldir->plugins(); + const auto qmldirPlugins = qmldir.plugins(); for (const QQmlDirParser::Plugin &plugin : qmldirPlugins) { QString resolvedFilePath = database->resolvePlugin(typeLoader, qmldirPath, plugin.path, plugin.name); if (!resolvedFilePath.isEmpty()) { @@ -1174,7 +1174,7 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, if (qmldirPluginCount > 1 && staticPluginsFound > 0) error.setDescription(QQmlImportDatabase::tr("could not resolve all plugins for module \"%1\"").arg(uri)); else - error.setDescription(QQmlImportDatabase::tr("module \"%1\" plugin \"%2\" not found").arg(uri).arg(qmldir->plugins()[dynamicPluginsFound].name)); + error.setDescription(QQmlImportDatabase::tr("module \"%1\" plugin \"%2\" not found").arg(uri).arg(qmldir.plugins()[dynamicPluginsFound].name)); error.setUrl(QUrl::fromLocalFile(qmldirFilePath)); errors->prepend(error); } @@ -1187,17 +1187,17 @@ bool QQmlImportsPrivate::importExtension(const QString &qmldirFilePath, } bool QQmlImportsPrivate::getQmldirContent(const QString &qmldirIdentifier, const QString &uri, - const QQmlTypeLoaderQmldirContent **qmldir, QList *errors) + QQmlTypeLoaderQmldirContent *qmldir, QList *errors) { Q_ASSERT(errors); Q_ASSERT(qmldir); *qmldir = typeLoader->qmldirContent(qmldirIdentifier); - if (*qmldir) { + if ((*qmldir).hasContent()) { // Ensure that parsing was successful - if ((*qmldir)->hasError()) { + if ((*qmldir).hasError()) { QUrl url = QUrl::fromLocalFile(qmldirIdentifier); - const QList qmldirErrors = (*qmldir)->errors(uri); + const QList qmldirErrors = (*qmldir).errors(uri); for (int i = 0; i < qmldirErrors.size(); ++i) { QQmlError error = qmldirErrors.at(i); error.setUrl(url); @@ -1323,14 +1323,14 @@ bool QQmlImportsPrivate::locateQmldir(const QString &uri, int vmaj, int vmin, QQ return false; } -bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent *qmldir, const QString &uri, int vmaj, int vmin, +bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri, int vmaj, int vmin, QList *errors) { int lowest_min = INT_MAX; int highest_min = INT_MIN; typedef QQmlDirComponents::const_iterator ConstIterator; - const QQmlDirComponents &components = qmldir->components(); + const QQmlDirComponents &components = qmldir.components(); ConstIterator cend = components.constEnd(); for (ConstIterator cit = components.constBegin(); cit != cend; ++cit) { @@ -1354,7 +1354,7 @@ bool QQmlImportsPrivate::validateQmldirVersion(const QQmlTypeLoaderQmldirContent } typedef QList::const_iterator SConstIterator; - const QQmlDirScripts &scripts = qmldir->scripts(); + const QQmlDirScripts &scripts = qmldir.scripts(); SConstIterator send = scripts.constEnd(); for (SConstIterator sit = scripts.constBegin(); sit != send; ++sit) { @@ -1446,14 +1446,14 @@ bool QQmlImportsPrivate::addLibraryImport(const QString& uri, const QString &pre Q_ASSERT(inserted); if (!incomplete) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!qmldirIdentifier.isEmpty()) { if (!getQmldirContent(qmldirIdentifier, uri, &qmldir, errors)) return false; - if (qmldir) { - if (!importExtension(qmldir->pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) + if (qmldir.hasContent()) { + if (!importExtension(qmldir.pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) return false; if (!inserted->setQmldirContent(qmldirUrl, qmldir, nameSpace, errors)) @@ -1471,7 +1471,7 @@ bool QQmlImportsPrivate::addLibraryImport(const QString& uri, const QString &pre error.setDescription(QQmlImportDatabase::tr("module \"%1\" is not installed").arg(uri)); errors->prepend(error); return false; - } else if ((vmaj >= 0) && (vmin >= 0) && qmldir) { + } else if ((vmaj >= 0) && (vmin >= 0) && qmldir.hasContent()) { // Verify that the qmldir content is valid for this version if (!validateQmldirVersion(qmldir, uri, vmaj, vmin, errors)) return false; @@ -1565,12 +1565,12 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix Q_ASSERT(inserted); if (!incomplete && !qmldirIdentifier.isEmpty()) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!getQmldirContent(qmldirIdentifier, importUri, &qmldir, errors)) return false; - if (qmldir) { - if (!importExtension(qmldir->pluginLocation(), importUri, vmaj, vmin, database, qmldir, errors)) + if (qmldir.hasContent()) { + if (!importExtension(qmldir.pluginLocation(), importUri, vmaj, vmin, database, qmldir, errors)) return false; if (!inserted->setQmldirContent(url, qmldir, nameSpace, errors)) @@ -1589,14 +1589,14 @@ bool QQmlImportsPrivate::updateQmldirContent(const QString &uri, const QString & Q_ASSERT(nameSpace); if (QQmlImportInstance *import = nameSpace->findImport(uri)) { - const QQmlTypeLoaderQmldirContent *qmldir = nullptr; + QQmlTypeLoaderQmldirContent qmldir; if (!getQmldirContent(qmldirIdentifier, uri, &qmldir, errors)) return false; - if (qmldir) { + if (qmldir.hasContent()) { int vmaj = import->majversion; int vmin = import->minversion; - if (!importExtension(qmldir->pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) + if (!importExtension(qmldir.pluginLocation(), uri, vmaj, vmin, database, qmldir, errors)) return false; if (import->setQmldirContent(qmldirUrl, qmldir, nameSpace, errors)) { diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h index b70bb5253c..2437979ef8 100644 --- a/src/qml/qml/qqmlimport_p.h +++ b/src/qml/qml/qqmlimport_p.h @@ -85,7 +85,7 @@ struct QQmlImportInstance QQmlDirComponents qmlDirComponents; // a copy of the components listed in the qmldir QQmlDirScripts qmlDirScripts; // a copy of the scripts in the qmldir - bool setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent *qmldir, + bool setQmldirContent(const QString &resolvedUrl, const QQmlTypeLoaderQmldirContent &qmldir, QQmlImportNamespace *nameSpace, QList *errors); static QQmlDirScripts getVersionedScripts(const QQmlDirScripts &qmldirscripts, int vmaj, int vmin); diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 5b954605e0..2b778b0b63 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1370,8 +1370,8 @@ bool QQmlTypeLoader::Blob::updateQmldir(QQmlQmldirData *data, const QV4::Compile if (!importQualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); - const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirIdentifier); - const auto qmldirScripts = qmldir->scripts(); + const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirIdentifier); + const auto qmldirScripts = qmldir.scripts(); for (const QQmlDirParser::Script &script : qmldirScripts) { QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName)); QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl); @@ -1418,8 +1418,8 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL if (!importQualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); - const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirFilePath); - const auto qmldirScripts = qmldir->scripts(); + const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirFilePath); + const auto qmldirScripts = qmldir.scripts(); for (const QQmlDirParser::Script &script : qmldirScripts) { QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName)); QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl); @@ -1584,6 +1584,7 @@ QString QQmlTypeLoaderQmldirContent::typeNamespace() const void QQmlTypeLoaderQmldirContent::setContent(const QString &location, const QString &content) { + m_hasContent = true; m_location = location; m_parser.parse(content); } @@ -1808,6 +1809,7 @@ QString QQmlTypeLoader::absoluteFilePath(const QString &path) int lastSlash = path.lastIndexOf(QLatin1Char('/')); QString dirPath(path.left(lastSlash)); + LockHolder holder(this); if (!m_importDirCache.contains(dirPath)) { bool exists = QDir(dirPath).exists(); QCache *entry = exists ? new QCache : nullptr; @@ -1871,6 +1873,7 @@ bool QQmlTypeLoader::directoryExists(const QString &path) --length; QString dirPath(path.left(length)); + LockHolder holder(this); if (!m_importDirCache.contains(dirPath)) { bool exists = QDir(dirPath).exists(); QCache *files = exists ? new QCache : nullptr; @@ -1889,8 +1892,10 @@ Return a QQmlTypeLoaderQmldirContent for absoluteFilePath. The QQmlTypeLoaderQm It can also be a remote path for a remote directory import, but it will have been cached by now in this case. */ -const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString &filePathIn) +const QQmlTypeLoaderQmldirContent QQmlTypeLoader::qmldirContent(const QString &filePathIn) { + LockHolder holder(this); + QString filePath; // Try to guess if filePathIn is already a URL. This is necessarily fragile, because @@ -1904,39 +1909,39 @@ const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString & filePath = filePathIn; } else { filePath = QQmlFile::urlToLocalFileOrQrc(url); - if (filePath.isEmpty()) // Can't load the remote here, but should be cached - return *(m_importQmlDirCache.value(filePathIn)); + if (filePath.isEmpty()) { // Can't load the remote here, but should be cached + if (auto entry = m_importQmlDirCache.value(filePathIn)) + return **entry; + else + return QQmlTypeLoaderQmldirContent(); + } } - QQmlTypeLoaderQmldirContent *qmldir; QQmlTypeLoaderQmldirContent **val = m_importQmlDirCache.value(filePath); - if (!val) { - qmldir = new QQmlTypeLoaderQmldirContent; + if (val) + return **val; + QQmlTypeLoaderQmldirContent *qmldir = new QQmlTypeLoaderQmldirContent; #define ERROR(description) { QQmlError e; e.setDescription(description); qmldir->setError(e); } #define NOT_READABLE_ERROR QString(QLatin1String("module \"$$URI$$\" definition \"%1\" not readable")) #define CASE_MISMATCH_ERROR QString(QLatin1String("cannot load module \"$$URI$$\": File name case mismatch for \"%1\"")) - QFile file(filePath); - if (!QQml_isFileCaseCorrect(filePath)) { - ERROR(CASE_MISMATCH_ERROR.arg(filePath)); - } else if (file.open(QFile::ReadOnly)) { - QByteArray data = file.readAll(); - qmldir->setContent(filePath, QString::fromUtf8(data)); - } else { - ERROR(NOT_READABLE_ERROR.arg(filePath)); - } + QFile file(filePath); + if (!QQml_isFileCaseCorrect(filePath)) { + ERROR(CASE_MISMATCH_ERROR.arg(filePath)); + } else if (file.open(QFile::ReadOnly)) { + QByteArray data = file.readAll(); + qmldir->setContent(filePath, QString::fromUtf8(data)); + } else { + ERROR(NOT_READABLE_ERROR.arg(filePath)); + } #undef ERROR #undef NOT_READABLE_ERROR #undef CASE_MISMATCH_ERROR - m_importQmlDirCache.insert(filePath, qmldir); - } else { - qmldir = *val; - } - - return qmldir; + m_importQmlDirCache.insert(filePath, qmldir); + return *qmldir; } void QQmlTypeLoader::setQmldirContent(const QString &url, const QString &content) diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index df79d13f1c..4665d342c1 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -228,12 +228,16 @@ class QQmlTypeLoaderQmldirContent { private: friend class QQmlTypeLoader; - QQmlTypeLoaderQmldirContent(); void setContent(const QString &location, const QString &content); void setError(const QQmlError &); public: + QQmlTypeLoaderQmldirContent(); + QQmlTypeLoaderQmldirContent(const QQmlTypeLoaderQmldirContent &) = default; + QQmlTypeLoaderQmldirContent &operator=(const QQmlTypeLoaderQmldirContent &) = default; + + bool hasContent() const { return m_hasContent; } bool hasError() const; QList errors(const QString &uri) const; @@ -250,6 +254,7 @@ public: private: QQmlDirParser m_parser; QString m_location; + bool m_hasContent = false; }; class Q_QML_PRIVATE_EXPORT QQmlTypeLoader @@ -304,7 +309,7 @@ public: QString absoluteFilePath(const QString &path); bool directoryExists(const QString &path); - const QQmlTypeLoaderQmldirContent *qmldirContent(const QString &filePath); + const QQmlTypeLoaderQmldirContent qmldirContent(const QString &filePath); void setQmldirContent(const QString &filePath, const QString &content); void clearCache(); -- cgit v1.2.3 From 958e412a25523cc031564faae81c569aa6c3b01f Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 19 Mar 2018 10:47:24 +0100 Subject: QML debugger: Don't crash when creating objects on engine destruction You can create further objects while the QML engine is being destroyed. The debug service is not interested in those because they will be rather short lived anyway. Task-number: QTBUG-62458 Change-Id: If5395ef058268e0e956d159bc636495da1c0c98f Reviewed-by: Simon Hausmann --- .../qmldbg_debugger/qqmlenginedebugservice.cpp | 3 ++- .../tst_qqmlenginedebugservice.cpp | 24 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/plugins/qmltooling/qmldbg_debugger/qqmlenginedebugservice.cpp b/src/plugins/qmltooling/qmldbg_debugger/qqmlenginedebugservice.cpp index 288ad243ce..236109d041 100644 --- a/src/plugins/qmltooling/qmldbg_debugger/qqmlenginedebugservice.cpp +++ b/src/plugins/qmltooling/qmldbg_debugger/qqmlenginedebugservice.cpp @@ -812,7 +812,8 @@ void QQmlEngineDebugServiceImpl::engineAboutToBeRemoved(QJSEngine *engine) void QQmlEngineDebugServiceImpl::objectCreated(QJSEngine *engine, QObject *object) { Q_ASSERT(engine); - Q_ASSERT(m_engines.contains(engine)); + if (!m_engines.contains(engine)) + return; int engineId = QQmlDebugService::idForObject(engine); int objectId = QQmlDebugService::idForObject(object); diff --git a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp index d01d9a6791..89217e7556 100644 --- a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp +++ b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp @@ -141,6 +141,7 @@ private slots: void queryObjectWithNonStreamableTypes(); void asynchronousCreate(); void invalidContexts(); + void createObjectOnDestruction(); }; QmlDebugObjectReference tst_QQmlEngineDebugService::findRootObject( @@ -1345,6 +1346,29 @@ void tst_QQmlEngineDebugService::invalidContexts() QCOMPARE(m_dbg->rootContext().contexts.count(), 0); } +void tst_QQmlEngineDebugService::createObjectOnDestruction() +{ + QSignalSpy spy(m_dbg, SIGNAL(newObject(int))); + { + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData( + "import QtQml 2.0;" + "QtObject {" + "property Component x:" + "Qt.createQmlObject('import QtQml 2.0; Component { QtObject { } }'," + "this, 'x.qml');" + "Component.onDestruction: x.createObject(this, {});" + "}", QUrl::fromLocalFile("x.qml")); + QVERIFY(component.isReady()); + QVERIFY(component.create()); + QTRY_COMPARE(spy.count(), 2); + } + // Doesn't crash and doesn't give us another signal for the object created on destruction. + QTest::qWait(500); + QCOMPARE(spy.count(), 2); +} + int main(int argc, char *argv[]) { int _argc = argc + 1; -- cgit v1.2.3 From e1d32c80665c7d90a21138b26cb74dbfc86a63ba Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 15 Mar 2018 13:52:07 +0100 Subject: Fix CONFIG+=qtquickcompiler with Q_CLEANUP_RESOURCE As we provide the init resources wrapper, we must also provide the cleanup wrapper. Change-Id: I7e45ae48ba955e70ffd8e253d4d2c15d0a50dabe Task-number: QTBUG-67087 Reviewed-by: Lars Knoll --- tools/qmlcachegen/generateloader.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/qmlcachegen/generateloader.cpp b/tools/qmlcachegen/generateloader.cpp index 1a0b987c64..a2e673d15a 100644 --- a/tools/qmlcachegen/generateloader.cpp +++ b/tools/qmlcachegen/generateloader.cpp @@ -358,15 +358,23 @@ bool generateLoader(const QStringList &compiledFiles, const QString &outputFileN originalResourceFile.truncate(mappingSplit); } - const QString function = QLatin1String("qInitResources_") + qtResourceNameForFile(originalResourceFile); + const QString suffix = qtResourceNameForFile(originalResourceFile); + const QString initFunction = QLatin1String("qInitResources_") + suffix; - stream << QStringLiteral("int QT_MANGLE_NAMESPACE(%1)() {\n").arg(function); + stream << QStringLiteral("int QT_MANGLE_NAMESPACE(%1)() {\n").arg(initFunction); stream << " ::unitRegistry();\n"; if (!newResourceFile.isEmpty()) stream << " Q_INIT_RESOURCE(" << qtResourceNameForFile(newResourceFile) << ");\n"; stream << " return 1;\n"; stream << "}\n"; - stream << "Q_CONSTRUCTOR_FUNCTION(QT_MANGLE_NAMESPACE(" << function << "));\n"; + stream << "Q_CONSTRUCTOR_FUNCTION(QT_MANGLE_NAMESPACE(" << initFunction << "));\n"; + + const QString cleanupFunction = QLatin1String("qCleanupResources_") + suffix; + stream << QStringLiteral("int QT_MANGLE_NAMESPACE(%1)() {\n").arg(cleanupFunction); + if (!newResourceFile.isEmpty()) + stream << " Q_CLEANUP_RESOURCE(" << qtResourceNameForFile(newResourceFile) << ");\n"; + stream << " return 1;\n"; + stream << "}\n"; } } -- cgit v1.2.3 From f495d4b660107536d0a67ba48e88550278f13893 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 19 Mar 2018 15:07:28 +0100 Subject: Fix out of bounds reads in Array.concat In some cases, when our simple array data had an offset and data would wrap around, ArrayData::append would write out of bounds data into the new array, leading to crashes. Task-number: QTBUG-51581 Change-Id: I55172542ef0b94d263cfc9a17d7ca49ec6c3a565 Reviewed-by: Simon Hausmann --- src/qml/jsruntime/qv4arraydata.cpp | 2 +- tests/auto/qml/qjsengine/tst_qjsengine.cpp | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/qml/jsruntime/qv4arraydata.cpp b/src/qml/jsruntime/qv4arraydata.cpp index 30c8527f21..b9c0e12305 100644 --- a/src/qml/jsruntime/qv4arraydata.cpp +++ b/src/qml/jsruntime/qv4arraydata.cpp @@ -617,7 +617,7 @@ uint ArrayData::append(Object *obj, ArrayObject *otherObj, uint n) uint toCopy = n; uint chunk = toCopy; if (chunk > os->values.alloc - os->offset) - chunk -= os->values.alloc - os->offset; + chunk = os->values.alloc - os->offset; obj->arrayPut(oldSize, os->values.data() + os->offset, chunk); toCopy -= chunk; if (toCopy) diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp index 8ffaa96569..8a017fd573 100644 --- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp +++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp @@ -128,6 +128,7 @@ private slots: void JSONparse(); void arraySort(); void lookupOnDisappearingProperty(); + void arrayConcat(); void qRegExpInport_data(); void qRegExpInport(); @@ -3009,6 +3010,19 @@ void tst_QJSEngine::lookupOnDisappearingProperty() QVERIFY(func.call(QJSValueList()<< o).isUndefined()); } +void tst_QJSEngine::arrayConcat() +{ + QJSEngine eng; + QJSValue v = eng.evaluate("var x = [1, 2, 3, 4, 5, 6];" + "var y = [];" + "for (var i = 0; i < 5; ++i)" + " x.shift();" + "for (var i = 10; i < 13; ++i)" + " x.push(i);" + "x.toString();"); + QCOMPARE(v.toString(), QString::fromLatin1("6,10,11,12")); +} + static QRegExp minimal(QRegExp r) { r.setMinimal(true); return r; } void tst_QJSEngine::qRegExpInport_data() -- cgit v1.2.3 From f01edde1f282b0de2112ddea31853d876d3c5727 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 13:51:14 +0100 Subject: Fix grammar in "Qt Quick Best Practices" Change-Id: I3ba63110a3674675f44feca51ba98128844e1904 Reviewed-by: Simon Hausmann --- src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index 6327ea67e6..82b5a10b60 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -28,7 +28,7 @@ /*! \page qtquick-bestpractices.html \title Qt Quick Best Practices -\brief Lists the practices that works best for Qt Quick +\brief Lists the practices that work best for Qt Quick Besides all the benefits that Qt Quick offers, it can be challenging in certain situations. For example, a Qt Quick application with a large codebase can be @@ -48,7 +48,7 @@ controls are also available with Qt Quick Controls 2. They cater to the most common use cases without any change, and offer a lot more possibilities with their customization options. In particular, Qt Quick Controls 2 provides styling options that align with the latest UI design trends. If these UI controls do not -satisfy to your application's needs, only then it is recommended to create a +satisfy your application's needs, only then it is recommended to create a custom control. -- cgit v1.2.3 From a07d2f21a4d445fae7a7cc130e45a8fd72997713 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Mon, 19 Mar 2018 12:01:06 +0100 Subject: tests: un-blacklist tst_qquickitem::ignoreButtonPressNotInAcceptedMouseButtons() This change reverts 8740f35e69 and 8ef46910fc. The qtestlib limitation was fixed by b3e91b66b9175c1c3ff5f73f3ac231f74f9bf932 Task-number: QTBUG-63957 Change-Id: I2e12b1eff25c5ea8005db0893477a9732f24d211 Reviewed-by: Liang Qi --- tests/auto/quick/qquickitem/BLACKLIST | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/auto/quick/qquickitem/BLACKLIST b/tests/auto/quick/qquickitem/BLACKLIST index f00b061356..d94a3ef102 100644 --- a/tests/auto/quick/qquickitem/BLACKLIST +++ b/tests/auto/quick/qquickitem/BLACKLIST @@ -1,4 +1,2 @@ [contains:hollow square: testing points inside] xcb -[ignoreButtonPressNotInAcceptedMouseButtons] -* -- cgit v1.2.3 From 1de4d413c9dd5c8ad5859f944d9ded7d8778f777 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 19 Mar 2018 17:01:21 +0100 Subject: Fix assigning objects to QJSValue properties We support simple object bindings such as someProperty: Rectangle { ... } when the type of "someProperty" is QVariant, but we produce an error when it's QJSValue. There is no good reason for that, and the fix for QTBUG-67118 requires this. Change-Id: Ia5dc88749bcba0b5c781a6ab2b4a9fb92299e0ac Reviewed-by: Mitch Curtis Reviewed-by: Lars Knoll --- src/qml/compiler/qqmlpropertyvalidator.cpp | 2 +- src/qml/qml/qqmlobjectcreator.cpp | 12 ++++++++++++ tests/auto/qml/qqmllanguage/data/assignLiteralToJSValue.qml | 1 + tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp | 5 +++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/qml/compiler/qqmlpropertyvalidator.cpp b/src/qml/compiler/qqmlpropertyvalidator.cpp index 00bb694ef4..ffd3b5975a 100644 --- a/src/qml/compiler/qqmlpropertyvalidator.cpp +++ b/src/qml/compiler/qqmlpropertyvalidator.cpp @@ -653,7 +653,7 @@ QQmlCompileError QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData * // Can only check at instantiation time if the created sub-object successfully casts to the // target interface. return noError; - } else if (property->propType() == QMetaType::QVariant) { + } else if (property->propType() == QMetaType::QVariant || property->propType() == qMetaTypeId()) { // We can convert everything to QVariant :) return noError; } else if (property->isQList()) { diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 90f3beb40b..7051fb51da 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -55,6 +55,7 @@ #include #include #include +#include QT_USE_NAMESPACE @@ -1039,6 +1040,17 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper argv[0] = &value; QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv); } + } else if (bindingProperty->propType() == qMetaTypeId()) { + QV4::Scope scope(v4); + QV4::ScopedValue wrappedObject(scope, QV4::QObjectWrapper::wrap(engine->handle(), createdSubObject)); + if (bindingProperty->isVarProperty()) { + _vmeMetaObject->setVMEProperty(bindingProperty->coreIndex(), wrappedObject); + } else { + QJSValue value; + QJSValuePrivate::setValue(&value, v4, wrappedObject); + argv[0] = &value; + QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv); + } } else if (bindingProperty->isQList()) { Q_ASSERT(_currentList.object); diff --git a/tests/auto/qml/qqmllanguage/data/assignLiteralToJSValue.qml b/tests/auto/qml/qqmllanguage/data/assignLiteralToJSValue.qml index fce248a381..69b0096a5e 100644 --- a/tests/auto/qml/qqmllanguage/data/assignLiteralToJSValue.qml +++ b/tests/auto/qml/qqmllanguage/data/assignLiteralToJSValue.qml @@ -30,6 +30,7 @@ QtObject { MyQmlObject { id: testObj22; objectName: "test22"; qjsvalue: null }, MyQmlObject { id: testObj1Bound; objectName: "test1Bound"; qjsvalue: testObj1.qjsvalue + 4 }, // 1 + 4 + 4 = 9 MyQmlObject { id: testObj20Bound; objectName: "test20Bound"; qjsvalue: testObj20.qjsvalue(testObj1Bound.qjsvalue) }, // 9 * 3 = 27 + MyQmlObject { id: testObj23; objectName: "test23"; qjsvalue: QtObject { objectName: "blah" } }, QtObject { id: varProperties objectName: "varProperties" diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp index f1f35f9fd4..18b1718ddf 100644 --- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp +++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp @@ -934,6 +934,11 @@ void tst_qqmllanguage::assignLiteralToJSValue() QJSValue value = object->qjsvalue(); QVERIFY(value.isNumber()); QCOMPARE(value.toNumber(), qreal(27)); + } { + MyQmlObject *object = root->findChild("test23"); + QJSValue value = object->qjsvalue(); + QVERIFY(value.isQObject()); + QCOMPARE(value.toQObject()->objectName(), "blah"); } } -- cgit v1.2.3 From a4adaee3a8c6a1048bbfe8286fef9009150c781c Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 14:27:32 +0100 Subject: Doc: reorganize "Bundle Application Resources" section Start off with the "direct" syntax, as that's the most straight-forward. Then, explain why it can be inefficient and introduce the concept of separate .qrc files. Change-Id: I63c2c3e188db04ed58e816f7e69ab98a42196ff1 Reviewed-by: hjk Reviewed-by: Simon Hausmann --- .../doc/src/guidelines/qtquick-bestpractices.qdoc | 48 ++++++++++++++-------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index 82b5a10b60..20611f5b04 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -80,34 +80,46 @@ rich user experience. It can often be a challenge to make these resources available to the application regardless of the target OS. Most popular OS-es employ stricter security policies that restrict access to the file system, making it harder to load these resources. As an alternative, Qt offers its own -resource system that is built into the application binary, enabling access to -the application's resources regardless of the target OS. +\l {The Qt Resource System}{resource system} that is built into the +application binary, enabling access to the application's resources regardless +of the target OS. -It is recommended to bundle your application's resources (including the -\c .qml files) into a resource file (\c.qrc). For example, the following entry -in the qmake project file ensures that the resources are built into the -application binary, making them available when needed: +For example, the following entry in the qmake project file ensures that the +resources are built into the application binary, making them available when +needed: \badcode - RESOURCES += resources.qrc + RESOURCES += a.qml b.png \endcode -If your application depends on a limited number of resources, you could list -them directly in the project file. +It's also possible to use a +\l {files(pattern[, recursive=false])}{wildcard syntax} to select several files +at once: \badcode - RESOURCES += a.qml b.png + RESOURCES += $$files(*.qml) $$files(*.png) +\endcode + +This approach is convenient for applications that depend on a limited number +of resources. However, whenever a new file is added to \c RESOURCES using this +approach, it causes \e all of the other files in \c RESOURCES to be recompiled +as well. This can be inefficient, especially for large sets of files. +In this case, a better approach is to separate each type of resource into its +own \l {Resource Collection Files (.qrc)}{.qrc} file. For example, the snippet +above could be changed like so: + +\badcode + RESOURCES += qml.qrc images.qrc \endcode -In such a case, qmake creates the \c qmake_intermediate.qrc build artifact, -which you could rename and use the \c{RESOURCES += resource-set.qrc} entry -instead. +Now, whenever a QML file is changed in qml.qrc, only the QML files have to be +recompiled. -You could go a step further by using one \c .qrc file for each resource type. -For example, list the \c .qml files in \c files.qrc, images in -\c images.qrc, fonts in \c fonts.qrc, and so on. That way, you need not -recompile the QML files when you, for example, add an image to the list in -\c images.qrc. +If you want to conveniently switch from using the "direct" syntax to a .qrc +file, look for \c qmake_intermediate.qrc (a build artifact created by qmake) +in your project's build directory and rename it to, for example, +\c resources.qrc. Then, replace the old \c RESOURCES entry with +\c {RESOURCES += resources.qrc}. \section2 Related Information \list -- cgit v1.2.3 From 0f719a8fa5a91945e698af4f7295256653ef78e5 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 14:39:23 +0100 Subject: Doc: simplify "Qt Quick Best Practices" introduction paragraph We don't need to mention specific challenges in the introduction. Change-Id: I03eec5fe543fbf0b1859850424eba8cfa9f134a5 Reviewed-by: J-P Nurmi Reviewed-by: Simon Hausmann --- src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index 20611f5b04..20ea8ec48c 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -30,10 +30,10 @@ \title Qt Quick Best Practices \brief Lists the practices that work best for Qt Quick -Besides all the benefits that Qt Quick offers, it can be challenging in certain -situations. For example, a Qt Quick application with a large codebase can be -painful to maintain if not organized well. The following sections elaborate -on some of the best practices that will help you get better results. +Despite all of the benefits that Qt Quick offers, it can be challenging in +certain situations. The following sections elaborate on some of the best +practices that will help you get better results when developing Qt Quick +applications. \section1 Custom UI Controls -- cgit v1.2.3 From 971292128b292052ef935da67a5d04fb5a3753f4 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 15:46:28 +0100 Subject: Doc: improve "Separate UI from Logic" section Change-Id: I0d36e009b4551e45e5e7fda6c95fc3fbfabfe1a5 Reviewed-by: Simon Hausmann --- .../doc/src/guidelines/qtquick-bestpractices.qdoc | 48 ++++++++++++++-------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index 20ea8ec48c..9d411a581f 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -126,33 +126,47 @@ in your project's build directory and rename it to, for example, \li \l{The Qt Resource System} \endlist -\section1 Application UI and Business Logic +\section1 Separate UI from Logic One of the key goals that most application developers want to achieve is to create a maintainable application. One of the ways to achieve this goal is -to separate the UI from the business logic. The following are a few reasons -why application's UI should be in QML: +to separate the user interface from the business logic. The following are a few +reasons why an application's UI should be written in QML: \list - \li QML is a declarative language, which suits best for defining UIs. - \li It's easier to embed JavaScript in QML to respond to events, for example. - \li QML is faster to code as it is not strongly typed. + \li Declarative languages are strongly suited for defining UIs. + \li QML code is simpler to write, as it is less verbose than C++, and is not + strongly typed. This also results in it being an excellent language to + prototype in, a quality that is vital when collaborating with designers, + for example. + \li JavaScript can easily be used in QML to respond to events. \endlist -On the other hand, C++ being a strongly typed language, suits best for defining -business logic. Typically, such code performs tasks such as complex calculations -or larger data processing, which can be faster with C++ than with QML. +Being a strongly typed language, C++ is best suited for an application's logic. +Typically, such code performs tasks such as complex calculations +or data processing, which are faster in C++ than QML. Qt offers various approaches to integrate QML and C++ code in an application. -In most cases, the C++ part (business logic) provides the data model to the QML -part (UI), which presents it in a readable form. It is often a challenge to -decide when to use this approach. It is recommended to use QML -if the data model is static, simple, and small, as C++ could be overkill. -Use C++ if the application depends on a dynamic, large, and complex data model. +A typical use case is displaying a list of data in a user interface. +If the data set is static, simple, and/or small, a model written in QML can be +sufficient. -\omit -examples snippets of simpler and complex data models. -\endomit +The following snippet demonstrates examples of models written in QML: + +\qml + model: ListModel { + ListElement { name: "Item 1" } + ListElement { name: "Item 2" } + ListElement { name: "Item 3" } + } + + model: [ "Item 1", "Item 2", "Item 3" ] + + model: 10 +\endqml + +Use \l {QAbstractItemModel Subclass}{C++} for dynamic data sets that are large +or frequently modified. \section2 Interaction Path -- cgit v1.2.3 From 22b13921f8067f8a93164875a4ad59bed85b0400 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Mon, 19 Mar 2018 13:14:13 +0100 Subject: Handle function expressions as signal handlers There are two ways to use function expressions on the right-hand side of bindings: property var somethingPressed somethingPressed: function() { /* ..press something else.. */ } signal buttonPressed onButtonPressed: function() { /* ..handle buttonPress.. */ } In the former case, it declares a property that holds a function. So on initialization, the right-hand side of the binding returns a closure that gets assigned to the property 'somethingPressed'. In the latter case, the signal handler is explicitly marked as a function for clarity. So, the handler should not be returning the closure, but the handler should *be* the closure. In general, it is not possible to detect if the left-hand side is a property or a signal handler when generating QML cache files ahead of time. So for this case, we mark the function as only returning a closure. Then when instantiating the object, we check if it is a signal handler, and if the handler is marked as only returning a closure. If so, we set that closure to be the signal handler. Task-number: QTBUG-57043 Task-number: QTBUG-50328 Change-Id: I3008ddd847e30b7d0adef07344a326f84d85f1ba Reviewed-by: Simon Hausmann --- src/qml/compiler/qv4codegen.cpp | 8 ++++ src/qml/compiler/qv4compileddata_p.h | 2 +- src/qml/compiler/qv4compiler.cpp | 4 +- src/qml/compiler/qv4compilercontext_p.h | 1 + src/qml/jsruntime/qv4function_p.h | 7 +++ src/qml/qml/qqmlobjectcreator.cpp | 8 ++++ src/qml/types/qqmlconnections.cpp | 14 ++++-- tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp | 64 ++++++++++++++++++++++++++ 8 files changed, 103 insertions(+), 5 deletions(-) diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index bc4ca5d6f4..a262908960 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -2075,6 +2075,14 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast, _context->hasDirectEval |= (_context->compilationMode == EvalCode || _context->compilationMode == GlobalCode || _module->debugMode); // Conditional breakpoints are like eval in the function + // When a user writes the following QML signal binding: + // onSignal: function() { doSomethingUsefull } + // we will generate a binding function that just returns the closure. However, that's not useful + // at all, because if the onSignal is a signal handler, the user is actually making it explicit + // that the binding is a function, so we should execute that. However, we don't know that during + // AOT compilation, so mark the surrounding function as only-returning-a-closure. + _context->returnsClosure = cast(ast) && cast(cast(ast)->expression); + BytecodeGenerator bytecode(_context->line, _module->debugMode); BytecodeGenerator *savedBytecodeGenerator; savedBytecodeGenerator = bytecodeGenerator; diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index f1776f5772..e6c4225bbd 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -225,7 +225,7 @@ struct Function quint32_le localsOffset; quint32_le nLineNumbers; quint32_le lineNumberOffset; - quint32_le nInnerFunctions; + quint32_le nestedFunctionIndex; // for functions that only return a single closure, used in signal handlers quint32_le nRegisters; Location location; diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index f2e1f4a0de..ccc909c199 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -305,6 +305,9 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->flags |= CompiledData::Function::IsStrict; if (irFunction->hasTry || irFunction->hasWith) function->flags |= CompiledData::Function::HasCatchOrWith; + function->nestedFunctionIndex = + irFunction->returnsClosure ? quint32(module->functions.indexOf(irFunction->nestedContexts.first())) + : std::numeric_limits::max(); function->nFormals = irFunction->arguments.size(); function->formalsOffset = currentOffset; currentOffset += function->nFormals * sizeof(quint32); @@ -317,7 +320,6 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->lineNumberOffset = currentOffset; currentOffset += function->nLineNumbers * sizeof(CompiledData::CodeOffsetToLine); - function->nInnerFunctions = irFunction->nestedContexts.size(); function->nRegisters = irFunction->registerCount; diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h index a78a66db52..8fabf41c40 100644 --- a/src/qml/compiler/qv4compilercontext_p.h +++ b/src/qml/compiler/qv4compilercontext_p.h @@ -144,6 +144,7 @@ struct Context { bool usesThis = false; bool hasTry = false; bool hasWith = false; + bool returnsClosure = false; mutable bool argumentsCanEscape = false; enum UsesArgumentsObject { diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h index 4c8c790ca7..59a94e5dde 100644 --- a/src/qml/jsruntime/qv4function_p.h +++ b/src/qml/jsruntime/qv4function_p.h @@ -105,6 +105,13 @@ struct Q_QML_EXPORT Function { { return QQmlSourceLocation(sourceFile(), compiledFunction->location.line, compiledFunction->location.column); } + + Function *nestedFunction() const + { + if (compiledFunction->nestedFunctionIndex == std::numeric_limits::max()) + return nullptr; + return compilationUnit->runtimeFunctions[compiledFunction->nestedFunctionIndex]; + } }; } diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 7051fb51da..7c36f59035 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -879,6 +879,14 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper if (binding->type == QV4::CompiledData::Binding::Type_Script || binding->containsTranslations()) { if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression) { QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // When a user writes the following: + // onSignal: function() { doSomethingUsefull } + // then do not run the binding that returns the closure, but run the closure + // instead. + if (auto closure = runtimeFunction->nestedFunction()) + runtimeFunction = closure; + int signalIndex = _propertyCache->methodIndexToSignalIndex(bindingProperty->coreIndex()); QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine); QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(_bindingTarget, signalIndex, diff --git a/src/qml/types/qqmlconnections.cpp b/src/qml/types/qqmlconnections.cpp index a43562a7b8..d1a7aa9b6f 100644 --- a/src/qml/types/qqmlconnections.cpp +++ b/src/qml/types/qqmlconnections.cpp @@ -288,9 +288,17 @@ void QQmlConnections::connectSignals() new QQmlBoundSignal(target, signalIndex, this, qmlEngine(this)); signal->setEnabled(d->enabled); - QQmlBoundSignalExpression *expression = ctxtdata ? - new QQmlBoundSignalExpression(target, signalIndex, - ctxtdata, this, d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]) : nullptr; + auto f = d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // If the function is marked as having a nested function, then the user wrote: + // onSomeSignal: function() { /*....*/ } + // So take that nested function: + if (auto closure = f->nestedFunction()) + f = closure; + + QQmlBoundSignalExpression *expression = + ctxtdata ? new QQmlBoundSignalExpression(target, signalIndex, ctxtdata, this, f) + : nullptr; signal->takeExpression(expression); d->boundsignals += signal; } else { diff --git a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp index 2f5ff0022e..e36edca699 100644 --- a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp +++ b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp @@ -47,6 +47,7 @@ private slots: void signalHandlerParameters(); void errorOnArgumentsInSignalHandler(); void aheadOfTimeCompilation(); + void functionExpressions(); void workerScripts(); }; @@ -292,6 +293,69 @@ void tst_qmlcachegen::workerScripts() QTRY_VERIFY(obj->property("success").toBool()); } +void tst_qmlcachegen::functionExpressions() +{ + QTemporaryDir tempDir; + QVERIFY(tempDir.isValid()); + + const auto writeTempFile = [&tempDir](const QString &fileName, const char *contents) { + QFile f(tempDir.path() + '/' + fileName); + const bool ok = f.open(QIODevice::WriteOnly | QIODevice::Truncate); + Q_ASSERT(ok); + f.write(contents); + return f.fileName(); + }; + + const QString testFilePath = writeTempFile( + "test.qml", + "import QtQuick 2.0\n" + "Item {\n" + " id: di\n" + " \n" + " property var f\n" + " property bool f_called: false\n" + " f : function() { f_called = true }\n" + " \n" + " signal g\n" + " property bool g_handler_called: false\n" + " onG: function() { g_handler_called = true }\n" + " \n" + " signal h(int i)\n" + " property bool h_connections_handler_called: false\n" + " Connections {\n" + " target: di\n" + " onH: function(magic) { h_connections_handler_called = (magic == 42)\n }\n" + " }\n" + " \n" + " function runTest() { \n" + " f()\n" + " g()\n" + " h(42)\n" + " }\n" + "}"); + + QVERIFY(generateCache(testFilePath)); + + const QString cacheFilePath = testFilePath + QLatin1Char('c'); + QVERIFY(QFile::exists(cacheFilePath)); + QVERIFY(QFile::remove(testFilePath)); + + QQmlEngine engine; + CleanlyLoadingComponent component(&engine, QUrl::fromLocalFile(testFilePath)); + QScopedPointer obj(component.create()); + QVERIFY(!obj.isNull()); + + QCOMPARE(obj->property("f_called").toBool(), false); + QCOMPARE(obj->property("g_handler_called").toBool(), false); + QCOMPARE(obj->property("h_connections_handler_called").toBool(), false); + + QMetaObject::invokeMethod(obj.data(), "runTest"); + + QCOMPARE(obj->property("f_called").toBool(), true); + QCOMPARE(obj->property("g_handler_called").toBool(), true); + QCOMPARE(obj->property("h_connections_handler_called").toBool(), true); +} + QTEST_GUILESS_MAIN(tst_qmlcachegen) #include "tst_qmlcachegen.moc" -- cgit v1.2.3 From 64e90f393146dadb382d154e7d67a9109ab2492a Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 9 Mar 2018 16:40:38 +0100 Subject: Fix QML data structure version checking for ahead-of-time generated files We must also do version checking for QML and JS files that were compiled ahead of time and are embedded in resources. If the lookup for the original source code fails, then we must generate an appropriate error message. As an upside we get better error reporting when trying to load an empty file and Qt.include() now reports the error message in the statusText field. The error reporting for imported scripts was not changed as importing an empty script is (oddly) allowed. Task-number: QTBUG-66986 Change-Id: Ie0ef81af371a51ecf8c66ae7954d43f5cc6c12de Reviewed-by: Erik Verbruggen --- src/qml/compiler/qv4compilationunitmapper.cpp | 32 --------------- src/qml/compiler/qv4compilationunitmapper_p.h | 2 - src/qml/compiler/qv4compilationunitmapper_unix.cpp | 2 +- src/qml/compiler/qv4compilationunitmapper_win.cpp | 2 +- src/qml/compiler/qv4compileddata.cpp | 38 ++++++++++++++++++ src/qml/compiler/qv4compileddata_p.h | 2 + src/qml/jsruntime/qv4include.cpp | 10 +++-- src/qml/jsruntime/qv4include_p.h | 3 +- src/qml/jsruntime/qv4script.cpp | 17 ++++++-- src/qml/jsruntime/qv4script_p.h | 2 +- src/qml/qml/qqmlmetatype.cpp | 35 +++++++++++++++- src/qml/qml/qqmlmetatype_p.h | 12 +++++- src/qml/qml/qqmltypeloader.cpp | 38 ++++++++++++++---- src/qml/qml/qqmltypeloader_p.h | 5 +++ src/qml/types/qquickworkerscript.cpp | 6 ++- tests/auto/qml/qmlcachegen/qmlcachegen.pro | 2 + tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp | 46 ++++++++++++++++++++++ tests/auto/qml/qmlcachegen/versionchecks.qml | 4 ++ .../qml/qqmlecmascript/data/include_callback.js | 2 +- tests/auto/qml/qqmllanguage/data/empty.errors.txt | 3 +- 20 files changed, 204 insertions(+), 59 deletions(-) create mode 100644 tests/auto/qml/qmlcachegen/versionchecks.qml diff --git a/src/qml/compiler/qv4compilationunitmapper.cpp b/src/qml/compiler/qv4compilationunitmapper.cpp index d94f7ac238..350f6f9485 100644 --- a/src/qml/compiler/qv4compilationunitmapper.cpp +++ b/src/qml/compiler/qv4compilationunitmapper.cpp @@ -59,36 +59,4 @@ CompilationUnitMapper::~CompilationUnitMapper() close(); } -bool CompilationUnitMapper::verifyHeader(const CompiledData::Unit *header, QDateTime sourceTimeStamp, QString *errorString) -{ - if (strncmp(header->magic, CompiledData::magic_str, sizeof(header->magic))) { - *errorString = QStringLiteral("Magic bytes in the header do not match"); - return false; - } - - if (header->version != quint32(QV4_DATA_STRUCTURE_VERSION)) { - *errorString = QString::fromUtf8("V4 data structure version mismatch. Found %1 expected %2").arg(header->version, 0, 16).arg(QV4_DATA_STRUCTURE_VERSION, 0, 16); - return false; - } - - if (header->qtVersion != quint32(QT_VERSION)) { - *errorString = QString::fromUtf8("Qt version mismatch. Found %1 expected %2").arg(header->qtVersion, 0, 16).arg(QT_VERSION, 0, 16); - return false; - } - - if (header->sourceTimeStamp) { - // Files from the resource system do not have any time stamps, so fall back to the application - // executable. - if (!sourceTimeStamp.isValid()) - sourceTimeStamp = QFileInfo(QCoreApplication::applicationFilePath()).lastModified(); - - if (sourceTimeStamp.isValid() && sourceTimeStamp.toMSecsSinceEpoch() != header->sourceTimeStamp) { - *errorString = QStringLiteral("QML source file has a different time stamp than cached file."); - return false; - } - } - - return true; -} - QT_END_NAMESPACE diff --git a/src/qml/compiler/qv4compilationunitmapper_p.h b/src/qml/compiler/qv4compilationunitmapper_p.h index b24f98df7c..80f914c141 100644 --- a/src/qml/compiler/qv4compilationunitmapper_p.h +++ b/src/qml/compiler/qv4compilationunitmapper_p.h @@ -72,8 +72,6 @@ public: void close(); private: - static bool verifyHeader(const QV4::CompiledData::Unit *header, QDateTime sourceTimeStamp, QString *errorString); - #if defined(Q_OS_UNIX) size_t length; #endif diff --git a/src/qml/compiler/qv4compilationunitmapper_unix.cpp b/src/qml/compiler/qv4compilationunitmapper_unix.cpp index 38dabc41cf..8348613888 100644 --- a/src/qml/compiler/qv4compilationunitmapper_unix.cpp +++ b/src/qml/compiler/qv4compilationunitmapper_unix.cpp @@ -73,7 +73,7 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co return nullptr; } - if (!verifyHeader(&header, sourceTimeStamp, errorString)) + if (!header.verifyHeader(sourceTimeStamp, errorString)) return nullptr; // Data structure and qt version matched, so now we can access the rest of the file safely. diff --git a/src/qml/compiler/qv4compilationunitmapper_win.cpp b/src/qml/compiler/qv4compilationunitmapper_win.cpp index d7a93ae233..8b000021f8 100644 --- a/src/qml/compiler/qv4compilationunitmapper_win.cpp +++ b/src/qml/compiler/qv4compilationunitmapper_win.cpp @@ -87,7 +87,7 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co return nullptr; } - if (!verifyHeader(&header, sourceTimeStamp, errorString)) + if (!header.verifyHeader(sourceTimeStamp, errorString)) return nullptr; const uint mappingFlags = header.flags & QV4::CompiledData::Unit::ContainsMachineCode diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index cc11b250f3..6220c1bc11 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -755,6 +755,44 @@ void Unit::generateChecksum() #endif } +bool Unit::verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) const +{ +#ifndef V4_BOOTSTRAP + if (strncmp(magic, CompiledData::magic_str, sizeof(magic))) { + *errorString = QStringLiteral("Magic bytes in the header do not match"); + return false; + } + + if (version != quint32(QV4_DATA_STRUCTURE_VERSION)) { + *errorString = QString::fromUtf8("V4 data structure version mismatch. Found %1 expected %2").arg(version, 0, 16).arg(QV4_DATA_STRUCTURE_VERSION, 0, 16); + return false; + } + + if (qtVersion != quint32(QT_VERSION)) { + *errorString = QString::fromUtf8("Qt version mismatch. Found %1 expected %2").arg(qtVersion, 0, 16).arg(QT_VERSION, 0, 16); + return false; + } + + if (sourceTimeStamp) { + // Files from the resource system do not have any time stamps, so fall back to the application + // executable. + if (!expectedSourceTimeStamp.isValid()) + expectedSourceTimeStamp = QFileInfo(QCoreApplication::applicationFilePath()).lastModified(); + + if (expectedSourceTimeStamp.isValid() && expectedSourceTimeStamp.toMSecsSinceEpoch() != sourceTimeStamp) { + *errorString = QStringLiteral("QML source file has a different time stamp than cached file."); + return false; + } + } + + return true; +#else + Q_UNUSED(expectedSourceTimeStamp) + Q_UNUSED(errorString) + return false; +#endif +} + } } diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index e6c4225bbd..49360bc3f1 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -727,6 +727,8 @@ struct Unit quint32_le padding; + bool verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) const; + const Import *importAt(int idx) const { return reinterpret_cast((reinterpret_cast(this)) + offsetToImports + idx * sizeof(Import)); } diff --git a/src/qml/jsruntime/qv4include.cpp b/src/qml/jsruntime/qv4include.cpp index aaf5e3b857..d0d66c9b9a 100644 --- a/src/qml/jsruntime/qv4include.cpp +++ b/src/qml/jsruntime/qv4include.cpp @@ -92,7 +92,8 @@ QV4Include::~QV4Include() #endif } -QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status status) +QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status status, + const QString &statusText) { QV4::Scope scope(v4); @@ -105,6 +106,8 @@ QV4::ReturnedValue QV4Include::resultValue(QV4::ExecutionEngine *v4, Status stat o->put((s = v4->newString(QStringLiteral("NETWORK_ERROR"))), (v = QV4::Primitive::fromInt32(NetworkError))); o->put((s = v4->newString(QStringLiteral("EXCEPTION"))), (v = QV4::Primitive::fromInt32(Exception))); o->put((s = v4->newString(QStringLiteral("status"))), (v = QV4::Primitive::fromInt32(status))); + if (!statusText.isEmpty()) + o->put((s = v4->newString(QStringLiteral("statusText"))), (v = v4->newString(statusText))); return o.asReturnedValue(); } @@ -227,7 +230,8 @@ QV4::ReturnedValue QV4Include::method_include(const QV4::FunctionObject *b, cons } else { QScopedPointer script; - script.reset(QV4::Script::createFromFileOrCache(scope.engine, qmlcontext, localFile, url)); + QString error; + script.reset(QV4::Script::createFromFileOrCache(scope.engine, qmlcontext, localFile, url, &error)); if (!script.isNull()) { script->parse(); @@ -242,7 +246,7 @@ QV4::ReturnedValue QV4Include::method_include(const QV4::FunctionObject *b, cons result = resultValue(scope.engine, Ok); } } else { - result = resultValue(scope.engine, NetworkError); + result = resultValue(scope.engine, NetworkError, error); } callback(callbackFunction, result); diff --git a/src/qml/jsruntime/qv4include_p.h b/src/qml/jsruntime/qv4include_p.h index 8015722afc..70ccfbf223 100644 --- a/src/qml/jsruntime/qv4include_p.h +++ b/src/qml/jsruntime/qv4include_p.h @@ -88,7 +88,8 @@ private: QV4::ReturnedValue result(); - static QV4::ReturnedValue resultValue(QV4::ExecutionEngine *v4, Status status = Loading); + static QV4::ReturnedValue resultValue(QV4::ExecutionEngine *v4, Status status = Loading, + const QString &statusText = QString()); static void callback(const QV4::Value &callback, const QV4::Value &status); QV4::ExecutionEngine *v4; diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp index 267c93952d..b4d9e11716 100644 --- a/src/qml/jsruntime/qv4script.cpp +++ b/src/qml/jsruntime/qv4script.cpp @@ -223,17 +223,28 @@ QQmlRefPointer Script::precompile(QV4::Compi return cg.generateCompilationUnit(/*generate unit data*/false); } -Script *Script::createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl) +Script *Script::createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl, QString *error) { - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(originalUrl)) { + if (error) + error->clear(); + + QQmlMetaType::CachedUnitLookupError cacheError = QQmlMetaType::CachedUnitLookupError::NoError; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(originalUrl, &cacheError)) { QQmlRefPointer jsUnit; jsUnit.adopt(new QV4::CompiledData::CompilationUnit(cachedUnit)); return new QV4::Script(engine, qmlContext, jsUnit); } QFile f(fileName); - if (!f.open(QIODevice::ReadOnly)) + if (!f.open(QIODevice::ReadOnly)) { + if (error) { + if (cacheError == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + *error = originalUrl.toString() + QString::fromUtf8(" was compiled ahead of time with an incompatible version of Qt and the original source code cannot be found. Please recompile"); + else + *error = QString::fromUtf8("Error opening source file %1: %2").arg(originalUrl.toString()).arg(f.errorString()); + } return nullptr; + } QByteArray data = f.readAll(); QString sourceCode = QString::fromUtf8(data); diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h index cb03c6b064..b4ac150044 100644 --- a/src/qml/jsruntime/qv4script_p.h +++ b/src/qml/jsruntime/qv4script_p.h @@ -101,7 +101,7 @@ struct Q_QML_EXPORT Script { QV4::Compiler::Module *module, Compiler::JSUnitGenerator *unitGenerator, const QString &fileName, const QString &finalUrl, const QString &source, QList *reportedErrors = nullptr, QQmlJS::Directives *directivesCollector = nullptr); - static Script *createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl); + static Script *createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlContext, const QString &fileName, const QUrl &originalUrl, QString *error); static ReturnedValue evaluate(ExecutionEngine *engine, const QString &script, QmlContext *qmlContext); }; diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp index 7754f0fddc..8fda7f6f77 100644 --- a/src/qml/qml/qqmlmetatype.cpp +++ b/src/qml/qml/qqmlmetatype.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #include #include @@ -63,6 +64,8 @@ #include #include "qqmlcomponent.h" +Q_DECLARE_LOGGING_CATEGORY(DBG_DISK_CACHE) + QT_BEGIN_NAMESPACE struct QQmlMetaTypeData @@ -2539,18 +2542,46 @@ QList QQmlMetaType::qmlSingletonTypes() return retn; } -const QV4::CompiledData::Unit *QQmlMetaType::findCachedCompilationUnit(const QUrl &uri) +const QV4::CompiledData::Unit *QQmlMetaType::findCachedCompilationUnit(const QUrl &uri, CachedUnitLookupError *status) { QMutexLocker lock(metaTypeDataLock()); QQmlMetaTypeData *data = metaTypeData(); for (const auto lookup : qAsConst(data->lookupCachedQmlUnit)) { - if (const QQmlPrivate::CachedQmlUnit *unit = lookup(uri)) + if (const QQmlPrivate::CachedQmlUnit *unit = lookup(uri)) { + QString error; + if (!unit->qmlData->verifyHeader(QDateTime(), &error)) { + qCDebug(DBG_DISK_CACHE) << "Error loading pre-compiled file " << uri << ":" << error; + if (status) + *status = CachedUnitLookupError::VersionMismatch; + return nullptr; + } + if (status) + *status = CachedUnitLookupError::NoError; return unit->qmlData; + } } + + if (status) + *status = CachedUnitLookupError::NoUnitFound; + return nullptr; } +void QQmlMetaType::prependCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler) +{ + QMutexLocker lock(metaTypeDataLock()); + QQmlMetaTypeData *data = metaTypeData(); + data->lookupCachedQmlUnit.prepend(handler); +} + +void QQmlMetaType::removeCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler) +{ + QMutexLocker lock(metaTypeDataLock()); + QQmlMetaTypeData *data = metaTypeData(); + data->lookupCachedQmlUnit.removeAll(handler); +} + /*! Returns the pretty QML type name (e.g. 'Item' instead of 'QtQuickItem') for the given object. */ diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h index 07bef526ba..cd7afc8a01 100644 --- a/src/qml/qml/qqmlmetatype_p.h +++ b/src/qml/qml/qqmlmetatype_p.h @@ -133,7 +133,17 @@ public: static QList parentFunctions(); - static const QV4::CompiledData::Unit *findCachedCompilationUnit(const QUrl &uri); + enum class CachedUnitLookupError { + NoError, + NoUnitFound, + VersionMismatch + }; + + static const QV4::CompiledData::Unit *findCachedCompilationUnit(const QUrl &uri, CachedUnitLookupError *status); + + // used by tst_qqmlcachegen.cpp + static void prependCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler); + static void removeCachedUnitLookupFunction(QQmlPrivate::QmlUnitCacheLookupFunction handler); static bool namespaceContainsRegistrations(const QString &, int majorVersion); diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 2b778b0b63..5572fdad44 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1259,6 +1259,7 @@ void QQmlTypeLoader::setData(QQmlDataBlob *blob, const QByteArray &data) QML_MEMORY_SCOPE_URL(blob->url()); QQmlDataBlob::SourceCodeData d; d.inlineSourceCode = QString::fromUtf8(data); + d.hasInlineSourceCode = true; setData(blob, d); } @@ -1670,9 +1671,11 @@ QQmlTypeData *QQmlTypeLoader::getType(const QUrl &url, Mode mode) typeData = new QQmlTypeData(url, this); // TODO: if (compiledData == 0), is it safe to omit this insertion? m_typeCache.insert(url, typeData); - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(typeData->url())) { + QQmlMetaType::CachedUnitLookupError error = QQmlMetaType::CachedUnitLookupError::NoError; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(typeData->url(), &error)) { QQmlTypeLoader::loadWithCachedUnit(typeData, cachedUnit, mode); } else { + typeData->setCachedUnitStatus(error); QQmlTypeLoader::load(typeData, mode); } } else if ((mode == PreferSynchronous || mode == Synchronous) && QQmlFile::isSynchronous(url)) { @@ -1727,9 +1730,11 @@ QQmlScriptBlob *QQmlTypeLoader::getScript(const QUrl &url) scriptBlob = new QQmlScriptBlob(url, this); m_scriptCache.insert(url, scriptBlob); - if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(scriptBlob->url())) { + QQmlMetaType::CachedUnitLookupError error; + if (const QV4::CompiledData::Unit *cachedUnit = QQmlMetaType::findCachedCompilationUnit(scriptBlob->url(), &error)) { QQmlTypeLoader::loadWithCachedUnit(scriptBlob, cachedUnit); } else { + scriptBlob->setCachedUnitStatus(error); QQmlTypeLoader::load(scriptBlob); } } @@ -2428,8 +2433,13 @@ void QQmlTypeData::dataReceived(const SourceCodeData &data) if (isError()) return; - if (!m_backupSourceCode.exists()) { - setError(QQmlTypeLoader::tr("No such file or directory")); + if (!m_backupSourceCode.exists() || m_backupSourceCode.isEmpty()) { + if (m_cachedUnitStatus == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + setError(QQmlTypeLoader::tr("File was compiled ahead of time with an incompatible version of Qt and the original file cannot be found. Please recompile")); + else if (!m_backupSourceCode.exists()) + setError(QQmlTypeLoader::tr("No such file or directory")); + else + setError(QQmlTypeLoader::tr("File is empty")); return; } @@ -2981,6 +2991,13 @@ void QQmlScriptBlob::dataReceived(const SourceCodeData &data) } } + if (!data.exists()) { + if (m_cachedUnitStatus == QQmlMetaType::CachedUnitLookupError::VersionMismatch) + setError(QQmlTypeLoader::tr("File was compiled ahead of time with an incompatible version of Qt and the original file cannot be found. Please recompile")); + else + setError(QQmlTypeLoader::tr("No such file or directory")); + return; + } QmlIR::Document irUnit(isDebugging()); @@ -3178,7 +3195,7 @@ void QQmlQmldirData::initializeFromCachedUnit(const QV4::CompiledData::Unit *) QString QQmlDataBlob::SourceCodeData::readAll(QString *error) const { error->clear(); - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return inlineSourceCode; QFile f(fileInfo.absoluteFilePath()); @@ -3205,7 +3222,7 @@ QString QQmlDataBlob::SourceCodeData::readAll(QString *error) const QDateTime QQmlDataBlob::SourceCodeData::sourceTimeStamp() const { - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return QDateTime(); QDateTime timeStamp = fileInfo.lastModified(); @@ -3220,11 +3237,18 @@ QDateTime QQmlDataBlob::SourceCodeData::sourceTimeStamp() const bool QQmlDataBlob::SourceCodeData::exists() const { - if (!inlineSourceCode.isEmpty()) + if (hasInlineSourceCode) return true; return fileInfo.exists(); } +bool QQmlDataBlob::SourceCodeData::isEmpty() const +{ + if (hasInlineSourceCode) + return inlineSourceCode.isEmpty(); + return fileInfo.size() == 0; +} + QT_END_NAMESPACE #include "qqmltypeloader.moc" diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index 4665d342c1..5988632547 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -140,11 +140,13 @@ public: QString readAll(QString *error) const; QDateTime sourceTimeStamp() const; bool exists() const; + bool isEmpty() const; private: friend class QQmlDataBlob; friend class QQmlTypeLoader; QString inlineSourceCode; QFileInfo fileInfo; + bool hasInlineSourceCode = false; }; protected: @@ -271,6 +273,8 @@ public: const QQmlImports &imports() const { return m_importCache; } + void setCachedUnitStatus(QQmlMetaType::CachedUnitLookupError status) { m_cachedUnitStatus = status; } + protected: bool addImport(const QV4::CompiledData::Import *import, QList *errors); @@ -293,6 +297,7 @@ public: QQmlImports m_importCache; QHash m_unresolvedImports; QList m_qmldirs; + QQmlMetaType::CachedUnitLookupError m_cachedUnitStatus = QQmlMetaType::CachedUnitLookupError::NoError; }; QQmlTypeLoader(QQmlEngine *); diff --git a/src/qml/types/qquickworkerscript.cpp b/src/qml/types/qquickworkerscript.cpp index 80c2d3a4bc..ef92e4108d 100644 --- a/src/qml/types/qquickworkerscript.cpp +++ b/src/qml/types/qquickworkerscript.cpp @@ -397,9 +397,11 @@ void QQuickWorkerScriptEnginePrivate::processLoad(int id, const QUrl &url) QV4::Scoped qmlContext(scope, getWorker(script)); Q_ASSERT(!!qmlContext); - program.reset(QV4::Script::createFromFileOrCache(v4, qmlContext, fileName, url)); + QString error; + program.reset(QV4::Script::createFromFileOrCache(v4, qmlContext, fileName, url, &error)); if (program.isNull()) { - qWarning().nospace() << "WorkerScript: Cannot find source file " << url.toString(); + if (!error.isEmpty()) + qWarning().nospace() << error; return; } diff --git a/tests/auto/qml/qmlcachegen/qmlcachegen.pro b/tests/auto/qml/qmlcachegen/qmlcachegen.pro index a8b72224ba..bad912c781 100644 --- a/tests/auto/qml/qmlcachegen/qmlcachegen.pro +++ b/tests/auto/qml/qmlcachegen/qmlcachegen.pro @@ -8,4 +8,6 @@ workerscripts_test.files = worker.js worker.qml workerscripts_test.prefix = /workerscripts RESOURCES += workerscripts_test +RESOURCES += versionchecks.qml + QT += core-private qml-private testlib diff --git a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp index e36edca699..1c05005c90 100644 --- a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp +++ b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include class tst_qmlcachegen: public QObject @@ -48,6 +49,7 @@ private slots: void errorOnArgumentsInSignalHandler(); void aheadOfTimeCompilation(); void functionExpressions(); + void versionChecksForAheadOfTimeUnits(); void workerScripts(); }; @@ -280,6 +282,50 @@ void tst_qmlcachegen::aheadOfTimeCompilation() QCOMPARE(result.toInt(), 42); } +static QQmlPrivate::CachedQmlUnit *temporaryModifiedCachedUnit = nullptr; + +void tst_qmlcachegen::versionChecksForAheadOfTimeUnits() +{ + QVERIFY(QFile::exists(":/versionchecks.qml")); + QCOMPARE(QFileInfo(":/versionchecks.qml").size(), 0); + + Q_ASSERT(!temporaryModifiedCachedUnit); + QQmlMetaType::CachedUnitLookupError error = QQmlMetaType::CachedUnitLookupError::NoError; + const QV4::CompiledData::Unit *originalUnit = QQmlMetaType::findCachedCompilationUnit(QUrl("qrc:/versionchecks.qml"), &error); + QVERIFY(originalUnit); + QV4::CompiledData::Unit *tweakedUnit = (QV4::CompiledData::Unit *)malloc(originalUnit->unitSize); + memcpy(reinterpret_cast(tweakedUnit), reinterpret_cast(originalUnit), originalUnit->unitSize); + tweakedUnit->version = QV4_DATA_STRUCTURE_VERSION - 1; + temporaryModifiedCachedUnit = new QQmlPrivate::CachedQmlUnit{tweakedUnit, nullptr, nullptr}; + + auto testHandler = [](const QUrl &url) -> const QQmlPrivate::CachedQmlUnit * { + if (url == QUrl("qrc:/versionchecks.qml")) + return temporaryModifiedCachedUnit; + return nullptr; + }; + QQmlMetaType::prependCachedUnitLookupFunction(testHandler); + + { + QQmlMetaType::CachedUnitLookupError error = QQmlMetaType::CachedUnitLookupError::NoError; + QVERIFY(!QQmlMetaType::findCachedCompilationUnit(QUrl("qrc:/versionchecks.qml"), &error)); + QCOMPARE(error, QQmlMetaType::CachedUnitLookupError::VersionMismatch); + } + + { + QQmlEngine engine; + QQmlComponent component(&engine, QUrl("qrc:/versionchecks.qml")); + QCOMPARE(component.status(), QQmlComponent::Error); + QCOMPARE(component.errorString(), QString("qrc:/versionchecks.qml:-1 File was compiled ahead of time with an incompatible version of Qt and the original file cannot be found. Please recompile\n")); + } + + Q_ASSERT(temporaryModifiedCachedUnit); + free(const_cast(temporaryModifiedCachedUnit->qmlData)); + delete temporaryModifiedCachedUnit; + temporaryModifiedCachedUnit = nullptr; + + QQmlMetaType::removeCachedUnitLookupFunction(testHandler); +} + void tst_qmlcachegen::workerScripts() { QVERIFY(QFile::exists(":/workerscripts/worker.js")); diff --git a/tests/auto/qml/qmlcachegen/versionchecks.qml b/tests/auto/qml/qmlcachegen/versionchecks.qml new file mode 100644 index 0000000000..77d67e7da4 --- /dev/null +++ b/tests/auto/qml/qmlcachegen/versionchecks.qml @@ -0,0 +1,4 @@ +import QtQml 2.0 +QtObject { + property bool ok: true +} diff --git a/tests/auto/qml/qqmlecmascript/data/include_callback.js b/tests/auto/qml/qqmlecmascript/data/include_callback.js index ea19eba300..7f3195bb1f 100644 --- a/tests/auto/qml/qqmlecmascript/data/include_callback.js +++ b/tests/auto/qml/qqmlecmascript/data/include_callback.js @@ -1,6 +1,6 @@ function go() { var a = Qt.include("missing.js", function(o) { test2 = o.status == o.NETWORK_ERROR }); - test1 = a.status == a.NETWORK_ERROR + test1 = (a.status == a.NETWORK_ERROR) && (a.statusText.indexOf("Error opening source file") != -1); var b = Qt.include("blank.js", function(o) { test4 = o.status == o.OK }); test3 = b.status == b.OK diff --git a/tests/auto/qml/qqmllanguage/data/empty.errors.txt b/tests/auto/qml/qqmllanguage/data/empty.errors.txt index 620db2bbba..ba685d78ae 100644 --- a/tests/auto/qml/qqmllanguage/data/empty.errors.txt +++ b/tests/auto/qml/qqmllanguage/data/empty.errors.txt @@ -1,2 +1 @@ -1:1:Expected token `numeric literal' -1:1:Expected a qualified name id +-1:-1:File is empty -- cgit v1.2.3 From 80592dcf0df3bce6177e8e051e88af7b9e6b3f6e Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 12 Mar 2018 16:33:01 +0100 Subject: Tighten QML cache version checking Don't just include the "compile hash" of QtQml in the dependencies hash of QML files but use a dedicated field in the data structure, that we will also fill in when generating cache files ahead of time. This ensures that AOT generated cache files are considered invalid even when switching between different sha1s of declarative. Task-number: QTBUG-66986 Change-Id: I3d8ee103fd1a33a5b4c4576b3a2703fcd09712dd Reviewed-by: Erik Verbruggen --- src/qml/compiler/compiler.pri | 2 -- src/qml/compiler/qv4compileddata.cpp | 45 +++++++++--------------------------- src/qml/compiler/qv4compileddata_p.h | 6 +++-- src/qml/compiler/qv4compiler.cpp | 4 ++++ src/qml/qml.pro | 3 ++- 5 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/qml/compiler/compiler.pri b/src/qml/compiler/compiler.pri index 2ca0c39acc..95096db51d 100644 --- a/src/qml/compiler/compiler.pri +++ b/src/qml/compiler/compiler.pri @@ -40,8 +40,6 @@ SOURCES += \ unix: SOURCES += $$PWD/qv4compilationunitmapper_unix.cpp else: SOURCES += $$PWD/qv4compilationunitmapper_win.cpp - -qtConfig(private_tests):qtConfig(dlopen): QMAKE_USE_PRIVATE += libdl } gcc { diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index 6220c1bc11..8dcc068a06 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -70,18 +70,14 @@ #include -#if defined(QT_BUILD_INTERNAL) -#if defined(Q_OS_UNIX) && !defined(QT_NO_DYNAMIC_CAST) -#include -#endif -#endif - QT_BEGIN_NAMESPACE namespace QV4 { namespace CompiledData { +static_assert(sizeof(Unit::libraryVersionHash) >= QML_COMPILE_HASH_LENGTH + 1, "Compile hash length exceeds reserved size in data structure. Please adjust and bump the format version"); + #if !defined(V4_BOOTSTRAP) static QString cacheFilePath(const QUrl &url) { @@ -686,32 +682,6 @@ void ResolvedTypeReference::doDynamicTypeCheck() isFullyDynamicType = qtTypeInherits(mo); } -static QByteArray ownLibraryChecksum() -{ - static QByteArray libraryChecksum; - static bool checksumInitialized = false; - if (checksumInitialized) - return libraryChecksum; - checksumInitialized = true; -#if defined(QT_BUILD_INTERNAL) && !defined(QT_NO_DYNAMIC_CAST) && QT_CONFIG(dlopen) - // This is a bit of a hack to make development easier. When hacking on the code generator - // the cache files may end up being re-used. To avoid that we also add the checksum of - // the QtQml library. - Dl_info libInfo; - if (dladdr(reinterpret_cast(&ownLibraryChecksum), &libInfo) != 0) { - QFile library(QFile::decodeName(libInfo.dli_fname)); - if (library.open(QIODevice::ReadOnly)) { - QCryptographicHash hash(QCryptographicHash::Md5); - hash.addData(&library); - libraryChecksum = hash.result(); - } - } -#else - libraryChecksum = QByteArray(QML_COMPILE_HASH); -#endif - return libraryChecksum; -} - bool ResolvedTypeReferenceMap::addToHash(QCryptographicHash *hash, QQmlEngine *engine) const { for (auto it = constBegin(), end = constEnd(); it != end; ++it) { @@ -719,8 +689,6 @@ bool ResolvedTypeReferenceMap::addToHash(QCryptographicHash *hash, QQmlEngine *e return false; } - hash->addData(ownLibraryChecksum()); - return true; } @@ -785,6 +753,15 @@ bool Unit::verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString) } } +#if defined(QML_COMPILE_HASH) + if (qstrcmp(QML_COMPILE_HASH, libraryVersionHash) != 0) { + *errorString = QStringLiteral("QML library version mismatch. Expected compile hash does not match"); + return false; + } +#else +#error "QML_COMPILE_HASH must be defined for the build of QtDeclarative to ensure version checking for cache files" +#endif + return true; #else Q_UNUSED(expectedSourceTimeStamp) diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index 49360bc3f1..1df9d6794f 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -72,7 +72,7 @@ QT_BEGIN_NAMESPACE // Bump this whenever the compiler data structures change in an incompatible way. -#define QV4_DATA_STRUCTURE_VERSION 0x18 +#define QV4_DATA_STRUCTURE_VERSION 0x19 class QIODevice; class QQmlPropertyCache; @@ -688,6 +688,8 @@ struct Unit quint32_le unitSize; // Size of the Unit and any depending data. // END DO NOT CHANGE THESE FIELDS EVER + char libraryVersionHash[48]; + char md5Checksum[16]; // checksum of all bytes following this field. void generateChecksum(); @@ -793,7 +795,7 @@ struct Unit } }; -static_assert(sizeof(Unit) == 144, "Unit structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); +static_assert(sizeof(Unit) == 192, "Unit structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); struct TypeReference { diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index ccc909c199..c9e535c93f 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -48,6 +48,9 @@ #include #include +// generated by qmake: +#include "qml_compile_hash_p.h" + QV4::Compiler::StringTableGenerator::StringTableGenerator() { clear(); @@ -396,6 +399,7 @@ QV4::CompiledData::Unit QV4::Compiler::JSUnitGenerator::generateHeader(QV4::Comp unit.flags |= module->unitFlags; unit.version = QV4_DATA_STRUCTURE_VERSION; unit.qtVersion = QT_VERSION; + qstrcpy(unit.libraryVersionHash, QML_COMPILE_HASH); memset(unit.md5Checksum, 0, sizeof(unit.md5Checksum)); memset(unit.dependencyMD5Checksum, 0, sizeof(unit.dependencyMD5Checksum)); diff --git a/src/qml/qml.pro b/src/qml/qml.pro index f75bfa0313..2137877427 100644 --- a/src/qml/qml.pro +++ b/src/qml/qml.pro @@ -36,7 +36,8 @@ DEFINES += QT_NO_FOREACH } compile_hash_contents = \ "// Generated file, DO NOT EDIT" \ - "$${LITERAL_HASH}define QML_COMPILE_HASH \"$$QML_COMPILE_HASH\"" + "$${LITERAL_HASH}define QML_COMPILE_HASH \"$$QML_COMPILE_HASH\"" \ + "$${LITERAL_HASH}define QML_COMPILE_HASH_LENGTH $$str_size($$QML_COMPILE_HASH)" write_file("$$OUT_PWD/qml_compile_hash_p.h", compile_hash_contents)|error() } -- cgit v1.2.3 From 1953f3cac57acc9ee28809a9bee1962aeebbc35e Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 16:09:42 +0100 Subject: Remove qtquick-guidelines, move contents into qtquick-bestpractices This page doesn't offer much on its own, and qtquick-bestpractices.qdoc is already starting to gather useful guidelines, so move its contents there. Having everything related to best practices/guidelines on one page means less clicking between these "overview" pages, which I think is a bit of a problem currently. Change-Id: I18316dc177a6a7eb5a031e178cd0aed31dfa63ae Reviewed-by: J-P Nurmi Reviewed-by: Simon Hausmann --- .../doc/src/guidelines/qtquick-bestpractices.qdoc | 15 +++++++ .../doc/src/guidelines/qtquick-guidelines.qdoc | 50 ---------------------- 2 files changed, 15 insertions(+), 50 deletions(-) delete mode 100644 src/quick/doc/src/guidelines/qtquick-guidelines.qdoc diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index 9d411a581f..ca9bfc9ebb 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -227,4 +227,19 @@ properties are enough. \li \l{Item Positioners} \li \l{Qt Quick Layouts Overview} \endlist + +\section1 Performance + +For information on performance in QML and Qt Quick, +see \l {Performance Considerations And Suggestions}. + +\section1 Tools and Utilities + +For information on useful tools and utilies that make working with QML and +Qt Quick easier, see \l {Qt Quick Tools and Utilities}. + +\section1 Scene Graph + +For information on Qt Quick's scene graph, see \l {Qt Quick Scene Graph}. + */ diff --git a/src/quick/doc/src/guidelines/qtquick-guidelines.qdoc b/src/quick/doc/src/guidelines/qtquick-guidelines.qdoc deleted file mode 100644 index b8432fd9ca..0000000000 --- a/src/quick/doc/src/guidelines/qtquick-guidelines.qdoc +++ /dev/null @@ -1,50 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2018 The Qt Company Ltd. -** Contact: https://www.qt.io/licensing/ -** -** This file is part of the documentation of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:FDL$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see https://www.qt.io/terms-conditions. For further -** information use the contact form at https://www.qt.io/contact-us. -** -** GNU Free Documentation License Usage -** Alternatively, this file may be used under the terms of the GNU Free -** Documentation License version 1.3 as published by the Free Software -** Foundation and appearing in the file included in the packaging of -** this file. Please review the following information to ensure -** the GNU Free Documentation License version 1.3 requirements -** will be met: https://www.gnu.org/licenses/fdl-1.3.html. -** $QT_END_LICENSE$ -** -****************************************************************************/ - -/*! -\page qtquick-guidelines.html -\title Qt Quick Guidelines -\brief Provides best practices and conventions for application developers - -Qt Quick has been the enabler that has helped developers achieve UI design -goals faster and relieve from the monotony of imperative-coding. It is one of -the very few UI design technologies that can be modified easily either using -a text editor or a graphical designer tool. - -As an application developer, it is important to understand the limitations -of any technology such as Qt Quick. This topic provides you the necessary insight -into Qt Quick, and how it affects your application. It also provides pointers to best -practices that you should try to follow, and the list of tools that helps you -understand and improve your application. - -\list -\li \l{Qt Quick Best practices}{Best Practices} -\li \l{Qt Quick Tools and Utilities} -\li \l{Performance Considerations And Suggestions}{Performance improvement tips} -\li \l{Qt Quick Scene Graph} -\endlist -*/ -- cgit v1.2.3 From 2b0e0bee0c6153a8912778de5ff1cabd728d98d8 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 14 Mar 2018 16:00:02 +0100 Subject: Rename best practices page to "Best Practices for QML and Qt Quick" Qt Quick is most often used via its QML types, and since some of the tips on this page are not specific to Qt Quick but apply to QML in general, it makes sense to list them both on the same page for convenience and completeness. Change-Id: I6d61b98ffd7e52dc28b33ef00a78dd745f39820a Reviewed-by: Frederik Gladhorn Reviewed-by: Simon Hausmann --- src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index ca9bfc9ebb..ea78c88028 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -27,12 +27,12 @@ /*! \page qtquick-bestpractices.html -\title Qt Quick Best Practices -\brief Lists the practices that work best for Qt Quick +\title Best Practices for QML and Qt Quick +\brief Lists best practices for working with QML and Qt Quick -Despite all of the benefits that Qt Quick offers, it can be challenging in -certain situations. The following sections elaborate on some of the best -practices that will help you get better results when developing Qt Quick +Despite all of the benefits that QML and Qt Quick offer, they can be +challenging in certain situations. The following sections elaborate on some of +the best practices that will help you get better results when developing applications. \section1 Custom UI Controls -- cgit v1.2.3 From cc62a35bbebc9d069c9dbaeb703dfd6afea548e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Vr=C3=A1til?= Date: Tue, 20 Mar 2018 18:14:20 +0100 Subject: Fix QQmlListModel crash when appending an empty array in debug mode Calling QQmlListModel::append() with an empty JS array triggers an assert in QAbstractItemModel::beginInsertRows() because it's called with negative "last" parameter. Change-Id: I202da260d79f2e6677c663c5785ff754c715fef8 Reviewed-by: Simon Hausmann --- src/qml/types/qqmllistmodel.cpp | 23 +++++++++++----------- tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp | 22 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp index d4fc02cd3e..c4e33d572d 100644 --- a/src/qml/types/qqmllistmodel.cpp +++ b/src/qml/types/qqmllistmodel.cpp @@ -2351,21 +2351,22 @@ void QQmlListModel::append(QQmlV4Function *args) QV4::ScopedObject argObject(scope); int objectArrayLength = objectArray->getLength(); + if (objectArrayLength > 0) { + int index = count(); + emitItemsAboutToBeInserted(index, objectArrayLength); - int index = count(); - emitItemsAboutToBeInserted(index, objectArrayLength); + for (int i=0 ; i < objectArrayLength ; ++i) { + argObject = objectArray->getIndexed(i); - for (int i=0 ; i < objectArrayLength ; ++i) { - argObject = objectArray->getIndexed(i); - - if (m_dynamicRoles) { - m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this)); - } else { - m_listModel->append(argObject); + if (m_dynamicRoles) { + m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this)); + } else { + m_listModel->append(argObject); + } } - } - emitItemsInserted(); + emitItemsInserted(); + } } else if (argObject) { int index; diff --git a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp index 4618ea4071..9fdc54f067 100644 --- a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp +++ b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp @@ -125,6 +125,7 @@ private slots: void bindingsOnGetResult(); void stringifyModelEntry(); void qobjectTrackerForDynamicModelObjects(); + void crash_append_empty_array(); }; bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object) @@ -1534,6 +1535,27 @@ void tst_qqmllistmodel::qobjectTrackerForDynamicModelObjects() QVERIFY(!ddata->jsWrapper.isNullOrUndefined()); } +void tst_qqmllistmodel::crash_append_empty_array() +{ + QQmlEngine engine; + QQmlComponent component(&engine); + component.setData( + "import QtQuick 2.0\n" + "Item {\n" + " ListModel {\n" + " id: testModel\n" + " objectName: \"testModel\"" + " }\n" + "}\n", QUrl()); + QScopedPointer scene(component.create()); + QQmlListModel *model = scene->findChild("testModel"); + QSignalSpy spy(model, &QQmlListModel::rowsAboutToBeInserted); + QQmlExpression expr(engine.rootContext(), model, "append(new Array())"); + expr.evaluate(); + QVERIFY2(!expr.hasError(), QTest::toString(expr.error().toString())); + QCOMPARE(spy.count(), 0); +} + QTEST_MAIN(tst_qqmllistmodel) #include "tst_qqmllistmodel.moc" -- cgit v1.2.3 From 8f4452fc72cd7a175ae5c89ffffe3a6ca6979392 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Tue, 20 Mar 2018 12:35:26 +0100 Subject: Doc: mention alternative syntax for resource files This syntax allows creating distinct .qrc files without having to create them manually. Change-Id: Iab7c76fd162bb7f39b42fb983f85d74fce3036d4 Reviewed-by: Simon Hausmann --- .../doc/src/guidelines/qtquick-bestpractices.qdoc | 50 +++++++++++++++------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index ea78c88028..f842f1b6aa 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -84,20 +84,36 @@ making it harder to load these resources. As an alternative, Qt offers its own application binary, enabling access to the application's resources regardless of the target OS. -For example, the following entry in the qmake project file ensures that the -resources are built into the application binary, making them available when -needed: +For example, consider the following project directory structure: \badcode - RESOURCES += a.qml b.png +project +├── images +│ ├── image1.png +│ └── image2.png +├── project.pro +└── qml + └── main.qml \endcode -It's also possible to use a +The following entry in \c project.pro ensures that the resources are built into +the application binary, making them available when needed: + +\badcode + RESOURCES += \ + qml/main.qml \ + images/image1.png \ + images/image2.png +\endcode + +A more convenient approach is to use the \l {files(pattern[, recursive=false])}{wildcard syntax} to select several files at once: \badcode - RESOURCES += $$files(*.qml) $$files(*.png) + RESOURCES += \ + $$files(qml/*.qml) \ + $$files(images/*.png) \endcode This approach is convenient for applications that depend on a limited number @@ -106,20 +122,24 @@ approach, it causes \e all of the other files in \c RESOURCES to be recompiled as well. This can be inefficient, especially for large sets of files. In this case, a better approach is to separate each type of resource into its own \l {Resource Collection Files (.qrc)}{.qrc} file. For example, the snippet -above could be changed like so: +above could be changed to the following: \badcode - RESOURCES += qml.qrc images.qrc + qml.files = $$files(*.qml) + qml.prefix = /qml + RESOURCES += qml + + images.files = $$files(*.png) + images.prefix = /images + RESOURCES += images \endcode -Now, whenever a QML file is changed in qml.qrc, only the QML files have to be -recompiled. +Now, whenever a QML file is changed, only the QML files have to be recompiled. -If you want to conveniently switch from using the "direct" syntax to a .qrc -file, look for \c qmake_intermediate.qrc (a build artifact created by qmake) -in your project's build directory and rename it to, for example, -\c resources.qrc. Then, replace the old \c RESOURCES entry with -\c {RESOURCES += resources.qrc}. +Sometimes it can be necessary to have more control over the path for a +specific file managed by the resource system. For example, if we wanted to give +\c image2.png an alias, we would need to switch to an explicit \c .qrc file. +\l {Creating Resource Files} explains how to do this in detail. \section2 Related Information \list -- cgit v1.2.3 From 1a816cd8dcc3499351ce6dfb6ad3bdfb34c31ede Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Tue, 20 Mar 2018 14:48:03 +0100 Subject: Doc: provide an example of the C++ - QML interaction path Change-Id: Ib65bb9edbcbd1172cc620243b078c9691d961828 Reviewed-by: Simon Hausmann --- .../doc/src/guidelines/qtquick-bestpractices.qdoc | 126 +++++++++++++++++++-- 1 file changed, 115 insertions(+), 11 deletions(-) diff --git a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc index f842f1b6aa..7d4f564a6f 100644 --- a/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc +++ b/src/quick/doc/src/guidelines/qtquick-bestpractices.qdoc @@ -188,17 +188,121 @@ The following snippet demonstrates examples of models written in QML: Use \l {QAbstractItemModel Subclass}{C++} for dynamic data sets that are large or frequently modified. -\section2 Interaction Path - -Although Qt enables you to manipulate QML from C++, it is not a recommended -approach, as debugging such code can be painful. The QML engine works out -a lot of the details on the fly, so manipulating the QML items from C++ could -lead to unexpected results. This approach also makes the C++ code rely on the -QML code to provide certain properties or objects, making it difficult to -refactor the QML code without breaking the C++ code. Moreover, such C++ code -cannot reused with other QML code. To avoid all of these hassles and have a -maintainable application, it is recommended to always use the C++ to QML path -and not the other way around. +\section2 Interacting with QML from C++ + +Although Qt enables you to manipulate QML from C++, it is not recommended to do +so. To explain why, let's take a look at a simplified example. Suppose we were +writing the UI for a settings page: + +\qml + import QtQuick 2.11 + import QtQuick.Controls 2.4 + + Page { + Button { + text: qsTr("Restore default settings") + } + } +\endqml + +We want the button to do something in C++ when it is clicked. We know objects +in QML can emit change signals just like they can in C++, so we give the button +an \l [QML]{QtObject::}{objectName} so that we can find it from C++: + +\qml + Button { + objectName: "restoreDefaultsButton" + text: qsTr("Restore default settings") + } +\endqml + +Then, in C++, we find that object and connect to its change signal: + +\code + #include + #include + #include + + class Backend : public QObject + { + Q_OBJECT + + public: + Backend() {} + + public slots: + void restoreDefaults() { + settings.setValue("loadLastProject", QVariant(false)); + } + + private: + QSettings settings; + }; + + int main(int argc, char *argv[]) + { + QGuiApplication app(argc, argv); + + QQmlApplicationEngine engine; + engine.load(QUrl(QStringLiteral("qrc:/main.qml"))); + if (engine.rootObjects().isEmpty()) + return -1; + + Backend backend; + + QObject *rootObject = engine.rootObjects().first(); + QObject *restoreDefaultsButton = rootObject->findChild("restoreDefaultsButton"); + QObject::connect(restoreDefaultsButton, SIGNAL(clicked()), + &backend, SLOT(restoreDefaults())); + + return app.exec(); + } + + #include "main.moc" +\endcode + +The problem with this approach is that the C++ logic layer depends on the QML +presentation layer. If we were to refactor the QML in such a way that the +\c objectName changes, or some other change breaks the ability for the C++ +to find the QML object, our workflow becomes much more complicated and tedious. + +Refactoring QML is a lot easier than refactoring C++, so in order to make +maintenance pain-free, we should strive to keep C++ types unaware of QML as +much as possible. This can be achieved by exposing the C++ types to QML: + +\code + int main(int argc, char *argv[]) + { + QGuiApplication app(argc, argv); + + Backend backend; + + QQmlApplicationEngine engine; + engine.rootContext()->setContextProperty("backend", &backend); + engine.load(QUrl(QStringLiteral("qrc:/main.qml"))); + if (engine.rootObjects().isEmpty()) + return -1; + + return app.exec(); + } +\endcode + +The QML then calls the C++ slot directly: + +\qml + import QtQuick 2.11 + import QtQuick.Controls 2.4 + + Page { + Button { + text: qsTr("Restore default settings") + onClicked: backend.restoreDefaults() + } + } +\endqml + +With this approach, the C++ remains unchanged in the event that the QML needs +to be refactored in the future. \section2 Related Information \list -- cgit v1.2.3