mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix: SCContentSharingPicker crash in iframes (#44228)

Browse source 
fix: SCContentSharingPicker crash in iframes (#43905)

When running `getDisplayMedia` from an `iframe` and then destroying that
`iframe`, the destrutcor for ScreenCaptureKitDeviceMac gets run, but it
doesn't clean up the observers for the shared macOS picker.

In this change we remove the observer on cleanup and fix the associated
crash:

    *** Terminating app due to uncaught exception 'NSGenericException', reason: '*** Collection <__NSArrayM: 0x12c024123a0> was mutated while being enumerated.'
     0   CoreFoundation                      0x000000018eda0ec0 __exceptionPreprocess + 176
     1   libobjc.A.dylib                     0x000000018e886cd8 objc_exception_throw + 88
     2   CoreFoundation                      0x000000018ee47f80 -[__NSSingleObjectEnumerator init] + 0
     3   ScreenCaptureKit                    0x000000022971f3e0 -[SCContentSharingPicker contentPickerDidSelectFilter:forStream:] + 436
     4   ReplayKit                           0x00000001d7b879d8 -[RPScreenRecorder contentPickerDidSelectFilter:forStream:] + 84
     5   ReplayKit                           0x00000001d7b95ce0 -[RPDaemonProxy contentPickerDidSelectFilter:forStream:] + 224
     6   CoreFoundation                      0x000000018ed0e8b4 __invoking___ + 148
     7   CoreFoundation                      0x000000018ed0e72c -[NSInvocation invoke] + 428
     8   ReplayKit                           0x00000001d7b91678 -[RPDaemonProxy connection:handleInvocation:isReply:] + 316
     9   Foundation                          0x0000000190899f64 -[NSXPCConnection _decodeAndInvokeMessageWithEvent:reply:flags:] + 1108
     10  Foundation                          0x000000019089b7d0 message_handler_message + 88
     11  Foundation                          0x000000018feb0160 message_handler + 152
     12  libxpc.dylib                        0x000000018e954460 _xpc_connection_call_event_handler + 144
     13  libxpc.dylib                        0x000000018e952bb4 _xpc_connection_mach_event + 1120
     14  libdispatch.dylib                   0x000000018ea94718 _dispatch_client_callout4 + 20
     15  libdispatch.dylib                   0x000000018eab0c00 _dispatch_mach_msg_invoke + 464
     16  libdispatch.dylib                   0x000000018ea9bae8 _dispatch_lane_serial_drain + 368
     17  libdispatch.dylib                   0x000000018eab1954 _dispatch_mach_invoke + 456
     18  libdispatch.dylib                   0x000000018ea9bae8 _dispatch_lane_serial_drain + 368
     19  libdispatch.dylib                   0x000000018ea9c79c _dispatch_lane_invoke + 432
    20  libdispatch.dylib                   0x000000018eaa77e8 _dispatch_root_queue_drain_deferred_wlh + 288
    21  libdispatch.dylib                   0x000000018eaa7034 _dispatch_workloop_worker_thread + 540
    22  libsystem_pthread.dylib             0x000000018ec433d8 _pthread_wqthread + 288
    23  libsystem_pthread.dylib             0x000000018ec420f0 start_wqthread + 8

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
This commit is contained in:
Keeley Hammond 2024-10-14 10:51:20 -07:00 • committed by GitHub
parent dd5bce17cb
commit 63d1d9cb65
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 59 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
patches/chromium/.patches
View file

@ -130,4 +130,3 @@ chore_remove_reference_to_chrome_browser_themes.patch
feat_enable_customizing_symbol_color_in_framecaptionbutton.patch feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
build_expose_webplugininfo_interface_to_electron.patch build_expose_webplugininfo_interface_to_electron.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

63
patches/chromium/feat_allow_-4_as_a_macos_screen_share_id.patch
View file

@ -1,63 +0,0 @@
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 1c2d0c6dd4101fe0bac69e3018bbbedadce224cc..e407382a3463374fd57a83d70c5f96dca5825faf 100644
--- a/content/browser/media/capture/screen_capture_kit_device_mac.mm
+++ b/content/browser/media/capture/screen_capture_kit_device_mac.mm
@@ -510,7 +510,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 d162612dc70a2b57190aaf558aca8f46cbdedcad..bab2f0282b191a4263fc964125e199e52c62554b 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
@@ -332,8 +332,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.

73
patches/chromium/feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
View file

@ -46,7 +46,7 @@ index 8ac12480f663a74dfbdcf7128a582a81b4474d25..db6802a2603e1d3c3039e49737438124
// OnStop is called by StopAndDeAllocate. // OnStop is called by StopAndDeAllocate.
virtual void OnStop() = 0; virtual void OnStop() = 0;
diff --git a/content/browser/media/capture/screen_capture_kit_device_mac.mm b/content/browser/media/capture/screen_capture_kit_device_mac.mm 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 b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbedadce224cc 100644 index b6129282c6807702cf88e0a3e2ba233e41a20960..87a00302de8db47299b185471e303b9e172e9c76 100644
--- a/content/browser/media/capture/screen_capture_kit_device_mac.mm --- a/content/browser/media/capture/screen_capture_kit_device_mac.mm
+++ b/content/browser/media/capture/screen_capture_kit_device_mac.mm +++ b/content/browser/media/capture/screen_capture_kit_device_mac.mm
@@ -24,24 +24,83 @@ @@ -24,24 +24,83 @@
@ -143,7 +143,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
public: public:
explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source, explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source,
SCContentFilter* filter) SCContentFilter* filter)
@@ -152,18 +212,41 @@ explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source, @@ -152,18 +212,42 @@ explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source,
device_task_runner_, device_task_runner_,
base::BindRepeating(&ScreenCaptureKitDeviceMac::OnStreamSample, base::BindRepeating(&ScreenCaptureKitDeviceMac::OnStreamSample,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
@ -165,8 +165,8 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
+ device_task_runner_, + device_task_runner_,
+ base::BindRepeating(&ScreenCaptureKitDeviceMac::OnContentFilterReady, weak_factory_.GetWeakPtr()) + base::BindRepeating(&ScreenCaptureKitDeviceMac::OnContentFilterReady, weak_factory_.GetWeakPtr())
+ ); + );
+ auto* picker_observer = [[ScreenCaptureKitPickerHelper alloc] initWithStreamPickCallback:picker_callback cancelCallback:cancel_callback errorCallback:error_callback]; + picker_helper_ = [[ScreenCaptureKitPickerHelper alloc] initWithStreamPickCallback:picker_callback cancelCallback:cancel_callback errorCallback:error_callback];
+ [[SCContentSharingPicker sharedPicker] addObserver:picker_observer]; + [[SCContentSharingPicker sharedPicker] addObserver:picker_helper_];
+ } + }
} }
ScreenCaptureKitDeviceMac(const ScreenCaptureKitDeviceMac&) = delete; ScreenCaptureKitDeviceMac(const ScreenCaptureKitDeviceMac&) = delete;
@ -180,13 +180,14 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
+ picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_); + picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_);
+ if (ScreenCaptureKitDeviceMac::active_streams_ == 0 && picker.active) { + if (ScreenCaptureKitDeviceMac::active_streams_ == 0 && picker.active) {
+ picker.active = false; + picker.active = false;
+ [[SCContentSharingPicker sharedPicker] removeObserver:picker_helper_];
+ } + }
+ } + }
+ } + }
void OnShareableContentCreated(SCShareableContent* content) { void OnShareableContentCreated(SCShareableContent* content) {
DCHECK(device_task_runner_->RunsTasksInCurrentSequence()); DCHECK(device_task_runner_->RunsTasksInCurrentSequence());
@@ -232,7 +315,7 @@ void CreateStream(SCContentFilter* filter) { @@ -232,7 +316,7 @@ void CreateStream(SCContentFilter* filter) {
return; return;
} }
@ -195,7 +196,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
// Update the content size. This step is neccessary when used together // Update the content size. This step is neccessary when used together
// with SCContentSharingPicker. If the Chrome picker is used, it will // with SCContentSharingPicker. If the Chrome picker is used, it will
// change to retina resolution if applicable. // change to retina resolution if applicable.
@@ -241,6 +324,9 @@ void CreateStream(SCContentFilter* filter) { @@ -241,6 +325,9 @@ void CreateStream(SCContentFilter* filter) {
filter.contentRect.size.height * filter.pointPixelScale); filter.contentRect.size.height * filter.pointPixelScale);
} }
@ -205,7 +206,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
gfx::RectF dest_rect_in_frame; gfx::RectF dest_rect_in_frame;
actual_capture_format_ = capture_params().requested_format; actual_capture_format_ = capture_params().requested_format;
actual_capture_format_.pixel_format = media::PIXEL_FORMAT_NV12; actual_capture_format_.pixel_format = media::PIXEL_FORMAT_NV12;
@@ -254,6 +340,7 @@ void CreateStream(SCContentFilter* filter) { @@ -254,6 +341,7 @@ void CreateStream(SCContentFilter* filter) {
stream_ = [[SCStream alloc] initWithFilter:filter stream_ = [[SCStream alloc] initWithFilter:filter
configuration:config configuration:config
delegate:helper_]; delegate:helper_];
@ -213,7 +214,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
{ {
NSError* error = nil; NSError* error = nil;
bool add_stream_output_result = bool add_stream_output_result =
@@ -395,7 +482,7 @@ void OnStreamError() { @@ -395,7 +483,7 @@ void OnStreamError() {
if (fullscreen_module_) { if (fullscreen_module_) {
fullscreen_module_->Reset(); fullscreen_module_->Reset();
} }
@ -222,7 +223,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
} else { } else {
client()->OnError(media::VideoCaptureError::kScreenCaptureKitStreamError, client()->OnError(media::VideoCaptureError::kScreenCaptureKitStreamError,
FROM_HERE, "Stream delegate called didStopWithError"); FROM_HERE, "Stream delegate called didStopWithError");
@@ -418,23 +505,39 @@ void OnUpdateConfigurationError() { @@ -418,23 +506,41 @@ void OnUpdateConfigurationError() {
} }
// IOSurfaceCaptureDeviceBase: // IOSurfaceCaptureDeviceBase:
@ -246,7 +247,9 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
+ +
+ if (@available(macOS 15.0, *)) { + if (@available(macOS 15.0, *)) {
+ constexpr bool DefaultUseNativePicker = true; + 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]; + auto* picker = [SCContentSharingPicker sharedPicker];
+ ScreenCaptureKitDeviceMac::active_streams_++; + ScreenCaptureKitDeviceMac::active_streams_++;
+ picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_); + picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_);
@ -277,7 +280,7 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
} }
void OnStop() override { void OnStop() override {
DCHECK(device_task_runner_->RunsTasksInCurrentSequence()); DCHECK(device_task_runner_->RunsTasksInCurrentSequence());
@@ -492,6 +595,8 @@ void ResetStreamTo(SCWindow* window) override { @@ -492,6 +598,8 @@ void ResetStreamTo(SCWindow* window) override {
} }
private: private:
@ -286,7 +289,18 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
const DesktopMediaID source_; const DesktopMediaID source_;
SCContentFilter* const filter_; SCContentFilter* const filter_;
const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_; const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
@@ -521,6 +626,8 @@ void ResetStreamTo(SCWindow* window) override { @@ -509,6 +617,10 @@ void ResetStreamTo(SCWindow* window) override {
// Helper class that acts as output and delegate for `stream_`.
ScreenCaptureKitDeviceHelper* __strong helper_;
+ // Helper class that acts as an observer for SCContentSharingPicker
+ API_AVAILABLE(macos(15.0))
+ ScreenCaptureKitPickerHelper* __strong picker_helper_;
+
// This is used to detect when a captured presentation enters fullscreen mode.
// If this happens, the module will call the ResetStreamTo function.
std::unique_ptr<ScreenCaptureKitFullscreenModule> fullscreen_module_;
@@ -521,6 +633,8 @@ void ResetStreamTo(SCWindow* window) override {
base::WeakPtrFactory<ScreenCaptureKitDeviceMac> weak_factory_{this}; base::WeakPtrFactory<ScreenCaptureKitDeviceMac> weak_factory_{this};
}; };
@ -296,10 +310,28 @@ index b6129282c6807702cf88e0a3e2ba233e41a20960..1c2d0c6dd4101fe0bac69e3018bbbeda
// Although ScreenCaptureKit is available in 12.3 there were some bugs that // Although ScreenCaptureKit is available in 12.3 there were some bugs that
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 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 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..d162612dc70a2b57190aaf558aca8f46cbdedcad 100644 index 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..bab2f0282b191a4263fc964125e199e52c62554b 100644
--- a/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc --- 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 +++ b/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
@@ -360,13 +360,15 @@ void InProcessVideoCaptureDeviceLauncher::LaunchDeviceAsync( @@ -332,8 +332,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.
@@ -360,13 +368,15 @@ void InProcessVideoCaptureDeviceLauncher::LaunchDeviceAsync(
std::move(after_start_capture_callback)); std::move(after_start_capture_callback));
break; break;
#else #else
@ -316,6 +348,19 @@ index 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..d162612dc70a2b57190aaf558aca8f46
CreateDeviceClient(media::VideoCaptureBufferType::kSharedMemory, CreateDeviceClient(media::VideoCaptureBufferType::kSharedMemory,
kMaxNumberOfBuffers, std::move(receiver), kMaxNumberOfBuffers, std::move(receiver),
std::move(receiver_on_io_thread)), std::move(receiver_on_io_thread)),
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.
diff --git a/media/capture/video_capture_types.h b/media/capture/video_capture_types.h diff --git a/media/capture/video_capture_types.h b/media/capture/video_capture_types.h
index f2b75f5b2f547ad135c1288bf3639b26dedc8053..ef18724d9f2ea68a47b66fc3981f58a73ac1b51d 100644 index f2b75f5b2f547ad135c1288bf3639b26dedc8053..ef18724d9f2ea68a47b66fc3981f58a73ac1b51d 100644
--- a/media/capture/video_capture_types.h --- a/media/capture/video_capture_types.h