From f21a48ff36a1288a592522e575a7f71fe44b35f9 Mon Sep 17 00:00:00 2001
From: jp9000 <obs.jim@gmail.com>
Date: Wed, 1 May 2019 12:13:35 -0700
Subject: [PATCH] UI: Remove mac browser workarounds, improve stability

The workarounds were made because of conflicts with running multiple UI
threads at once on macOS, which macOS can't do very well, and would be
susceptible to crashes.  This would cause crashes not only on startup
but seemingly at random when using the browser source on macOS.  The
original "fix" was a hack to try to minimize UI code and browser UI code
from executing at the same time.  The macOS initial scene loading was
deferred until all Qt-related and main window initialization was
completed.  Although this worked to some extent to prevent conflicts, it
made it so that there was an initial period on startup where the entire
UI seemed "blank" for users, and it was still possible for the main UI
thread and the browser UI thread to clash, causing crashes seemingly at
random for users.

The external message pump method of CEF is the solution to the problem,
which is the method which allows the main UI thread to share events with
CEF.  To do this, all CEF operations need to be performed in the UI
thread (Qt's main thread), and CefDoMessageLoopWork() needs to be called
when CefApp::OnScheduleMessagePumpWork callback is triggered.  A number
of other issues had to be solved as well, such as CefBrowser references
getting "stuck" in the Qt event queue.

With this, macOS no longer needs to do the "deferred load" hack, and
browsers are now much more stable and no longer as susceptible to
seemingly random crashes, improving overall program stability when
browsers are used.
---
 UI/CMakeLists.txt        |  2 +-
 UI/window-basic-main.cpp | 35 +++++------------------------------
 UI/window-basic-main.hpp |  2 +-
 plugins/obs-browser      |  2 +-
 4 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/UI/CMakeLists.txt b/UI/CMakeLists.txt
index 8270baba..ac541105 100644
--- a/UI/CMakeLists.txt
+++ b/UI/CMakeLists.txt
@@ -7,7 +7,7 @@ else()
 	set(FIND_MODE QUIET)
 endif()
 
-if(BROWSER_AVAILABLE_INTERNAL AND WIN32)
+if(BROWSER_AVAILABLE_INTERNAL)
 	add_definitions(-DBROWSER_AVAILABLE)
 endif()
 
diff --git a/UI/window-basic-main.cpp b/UI/window-basic-main.cpp
index a53ff328..10158782 100644
--- a/UI/window-basic-main.cpp
+++ b/UI/window-basic-main.cpp
@@ -1640,14 +1640,12 @@ void OBSBasic::OBSInit()
 	SET_VISIBILITY("ShowStatusBar", toggleStatusBar);
 #undef SET_VISIBILITY
 
-#ifndef __APPLE__
 	{
 		ProfileScope("OBSBasic::Load");
 		disableSaving--;
 		Load(savePath);
 		disableSaving++;
 	}
-#endif
 
 	TimedCheckForUpdates();
 	loaded = true;
@@ -1669,9 +1667,7 @@ void OBSBasic::OBSInit()
 	}
 #endif
 
-#ifndef __APPLE__
 	RefreshSceneCollections();
-#endif
 	RefreshProfiles();
 	disableSaving--;
 
@@ -1809,28 +1805,12 @@ void OBSBasic::OBSInit()
 	ui->actionCheckForUpdates = nullptr;
 #endif
 
+	OnFirstLoad();
+
 #ifdef __APPLE__
-	/* This is an incredibly unpleasant hack for macOS to isolate CEF
-	 * initialization until after all tasks related to Qt startup and main
-	 * window initialization have completed.  There is a macOS-specific bug
-	 * within either CEF and/or Qt that can cause a crash if both Qt and
-	 * CEF are loading at the same time.
-	 *
-	 * CEF will typically load fine after about two iterations from this
-	 * point, and all Qt tasks are typically fully completed after about
-	 * four or five iterations, but to be "ultra" safe, an arbitrarily
-	 * large number such as 10 is used.  This hack is extremely unpleasant,
-	 * but is worth doing instead of being forced to isolate the entire
-	 * browser plugin in to a separate process as before.
-	 *
-	 * Again, this hack is specific to macOS only.  Fortunately, on other
-	 * operating systems, such issues do not occur. */
-	QMetaObject::invokeMethod(this, "DeferredLoad",
+	QMetaObject::invokeMethod(this, "DeferredSysTrayLoad",
 			Qt::QueuedConnection,
-			Q_ARG(QString, QT_UTF8(savePath)),
 			Q_ARG(int, 10));
-#else
-	OnFirstLoad();
 #endif
 }
 
@@ -1857,20 +1837,15 @@ void OBSBasic::OnFirstLoad()
 	Auth::Load();
 }
 
-void OBSBasic::DeferredLoad(const QString &file, int requeueCount)
+void OBSBasic::DeferredSysTrayLoad(int requeueCount)
 {
 	if (--requeueCount > 0) {
-		QMetaObject::invokeMethod(this, "DeferredLoad",
+		QMetaObject::invokeMethod(this, "DeferredSysTrayLoad",
 				Qt::QueuedConnection,
-				Q_ARG(QString, file),
 				Q_ARG(int, requeueCount));
 		return;
 	}
 
-	Load(QT_TO_UTF8(file));
-	RefreshSceneCollections();
-	OnFirstLoad();
-
 	/* Minimizng to tray on initial startup does not work on mac
 	 * unless it is done in the deferred load */
 	SystemTray(true);
diff --git a/UI/window-basic-main.hpp b/UI/window-basic-main.hpp
index 120408dc..d624dcc8 100644
--- a/UI/window-basic-main.hpp
+++ b/UI/window-basic-main.hpp
@@ -796,7 +796,7 @@ private slots:
 	void OpenMultiviewWindow();
 	void OpenSceneWindow();
 
-	void DeferredLoad(const QString &file, int requeueCount);
+	void DeferredSysTrayLoad(int requeueCount);
 
 	void StackedMixerAreaContextMenuRequested();
 
diff --git a/plugins/obs-browser b/plugins/obs-browser
index f94f9096..c1123b98 160000
--- a/plugins/obs-browser
+++ b/plugins/obs-browser
@@ -1 +1 @@
-Subproject commit f94f909690b2acc57d71ed767bd527d86321dad3
+Subproject commit c1123b9806df9bf605f5b14054c5fb6279dff45e
-- 
GitLab