fix: third time screen sharing on macOS (#43767)

Because we used decrementing negative source ids for fake video id when
instantating a native macOS screen share picker, we eventually hit the
`DesktopMediaID::kFakeId = -3` in Chromium source code which displayed a
test green screen.

In this change we reserve our own fake id of `-4` and decrement the
window id integer for uniqueness instead.

Co-authored-by: Fedor Indutny <238531+indutny@users.noreply.github.com>
This commit is contained in:
Fedor Indutny 2024-09-19 18:28:28 -07:00 committed by GitHub
parent d100921289
commit 6aa6bada79
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 69 additions and 3 deletions

View file

@ -3,14 +3,16 @@ import { net } from 'electron/main';
const { fromPartition, fromPath, Session } = process._linkedBinding('electron_browser_session'); const { fromPartition, fromPath, Session } = process._linkedBinding('electron_browser_session');
const { isDisplayMediaSystemPickerAvailable } = process._linkedBinding('electron_browser_desktop_capturer'); const { isDisplayMediaSystemPickerAvailable } = process._linkedBinding('electron_browser_desktop_capturer');
// Fake video source that activates the native system picker // Fake video window that activates the native system picker
// This is used to get around the need for a screen/window // This is used to get around the need for a screen/window
// id in Chrome's desktopCapturer. // id in Chrome's desktopCapturer.
let fakeVideoSourceId = -1; let fakeVideoWindowId = -1;
// See content/public/browser/desktop_media_id.h
const kMacOsNativePickerId = -4;
const systemPickerVideoSource = Object.create(null); const systemPickerVideoSource = Object.create(null);
Object.defineProperty(systemPickerVideoSource, 'id', { Object.defineProperty(systemPickerVideoSource, 'id', {
get () { get () {
return `window:${fakeVideoSourceId--}:0`; return `window:${kMacOsNativePickerId}:${fakeVideoWindowId--}`;
} }
}); });
systemPickerVideoSource.name = ''; systemPickerVideoSource.name = '';

View file

@ -131,3 +131,4 @@ feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
build_expose_webplugininfo_interface_to_electron.patch build_expose_webplugininfo_interface_to_electron.patch
osr_shared_texture_remove_keyed_mutex_on_win_dxgi.patch osr_shared_texture_remove_keyed_mutex_on_win_dxgi.patch
feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
feat_allow_-4_as_a_macos_screen_share_id.patch

View file

@ -0,0 +1,63 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <indutny@signal.org>
Date: Tue, 17 Sep 2024 17:51:46 -0700
Subject: feat: allow -4 as a macos screen share id
We use fake video source ids for native macOS screen share window picker
of the following form:
window:-4:-1
Where the last digit represents the window id and decrements with each
requested screen share.
diff --git a/content/browser/media/capture/screen_capture_kit_device_mac.mm b/content/browser/media/capture/screen_capture_kit_device_mac.mm
index 27b7edd2e99f36ebf3381781f2d2b3e7aff3eca1..30b3c896d5d6f12d63a7e12df0c90c767a5d5a71 100644
--- a/content/browser/media/capture/screen_capture_kit_device_mac.mm
+++ b/content/browser/media/capture/screen_capture_kit_device_mac.mm
@@ -503,7 +503,9 @@ void OnStart(std::optional<bool> use_native_picker) override {
if (@available(macOS 15.0, *)) {
constexpr bool DefaultUseNativePicker = true;
- if (use_native_picker.value_or(DefaultUseNativePicker) && source_.id < 0 && source_.window_id == 0) {
+ if (use_native_picker.value_or(DefaultUseNativePicker) &&
+ source_.id == DesktopMediaID::kMacOsNativePickerId &&
+ source_.window_id < 0) {
auto* picker = [SCContentSharingPicker sharedPicker];
ScreenCaptureKitDeviceMac::active_streams_++;
picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_);
diff --git a/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc b/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
index f38ea5df3b6c694aed3a54486733130a2bec606b..f34ea831e3f0988b85940b11ca5484069f3013cb 100644
--- a/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
+++ b/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
@@ -316,8 +316,16 @@ void InProcessVideoCaptureDeviceLauncher::LaunchDeviceAsync(
break;
}
+#if defined(USE_AURA)
+ bool allow_window_id = false;
+#elif BUILDFLAG(IS_MAC)
+ bool allow_window_id =
+ desktop_id.id == DesktopMediaID::kMacOsNativePickerId;
+#endif
+
#if defined(USE_AURA) || BUILDFLAG(IS_MAC)
- if (desktop_id.window_id != DesktopMediaID::kNullId) {
+ if (!allow_window_id &&
+ desktop_id.window_id != DesktopMediaID::kNullId) {
// For the other capturers, when a bug reports the type of capture it's
// easy enough to determine which capturer was used, but it's a little
// fuzzier with window capture.
diff --git a/content/public/browser/desktop_media_id.h b/content/public/browser/desktop_media_id.h
index 415156d403a59bf426cf4561a9d58ecdb27524b4..78aa7b2359c684d5305bf6352751dfbb7ca00d29 100644
--- a/content/public/browser/desktop_media_id.h
+++ b/content/public/browser/desktop_media_id.h
@@ -27,6 +27,8 @@ struct CONTENT_EXPORT DesktopMediaID {
static constexpr Id kNullId = 0;
// Represents a fake id to create a dummy capturer for autotests.
static constexpr Id kFakeId = -3;
+ // Represents an id to use native macOS picker for screenshare
+ static constexpr Id kMacOsNativePickerId = -4;
#if defined(USE_AURA) || BUILDFLAG(IS_MAC)
// Assigns integer identifier to the |window| and returns its DesktopMediaID.