QtQml: Move locateLocalQmldir and make it a template

It conceptually belongs into QQmlImportDatabase because it messes with
the qmldir cache.

This way, if we reject a qmldir file for whatever reason, we can still
check for further matching qmldir files in other places. We might, for
example determine that we need to load a plugin, but we have been given
a qmldir file in the resource file system. In that case, we want to
check for other instances of the same module in the host file system.

Change-Id: I8fe4a0f188f3732b9d10d017a94562e8bd1fb242
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit dbca762f45d3249974652cccb5d6421440f852ae)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

8 files changed, 169 insertions, 119 deletions

diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 8cabaf3d90..a03fc00d16 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1081,6 +1081,15 @@ QUrl QQmlEngine::interceptUrl(const QUrl &url, QQmlAbstractUrlInterceptor::DataT
     return result;
 }
 
+/*!
+  Returns the list of currently active URL interceptors.
+ */
+QList<QQmlAbstractUrlInterceptor *> QQmlEngine::urlInterceptors() const
+{
+    Q_D(const QQmlEngine);
+    return d->urlInterceptors;
+}
+
 void QQmlEnginePrivate::registerFinalizeCallback(QObject *obj, int index)
 {
     if (activeObjectCreator) {
diff --git a/src/qml/qml/qqmlengine.h b/src/qml/qml/qqmlengine.h
index 37ca25779f..64e4a82b03 100644
--- a/src/qml/qml/qqmlengine.h
+++ b/src/qml/qml/qqmlengine.h
@@ -136,6 +136,7 @@ public:
 
     void addUrlInterceptor(QQmlAbstractUrlInterceptor *urlInterceptor);
     void removeUrlInterceptor(QQmlAbstractUrlInterceptor *urlInterceptor);
+    QList<QQmlAbstractUrlInterceptor *> urlInterceptors() const;
     QUrl interceptUrl(const QUrl &url, QQmlAbstractUrlInterceptor::DataType type) const;
 
     void addImageProvider(const QString &id, QQmlImageProviderBase *);
diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp
index 28f9764a06..6808de2262 100644
--- a/src/qml/qml/qqmlimport.cpp
+++ b/src/qml/qml/qqmlimport.cpp
@@ -242,10 +242,6 @@ public:
 
     QQmlTypeLoader *typeLoader;
 
-    static QQmlImports::LocalQmldirResult locateLocalQmldir(
-            const QString &uri, QTypeRevision version, QQmlImportDatabase *database,
-            QString *outQmldirFilePath, QString *outUrl);
-
     static QTypeRevision matchingQmldirVersion(
             const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri,
             QTypeRevision version, QList<QQmlError> *errors);
@@ -1155,92 +1151,22 @@ QString QQmlImportsPrivate::resolvedUri(const QString &dir_arg, QQmlImportDataba
     return stableRelativePath;
 }
 
-/*
-Locates the qmldir file for \a uri version \a vmaj.vmin.  Returns true if found,
-and fills in outQmldirFilePath and outQmldirUrl appropriately.  Otherwise returns
-false.
-*/
-QQmlImports::LocalQmldirResult QQmlImportsPrivate::locateLocalQmldir(
-        const QString &uri, QTypeRevision version, QQmlImportDatabase *database,
-        QString *outQmldirFilePath, QString *outQmldirPathUrl)
-{
-    // Check cache first
-
-    QQmlImportDatabase::QmldirCache *cacheHead = nullptr;
-    {
-        QQmlImportDatabase::QmldirCache **cachePtr = database->qmldirCache.value(uri);
-        if (cachePtr) {
-            cacheHead = *cachePtr;
-            QQmlImportDatabase::QmldirCache *cache = cacheHead;
-            while (cache) {
-                if (cache->version == version) {
-                    *outQmldirFilePath = cache->qmldirFilePath;
-                    *outQmldirPathUrl = cache->qmldirPathUrl;
-                    return cache->qmldirFilePath.isEmpty() ? QQmlImports::QmldirNotFound
-                                                           : QQmlImports::QmldirFound;
-                }
-                cache = cache->next;
-            }
-        }
-    }
-
-    QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(database->engine);
-    QQmlTypeLoader &typeLoader = enginePrivate->typeLoader;
-    const bool hasInterceptors = !enginePrivate->urlInterceptors.isEmpty();
-
-    // Interceptor might redirect remote files to local ones.
-    QStringList localImportPaths = database->importPathList(
-                hasInterceptors ? QQmlImportDatabase::LocalOrRemote : QQmlImportDatabase::Local);
-
-    // Search local import paths for a matching version
-    const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(
-                uri, localImportPaths, version);
-    bool pathTurnedRemote = false;
-    for (QString qmldirPath : qmlDirPaths) {
-        if (hasInterceptors) {
-            const QUrl intercepted = database->engine->interceptUrl(
-                        QQmlImports::urlFromLocalFileOrQrcOrUrl(qmldirPath),
-                        QQmlAbstractUrlInterceptor::QmldirFile);
-            qmldirPath = QQmlFile::urlToLocalFileOrQrc(intercepted);
-            if (!pathTurnedRemote && qmldirPath.isEmpty() && !QQmlFile::isLocalFile(intercepted))
-                pathTurnedRemote = true;
-        }
-
-        QString absoluteFilePath = typeLoader.absoluteFilePath(qmldirPath);
-        if (!absoluteFilePath.isEmpty()) {
-            QString url;
-            const QStringView absolutePath = QStringView{absoluteFilePath}.left(absoluteFilePath.lastIndexOf(Slash) + 1);
-            if (absolutePath.at(0) == Colon) {
-                url = QLatin1String("qrc") + absolutePath;
-            } else {
-                url = QUrl::fromLocalFile(absolutePath.toString()).toString();
-                // This handles the UNC path case as when the path is retrieved from the QUrl it
-                // will convert the host name from upper case to lower case. So the absoluteFilePath
-                // is changed at this point to make sure it will match later on in that case.
-                if (absoluteFilePath.startsWith(QLatin1String("//")))
-                    absoluteFilePath = QUrl::fromLocalFile(absoluteFilePath).toString(QUrl::RemoveScheme);
-            }
-            QQmlImportDatabase::QmldirCache *cache = new QQmlImportDatabase::QmldirCache;
-            cache->version = version;
-            cache->qmldirFilePath = absoluteFilePath;
-            cache->qmldirPathUrl = url;
-            cache->next = cacheHead;
-            database->qmldirCache.insert(uri, cache);
+/*!
+  \internal
 
-            *outQmldirFilePath = absoluteFilePath;
-            *outQmldirPathUrl = url;
+  \fn template<typename Callback>
+      QQmlImportDatabase::LocalQmldirResult QQmlImportDatabase::locateLocalQmldir(
+            const QString &uri, QTypeRevision version, const Callback &callback)
 
-            return QQmlImports::QmldirFound;
-        }
-    }
+  Locates the qmldir files for \a uri version \a version. For each one, calls
+  the \a callback. If the \a callback returns \c true, returns QmldirFound.
 
-    QQmlImportDatabase::QmldirCache *cache = new QQmlImportDatabase::QmldirCache;
-    cache->version = version;
-    cache->next = cacheHead;
-    database->qmldirCache.insert(uri, cache);
+  If at least one callback invocation returned \c false and there are no qmldir
+  files left to check, returns QmldirRejected.
 
-    return pathTurnedRemote ? QQmlImports::QmldirInterceptedToRemote : QQmlImports::QmldirNotFound;
-}
+  Otherwise, if interception redirects a previously local qmldir URL to a remote
+  one, returns QmldirInterceptedToRemote. Otherwise, returns QmldirNotFound.
+*/
 
 QTypeRevision QQmlImportsPrivate::matchingQmldirVersion(
         const QQmlTypeLoaderQmldirContent &qmldir, const QString &uri, QTypeRevision version,
@@ -1686,13 +1612,6 @@ QTypeRevision QQmlImports::updateQmldirContent(
     return d->updateQmldirContent(uri, prefix, qmldirIdentifier, qmldirUrl, importDb, errors);
 }
 
-QQmlImports::LocalQmldirResult QQmlImports::locateLocalQmldir(
-        QQmlImportDatabase *importDb, const QString &uri, QTypeRevision version,
-        QString *qmldirFilePath, QString *url)
-{
-    return d->locateLocalQmldir(uri, version, importDb, qmldirFilePath, url);
-}
-
 bool QQmlImports::isLocal(const QString &url)
 {
     return !QQmlFile::urlToLocalFileOrQrc(url).isEmpty();
@@ -1821,6 +1740,11 @@ void QQmlImportDatabase::addPluginPath(const QString& path)
     }
 }
 
+QString QQmlImportDatabase::absoluteFilePath(const QString &path) const
+{
+    return QQmlEnginePrivate::get(engine)->typeLoader.absoluteFilePath(path);
+}
+
 /*!
     \internal
 */
diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h
index 33f5eaf04a..946f36b398 100644
--- a/src/qml/qml/qqmlimport_p.h
+++ b/src/qml/qml/qqmlimport_p.h
@@ -44,7 +44,9 @@
 #include <QtCore/qcoreapplication.h>
 #include <QtCore/qset.h>
 #include <QtCore/qstringlist.h>
+#include <QtQml/qqmlengine.h>
 #include <QtQml/qqmlerror.h>
+#include <QtQml/qqmlfile.h>
 #include <private/qqmldirparser_p.h>
 #include <private/qqmltype_p.h>
 #include <private/qstringhash_p.h>
@@ -174,16 +176,6 @@ public:
             QQmlImportDatabase *importDb, const QString &uri, const QString &prefix,
             const QString &qmldirIdentifier, const QString &qmldirUrl, QList<QQmlError> *errors);
 
-    enum LocalQmldirResult {
-        QmldirFound,
-        QmldirNotFound,
-        QmldirInterceptedToRemote
-    };
-
-    LocalQmldirResult locateLocalQmldir(
-            QQmlImportDatabase *, const QString &uri, QTypeRevision version,
-            QString *qmldirFilePath, QString *url);
-
     void populateCache(QQmlTypeNameCache *cache) const;
 
     struct ScriptReference
@@ -225,6 +217,13 @@ class Q_QML_PRIVATE_EXPORT QQmlImportDatabase
 public:
     enum PathType { Local, Remote, LocalOrRemote };
 
+    enum LocalQmldirResult {
+        QmldirFound,
+        QmldirNotFound,
+        QmldirInterceptedToRemote,
+        QmldirRejected
+    };
+
     QQmlImportDatabase(QQmlEngine *);
     ~QQmlImportDatabase();
 
@@ -239,12 +238,18 @@ public:
     void setPluginPathList(const QStringList &paths);
     void addPluginPath(const QString& path);
 
+    template<typename Callback>
+    LocalQmldirResult locateLocalQmldir(
+            const QString &uri, QTypeRevision version, const Callback &callback);
+
     static QTypeRevision lockModule(const QString &uri, const QString &typeNamespace,
                                     QTypeRevision version, QList<QQmlError> *errors);
 
 private:
     friend class QQmlImportsPrivate;
     friend class QQmlPluginImporter;
+
+    QString absoluteFilePath(const QString &path) const;
     void clearDirCache();
 
     struct QmldirCache {
@@ -266,6 +271,106 @@ private:
     QQmlEngine *engine;
 };
 
+template<typename Callback>
+QQmlImportDatabase::LocalQmldirResult QQmlImportDatabase::locateLocalQmldir(
+        const QString &uri, QTypeRevision version, const Callback &callback)
+{
+    // Check cache first
+
+    LocalQmldirResult result = QmldirNotFound;
+    QmldirCache *cacheHead = nullptr;
+    {
+        QmldirCache **cachePtr = qmldirCache.value(uri);
+        if (cachePtr) {
+            cacheHead = *cachePtr;
+            QmldirCache *cache = cacheHead;
+            while (cache) {
+                if (cache->version == version) {
+                    if (cache->qmldirFilePath.isEmpty())
+                        return QmldirNotFound;
+                    if (callback(cache->qmldirFilePath, cache->qmldirPathUrl))
+                        return QmldirFound;
+                    result = QmldirRejected;
+                }
+                cache = cache->next;
+            }
+        }
+    }
+
+    // Do not try to construct the cache if it already had any entries for the URI.
+    // Otherwise we might duplicate cache entries.
+    if (result != QmldirNotFound)
+        return result;
+
+    const bool hasInterceptors = !engine->urlInterceptors().isEmpty();
+
+    // Interceptor might redirect remote files to local ones.
+    QStringList localImportPaths = importPathList(hasInterceptors ? LocalOrRemote : Local);
+
+    // Search local import paths for a matching version
+    const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(
+                uri, localImportPaths, version);
+
+    for (QString qmldirPath : qmlDirPaths) {
+        if (hasInterceptors) {
+            const QUrl intercepted = engine->interceptUrl(
+                        QQmlImports::urlFromLocalFileOrQrcOrUrl(qmldirPath),
+                        QQmlAbstractUrlInterceptor::QmldirFile);
+            qmldirPath = QQmlFile::urlToLocalFileOrQrc(intercepted);
+            if (result != QmldirInterceptedToRemote
+                    && qmldirPath.isEmpty()
+                    && !QQmlFile::isLocalFile(intercepted)) {
+                result = QmldirInterceptedToRemote;
+            }
+        }
+
+        QString qmldirAbsoluteFilePath = absoluteFilePath(qmldirPath);
+        if (!qmldirAbsoluteFilePath.isEmpty()) {
+            QString url;
+            const QString absolutePath = qmldirAbsoluteFilePath.left(
+                        qmldirAbsoluteFilePath.lastIndexOf(u'/') + 1);
+            if (absolutePath.at(0) == u':') {
+                url = QStringLiteral("qrc") + absolutePath;
+            } else {
+                url = QUrl::fromLocalFile(absolutePath).toString();
+                // This handles the UNC path case as when the path is retrieved from the QUrl it
+                // will convert the host name from upper case to lower case. So the absoluteFilePath
+                // is changed at this point to make sure it will match later on in that case.
+                if (qmldirAbsoluteFilePath.startsWith(QStringLiteral("//"))) {
+                    qmldirAbsoluteFilePath = QUrl::fromLocalFile(qmldirAbsoluteFilePath)
+                            .toString(QUrl::RemoveScheme);
+                }
+            }
+
+            QmldirCache *cache = new QmldirCache;
+            cache->version = version;
+            cache->qmldirFilePath = qmldirAbsoluteFilePath;
+            cache->qmldirPathUrl = url;
+            cache->next = cacheHead;
+            qmldirCache.insert(uri, cache);
+            cacheHead = cache;
+
+            if (result != QmldirFound) {
+                result = callback(qmldirAbsoluteFilePath, url)
+                        ? QmldirFound
+                        : QmldirRejected;
+            }
+
+            // Do not return here. Rather, construct the complete cache for this URI.
+        }
+    }
+
+    // Nothing found? Add an empty cache entry to signal that for further requests.
+    if (result == QmldirNotFound) {
+        QmldirCache *cache = new QmldirCache;
+        cache->version = version;
+        cache->next = cacheHead;
+        qmldirCache.insert(uri, cache);
+    }
+
+    return result;
+}
+
 void qmlClearEnginePlugins();// For internal use by qmlClearRegisteredProperties
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp
index ab2d83e57a..1c16afc557 100644
--- a/src/qml/qml/qqmltypedata.cpp
+++ b/src/qml/qml/qqmltypedata.cpp
@@ -695,11 +695,11 @@ void QQmlTypeData::continueLoadFromIR()
     for (const QV4::CompiledData::Import *import : qAsConst(m_document->imports)) {
         if (!addImport(import, {}, &errors)) {
             Q_ASSERT(errors.size());
-            QQmlError error(errors.takeFirst());
-            error.setUrl(m_importCache.baseUrl());
-            error.setLine(qmlConvertSourceCoordinate<quint32, int>(import->location.line));
-            error.setColumn(qmlConvertSourceCoordinate<quint32, int>(import->location.column));
-            errors.prepend(error); // put it back on the list after filling out information.
+            for (QQmlError &error : errors) {
+                error.setUrl(m_importCache.baseUrl());
+                error.setLine(qmlConvertSourceCoordinate<quint32, int>(import->location.line));
+                error.setColumn(qmlConvertSourceCoordinate<quint32, int>(import->location.column));
+            }
             setError(errors);
             return;
         }
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index ce1625fb63..7a3d52b4a5 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -581,13 +581,10 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
 
         scriptImported(blob, import->location, import->qualifier, QString());
     } else if (import->type == QV4::CompiledData::Import::ImportLibrary) {
-        QString qmldirFilePath;
-        QString qmldirUrl;
-
-        const QQmlImports::LocalQmldirResult qmldirResult = m_importCache.locateLocalQmldir(
-                    importDatabase, import->uri, import->version,
-                    &qmldirFilePath, &qmldirUrl);
-        if (qmldirResult == QQmlImports::QmldirFound) {
+        const QQmlImportDatabase::LocalQmldirResult qmldirResult
+                = importDatabase->locateLocalQmldir(
+                    import->uri, import->version,
+                    [&](const QString &qmldirFilePath, const QString &qmldirUrl) {
             // This is a local library import
             const QTypeRevision actualVersion = m_importCache.addLibraryImport(
                         importDatabase, import->uri, import->qualifier,
@@ -623,13 +620,26 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
                 if (!module)
                     QQmlMetaType::qmlRegisterModuleTypes(import->uri);
             }
-        } else if (
+            return true;
+        });
+
+        switch (qmldirResult) {
+        case QQmlImportDatabase::QmldirFound:
+            return true;
+        case QQmlImportDatabase::QmldirNotFound:
+        case QQmlImportDatabase::QmldirInterceptedToRemote:
+            break;
+        case QQmlImportDatabase::QmldirRejected:
+            return false;
+        }
+
+        if (
                 // Major version of module already registered:
                 // We believe that the registration is complete.
                 QQmlMetaType::typeModule(import->uri, import->version)
 
                 // Otherwise, try to register further module types.
-                || (qmldirResult != QQmlImports::QmldirInterceptedToRemote
+                || (qmldirResult != QQmlImportDatabase::QmldirInterceptedToRemote
                     && QQmlMetaType::qmlRegisterModuleTypes(import->uri))
 
                 // Otherwise, there is no way to register any further types.
diff --git a/tests/auto/qml/qqmllanguage/data/importJs.7.errors.txt b/tests/auto/qml/qqmllanguage/data/importJs.7.errors.txt
index 8a5f3c6721..f555ed43ae 100644
--- a/tests/auto/qml/qqmllanguage/data/importJs.7.errors.txt
+++ b/tests/auto/qml/qqmllanguage/data/importJs.7.errors.txt
@@ -1 +1,2 @@
 2:1:"org.qtproject.PureJsModule" is ambiguous.
+2:1:"org.qtproject.PureJsModule" is ambiguous.
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 478f236ee2..d09f903ccb 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -3422,11 +3422,11 @@ void tst_qqmllanguage::importJs()
 
     {
         DETERMINE_ERRORS(errorFile,expected,actual);
-        QCOMPARE(expected.size(), actual.size());
+        QCOMPARE(actual.size(), expected.size());
         for (int i = 0; i < expected.size(); ++i)
         {
             const int compareLen = qMin(expected.at(i).length(), actual.at(i).length());
-            QCOMPARE(expected.at(i).left(compareLen), actual.at(i).left(compareLen));
+            QCOMPARE(actual.at(i).left(compareLen), expected.at(i).left(compareLen));
         }
     }