From add92a551cf601b5c9e074046326f95ccc38062e Mon Sep 17 00:00:00 2001 From: Albert Astals Cid 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. Pick-to: 5.12 Pick-to: 5.15 Fixes: QTBUG-74252 Change-Id: I65268fc3d3d0bd282d76c76cf75e495bcc9d1a30 Done-with: Albert Astals Cid Reviewed-by: Albert Astals Cid Reviewed-by: Shawn Rutledge --- src/gui/image/qicon.cpp | 3 +++ src/gui/image/qiconloader.cpp | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp index 36f499711e9..91da21c477d 100644 --- a/src/gui/image/qicon.cpp +++ b/src/gui/image/qicon.cpp @@ -1238,6 +1238,9 @@ QString QIcon::fallbackThemeName() 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 15ab1b3cd90..3fa3bb9c598 100644 --- a/src/gui/image/qiconloader.cpp +++ b/src/gui/image/qiconloader.cpp @@ -112,10 +112,9 @@ extern QFactoryLoader *qt_iconEngineFactoryLoader(); // qicon.cpp 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 @@ void QIconLoader::ensureInitialized() } } +/*! + \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(); -- 2.16.3