Clean up QQmlFileSelector

The global static map of file selectors is not thread safe. If you add a
file selector to engine A running in thread 1, you cannot at the same
time retrieve the file selector of engine B running in thread 2. Alas,
we only need the static map to discern file selectors from other URL
interceptors, and we only need to do this on QQmlFileSelector::get(),
which we don't use. Unfortunately it is public API, though.

Deprecate QQmlFileSelector::get() and add a different hack to discern
the interceptors for the case that it's still called. Also remove the
duplicate setExtraSelectors() method.

For this to work, we also add a method to QQmlApplicationEngine that
allows us to pass extra file selectors and delay the initialization of
QQmlApplicationEngine's QQmlFileSelector until the first file is loaded.

[ChangeLog][QML] QQmlFileSelector::get() is deprecated. You can use the
new method QQmlAplicationEngine::setExtraFileSelectors() to pass extra
selectors to QQmlApplicationEngine's internal QQmlFileSelector.
Manually created QQmlFileSelectors should be configured immediately
after creation, before they are used by the QQmlEngine.

Change-Id: Ia61a93777dc910b441a03ffb42d35a2a224c0e26
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 73 insertions, 17 deletions

diff --git a/src/qml/qml/qqmlapplicationengine.cpp b/src/qml/qml/qqmlapplicationengine.cpp
index 7d961cd0c7..a633d3f58b 100644
--- a/src/qml/qml/qqmlapplicationengine.cpp
+++ b/src/qml/qml/qqmlapplicationengine.cpp
@@ -81,7 +81,8 @@ void QQmlApplicationEnginePrivate::init()
     else
         delete qtTranslator;
 #endif
-    new QQmlFileSelector(q,q);
+    auto *selector = new QQmlFileSelector(q,q);
+    selector->setExtraSelectors(extraFileSelectors);
     QCoreApplication::instance()->setProperty("__qml_using_qqmlapplicationengine", QVariant(true));
 }
 
@@ -115,6 +116,11 @@ void QQmlApplicationEnginePrivate::startLoad(const QUrl &url, const QByteArray &
 {
     Q_Q(QQmlApplicationEngine);
 
+    if (!isInitialized) {
+        init();
+        isInitialized = true;
+    }
+
     if (url.scheme() == QLatin1String("file") || url.scheme() == QLatin1String("qrc")) {
         QFileInfo fi(QQmlFile::urlToLocalFileOrQrc(url));
         translationsDirectory = fi.path() + QLatin1String("/i18n");
@@ -227,8 +233,6 @@ void QQmlApplicationEnginePrivate::finishLoad(QQmlComponent *c)
 QQmlApplicationEngine::QQmlApplicationEngine(QObject *parent)
 : QQmlEngine(*(new QQmlApplicationEnginePrivate(this)), parent)
 {
-    Q_D(QQmlApplicationEngine);
-    d->init();
     QJSEnginePrivate::addToDebugServer(this);
 }
 
@@ -312,6 +316,26 @@ void QQmlApplicationEngine::setInitialProperties(const QVariantMap &initialPrope
 }
 
 /*!
+  Sets the \a extraFileSelectors to be passed to the internal QQmlFileSelector
+  used for resolving URLs to local files. The \a extraFileSelectors are applied
+  when the first QML file is loaded. Setting them afterwards has no effect.
+
+  \sa QQmlFileSelector
+  \sa QFileSelector::setExtraSelectors
+  \since 6.0
+*/
+void QQmlApplicationEngine::setExtraFileSelectors(const QStringList &extraFileSelectors)
+{
+    Q_D(QQmlApplicationEngine);
+    if (d->isInitialized) {
+        qWarning() << "QQmlApplicationEngine::setExtraFileSelectors()"
+                   << "called after loading QML files. This has no effect.";
+    } else {
+        d->extraFileSelectors = extraFileSelectors;
+    }
+}
+
+/*!
   Loads the QML given in \a data. The object tree defined by \a data is
   instantiated immediately.
 
diff --git a/src/qml/qml/qqmlapplicationengine.h b/src/qml/qml/qqmlapplicationengine.h
index 37f75d5068..7faa51892e 100644
--- a/src/qml/qml/qqmlapplicationengine.h
+++ b/src/qml/qml/qqmlapplicationengine.h
@@ -67,6 +67,7 @@ public Q_SLOTS:
     void load(const QUrl &url);
     void load(const QString &filePath);
     void setInitialProperties(const QVariantMap &initialProperties);
+    void setExtraFileSelectors(const QStringList &extraFileSelectors);
     void loadData(const QByteArray &data, const QUrl &url = QUrl());
 
 Q_SIGNALS:
diff --git a/src/qml/qml/qqmlapplicationengine_p.h b/src/qml/qml/qqmlapplicationengine_p.h
index c514e3daf9..70232b3d52 100644
--- a/src/qml/qml/qqmlapplicationengine_p.h
+++ b/src/qml/qml/qqmlapplicationengine_p.h
@@ -74,8 +74,10 @@ public:
     void finishLoad(QQmlComponent *component);
     QList<QObject *> objects;
     QVariantMap initialProperties;
+    QStringList extraFileSelectors;
     QString translationsDirectory;
     QScopedPointer<QTranslator> activeTranslator;
+    bool isInitialized = false;
 };
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmlfileselector.cpp b/src/qml/qml/qqmlfileselector.cpp
index 1ea2b23153..aaddfa628c 100644
--- a/src/qml/qml/qqmlfileselector.cpp
+++ b/src/qml/qml/qqmlfileselector.cpp
@@ -39,6 +39,8 @@
 
 #include <QtCore/QFileSelector>
 #include <QtQml/QQmlAbstractUrlInterceptor>
+#include <QtQml/private/qqmlengine_p.h>
+#include <QtQml/private/qqmlapplicationengine_p.h>
 #include <qobjectdefs.h>
 #include "qqmlfileselector.h"
 #include "qqmlfileselector_p.h"
@@ -47,8 +49,6 @@
 
 QT_BEGIN_NAMESPACE
 
-typedef QHash<QQmlAbstractUrlInterceptor*, QQmlFileSelector*> interceptorSelectorMap;
-Q_GLOBAL_STATIC(interceptorSelectorMap, interceptorInstances);
 /*!
    \class QQmlFileSelector
    \since 5.2
@@ -105,7 +105,6 @@ QQmlFileSelector::QQmlFileSelector(QQmlEngine* engine, QObject* parent)
 {
     Q_D(QQmlFileSelector);
     d->engine = engine;
-    interceptorInstances()->insert(d->myInstance.data(), this);
     d->engine->addUrlInterceptor(d->myInstance.data());
 }
 
@@ -115,11 +114,10 @@ QQmlFileSelector::QQmlFileSelector(QQmlEngine* engine, QObject* parent)
 QQmlFileSelector::~QQmlFileSelector()
 {
     Q_D(QQmlFileSelector);
-    if (d->engine && QQmlFileSelector::get(d->engine) == this) {
+    if (d->engine) {
         d->engine->removeUrlInterceptor(d->myInstance.data());
         d->engine = nullptr;
     }
-    interceptorInstances()->remove(d->myInstance.data());
 }
 
 /*!
@@ -180,19 +178,44 @@ void QQmlFileSelector::setExtraSelectors(const QStringList &strings)
     d->selector->setExtraSelectors(strings);
 }
 
+#if QT_DEPRECATED_SINCE(6, 0)
 /*!
+  \deprecated
   Gets the QQmlFileSelector currently active on the target \a engine.
+
+  This method is deprecated. You should not retrieve the files selector from an
+  engine after setting it. It may be in use.
+
+  If the \a engine passed here is a QQmlApplicationEngine that hasn't loaded any
+  QML files, yet, it will be initialized. Any later calls to
+  QQmlApplicationEngine::setExtraFileSelectors() will have no effect.
+
+  \sa QQmlApplicationEngine
 */
 QQmlFileSelector* QQmlFileSelector::get(QQmlEngine* engine)
 {
-    //Since I think we still can't use dynamic_cast inside Qt...
-    const QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
-    for (QQmlAbstractUrlInterceptor *current : enginePrivate->urlInterceptors) {
-        if (interceptorInstances()->contains(current))
-            return interceptorInstances()->value(current);
+    QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
+
+    if (qobject_cast<QQmlApplicationEngine *>(engine)) {
+        auto *appEnginePrivate = static_cast<QQmlApplicationEnginePrivate *>(enginePrivate);
+        if (!appEnginePrivate->isInitialized) {
+            appEnginePrivate->init();
+            appEnginePrivate->isInitialized = true;
+        }
+    }
+
+    const QUrl nonEmptyInvalid(QLatin1String(":"));
+    for (QQmlAbstractUrlInterceptor *interceptor : enginePrivate->urlInterceptors) {
+        const QUrl result = interceptor->intercept(
+                    nonEmptyInvalid, QQmlAbstractUrlInterceptor::UrlString);
+        if (result.scheme() == QLatin1String("type")
+                && result.path() == QLatin1String("fileselector")) {
+            return static_cast<QQmlFileSelectorInterceptor *>(interceptor)->d->q_func();
+        }
     }
     return nullptr;
 }
+#endif
 
 /*!
   \internal
@@ -207,9 +230,12 @@ QQmlFileSelectorInterceptor::QQmlFileSelectorInterceptor(QQmlFileSelectorPrivate
 */
 QUrl QQmlFileSelectorInterceptor::intercept(const QUrl &path, DataType type)
 {
-    if ( type ==  QQmlAbstractUrlInterceptor::QmldirFile ) //Don't intercept qmldir files, to prevent double interception
-        return path;
-    return d->selector->select(path);
+    if (!path.isEmpty() && !path.isValid())
+        return QUrl(QLatin1String("type:fileselector"));
+
+    return type == QQmlAbstractUrlInterceptor::QmldirFile
+            ? path // Don't intercept qmldir files, to prevent double interception
+            : d->selector->select(path);
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmlfileselector.h b/src/qml/qml/qqmlfileselector.h
index 362a15a27e..0aeea6775f 100644
--- a/src/qml/qml/qqmlfileselector.h
+++ b/src/qml/qml/qqmlfileselector.h
@@ -59,7 +59,10 @@ public:
     QFileSelector *selector() const Q_DECL_NOTHROW;
     void setSelector(QFileSelector *selector);
     void setExtraSelectors(const QStringList &strings);
-    static QQmlFileSelector* get(QQmlEngine*);
+
+#if QT_DEPRECATED_SINCE(6, 0)
+    QT_DEPRECATED static QQmlFileSelector *get(QQmlEngine*);
+#endif
 
 private:
     Q_DISABLE_COPY(QQmlFileSelector)