From 410c22978c2add5f9427af08d97356d6e959e412 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Tue, 7 Aug 2018 09:15:53 +0200
Subject: [PATCH] MediaCaptureDevicesDispatcher: Disable getDefaultScreenId on
X11
Not needed and triggers race condition.
Task-number: QTBUG-69007
Change-Id: Id57ba527387e5dbe44a8dd6c5a49e7278403ce64
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
src/core/media_capture_devices_dispatcher.cpp | 11 ++++-
.../qwebenginepage/tst_qwebenginepage.cpp | 40 +++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/core/media_capture_devices_dispatcher.cpp b/src/core/media_capture_devices_dispatcher.cpp
index 87fe543b1..7f521dd10 100644
--- a/src/core/media_capture_devices_dispatcher.cpp
+++ b/src/core/media_capture_devices_dispatcher.cpp
@@ -123,7 +123,16 @@ std::unique_ptr<content::MediaStreamUI> getDevicesForDesktopCapture(content::Med
content::DesktopMediaID getDefaultScreenId()
{
-#if BUILDFLAG(ENABLE_WEBRTC)
+ // While this function is executing another thread may also want to create a
+ // DesktopCapturer [1]. Unfortunately, creating a DesktopCapturer is not
+ // thread safe on X11 due to the use of webrtc::XErrorTrap. It's safe to
+ // disable this code on X11 since we don't actually need to create a
+ // DesktopCapturer to get the screen id anyway
+ // (ScreenCapturerLinux::GetSourceList always returns 0 as the id).
+ //
+ // [1]: webrtc::InProcessVideoCaptureDeviceLauncher::DoStartDesktopCaptureOnDeviceThread
+
+#if BUILDFLAG(ENABLE_WEBRTC) && !defined(USE_X11)
// Source id patterns are different across platforms.
// On Linux, the hardcoded value "0" is used.
// On Windows, the screens are enumerated consecutively in increasing order from 0.
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index 47a94374a..7bdad1bda 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -125,6 +125,8 @@ private Q_SLOTS:
void getUserMediaRequest();
void getUserMediaRequestDesktopAudio();
void getUserMediaRequestSettingDisabled();
+ void getUserMediaRequestDesktopVideoManyPages();
+ void getUserMediaRequestDesktopVideoManyRequests();
void savePage();
void crashTests_LazyInitializationOfMainFrame();
@@ -2447,6 +2449,44 @@ void tst_QWebEnginePage::getUserMediaRequestSettingDisabled()
QTRY_VERIFY(!page.jsPromiseFulfilled() && page.jsPromiseRejected());
}
+// Try to trigger any possible race condition between the UI thread (permission
+// management) and the audio/device thread (desktop capture initialization).
+void tst_QWebEnginePage::getUserMediaRequestDesktopVideoManyPages()
+{
+ const QString constraints = QStringLiteral("{video: { mandatory: { chromeMediaSource: 'desktop' }}}");
+ const QWebEnginePage::Feature feature = QWebEnginePage::DesktopVideoCapture;
+ std::vector<GetUserMediaTestPage> pages(10);
+ for (GetUserMediaTestPage &page : pages)
+ QTRY_VERIFY_WITH_TIMEOUT(page.loadSucceeded(), 20000);
+ for (GetUserMediaTestPage &page : pages)
+ page.settings()->setAttribute(QWebEngineSettings::ScreenCaptureEnabled, true);
+ for (GetUserMediaTestPage &page : pages)
+ page.jsGetUserMedia(constraints);
+ for (GetUserMediaTestPage &page : pages)
+ QTRY_VERIFY(page.gotFeatureRequest(feature));
+ for (GetUserMediaTestPage &page : pages)
+ page.acceptPendingRequest();
+ for (GetUserMediaTestPage &page : pages)
+ QTRY_VERIFY(page.jsPromiseFulfilled() || page.jsPromiseRejected());
+}
+
+// Try to trigger any possible race condition between the UI or audio/device
+// threads and the desktop capture thread, where the capture actually happens.
+void tst_QWebEnginePage::getUserMediaRequestDesktopVideoManyRequests()
+{
+ const QString constraints = QStringLiteral("{video: { mandatory: { chromeMediaSource: 'desktop' }}}");
+ const QWebEnginePage::Feature feature = QWebEnginePage::DesktopVideoCapture;
+ GetUserMediaTestPage page;
+ QTRY_VERIFY_WITH_TIMEOUT(page.loadSucceeded(), 20000);
+ page.settings()->setAttribute(QWebEngineSettings::ScreenCaptureEnabled, true);
+ for (int i = 0; i != 100; ++i) {
+ page.jsGetUserMedia(constraints);
+ QTRY_VERIFY(page.gotFeatureRequest(feature));
+ page.acceptPendingRequest();
+ QTRY_VERIFY(page.jsPromiseFulfilled() || page.jsPromiseRejected());
+ }
+}
+
void tst_QWebEnginePage::savePage()
{
QWebEngineView view;
--
GitLab