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
This commit is contained in:
Fedor Indutny
GitHub
parent
ff36ff84b7
commit
15db63e26d
3 changed files with 59 additions and 78 deletions
|
|
@ -131,4 +131,3 @@ feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
|
|||
build_expose_webplugininfo_interface_to_electron.patch
|
||||
osr_shared_texture_remove_keyed_mutex_on_win_dxgi.patch
|
||||
feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
|
||||
feat_allow_-4_as_a_macos_screen_share_id.patch
|
||||
|
|
|
|||
|
|
@ -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 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.
|
||||
|
|
@ -46,7 +46,7 @@ index 8ac12480f663a74dfbdcf7128a582a81b4474d25..db6802a2603e1d3c3039e49737438124
|
|||
// OnStop is called by StopAndDeAllocate.
|
||||
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
|
||||
index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073fa634e6a 100644
|
||||
index 128bca8bb64113418551a20164f67ef5e5a88253..93f86ec79364db1f46c5d2eb015a11cf241d587a 100644
|
||||
--- a/content/browser/media/capture/screen_capture_kit_device_mac.mm
|
||||
+++ b/content/browser/media/capture/screen_capture_kit_device_mac.mm
|
||||
@@ -26,6 +26,61 @@
|
||||
|
|
@ -144,7 +144,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
public:
|
||||
explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source,
|
||||
SCContentFilter* filter)
|
||||
@@ -198,18 +258,41 @@ explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source,
|
||||
@@ -198,18 +258,42 @@ explicit ScreenCaptureKitDeviceMac(const DesktopMediaID& source,
|
||||
device_task_runner_,
|
||||
base::BindRepeating(&ScreenCaptureKitDeviceMac::OnStreamSample,
|
||||
weak_factory_.GetWeakPtr()));
|
||||
|
|
@ -166,8 +166,8 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
+ device_task_runner_,
|
||||
+ base::BindRepeating(&ScreenCaptureKitDeviceMac::OnContentFilterReady, weak_factory_.GetWeakPtr())
|
||||
+ );
|
||||
+ auto* picker_observer = [[ScreenCaptureKitPickerHelper alloc] initWithStreamPickCallback:picker_callback cancelCallback:cancel_callback errorCallback:error_callback];
|
||||
+ [[SCContentSharingPicker sharedPicker] addObserver:picker_observer];
|
||||
+ picker_helper_ = [[ScreenCaptureKitPickerHelper alloc] initWithStreamPickCallback:picker_callback cancelCallback:cancel_callback errorCallback:error_callback];
|
||||
+ [[SCContentSharingPicker sharedPicker] addObserver:picker_helper_];
|
||||
+ }
|
||||
}
|
||||
ScreenCaptureKitDeviceMac(const ScreenCaptureKitDeviceMac&) = delete;
|
||||
|
|
@ -181,13 +181,14 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
+ picker.maximumStreamCount = @(ScreenCaptureKitDeviceMac::active_streams_);
|
||||
+ if (ScreenCaptureKitDeviceMac::active_streams_ == 0 && picker.active) {
|
||||
+ picker.active = false;
|
||||
+ [[SCContentSharingPicker sharedPicker] removeObserver:picker_helper_];
|
||||
+ }
|
||||
+ }
|
||||
+ }
|
||||
|
||||
void OnShareableContentCreated(SCShareableContent* content) {
|
||||
DCHECK(device_task_runner_->RunsTasksInCurrentSequence());
|
||||
@@ -278,7 +361,7 @@ void CreateStream(SCContentFilter* filter) {
|
||||
@@ -278,7 +362,7 @@ void CreateStream(SCContentFilter* filter) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -196,7 +197,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
// Update the content size. This step is neccessary when used together
|
||||
// with SCContentSharingPicker. If the Chrome picker is used, it will
|
||||
// change to retina resolution if applicable.
|
||||
@@ -287,6 +370,9 @@ void CreateStream(SCContentFilter* filter) {
|
||||
@@ -287,6 +371,9 @@ void CreateStream(SCContentFilter* filter) {
|
||||
filter.contentRect.size.height * filter.pointPixelScale);
|
||||
}
|
||||
|
||||
|
|
@ -206,7 +207,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
gfx::RectF dest_rect_in_frame;
|
||||
actual_capture_format_ = capture_params().requested_format;
|
||||
actual_capture_format_.pixel_format = media::PIXEL_FORMAT_NV12;
|
||||
@@ -300,6 +386,7 @@ void CreateStream(SCContentFilter* filter) {
|
||||
@@ -300,6 +387,7 @@ void CreateStream(SCContentFilter* filter) {
|
||||
stream_ = [[SCStream alloc] initWithFilter:filter
|
||||
configuration:config
|
||||
delegate:helper_];
|
||||
|
|
@ -214,7 +215,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
{
|
||||
NSError* error = nil;
|
||||
bool add_stream_output_result =
|
||||
@@ -453,7 +540,7 @@ void OnStreamError() {
|
||||
@@ -453,7 +541,7 @@ void OnStreamError() {
|
||||
if (fullscreen_module_) {
|
||||
fullscreen_module_->Reset();
|
||||
}
|
||||
|
|
@ -223,7 +224,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
} else {
|
||||
client()->OnError(media::VideoCaptureError::kScreenCaptureKitStreamError,
|
||||
FROM_HERE, "Stream delegate called didStopWithError");
|
||||
@@ -476,23 +563,39 @@ void OnUpdateConfigurationError() {
|
||||
@@ -476,23 +564,41 @@ void OnUpdateConfigurationError() {
|
||||
}
|
||||
|
||||
// IOSurfaceCaptureDeviceBase:
|
||||
|
|
@ -247,7 +248,9 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
+
|
||||
+ 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_);
|
||||
|
|
@ -278,7 +281,7 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
}
|
||||
void OnStop() override {
|
||||
DCHECK(device_task_runner_->RunsTasksInCurrentSequence());
|
||||
@@ -550,6 +653,8 @@ void ResetStreamTo(SCWindow* window) override {
|
||||
@@ -550,6 +656,8 @@ void ResetStreamTo(SCWindow* window) override {
|
||||
}
|
||||
|
||||
private:
|
||||
|
|
@ -287,7 +290,18 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
const DesktopMediaID source_;
|
||||
SCContentFilter* const filter_;
|
||||
const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
|
||||
@@ -579,6 +684,8 @@ void ResetStreamTo(SCWindow* window) override {
|
||||
@@ -567,6 +675,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_;
|
||||
@@ -579,6 +691,8 @@ void ResetStreamTo(SCWindow* window) override {
|
||||
base::WeakPtrFactory<ScreenCaptureKitDeviceMac> weak_factory_{this};
|
||||
};
|
||||
|
||||
|
|
@ -297,10 +311,28 @@ index 128bca8bb64113418551a20164f67ef5e5a88253..705ac9d144b092f4b5558e06b3604073
|
|||
|
||||
// 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
|
||||
index 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..d162612dc70a2b57190aaf558aca8f46cbdedcad 100644
|
||||
index 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..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
|
||||
@@ -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));
|
||||
break;
|
||||
#else
|
||||
|
|
@ -317,6 +349,19 @@ index 7adf8264cfa9980c4a8414bf0f8bfa9ad70ec0b3..d162612dc70a2b57190aaf558aca8f46
|
|||
CreateDeviceClient(media::VideoCaptureBufferType::kSharedMemory,
|
||||
kMaxNumberOfBuffers, std::move(receiver),
|
||||
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
|
||||
index f2b75f5b2f547ad135c1288bf3639b26dedc8053..ef18724d9f2ea68a47b66fc3981f58a73ac1b51d 100644
|
||||
--- a/media/capture/video_capture_types.h
|
||||
|
|
|
|||
Loading…
Reference in a new issue