QFactoryLoader: add setExtraSearchPath() (for QPA plugins' use)

This is added specifically for the QPA platform and theme plugins, to
honor the QT_QPA_PLATFORM_PLUGIN_PATH environment variable and the
(inadvisable) -platformpluginpath command-line argument.

This removes the last QFactoryLoader used with an empty path (also the
only two that could be reached), which were causing a scan of the
application's binary directory whenever the platform plugin path was
set. In case of applications installed to /usr/bin, the entire /usr/bin
was scanned, which can be qualified as "not good".

Fixes: QTBUG-97950
Pick-to: 6.3
Change-Id: Ice04365c72984d07a64dfffd16b47fe1d22f26d3
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/corelib/plugin/qfactoryloader.cpp

@@ -1,7 +1,7 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
-** Copyright (C) 2018 Intel Corporation.
+** Copyright (C) 2022 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -140,6 +140,7 @@ public:
     QList<QLibraryPrivate*> libraryList;
     QMap<QString,QLibraryPrivate*> keyMap;
     QString suffix;
+    QString extraSearchPath;
     Qt::CaseSensitivity cs;
 
     void updateSinglePath(const QString &pluginDir);
@@ -276,6 +277,8 @@ void QFactoryLoader::update()
 
         d->updateSinglePath(path);
     }
+    if (!d->extraSearchPath.isEmpty())
+        d->updateSinglePath(d->extraSearchPath);
 #else
     Q_D(QFactoryLoader);
     qCDebug(lcFactoryLoader) << "ignoring" << d->iid
@@ -314,6 +317,9 @@ QFactoryLoader::QFactoryLoader(const char *iid,
                                Qt::CaseSensitivity cs)
     : QObject(*new QFactoryLoaderPrivate)
 {
+    Q_ASSERT_X(suffix.startsWith(u'/'), "QFactoryLoader",
+               "For historical reasons, the suffix must start with '/' (and it can't be empty)");
+
     moveToThread(QCoreApplicationPrivate::mainThread());
     Q_D(QFactoryLoader);
     d->iid = iid;
@@ -334,6 +340,30 @@ QFactoryLoader::QFactoryLoader(const char *iid,
 #endif
 }
 
+void QFactoryLoader::setExtraSearchPath(const QString &path)
+{
+#if QT_CONFIG(library)
+    Q_D(QFactoryLoader);
+    if (d->extraSearchPath == path)
+        return;             // nothing to do
+
+    QMutexLocker locker(qt_factoryloader_mutex());
+    QString oldPath = qExchange(d->extraSearchPath, path);
+    if (oldPath.isEmpty()) {
+        // easy case, just update this directory
+        d->updateSinglePath(d->extraSearchPath);
+    } else {
+        // must re-scan everything
+        d->loadedPaths.clear();
+        d->libraryList.clear();
+        d->keyMap.clear();
+        update();
+    }
+#else
+    Q_UNUSED(path);
+#endif
+}
+
 QFactoryLoader::MetaDataList QFactoryLoader::metaData() const
 {
     Q_D(const QFactoryLoader);

src/corelib/plugin/qfactoryloader_p.h

@@ -1,7 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2017 The Qt Company Ltd.
-** Copyright (C) 2021 Intel Corporation.
+** Copyright (C) 2022 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -115,6 +115,7 @@ public:
 #endif // Q_OS_UNIX && !Q_OS_MAC
 #endif // QT_CONFIG(library)
 
+    void setExtraSearchPath(const QString &path);
     QMultiMap<int, QString> keyMap() const;
     int indexOf(const QString &needle) const;
 

src/gui/kernel/qplatformintegrationfactory.cpp

@@ -51,23 +51,9 @@ QT_BEGIN_NAMESPACE
 Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, loader,
     (QPlatformIntegrationFactoryInterface_iid, QLatin1String("/platforms"), Qt::CaseInsensitive))
 
-#if QT_CONFIG(library)
-Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, directLoader,
-                          (QPlatformIntegrationFactoryInterface_iid, QLatin1String(""), Qt::CaseInsensitive))
-#endif // QT_CONFIG(library)
-
 QPlatformIntegration *QPlatformIntegrationFactory::create(const QString &platform, const QStringList &paramList, int &argc, char **argv, const QString &platformPluginPath)
 {
-#if QT_CONFIG(library)
+    loader->setExtraSearchPath(platformPluginPath);
-    // Try loading the plugin from platformPluginPath first:
-    if (!platformPluginPath.isEmpty()) {
-        QCoreApplication::addLibraryPath(platformPluginPath);
-        if (QPlatformIntegration *ret = qLoadPlugin<QPlatformIntegration, QPlatformIntegrationPlugin>(directLoader(), platform, paramList, argc, argv))
-            return ret;
-    }
-#else
-    Q_UNUSED(platformPluginPath);
-#endif
     return qLoadPlugin<QPlatformIntegration, QPlatformIntegrationPlugin>(loader(), platform, paramList, argc, argv);
 }
 
@@ -80,25 +66,8 @@ QPlatformIntegration *QPlatformIntegrationFactory::create(const QString &platfor
 
 QStringList QPlatformIntegrationFactory::keys(const QString &platformPluginPath)
 {
-    QStringList list;
+    loader->setExtraSearchPath(platformPluginPath);
-#if QT_CONFIG(library)
+    return loader->keyMap().values();
-    if (!platformPluginPath.isEmpty()) {
-        QCoreApplication::addLibraryPath(platformPluginPath);
-        list = directLoader()->keyMap().values();
-        if (!list.isEmpty()) {
-            const QString postFix = QLatin1String(" (from ")
-                                    + QDir::toNativeSeparators(platformPluginPath)
-                                    + QLatin1Char(')');
-            const QStringList::iterator end = list.end();
-            for (QStringList::iterator it = list.begin(); it != end; ++it)
-                (*it).append(postFix);
-        }
-    }
-#else
-    Q_UNUSED(platformPluginPath);
-#endif
-    list.append(loader()->keyMap().values());
-    return list;
 }
 
 QT_END_NAMESPACE

src/gui/kernel/qplatformthemefactory.cpp

@@ -51,25 +51,11 @@ QT_BEGIN_NAMESPACE
 Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, loader,
     (QPlatformThemeFactoryInterface_iid, QLatin1String("/platformthemes"), Qt::CaseInsensitive))
 
-#if QT_CONFIG(library)
-Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, directLoader,
-                          (QPlatformThemeFactoryInterface_iid, QLatin1String(""), Qt::CaseInsensitive))
-#endif
-
 QPlatformTheme *QPlatformThemeFactory::create(const QString& key, const QString &platformPluginPath)
 {
     QStringList paramList = key.split(QLatin1Char(':'));
     const QString platform = paramList.takeFirst().toLower();
-#if QT_CONFIG(library)
+    loader->setExtraSearchPath(platformPluginPath);
-    // Try loading the plugin from platformPluginPath first:
-    if (!platformPluginPath.isEmpty()) {
-        QCoreApplication::addLibraryPath(platformPluginPath);
-        if (QPlatformTheme *ret = qLoadPlugin<QPlatformTheme, QPlatformThemePlugin>(directLoader(), platform, paramList))
-            return ret;
-    }
-#else
-    Q_UNUSED(platformPluginPath);
-#endif
     return qLoadPlugin<QPlatformTheme, QPlatformThemePlugin>(loader(), platform, paramList);
 }
 
@@ -81,26 +67,8 @@ QPlatformTheme *QPlatformThemeFactory::create(const QString& key, const QString
 */
 QStringList QPlatformThemeFactory::keys(const QString &platformPluginPath)
 {
-    QStringList list;
+    loader->setExtraSearchPath(platformPluginPath);
-
+    return loader->keyMap().values();
-#if QT_CONFIG(library)
-    if (!platformPluginPath.isEmpty()) {
-        QCoreApplication::addLibraryPath(platformPluginPath);
-        list += directLoader()->keyMap().values();
-        if (!list.isEmpty()) {
-            const QString postFix = QLatin1String(" (from ")
-                    + QDir::toNativeSeparators(platformPluginPath)
-                    + QLatin1Char(')');
-            const QStringList::iterator end = list.end();
-            for (QStringList::iterator it = list.begin(); it != end; ++it)
-                (*it).append(postFix);
-        }
-    }
-#else
-    Q_UNUSED(platformPluginPath);
-#endif
-    list += loader()->keyMap().values();
-    return list;
 }
 
 QT_END_NAMESPACE

tests/auto/corelib/plugin/qfactoryloader/tst_qfactoryloader.cpp

@@ -47,11 +47,13 @@ class tst_QFactoryLoader : public QObject
     QSharedPointer<QTemporaryDir> directory;
 #endif
 
+    QString binFolder;
 public slots:
     void initTestCase();
 
 private slots:
     void usingTwoFactoriesFromSameDir();
+    void extraSearchPath();
 };
 
 static const char binFolderC[] = "bin";
@@ -64,15 +66,17 @@ void tst_QFactoryLoader::initTestCase()
     QVERIFY(directory->isValid());
     QVERIFY2(QDir::setCurrent(directory->path()), qPrintable("Could not chdir to " + directory->path()));
 #endif
-    const QString binFolder = QFINDTESTDATA(binFolderC);
+    binFolder = QFINDTESTDATA(binFolderC);
     QVERIFY2(!binFolder.isEmpty(), "Unable to locate 'bin' folder");
-#if QT_CONFIG(library)
-    QCoreApplication::setLibraryPaths(QStringList(QFileInfo(binFolder).absolutePath()));
-#endif
 }
 
 void tst_QFactoryLoader::usingTwoFactoriesFromSameDir()
 {
+#if QT_CONFIG(library)
+    // set the library path to contain the directory where the 'bin' dir is located
+    QCoreApplication::setLibraryPaths( { QFileInfo(binFolder).absolutePath() });
+#endif
+
     const QString suffix = QLatin1Char('/') + QLatin1String(binFolderC);
     QFactoryLoader loader1(PluginInterface1_iid, suffix);
 
@@ -92,5 +96,32 @@ void tst_QFactoryLoader::usingTwoFactoriesFromSameDir()
     QCOMPARE(plugin2->pluginName(), QLatin1String("Plugin2 ok"));
 }
 
+void tst_QFactoryLoader::extraSearchPath()
+{
+#if defined(Q_OS_ANDROID) && !QT_CONFIG(library)
+    QSKIP("Test not applicable in this configuration.");
+#else
+    QCoreApplication::setLibraryPaths(QStringList());
+
+    QString absoluteBinPath = QFileInfo(binFolder).absoluteFilePath();
+    QFactoryLoader loader1(PluginInterface1_iid, "/nonexistent");
+
+    // it shouldn't have scanned anything because we haven't given it a path yet
+    QVERIFY(loader1.metaData().isEmpty());
+
+    loader1.setExtraSearchPath(absoluteBinPath);
+    PluginInterface1 *plugin1 = qobject_cast<PluginInterface1 *>(loader1.instance(0));
+    QVERIFY2(plugin1,
+             qPrintable(QString::fromLatin1("Cannot load plugin '%1'")
+                        .arg(QLatin1String(PluginInterface1_iid))));
+
+    QCOMPARE(plugin1->pluginName(), QLatin1String("Plugin1 ok"));
+
+    // check that it forgets that plugin
+    loader1.setExtraSearchPath(QString());
+    QVERIFY(loader1.metaData().isEmpty());
+#endif
+}
+
 QTEST_MAIN(tst_QFactoryLoader)
 #include "tst_qfactoryloader.moc"