Make sure we load translations on the main thread.

BUG: 346188
REVIEW: 123726

11 files changed, 277 insertions, 46 deletions

diff --git a/modules/ECMPoQmTools.cmake b/modules/ECMPoQmTools.cmake
index 12bcf6b6..22258dc8 100644
--- a/modules/ECMPoQmTools.cmake
+++ b/modules/ECMPoQmTools.cmake
@@ -99,9 +99,20 @@ endfunction()
 
 
 function(ecm_create_qm_loader out_var catalog_name)
+    set(loader_cpp ${CMAKE_CURRENT_BINARY_DIR}/ECMQmLoader.cpp)
+    set(loader_moc ${CMAKE_CURRENT_BINARY_DIR}/ECMQmLoader.moc)
+
     # catalog_name is used in ECMQmLoader.cpp.in
-    configure_file(${ECM_MODULE_DIR}/ECMQmLoader.cpp.in ECMQmLoader.cpp @ONLY)
-    set(${out_var} ${${out_var}} ${CMAKE_CURRENT_BINARY_DIR}/ECMQmLoader.cpp PARENT_SCOPE)
+    configure_file(${ECM_MODULE_DIR}/ECMQmLoader.cpp.in "${loader_cpp}" @ONLY)
+    set(${out_var} "${${out_var}}" "${loader_cpp}" "${loader_moc}" PARENT_SCOPE)
+
+    # can't assume target has AUTOMOC turned on
+    if(NOT Qt5Core_FOUND)
+        find_package(Qt5Core)
+    endif()
+    if(Qt5Core_FOUND)
+        qt5_generate_moc("${loader_cpp}" "${loader_moc}")
+    endif()
 endfunction()
 
 
diff --git a/modules/ECMQmLoader.cpp.in b/modules/ECMQmLoader.cpp.in
index bc01bf98..f6b98e7b 100644
--- a/modules/ECMQmLoader.cpp.in
+++ b/modules/ECMQmLoader.cpp.in
@@ -5,41 +5,77 @@
  */
 #include <QCoreApplication>
 #include <QLocale>
+#include <QMetaObject>
 #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/@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"));
-
-    QLocale locale = QLocale::system();
-    if (locale.name() != QStringLiteral("en")) {
-        if (!loadTranslation(locale.name())) {
-            loadTranslation(locale.bcp47Name());
+    // helper to call load() on the correct thread
+    class Loader : public QObject
+    {
+        Q_OBJECT
+
+    public Q_SLOTS:
+        void callLoadAndDeleteSelf()
+        {
+            load();
+            this->deleteLater();
+        }
+    };
+
+    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());
+            QMetaObject::invokeMethod(loader, "callLoadAndDeleteSelf", Qt::AutoConnection);
         }
     }
+
 }
 
-Q_COREAPP_STARTUP_FUNCTION(load)
+Q_COREAPP_STARTUP_FUNCTION(loadOnMainThread)
+
+#include "ECMQmLoader.moc"
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 8a75ae61..9e6de12f 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -101,13 +101,13 @@ add_test_macro(ECMInstallIconsTest
     ${CMAKE_COMMAND} -P "${CMAKE_CURRENT_BINARY_DIR}/ECMInstallIconsTest/check_tree.cmake"
 )
 
-if (Qt5LinguistTools_FOUND)
+if (Qt5Core_FOUND AND Qt5LinguistTools_FOUND)
     set(ECMPoQmToolsTest_EXTRA_OPTIONS
         --build-target install
         --build-options
             "-DCMAKE_INSTALL_PREFIX:PATH=${CMAKE_CURRENT_BINARY_DIR}/ECMPoQmToolsTest/InstallDirectory"
     )
     add_test_macro(ECMPoQmToolsTest
-        ${CMAKE_COMMAND} -P "${CMAKE_CURRENT_BINARY_DIR}/ECMPoQmToolsTest/check_tree.cmake"
+        ${CMAKE_COMMAND} -P "${CMAKE_CURRENT_BINARY_DIR}/ECMPoQmToolsTest/check.cmake"
     )
 endif()
diff --git a/tests/ECMPoQmToolsTest/CMakeLists.txt b/tests/ECMPoQmToolsTest/CMakeLists.txt
index 15351d2f..e08a2b8c 100644
--- a/tests/ECMPoQmToolsTest/CMakeLists.txt
+++ b/tests/ECMPoQmToolsTest/CMakeLists.txt
@@ -9,9 +9,14 @@ file(REMOVE_RECURSE "${CMAKE_INSTALL_PREFIX}")
 
 include(ECMPoQmTools)
 
-# Should create ${CMAKE_CURRENT_BINARY_DIR}/qmloader.cpp and set QMLOADER_PATH
-# to its path
-ecm_create_qm_loader(QMLOADER_PATH catalog)
+include(../test_helpers.cmake)
+
+unset(QMLOADER_FILES)
+ecm_create_qm_loader(QMLOADER_FILES catalog)
+assert_var_defined(QMLOADER_FILES)
+
+# These will be used to test the above-generated loader
+ecm_install_po_files_as_qm(tr_test-po)
 
 # Should create a process-and-install.qm file and install it
 ecm_process_po_files_as_qm(fr ALL
@@ -39,5 +44,22 @@ ecm_install_po_files_as_qm(po-custom-dir1)
 set(LOCALE_INSTALL_DIR custom-dir2)
 ecm_install_po_files_as_qm(po-custom-dir2)
 
-# this will be run by CTest
-configure_file(check_tree.cmake.in "${CMAKE_CURRENT_BINARY_DIR}/check_tree.cmake" @ONLY)
+find_package(Qt5Core CONFIG REQUIRED)
+
+add_executable(tr_test tr_test.cpp ${QMLOADER_FILES})
+target_link_libraries(tr_test PRIVATE Qt5::Core)
+
+add_library(tr_thread_module MODULE tr_thread_test_module.cpp ${QMLOADER_FILES})
+target_link_libraries(tr_thread_module PRIVATE Qt5::Core)
+
+add_executable(tr_thread_test tr_thread_test.cpp)
+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)
+
+file(GENERATE
+    OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/check_conf.cmake"
+    INPUT "${CMAKE_CURRENT_SOURCE_DIR}/check_conf.cmake.in"
+)
+configure_file(check.cmake.in "${CMAKE_CURRENT_BINARY_DIR}/check.cmake" @ONLY)
diff --git a/tests/ECMPoQmToolsTest/check_tree.cmake.in b/tests/ECMPoQmToolsTest/check.cmake.in
index 9f4f7c0d..2f0cc205 100644
--- a/tests/ECMPoQmToolsTest/check_tree.cmake.in
+++ b/tests/ECMPoQmToolsTest/check.cmake.in
@@ -1,6 +1,6 @@
 set(BINARY_DIR "@CMAKE_CURRENT_BINARY_DIR@")
 set(ACTUAL_TREE "@CMAKE_INSTALL_PREFIX@")
-set(QMLOADER_PATH "@QMLOADER_PATH@")
+include("${BINARY_DIR}/check_conf.cmake")
 
 set(fail OFF)
 
@@ -9,27 +9,21 @@ macro(mark_failed msg)
     set(fail ON)
 endmacro()
 
-macro(check_strequal var expected)
-    if (NOT "${${var}}" STREQUAL "${expected}")
-        mark_failed("${var} is:\n  \"${${var}}\"\nExpected:\n  \"${expected}\"")
-    endif()
-endmacro()
-
 macro(check_exists file)
+    message(STATUS "Checking for ${file}")
     if (NOT EXISTS ${file})
         mark_failed("File \"${file}\" does not exist")
     endif()
 endmacro()
 
-check_exists(${BINARY_DIR}/ECMQmLoader.cpp)
-check_strequal(QMLOADER_PATH "${BINARY_DIR}/ECMQmLoader.cpp")
-
 check_exists(${BINARY_DIR}/fr/only-process.qm)
 
 set(exp_files
     "share/locale/fr/LC_MESSAGES/process-and-install.qm"
     "share/locale/es/LC_MESSAGES/install-test.qm"
     "share/locale/fr/LC_MESSAGES/install-test.qm"
+    "share/locale/en/LC_MESSAGES/catalog.qm"
+    "share/locale/en_GB/LC_MESSAGES/catalog.qm"
     "custom-dir1/es/LC_MESSAGES/custom-dir1-install-test.qm"
     "custom-dir1/fr/LC_MESSAGES/custom-dir1-install-test.qm"
     "custom-dir2/es/LC_MESSAGES/custom-dir2-install-test.qm"
@@ -54,6 +48,32 @@ if(NOT exp_files STREQUAL actual_files)
             set(fail ON)
         endif()
     endforeach()
+else()
+    message(STATUS "Installed translations in expected locations")
+endif()
+
+# we know we can modify the executable environment on Linux
+if("@CMAKE_SYSTEM_NAME@" STREQUAL "Linux")
+    set(exp_output_en "english text:english plural form 5")
+    set(exp_output_en_GB "british english text:british english plural form 5")
+    # no french translation provided -> english fallback
+    set(exp_output_fr "${exp_output_en}")
+    foreach(exec TR_TEST TR_THREAD_TEST)
+        foreach(lang en en_GB fr)
+            execute_process(
+                COMMAND "${CMAKE_COMMAND}" -E env "XDG_DATA_DIRS=${ACTUAL_TREE}/share"
+                    LC_ALL=${lang} "${${exec}_EXEC}"
+                OUTPUT_VARIABLE output
+            )
+            string(STRIP "${output}" stripped_output)
+            if(NOT stripped_output STREQUAL exp_output_${lang})
+                message(WARNING "${exec}[${lang}] output was \"${stripped_output}\", but expected \"${exp_output_${lang}}\"")
+                set(fail ON)
+            else()
+                message(STATUS "${exec}[${lang}] output was \"${stripped_output}\", as expected")
+            endif()
+        endforeach()
+    endforeach()
 endif()
 
 if (fail)
diff --git a/tests/ECMPoQmToolsTest/check_conf.cmake.in b/tests/ECMPoQmToolsTest/check_conf.cmake.in
new file mode 100644
index 00000000..0bbab1d9
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/check_conf.cmake.in
@@ -0,0 +1,2 @@
+set(TR_TEST_EXEC "$<TARGET_FILE:tr_test>")
+set(TR_THREAD_TEST_EXEC "$<TARGET_FILE:tr_thread_test>")
diff --git a/tests/ECMPoQmToolsTest/tr_test-po/en/catalog.po b/tests/ECMPoQmToolsTest/tr_test-po/en/catalog.po
new file mode 100644
index 00000000..2a7b6d28
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_test-po/en/catalog.po
@@ -0,0 +1,22 @@
+msgid ""
+msgstr ""
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"X-Language: en\n"
+"X-Qt-Contexts: true\n"
+
+#: main.cpp:12
+msgctxt "testcontext|"
+msgid "test string"
+msgstr "english text"
+
+#: main.cpp:13
+#, qt-format
+#| msgid "test plural"
+msgctxt "testcontext|"
+msgid "test plural %n"
+msgid_plural "test plural %n"
+msgstr[0] "english singular form %n"
+msgstr[1] "english plural form %n"
diff --git a/tests/ECMPoQmToolsTest/tr_test-po/en_GB/catalog.po b/tests/ECMPoQmToolsTest/tr_test-po/en_GB/catalog.po
new file mode 100644
index 00000000..ec5ad857
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_test-po/en_GB/catalog.po
@@ -0,0 +1,22 @@
+msgid ""
+msgstr ""
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"X-Language: en_GB\n"
+"X-Qt-Contexts: true\n"
+
+#: main.cpp:12
+msgctxt "testcontext|"
+msgid "test string"
+msgstr "british english text"
+
+#: main.cpp:13
+#, qt-format
+#| msgid "test plural"
+msgctxt "testcontext|"
+msgid "test plural %n"
+msgid_plural "test plural %n"
+msgstr[0] "british english singular form %n"
+msgstr[1] "british english plural form %n"
diff --git a/tests/ECMPoQmToolsTest/tr_test.cpp b/tests/ECMPoQmToolsTest/tr_test.cpp
new file mode 100644
index 00000000..be5d3427
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_test.cpp
@@ -0,0 +1,16 @@
+#include <QCoreApplication>
+#include <QTextStream>
+
+#include <stdio.h>
+
+int main(int argc, char** argv)
+{
+    QCoreApplication app(argc, argv);
+
+    QTextStream output(stdout);
+
+    output << QCoreApplication::translate("testcontext", "test string") << ":";
+    output << QCoreApplication::translate("testcontext", "test plural %n", 0, 5) << '\n';
+
+    return 0;
+}
diff --git a/tests/ECMPoQmToolsTest/tr_thread_test.cpp b/tests/ECMPoQmToolsTest/tr_thread_test.cpp
new file mode 100644
index 00000000..3ed30ee1
--- /dev/null
+++ b/tests/ECMPoQmToolsTest/tr_thread_test.cpp
@@ -0,0 +1,68 @@
+#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()
+    {
+        // NB: this will run on the *main* event loop.
+        QFunctionPointer print_strings = m_lib->resolve("print_strings");
+        if (print_strings) {
+            print_strings();
+        } else {
+            qFatal("Could not resolve print_strings: %s", m_lib->errorString().toUtf8().data());
+        }
+
+        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';
+}