From 704c3cead9ab7c16c1e0673d42c37b4576ae4844 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Fri, 15 Mar 2019 11:02:02 +0100
Subject: Use MessagePumpForUIQt for UI thread only

In what almost seems like intentionally confusing terminology, Chromium draws a
distinction between the UI thread and UI type threads. The thread's type refers
mainly to the MessagePump implementation that it uses: currently
MessagePumpForUIQt for all UI type threads.

It turns out however that the desktop capture thread on macOS requires the
original MessagePumpMac implementation for some macOS specifics. So, with this
patch, MessagePumpForUIQt will be used only for the actual UI thread, and all
other UI type threads will use upstream message pump implementations.

Theoretically, this means that we cannot send events or async signals to these
other UI type threads any more (sending events *from* these threads should still
work). Practically though it seems safer to try, as far as possible, to not mix
different event/task frameworks on the same thread.

Change-Id: I81308d62c64354230796fccce2d3e0fa6cbb5013
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
---
 src/core/browser_main_parts_qt.cpp | 49 ++++++++++++++++++++++----------------
 src/core/browser_main_parts_qt.h   |  5 ++--
 2 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/core/browser_main_parts_qt.cpp b/src/core/browser_main_parts_qt.cpp
index 7e1617802..dacaf177f 100644
--- a/src/core/browser_main_parts_qt.cpp
+++ b/src/core/browser_main_parts_qt.cpp
@@ -67,7 +67,6 @@
 #include "service/service_qt.h"
 #include "web_engine_context.h"
 
-#include <QEventLoop>
 #include <QtGui/qtgui-config.h>
 
 #if QT_CONFIG(opengl)
@@ -115,36 +114,31 @@ public:
         : m_scheduler([this]() { handleScheduledWork(); })
     {}
 
+    void setDelegate(Delegate *delegate)
+    {
+        m_delegate = delegate;
+    }
+
     void Run(Delegate *delegate) override
     {
-        if (!m_delegate)
-            m_delegate = delegate;
-        else
-            Q_ASSERT(delegate == m_delegate);
         // This is used only when MessagePumpForUIQt is used outside of the GUI thread.
-        QEventLoop loop;
-        m_explicitLoop = &loop;
-        loop.exec();
-        m_explicitLoop = nullptr;
+        NOTIMPLEMENTED();
     }
 
     void Quit() override
     {
-        Q_ASSERT(m_explicitLoop);
-        m_explicitLoop->quit();
+        // This is used only when MessagePumpForUIQt is used outside of the GUI thread.
+        NOTIMPLEMENTED();
     }
 
     void ScheduleWork() override
     {
         // NOTE: This method may called from any thread at any time.
-        if (!m_delegate)
-            m_delegate = static_cast<base::MessageLoopImpl*>(base::MessageLoopCurrentForUI::Get().ToMessageLoopBaseDeprecated());
         m_scheduler.scheduleWork();
     }
 
     void ScheduleDelayedWork(const base::TimeTicks &delayed_work_time) override
     {
-        Q_ASSERT(m_delegate);
         m_scheduler.scheduleDelayedWork(GetTimeIntervalMilliseconds(delayed_work_time));
     }
 
@@ -196,6 +190,8 @@ private:
 
     void handleScheduledWork()
     {
+        Q_ASSERT(m_delegate);
+
         ScopedGLContextChecker glContextChecker;
 
         bool more_work_is_plausible = m_delegate->DoWork();
@@ -214,14 +210,26 @@ private:
     }
 
     Delegate *m_delegate = nullptr;
-    QEventLoop *m_explicitLoop = nullptr;
     QWebEngineMessagePumpScheduler m_scheduler;
 };
 
-std::unique_ptr<base::MessagePump> messagePumpFactory()
-{
-    return base::WrapUnique(new MessagePumpForUIQt);
-}
+// Needed to access protected constructor from MessageLoop.
+class MessageLoopForUIQt : public base::MessageLoop {
+public:
+    MessageLoopForUIQt() : MessageLoop(TYPE_UI, base::BindOnce(&messagePumpFactory))
+    {
+        BindToCurrentThread();
+
+        auto pump = static_cast<MessagePumpForUIQt *>(pump_);
+        auto backend = static_cast<base::MessageLoopImpl *>(backend_.get());
+        pump->setDelegate(backend);
+    }
+private:
+    static std::unique_ptr<base::MessagePump> messagePumpFactory()
+    {
+        return base::WrapUnique(new MessagePumpForUIQt);
+    }
+};
 
 BrowserMainPartsQt::BrowserMainPartsQt() : content::BrowserMainParts()
 { }
@@ -234,12 +242,13 @@ int BrowserMainPartsQt::PreEarlyInitialization()
 #if BUILDFLAG(ENABLE_EXTENSIONS)
     content::ChildProcessSecurityPolicy::GetInstance()->RegisterWebSafeScheme(extensions::kExtensionScheme);
 #endif //ENABLE_EXTENSIONS
-    base::MessageLoop::InitMessagePumpForUIFactory(messagePumpFactory);
     return 0;
 }
 
 void BrowserMainPartsQt::PreMainMessageLoopStart()
 {
+    // Overrides message loop creation in BrowserMainLoop::MainMessageLoopStart().
+    m_mainMessageLoop.reset(new MessageLoopForUIQt);
 }
 
 void BrowserMainPartsQt::PreMainMessageLoopRun()
diff --git a/src/core/browser_main_parts_qt.h b/src/core/browser_main_parts_qt.h
index 374220c3b..4eb10e379 100644
--- a/src/core/browser_main_parts_qt.h
+++ b/src/core/browser_main_parts_qt.h
@@ -43,7 +43,7 @@
 #include "content/public/browser/browser_main_parts.h"
 
 namespace base {
-class MessagePump;
+class MessageLoop;
 }
 
 namespace content {
@@ -56,8 +56,6 @@ class ProcessResourceCoordinator;
 
 namespace QtWebEngineCore {
 
-std::unique_ptr<base::MessagePump> messagePumpFactory();
-
 class BrowserMainPartsQt : public content::BrowserMainParts
 {
 public:
@@ -74,6 +72,7 @@ public:
 private:
     DISALLOW_COPY_AND_ASSIGN(BrowserMainPartsQt);
     std::unique_ptr<resource_coordinator::ProcessResourceCoordinator> m_processResourceCoordinator;
+    std::unique_ptr<base::MessageLoop> m_mainMessageLoop;
 };
 
 } // namespace QtWebEngineCore
-- 
cgit v1.2.3