Blob Blame History Raw
From 843a3b4bf1fd62a72646afaf0efb348b1f444b78 Mon Sep 17 00:00:00 2001
From: Albert Astals Cid <aacid@kde.org>
Date: Sat, 23 May 2020 01:35:18 +0200
Subject: [PATCH] Do not fully initialize QIconLoader when setting the fallback theme

We need this because without this patch you get bugs both
if you call QIcon::setFallbackThemeName before creating the QGuiApplication and
if you call QIcon::setFallbackThemeName after creating QGuiApplication

Why do you get a bug if you call QIconLoader::setFallbackThemeName
before creating the QGuiApplication:
 * QIcon::setFallbackThemeName calls QIconLoader::instance
 * QIconLoader::instance calls QIconLoader::ensureInitialized
 * QIconLoader::ensureInitialized calls systemThemeName
 * systemThemeName asks the current QPlatformTheme for its
   QPlatformTheme::SystemIconThemeName
 * But since we're calling this before creating the QGuiApplication
   there is no current QPlatformTheme yet, so systemThemeName
   is set to empty, which is obviously not what we want

Why do you get a bug if you call QIconLoader::setFallbackThemeName
after creating the QGuiApplication:
 * QGuiApplicationPrivate::init calls
   QGuiApplicationPrivate::createPlatformIntegration
 * QGuiApplicationPrivate::createPlatformIntegration sets the
   current QPlatformTheme and at the end of the very same function
   uses QIcon::fromTheme
 * Since we haven't called QIconLoader::setFallbackThemeName yet
   there is at least one icon lookup that doesn't take
   the fallback theme we would like to have into account

This patch makes it so calling QIconLoader::setFallbackThemeName
before creating the QGuiApplication works.

The only thing we want to do from QIcon::setFallbackThemeName is set
the internal m_userFallbackTheme, it doesn't care about doing
further initialization of QIconLoader, if it's done, great it's done,
if it is not initialized yet, great it will be initialized later
when someone actually tries to use the QIconloader.

So it's OK for ensureInitialized() to return early if there is no
platform theme yet, because it will be called again later.

Fixes: QTBUG-74252
Change-Id: I65268fc3d3d0bd282d76c76cf75e495bcc9d1a30
Done-with: Albert Astals Cid <albert.astals.cid@kdab.com>
Reviewed-by: Albert Astals Cid <albert.astals.cid@kdab.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit add92a551cf601b5c9e074046326f95ccc38062e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---

diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp
index 41fe649..91f3fbd 100644
--- a/src/gui/image/qicon.cpp
+++ b/src/gui/image/qicon.cpp
@@ -1256,6 +1256,9 @@
     themeSearchPath() containing an index.theme
     file describing its contents.
 
+    \note This should be done before creating \l QGuiApplication, to ensure
+    correct initialization.
+
     \sa fallbackThemeName(), themeSearchPaths(), themeName()
 */
 void QIcon::setFallbackThemeName(const QString &name)
diff --git a/src/gui/image/qiconloader.cpp b/src/gui/image/qiconloader.cpp
index 15ab1b3..3fa3bb9 100644
--- a/src/gui/image/qiconloader.cpp
+++ b/src/gui/image/qiconloader.cpp
@@ -112,10 +112,9 @@
 void QIconLoader::ensureInitialized()
 {
     if (!m_initialized) {
+        if (!QGuiApplicationPrivate::platformTheme())
+            return; // it's too early: try again later (QTBUG-74252)
         m_initialized = true;
-
-        Q_ASSERT(qApp);
-
         m_systemTheme = systemThemeName();
 
         if (m_systemTheme.isEmpty())
@@ -125,6 +124,16 @@
     }
 }
 
+/*!
+    \internal
+    Gets an instance.
+
+    \l QIcon::setFallbackThemeName() should be called before QGuiApplication is
+    created, to avoid a race condition (QTBUG-74252). When this function is
+    called from there, ensureInitialized() does not succeed because there
+    is no QPlatformTheme yet, so systemThemeName() is empty, and we don't want
+    m_systemTheme to get intialized to the fallback theme instead of the normal one.
+*/
 QIconLoader *QIconLoader::instance()
 {
    iconLoaderInstance()->ensureInitialized();