From 81609cb7bca0b0f664a4d017c676c0a0123dea9e Mon Sep 17 00:00:00 2001
From: Guido Urdaneta <[email protected]>
Date: Wed, 27 May 2026 07:57:28 -0700
Subject: [PATCH] [MediaCapture] Use Obj-C weak pointers for the takePhoto operation

Previously it was using WeakPtrFactory, but GetWeakPtr was being
called cross-thread in captureOutput, which is not allowed as
WeakPtrs are not thread safe.

This CL also updates the logic to avoid relying on InvalidateWeakPtr
to cancel the stopPhotoOutput operation when the number of pending
takePhoto operations changes from 0 to 1.
Now stopPhotoOutput exits early if there are pending takePhoto
operations.

Fixed: 513362710
Change-Id: Id7e3e4fcd68ae8f9a51297c5612cec1f637639c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7879144
Auto-Submit: Guido Urdaneta <[email protected]>
Reviewed-by: Tony Herre <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Tony Herre <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1636960}
---

diff --git a/media/capture/video/apple/video_capture_device_avfoundation.mm b/media/capture/video/apple/video_capture_device_avfoundation.mm
index f748a6e5..8a0ef2ff 100644
--- a/media/capture/video/apple/video_capture_device_avfoundation.mm
+++ b/media/capture/video/apple/video_capture_device_avfoundation.mm
@@ -269,7 +269,6 @@
   // pending until we're ready to take another photo, which involves a PostTask
   // back to the main thread after the photo was taken.
   size_t _pendingTakePhotos;
-  SelfHolder _weakPtrHolderForTakePhoto;
 
   // For testing.
   base::RepeatingCallback<void()> _onPhotoOutputStopped;
@@ -323,7 +322,6 @@
     _useGPUMemoryBuffer = true;
     _capturedFirstFrame = false;
     _weakPtrHolderForStallCheck.the_self = self;
-    _weakPtrHolderForTakePhoto.the_self = self;
     [self setFrameReceiver:frameReceiver];
     _captureSession = [[AVCaptureSession alloc] init];
     _sampleBufferTransformer = media::SampleBufferTransformer::Create();
@@ -628,11 +626,6 @@
     // the next takePhotoInternal(), so there is nothing more to do here.
     return;
   }
-  // `_pendingTakePhotos` just went from 0 to 1. In case the 60 second delayed
-  // task to perform stopPhotoOutput() is in-flight, invalidate weak ptrs to
-  // cancel any such operation.
-  _weakPtrHolderForTakePhoto.weak_ptr_factory.InvalidateWeakPtrs();
-
   // Ready to take a photo immediately?
   // Thread-safe because `_photoOutput` is only modified on the main thread.
   if (_photoOutput) {
@@ -664,16 +657,17 @@
   }
   // A delay is needed before taking the photo or else the photo may be dark.
   // 2 seconds was enough in manual testing; we delay by 3 for good measure.
+  __weak VideoCaptureDeviceAVFoundation* weakSelf = self;
   _mainThreadTaskRunner->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(
-          [](base::WeakPtr<SelfHolder> weakSelf) {
-            if (!weakSelf.get()) {
-              return;
+          [](VideoCaptureDeviceAVFoundation* __weak wSelf) {
+            VideoCaptureDeviceAVFoundation* sSelf = wSelf;
+            if (sSelf) {
+              [sSelf takePhotoInternal];
             }
-            [weakSelf.get()->the_self takePhotoInternal];
           },
-          _weakPtrHolderForTakePhoto.weak_ptr_factory.GetWeakPtr()),
+          weakSelf),
       base::Seconds(3));
 }
 
@@ -736,19 +730,23 @@
   }
   // Whether we succeeded or failed, we need to resolve the pending
   // takePhoto() operation.
+  __weak VideoCaptureDeviceAVFoundation* weakSelf = self;
   _mainThreadTaskRunner->PostTask(
       FROM_HERE, base::BindOnce(
-                     [](base::WeakPtr<SelfHolder> weakSelf) {
-                       if (!weakSelf.get()) {
-                         return;
+                     [](VideoCaptureDeviceAVFoundation* __weak wSelf) {
+                       VideoCaptureDeviceAVFoundation* sSelf = wSelf;
+                       if (sSelf) {
+                         [sSelf takePhotoResolved];
                        }
-                       [weakSelf.get()->the_self takePhotoResolved];
                      },
-                     _weakPtrHolderForTakePhoto.weak_ptr_factory.GetWeakPtr()));
+                     weakSelf));
 }
 
 - (void)takePhotoResolved {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
+  if (_pendingTakePhotos == 0) {
+    return;
+  }
   --_pendingTakePhotos;
   if (_pendingTakePhotos > 0u) {
     // Take another photo.
@@ -758,28 +756,31 @@
   // All pending takePhoto()s have completed. If no more photos are taken
   // within 60 seconds, stop photo output to avoid expensive MJPEG conversions
   // going forward.
+  __weak VideoCaptureDeviceAVFoundation* weakSelf = self;
   _mainThreadTaskRunner->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(
-          [](base::WeakPtr<SelfHolder> weakSelf) {
-            if (!weakSelf.get()) {
-              return;
+          [](VideoCaptureDeviceAVFoundation* __weak wSelf) {
+            VideoCaptureDeviceAVFoundation* sSelf = wSelf;
+            if (sSelf) {
+              [sSelf stopPhotoOutput];
             }
-            [weakSelf.get()->the_self stopPhotoOutput];
           },
-          _weakPtrHolderForTakePhoto.weak_ptr_factory.GetWeakPtr()),
+          weakSelf),
       base::Seconds(kTimeToWaitBeforeStoppingPhotoOutputInSeconds));
 }
 
 - (void)stopPhotoOutput {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
+  if (_pendingTakePhotos > 0u) {
+    return;
+  }
   // Already stopped?
   // Thread-safe because `_photoOutput` is only modified on the main thread.
   if (!_photoOutput) {
     return;
   }
   // Cancel all in-flight operations.
-  _weakPtrHolderForTakePhoto.weak_ptr_factory.InvalidateWeakPtrs();
   {
     base::AutoLock lock(_lock);
     if (_captureSession) {