Repository: electron/electron
Pull request: fix: SCContentSharingPicker crash in iframes

#### Description of Change

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


#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [ ] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [ ] relevant documentation, tutorials, templates and examples are changed or added
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: fix crash when running `getDisplayMedia` from multiple iframes

cc @VerteDinde @MarshallOfSound 

Changed files:
filename: patches/chromium/.patches
patch: @@ -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

filename: patches/chromium/feat_allow_-4_as_a_macos_screen_share_id.patch
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.

filename: patches/chromium/feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
patch: @@ -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,15 +207,15 @@ 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_];
 +
      {
        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