Browse Source

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

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: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Fedor Indutny <[email protected]>
trop[bot] 7 months ago
parent
commit
8ac9d16162

+ 5 - 3
lib/browser/api/session.ts

@@ -3,14 +3,16 @@ import { net } from 'electron/main';
 const { fromPartition, fromPath, Session } = process._linkedBinding('electron_browser_session');
 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
 // 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);
 Object.defineProperty(systemPickerVideoSource, 'id', {
   get () {
-    return `window:${fakeVideoSourceId--}:0`;
+    return `window:${kMacOsNativePickerId}:${fakeVideoWindowId--}`;
   }
 });
 systemPickerVideoSource.name = '';

+ 1 - 0
patches/chromium/.patches

@@ -132,3 +132,4 @@ chore_remove_reference_to_chrome_browser_themes.patch
 feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
 fix_potential_draggable_region_crash_when_no_mainframeimpl.patch
 feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
+feat_allow_-4_as_a_macos_screen_share_id.patch

+ 63 - 0
patches/chromium/feat_allow_-4_as_a_macos_screen_share_id.patch

@@ -0,0 +1,63 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fedor Indutny <[email protected]>
+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.