Make sure we load translations on the main thread.

Because the old implementation (accidentally) worked when you put the
ecm_create_qm_loader call in a different CMakeLists.txt file to the
target the file was added to, some projects did this.

This won't work with build-time-generated files, though, like moc files.
So we (ab)use QTimer events to make the loading happen on the main
thread.

BUG: 346188
REVIEW: 126000

7 files changed, 207 insertions, 33 deletions

diff --git a/modules/ECMPoQmTools.cmake b/modules/ECMPoQmTools.cmake
index 0af5b12f..2547df1a 100644
--- a/modules/ECMPoQmTools.cmake
+++ b/modules/ECMPoQmTools.cmake
@@ -28,8 +28,8 @@
 #
 #   ecm_create_qm_loader(<source_files_var> <catalog_name>)
 #
-# Generates a C++ file which ensures translations are automatically loaded at
-# startup. The path of the .cpp file is appended to ``<source_files_var>``.
+# Generates C++ code which ensures translations are automatically loaded at
+# startup. The generated files are appended to ``<source_files_var>``.
 #
 # It assumes that the .qm file for the language code ``<lang>`` is installed as
 # ``<sharedir>/locale/<lang>/LC_MESSAGES/<catalog_name>.qm``, where
@@ -99,9 +99,16 @@ endfunction()
 
 
 function(ecm_create_qm_loader out_var catalog_name)
-    # catalog_name is used in ECMQmLoader.cpp.in
-    configure_file(${ECM_MODULE_DIR}/ECMQmLoader.cpp.in ECMQmLoader-${catalog_name}.cpp @ONLY)
-    set(${out_var} ${${out_var}} ${CMAKE_CURRENT_BINARY_DIR}/ECMQmLoader-${catalog_name}.cpp PARENT_SCOPE)
+    set(loader_base ${CMAKE_CURRENT_BINARY_DIR}/ECMQmLoader-${catalog_name})
+
+    set(QM_LOADER_CATALOG_NAME "${catalog_name}")
+
+    configure_file(
+        ${ECM_MODULE_DIR}/ECMQmLoader.cpp.in
+        "${loader_base}.cpp"
+        @ONLY
+    )
+    set(${out_var} "${${out_var}}" "${loader_base}.cpp" PARENT_SCOPE)
 endfunction()
 
 
diff --git a/modules/ECMQmLoader.cpp.in b/modules/ECMQmLoader.cpp.in
index 423d1c93..97c5c826 100644
--- a/modules/ECMQmLoader.cpp.in
+++ b/modules/ECMQmLoader.cpp.in
@@ -33,40 +33,81 @@
 #include <QCoreApplication>
 #include <QLocale>
 #include <QStandardPaths>
+#include <QThread>
 #include <QTranslator>
 
-#include <QDebug>
+namespace {
 
-static bool loadTranslation(const QString &localeDirName)
-{
-    QString subPath = QStringLiteral("locale/") + localeDirName + QStringLiteral("/LC_MESSAGES/@catalog_name@.qm");
-    QString fullPath = QStandardPaths::locate(QStandardPaths::GenericDataLocation, subPath);
-    if (fullPath.isEmpty()) {
-        return false;
+    bool loadTranslation(const QString &localeDirName)
+    {
+        QString subPath = QStringLiteral("locale/") + localeDirName + QStringLiteral("/LC_MESSAGES/@QM_LOADER_CATALOG_NAME@.qm");
+        QString fullPath = QStandardPaths::locate(QStandardPaths::GenericDataLocation, subPath);
+        if (fullPath.isEmpty()) {
+            return false;
+        }
+        QTranslator *translator = new QTranslator(QCoreApplication::instance());
+        if (!translator->load(fullPath)) {
+            delete translator;
+            return false;
+        }
+        QCoreApplication::instance()->installTranslator(translator);
+        return true;
     }
-    QTranslator *translator = new QTranslator(QCoreApplication::instance());
-    if (!translator->load(fullPath)) {
-        delete translator;
-        return false;
+
+    void load()
+    {
+        // The way Qt translation system handles plural forms makes it necessary to
+        // have a translation file which contains only plural forms for `en`. That's
+        // why we load the `en` translation unconditionally, then load the
+        // translation for the current locale to overload it.
+        loadTranslation(QStringLiteral("en"));
+
+        QLocale locale = QLocale::system();
+        if (locale.name() != QStringLiteral("en")) {
+            if (!loadTranslation(locale.name())) {
+                loadTranslation(locale.bcp47Name());
+            }
+        }
     }
-    QCoreApplication::instance()->installTranslator(translator);
-    return true;
-}
 
-static void load()
-{
-    // The way Qt translation system handles plural forms makes it necessary to
-    // have a translation file which contains only plural forms for `en`. That's
-    // why we load the `en` translation unconditionally, then load the
-    // translation for the current locale to overload it.
-    loadTranslation(QStringLiteral("en"));
+    // Helper to call load() on the main thread.
+    //
+    // Calling functions on another thread without using moc is non-trivial in
+    // Qt until 5.4 (when some useful QTimer::singleShot overloads were added).
+    //
+    // Instead, we have to use QEvents. Ideally, we'd use a custom QEvent, but
+    // there's a chance this could cause trouble with applications that claim
+    // QEvent codes themselves, but don't register them with Qt (and we also
+    // want to avoid registering a new QEvent code for every plugin that might
+    // be loaded). We use QTimer because it's unlikely to be filtered by
+    // applications, and is also unlikely to cause Qt to do something it
+    // shouldn't.
+    class Loader : public QObject
+    {
+    protected:
+        void timerEvent(QTimerEvent *) Q_DECL_OVERRIDE
+        {
+            load();
+            this->deleteLater();
+        }
+    };
 
-    QLocale locale = QLocale::system();
-    if (locale.name() != QStringLiteral("en")) {
-        if (!loadTranslation(locale.name())) {
-            loadTranslation(locale.bcp47Name());
+    void loadOnMainThread()
+    {
+        // If this library is loaded after the QCoreApplication instance is
+        // created (eg: because it is brought in by a plugin), there is no
+        // guarantee this function will be called on the main thread.
+        // QCoreApplication::installTranslator needs to be called on the main
+        // thread, because it uses QCoreApplication::sendEvent.
+        if (QThread::currentThread() == QCoreApplication::instance()->thread()) {
+            load();
+        } else {
+            // QObjects inherit their parent object's thread
+            Loader *loader = new Loader();
+            loader->moveToThread(QCoreApplication::instance()->thread());
+            QCoreApplication::instance()->postEvent(loader, new QTimerEvent(0), Qt::HighEventPriority);
         }
     }
 }
 
-Q_COREAPP_STARTUP_FUNCTION(load)
+Q_COREAPP_STARTUP_FUNCTION(loadOnMainThread)
diff --git a/tests/ECMPoQmToolsTest/CMakeLists.txt b/tests/ECMPoQmToolsTest/CMakeLists.txt
index 73d2b427..2cd76f8f 100644
--- a/tests/ECMPoQmToolsTest/CMakeLists.txt
+++ b/tests/ECMPoQmToolsTest/CMakeLists.txt
@@ -82,7 +82,7 @@ target_link_libraries(tr_test PRIVATE Qt5::Core)
 
 
 #
-# single-threaded test (different catalog name)
+# single-threaded test (different catalog name, automoc)
 #
 # This is to check we don't overwrite previously-generated files.
 set(tr_test_2_SRCS
@@ -90,15 +90,55 @@ set(tr_test_2_SRCS
 )
 ecm_create_qm_loader(tr_test_2_SRCS catalog2)
 add_executable(tr_test_2 ${tr_test_2_SRCS})
+set_target_properties(tr_test_2 PROPERTIES AUTOMOC ON)
+target_include_directories(tr_test_2 PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
 target_link_libraries(tr_test_2 PRIVATE Qt5::Core)
 
 
 #
+# module for tr_thread_test
+#
+add_library(tr_thread_module MODULE tr_thread_test_module.cpp ${QMLOADER_FILES})
+target_link_libraries(tr_thread_module PRIVATE Qt5::Core)
+
+
+#
+# loading a module on a thread other than the main thread
+# (automoc)
+#
+set(tr_thread_test_SRCS
+    tr_thread_test.cpp
+)
+ecm_create_qm_loader(tr_thread_test_SRCS catalog)
+add_executable(tr_thread_test ${tr_thread_test_SRCS})
+set_target_properties(tr_thread_test PROPERTIES AUTOMOC ON)
+target_include_directories(tr_thread_test PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
+target_compile_definitions(tr_thread_test PRIVATE "MODULE_PATH=\"$<TARGET_FILE:tr_thread_module>\"")
+target_link_libraries(tr_thread_test PRIVATE Qt5::Core)
+
+
+#
+# loading a module on a thread other than the main thread
+# (different catalog, no AUTOMOC)
+#
+qt5_generate_moc(tr_thread_test.cpp "${CMAKE_CURRENT_BINARY_DIR}/tr_thread_test.moc")
+set(tr_thread_test_2_SRCS
+    tr_thread_test.cpp
+    "${CMAKE_CURRENT_BINARY_DIR}/tr_thread_test.moc"
+)
+ecm_create_qm_loader(tr_thread_test_2_SRCS catalog2)
+add_executable(tr_thread_test_2 ${tr_thread_test_2_SRCS})
+target_include_directories(tr_thread_test_2 PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
+target_compile_definitions(tr_thread_test_2 PRIVATE "MODULE_PATH=\"$<TARGET_FILE:tr_thread_module>\"")
+target_link_libraries(tr_thread_test_2 PRIVATE Qt5::Core)
+
+
+#
 # call to ecm_create_qm_loader is in a different CMakeLists.txt to where
 # the target it is added to is defined
 #
 # This is not something we want people to do, but it's unfortunately something
-# projects have done and we need to keep them building
+# projects have done and we need to keep them building.
 unset(QMLOADER_FILES)
 ecm_create_qm_loader(QMLOADER_FILES catalog)
 assert_var_defined(QMLOADER_FILES)
diff --git a/tests/ECMPoQmToolsTest/check.cmake.in b/tests/ECMPoQmToolsTest/check.cmake.in
index 5329b78d..4fe2ac33 100644
--- a/tests/ECMPoQmToolsTest/check.cmake.in
+++ b/tests/ECMPoQmToolsTest/check.cmake.in
@@ -87,6 +87,9 @@ if("@CMAKE_SYSTEM_NAME@" STREQUAL "Linux")
     check_translations(TR_TEST_2 "${TR_TEST_2_EXEC}" catalog2)
 
     check_translations(TR_TEST_SUBDIR "${TR_TEST_SUBDIR_EXEC}" catalog)
+
+    check_translations(TR_THREAD_TEST "${TR_THREAD_TEST_EXEC}" catalog)
+    check_translations(TR_THREAD_TEST_2 "${TR_THREAD_TEST_2_EXEC}" catalog2)
 endif()
 
 if (fail)
diff --git a/tests/ECMPoQmToolsTest/check_conf.cmake.in b/tests/ECMPoQmToolsTest/check_conf.cmake.in
index 7bd11c36..a752d5bd 100644
--- a/tests/ECMPoQmToolsTest/check_conf.cmake.in
+++ b/tests/ECMPoQmToolsTest/check_conf.cmake.in
@@ -1,3 +1,5 @@
 set(TR_TEST_EXEC "$<TARGET_FILE:tr_test>")
 set(TR_TEST_2_EXEC "$<TARGET_FILE:tr_test_2>")
 set(TR_TEST_SUBDIR_EXEC "$<TARGET_FILE:tr_test_subdir>")
+set(TR_THREAD_TEST_EXEC "$<TARGET_FILE:tr_thread_test>")
+set(TR_THREAD_TEST_2_EXEC "$<TARGET_FILE:tr_thread_test_2>")
diff --git a/tests/ECMPoQmToolsTest/tr_thread_test.cpp b/tests/ECMPoQmToolsTest/tr_thread_test.cpp
new file mode 100644
index 00000000..40e4d1eb
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_thread_test.cpp
@@ -0,0 +1,69 @@
+#include <QCoreApplication>
+#include <QLibrary>
+#include <QMetaObject>
+#include <QThread>
+
+class Thread : public QThread
+{
+    Q_OBJECT
+
+    QLibrary *m_lib;
+
+public:
+    Thread()
+        : m_lib(0)
+    {}
+    ~Thread()
+    {
+        delete m_lib;
+    }
+
+Q_SIGNALS:
+    void libraryLoaded();
+
+public Q_SLOTS:
+    void printStrings()
+    {
+        Q_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread());
+
+        QFunctionPointer print_strings = m_lib->resolve("print_strings");
+        if (print_strings) {
+            print_strings();
+        } else {
+            qFatal("Could not resolve print_strings: %s", qPrintable(m_lib->errorString()));
+        }
+
+        QCoreApplication::instance()->quit();
+    }
+protected:
+    void run()
+    {
+        m_lib = new QLibrary(MODULE_PATH);
+
+        if (!m_lib->load()) {
+            qFatal("Could not load module: %s", m_lib->errorString().toUtf8().data());
+        }
+
+        // Queue a call to printStrings() on the main event loop (giving the
+        // translations a chance to be loaded).
+        QMetaObject::invokeMethod(this, "printStrings", Qt::QueuedConnection);
+    }
+};
+
+int main(int argc, char** argv)
+{
+    QCoreApplication app(argc, argv);
+
+    Thread thread;
+
+    // Start the thread *after* QCoreApplication is started (otherwise the
+    // plugin's startup function won't be run on the Thread, and we won't test
+    // what we wanted to test).
+    QMetaObject::invokeMethod(&thread, "start", Qt::QueuedConnection);
+
+    app.exec();
+
+    return 0;
+}
+
+#include "tr_thread_test.moc"
diff --git a/tests/ECMPoQmToolsTest/tr_thread_test_module.cpp b/tests/ECMPoQmToolsTest/tr_thread_test_module.cpp
new file mode 100644
index 00000000..b9000ffa
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_thread_test_module.cpp
@@ -0,0 +1,12 @@
+#include <QCoreApplication>
+#include <QTextStream>
+
+#include <stdio.h>
+
+extern "C" Q_DECL_EXPORT void print_strings()
+{
+    QTextStream output(stdout);
+
+    output << QCoreApplication::translate("testcontext", "test string") << ":";
+    output << QCoreApplication::translate("testcontext", "test plural %n", 0, 5) << '\n';
+}