[macOS] Migrate to AVCapturePhotoOutput for ImageCapture.takePhoto().

AVCaptureStillImageOutput is deprecated, in preparation for its future
removal migrate to AVCapturePhotoOutput which is only available on
macOS 10.15. In order not to break backwards-compat, the old API is
still used as a fallback.

To avoid takePhoto() returning empty data in the event of multiple
pending takePhoto() calls, the logic for handling pending photos is
simplified: take as many photos as requested, but one at a time. This
means we only need a single variable to keep track of pending photos
(_pendingTakePhotos).

To test both the new and old path, TEST_P is used.

TESTED=https://googlechrome.github.io/samples/image-capture/grab-frame-take-photo.html

Bug: chromium:1124322
Change-Id: I802c80ebb06677d2309ca804b39a2d8703f548ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4128918
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Markus Handell <handellm@google.com>
Cr-Commit-Position: refs/heads/main@{#1088642}
diff --git a/media/capture/video/mac/video_capture_device_avfoundation_mac.h b/media/capture/video/mac/video_capture_device_avfoundation_mac.h
index ec48e672..d9d2217 100644
--- a/media/capture/video/mac/video_capture_device_avfoundation_mac.h
+++ b/media/capture/video/mac/video_capture_device_avfoundation_mac.h
@@ -81,7 +81,8 @@
 // "next generation" moniker.
 CAPTURE_EXPORT
 @interface VideoCaptureDeviceAVFoundation
-    : NSObject <AVCaptureVideoDataOutputSampleBufferDelegate> {
+    : NSObject <AVCaptureVideoDataOutputSampleBufferDelegate,
+                AVCapturePhotoCaptureDelegate> {
  @private
   // The following attributes are set via -setCaptureHeight:width:frameRate:.
   int _frameWidth;
@@ -130,17 +131,20 @@
   std::vector<std::unique_ptr<media::SampleBufferTransformer>>
       _scaledFrameTransformers;
 
-  // An AVDataOutput specialized for taking pictures out of |captureSession_|.
-  base::scoped_nsobject<AVCaptureStillImageOutput> _stillImageOutput;
-  size_t _takePhotoStartedCount;
-  size_t _takePhotoPendingCount;
-  size_t _takePhotoCompletedCount;
-  bool _stillImageOutputWarmupCompleted;
+  // On macOS 10.15 or later, this has type AVCapturePhotoOutput.
+  // On earlier versions, this has type AVCaptureStillImageOutput.
+  // You say tomato, I say potato.
+  base::scoped_nsobject<id> _photoOutput;
+  // Only accessed on the main thread. The takePhoto() operation is considered
+  // 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;
   std::unique_ptr<base::WeakPtrFactory<VideoCaptureDeviceAVFoundation>>
       _weakPtrFactoryForTakePhoto;
 
   // For testing.
-  base::RepeatingCallback<void()> _onStillImageOutputStopped;
+  base::RepeatingCallback<void()> _onPhotoOutputStopped;
+  bool _forceLegacyStillImageApi;
 
   scoped_refptr<base::SingleThreadTaskRunner> _mainThreadTaskRunner;
 }
@@ -206,8 +210,9 @@
 // formats. This implementation recognizes NV12.
 + (media::VideoPixelFormat)FourCCToChromiumPixelFormat:(FourCharCode)code;
 
-- (void)setOnStillImageOutputStoppedForTesting:
-    (base::RepeatingCallback<void()>)onStillImageOutputStopped;
+- (void)setOnPhotoOutputStoppedForTesting:
+    (base::RepeatingCallback<void()>)onPhotoOutputStopped;
+- (void)setForceLegacyStillImageApiForTesting:(bool)forceLegacyApi;
 
 // Use the below only for test.
 - (void)callLocked:(base::OnceClosure)lambda;
diff --git a/media/capture/video/mac/video_capture_device_avfoundation_mac.mm b/media/capture/video/mac/video_capture_device_avfoundation_mac.mm
index 82b18a8f..fa936dc 100644
--- a/media/capture/video/mac/video_capture_device_avfoundation_mac.mm
+++ b/media/capture/video/mac/video_capture_device_avfoundation_mac.mm
@@ -43,7 +43,7 @@
     gfx::ColorSpace::MatrixID::SMPTE170M,
     gfx::ColorSpace::RangeID::LIMITED);
 
-constexpr int kTimeToWaitBeforeStoppingStillImageCaptureInSeconds = 60;
+constexpr int kTimeToWaitBeforeStoppingPhotoOutputInSeconds = 60;
 constexpr FourCharCode kDefaultFourCCPixelFormat =
     kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;  // NV12 (a.k.a. 420v)
 
@@ -197,10 +197,10 @@
 }
 
 - (void)dealloc {
-  // Stopping a running still image output takes `_lock`. To avoid this
-  // happening inside stopCapture() below which would deadlock, we ensure that
-  // still image output is already stopped before taking `_lock`.
-  [self stopStillImageOutput];
+  // Stopping a running photo output takes `_lock`. To avoid this happening
+  // inside stopCapture() below which would deadlock, we ensure that the photo
+  // output is already stopped before taking `_lock`.
+  [self stopPhotoOutput];
   {
     // To avoid races with concurrent callbacks, grab the lock before stopping
     // capture and clearing all the variables.
@@ -239,7 +239,7 @@
     [self stopCapture];
     // Now remove the input and output from the capture session.
     [_captureSession removeOutput:_captureVideoDataOutput];
-    [self stopStillImageOutput];
+    [self stopPhotoOutput];
     if (_captureDeviceInput) {
       DCHECK(_captureDevice);
       [_captureSession stopRunning];
@@ -449,93 +449,101 @@
 - (void)stopCapture {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
   _weakPtrFactoryForStallCheck.reset();
-  [self stopStillImageOutput];
+  [self stopPhotoOutput];
   if ([_captureSession isRunning])
     [_captureSession stopRunning];  // Synchronous.
   [[NSNotificationCenter defaultCenter] removeObserver:self];
 }
 
+- (bool)useLegacyStillImageApi {
+  if (@available(macOS 10.15, *)) {
+    return _forceLegacyStillImageApi;
+  }
+  return true;
+}
+
+- (void)setForceLegacyStillImageApiForTesting:(bool)forceLegacyApi {
+  _forceLegacyStillImageApi = forceLegacyApi;
+}
+
 - (void)takePhoto {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
   DCHECK([_captureSession isRunning]);
 
-  ++_takePhotoStartedCount;
+  ++_pendingTakePhotos;
+  if (_pendingTakePhotos > 1u) {
+    // There is already pending takePhoto(). When it finishes it will kick off
+    // 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.
+  _weakPtrFactoryForTakePhoto->InvalidateWeakPtrs();
 
   // Ready to take a photo immediately?
-  if (_stillImageOutput && _stillImageOutputWarmupCompleted) {
+  // Thread-safe because `_photoOutput` is only modified on the main thread.
+  if (_photoOutput) {
     [self takePhotoInternal];
     return;
   }
 
-  // Lazily instantiate the |_stillImageOutput| the first time takePhoto() is
-  // called. When takePhoto() isn't called, this avoids JPEG compession work for
-  // every frame. This can save a lot of CPU in some cases (see
-  // https://crbug.com/1116241). However because it can take a couple of second
-  // for the 3A to stabilize, lazily instantiating like may result in noticeable
-  // delays. To avoid delays in future takePhoto() calls we don't delete
-  // |_stillImageOutput| until takePhoto() has not been called for 60 seconds.
-  if (!_stillImageOutput) {
-    // We use AVCaptureStillImageOutput for historical reasons, but note that it
-    // has been deprecated in macOS 10.15[1] in favor of
-    // AVCapturePhotoOutput[2].
-    //
-    // [1]
-    // https://developer.apple.com/documentation/avfoundation/avcapturestillimageoutput
-    // [2]
-    // https://developer.apple.com/documentation/avfoundation/avcapturephotooutput
-    // TODO(https://crbug.com/1124322): Migrate to the new API.
-    _stillImageOutput.reset([[AVCaptureStillImageOutput alloc] init]);
-    if (!_stillImageOutput ||
-        ![_captureSession canAddOutput:_stillImageOutput]) {
-      // Complete this started photo as error.
-      ++_takePhotoPendingCount;
-      {
-        base::AutoLock lock(_lock);
-        if (_frameReceiver) {
-          _frameReceiver->OnPhotoError();
-        }
-      }
-      [self takePhotoCompleted];
-      return;
+  // Lazily instantiate `_photoOutput` so that if the app never calls
+  // takePhoto() we don't have to pay the associated performance cost, see
+  // https://crbug.com/1116241. This procedure is purposefully delayed by 3
+  // seconds because the camera needs to ramp up after re-configuring itself in
+  // order for 3A to stabilize or else the photo is dark/black.
+  {
+    // `_lock` is needed since `_photoOutput` may be read from non-main thread.
+    base::AutoLock lock(_lock);
+    if ([self useLegacyStillImageApi]) {
+      _photoOutput.reset([[AVCaptureStillImageOutput alloc] init]);
+    } else if (@available(macOS 10.15, *)) {
+      _photoOutput.reset([[AVCapturePhotoOutput alloc] init]);
+    } else {
+      NOTREACHED();
     }
-    [_captureSession addOutput:_stillImageOutput];
-    // 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.
-    _mainThreadTaskRunner->PostDelayedTask(
-        FROM_HERE,
-        base::BindOnce(
-            [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf) {
-              [weakSelf.get() takePhotoInternal];
-            },
-            _weakPtrFactoryForTakePhoto->GetWeakPtr()),
-        base::Seconds(3));
   }
+  if (![_captureSession canAddOutput:_photoOutput]) {
+    {
+      base::AutoLock lock(_lock);
+      if (_frameReceiver) {
+        _frameReceiver->OnPhotoError();
+      }
+    }
+    [self takePhotoResolved];
+    return;
+  } else {
+    [_captureSession addOutput:_photoOutput];
+  }
+  // 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.
+  _mainThreadTaskRunner->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf) {
+            [weakSelf.get() takePhotoInternal];
+          },
+          _weakPtrFactoryForTakePhoto->GetWeakPtr()),
+      base::Seconds(3));
 }
 
-- (void)setOnStillImageOutputStoppedForTesting:
-    (base::RepeatingCallback<void()>)onStillImageOutputStopped {
+- (void)setOnPhotoOutputStoppedForTesting:
+    (base::RepeatingCallback<void()>)onPhotoOutputStopped {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
-  _onStillImageOutputStopped = onStillImageOutputStopped;
+  _onPhotoOutputStopped = onPhotoOutputStopped;
 }
 
 #pragma mark Private methods
 
 - (void)takePhotoInternal {
   DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
-  // stopStillImageOutput invalidates all weak ptrs, meaning in-flight
-  // operations are affectively cancelled. So if this method is running, still
-  // image output must be good to go.
   DCHECK([_captureSession isRunning]);
-  DCHECK(_stillImageOutput);
-  DCHECK([[_stillImageOutput connections] count] == 1);
-  AVCaptureConnection* const connection =
-      [[_stillImageOutput connections] firstObject];
-  DCHECK(connection);
-  _stillImageOutputWarmupCompleted = true;
-
-  // For all photos started that are not yet pending, take photos.
-  while (_takePhotoPendingCount < _takePhotoStartedCount) {
-    ++_takePhotoPendingCount;
+  // takePhotoInternal() can only happen when we have a `_photoOutput` because
+  // stopPhotoOutput() cancels in-flight operations by invalidating weak ptrs.
+  DCHECK(_photoOutput);
+  if ([self useLegacyStillImageApi]) {
+    // `_photoOutput` is of type AVCaptureStillImageOutput.
     const auto handler = ^(CMSampleBufferRef sampleBuffer, NSError* error) {
       {
         base::AutoLock lock(_lock);
@@ -552,7 +560,6 @@
             DCHECK_EQ(kCMVideoCodecType_JPEG,
                       CMFormatDescriptionGetMediaSubType(
                           CMSampleBufferGetFormatDescription(sampleBuffer)));
-
             char* baseAddress = 0;
             size_t length = 0;
             const bool sample_buffer_addressable =
@@ -567,75 +574,139 @@
           }
         }
       }
-      // Called both on success and failure.
+      // Whether we succeeded or failed, we need to resolve the pending
+      // takePhoto() operation.
       _mainThreadTaskRunner->PostTask(
           FROM_HERE,
           base::BindOnce(
               [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf) {
-                [weakSelf.get() takePhotoCompleted];
+                [weakSelf.get() takePhotoResolved];
               },
               _weakPtrFactoryForTakePhoto->GetWeakPtr()));
     };
-    [_stillImageOutput captureStillImageAsynchronouslyFromConnection:connection
-                                                   completionHandler:handler];
+    AVCaptureStillImageOutput* image_output =
+        static_cast<AVCaptureStillImageOutput*>(_photoOutput.get());
+    DCHECK([[image_output connections] count] == 1);
+    AVCaptureConnection* const connection =
+        [[image_output connections] firstObject];
+    DCHECK(connection);
+    [image_output captureStillImageAsynchronouslyFromConnection:connection
+                                              completionHandler:handler];
+  } else if (@available(macOS 10.15, *)) {
+    // `_photoOutput` is of type AVCapturePhotoOutput.
+    @try {
+      // Asynchronous success or failure is handled inside
+      // captureOutput:didFinishProcessingPhoto:error on an unknown thread.
+      // Synchronous failures are handled in the catch clause below.
+      [_photoOutput
+          capturePhotoWithSettings:[AVCapturePhotoSettings
+                                       photoSettingsWithFormat:@{
+                                         AVVideoCodecKey : AVVideoCodecTypeJPEG
+                                       }]
+                          delegate:self];
+    } @catch (id exception) {
+      {
+        base::AutoLock lock(_lock);
+        if (_frameReceiver) {
+          _frameReceiver->OnPhotoError();
+        }
+      }
+      [self takePhotoResolved];
+    }
+  } else {
+    NOTREACHED();
   }
 }
 
-- (void)takePhotoCompleted {
-  DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
-  ++_takePhotoCompletedCount;
-  if (_takePhotoStartedCount != _takePhotoCompletedCount)
-    return;
-  // All pending takePhoto()s have completed. If no more photos are taken
-  // within 60 seconds, stop still image output to avoid expensive MJPEG
-  // conversions going forward.
-  _mainThreadTaskRunner->PostDelayedTask(
-      FROM_HERE,
-      base::BindOnce(
-          [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf,
-             size_t takePhotoCount) {
-            VideoCaptureDeviceAVFoundation* strongSelf = weakSelf.get();
-            if (!strongSelf)
-              return;
-            // Don't stop the still image output if takePhoto() was called
-            // while the task was pending.
-            if (strongSelf->_takePhotoStartedCount != takePhotoCount)
-              return;
-            [strongSelf stopStillImageOutput];
-          },
-          _weakPtrFactoryForTakePhoto->GetWeakPtr(), _takePhotoStartedCount),
-      base::Seconds(kTimeToWaitBeforeStoppingStillImageCaptureInSeconds));
-}
-
-- (void)stopStillImageOutput {
-  DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
-  if (!_stillImageOutput) {
-    // Already stopped.
-    return;
-  }
-  if (_captureSession) {
-    [_captureSession removeOutput:_stillImageOutput];
-  }
-  _stillImageOutput.reset();
-  _stillImageOutputWarmupCompleted = false;
-
-  // Cancel all in-flight operations.
-  _weakPtrFactoryForTakePhoto->InvalidateWeakPtrs();
-  // Report error for all pending calls that were stopped.
-  size_t pendingCalls = _takePhotoStartedCount - _takePhotoCompletedCount;
-  _takePhotoCompletedCount = _takePhotoPendingCount = _takePhotoStartedCount;
-  {
+// Callback for the `_photoOutput` operation started in takePhotoInternal().
+- (void)captureOutput:(id)output        // AVCapturePhotoOutput*
+    didFinishProcessingPhoto:(id)photo  // AVCapturePhoto*
+                       error:(NSError*)error {
+  if (@available(macOS 10.15, *)) {
     base::AutoLock lock(_lock);
+    // If `output` is no longer current, ignore the result of this operation.
+    // `_frameReceiver->OnPhotoError()` will already have been called inside
+    // stopPhotoOutput().
+    if (output != _photoOutput) {
+      return;
+    }
     if (_frameReceiver) {
-      for (size_t i = 0; i < pendingCalls; ++i) {
+      // Always non-nil according to Apple's documentation.
+      DCHECK(photo);
+      NSData* data = static_cast<AVCapturePhoto*>(photo).fileDataRepresentation;
+      if (!error && data) {
+        _frameReceiver->OnPhotoTaken(
+            reinterpret_cast<const uint8_t*>(data.bytes), data.length,
+            "image/jpeg");
+      } else {
         _frameReceiver->OnPhotoError();
       }
     }
+    // Whether we succeeded or failed, we need to resolve the pending
+    // takePhoto() operation.
+    _mainThreadTaskRunner->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf) {
+              [weakSelf.get() takePhotoResolved];
+            },
+            _weakPtrFactoryForTakePhoto->GetWeakPtr()));
+  } else {
+    NOTREACHED();
+  }
+}
+
+- (void)takePhotoResolved {
+  DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
+  --_pendingTakePhotos;
+  if (_pendingTakePhotos > 0u) {
+    // Take another photo.
+    [self takePhotoInternal];
+    return;
+  }
+  // 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.
+  _mainThreadTaskRunner->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(
+          [](base::WeakPtr<VideoCaptureDeviceAVFoundation> weakSelf) {
+            [weakSelf.get() stopPhotoOutput];
+          },
+          _weakPtrFactoryForTakePhoto->GetWeakPtr()),
+      base::Seconds(kTimeToWaitBeforeStoppingPhotoOutputInSeconds));
+}
+
+- (void)stopPhotoOutput {
+  DCHECK(_mainThreadTaskRunner->BelongsToCurrentThread());
+  // Already stopped?
+  // Thread-safe because `_photoOutput` is only modified on the main thread.
+  if (!_photoOutput) {
+    return;
+  }
+  // Cancel all in-flight operations.
+  _weakPtrFactoryForTakePhoto->InvalidateWeakPtrs();
+  {
+    base::AutoLock lock(_lock);
+    if (_captureSession) {
+      [_captureSession removeOutput:_photoOutput];
+    }
+    // `_lock` is needed since `_photoOutput` may be read from non-main thread.
+    _photoOutput.reset();
+    // For every pending photo, report OnPhotoError().
+    if (_pendingTakePhotos) {
+      if (_frameReceiver) {
+        for (size_t i = 0; i < _pendingTakePhotos; ++i) {
+          _frameReceiver->OnPhotoError();
+        }
+      }
+      _pendingTakePhotos = 0u;
+    }
   }
 
-  if (_onStillImageOutputStopped) {
+  if (_onPhotoOutputStopped) {
     // Callback used by tests.
-    _onStillImageOutputStopped.Run();
+    _onPhotoOutputStopped.Run();
   }
 }
 
@@ -893,8 +964,7 @@
     // If we captured a frame since last check, then we aren't stalled.
     // We're also not considered stalled if takePhoto() is pending, avoiding
     // excessive capture restarts in unit tests with mock time.
-    if (_capturedFrameSinceLastStallCheck ||
-        _takePhotoStartedCount != _takePhotoCompletedCount) {
+    if (_capturedFrameSinceLastStallCheck || _pendingTakePhotos) {
       nextFailedCheckCount = 0;
     }
     _capturedFrameSinceLastStallCheck = NO;
diff --git a/media/capture/video/mac/video_capture_device_avfoundation_mac_unittest.mm b/media/capture/video/mac/video_capture_device_avfoundation_mac_unittest.mm
index cb890109..98286d3a 100644
--- a/media/capture/video/mac/video_capture_device_avfoundation_mac_unittest.mm
+++ b/media/capture/video/mac/video_capture_device_avfoundation_mac_unittest.mm
@@ -25,6 +25,9 @@
 #include "ui/gfx/color_space.h"
 
 using testing::_;
+using testing::Gt;
+using testing::Ne;
+using testing::Return;
 
 namespace media {
 
@@ -419,199 +422,228 @@
   }));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest, TakePhoto) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
-
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
+class VideoCaptureDeviceAVFoundationMacTakePhotoTest
+    : public testing::TestWithParam<bool> {
+ public:
+  base::scoped_nsobject<VideoCaptureDeviceAVFoundation> CreateCaptureDevice(
+      testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>*
+          frame_receiver) {
     base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
         [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+            initWithFrameReceiver:frame_receiver]);
+    [captureDevice setForceLegacyStillImageApiForTesting:GetParam()];
+    return captureDevice;
+  }
+};
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest, TakePhoto) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
-    EXPECT_CALL(frame_receiver, OnPhotoTaken)
-        .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
-    [captureDevice takePhoto];
-    run_loop.Run();
-  }));
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
+
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
+
+        base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+        EXPECT_CALL(frame_receiver, OnPhotoTaken)
+
+            .WillOnce([&run_loop](const uint8_t* image_data,
+                                  size_t image_length,
+                                  const std::string& mime_type) {
+              EXPECT_TRUE(image_data);
+              EXPECT_GT(image_length, 0u);
+              EXPECT_EQ(mime_type, "image/jpeg");
+              run_loop.Quit();
+            });
+        [captureDevice takePhoto];
+        run_loop.Run();
+      },
+      this));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest, StopCaptureWhileTakingPhoto) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+       StopCaptureWhileTakingPhoto) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
-    base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
-        [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
 
-    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
-    EXPECT_CALL(frame_receiver, OnPhotoError())
-        .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
-    [captureDevice takePhoto];
-    // There is no risk that takePhoto() has successfully finishes before
-    // stopCapture() because the takePhoto() call involes a PostDelayedTask()
-    // that cannot run until RunLoop::Run() below.
-    [captureDevice stopCapture];
-    run_loop.Run();
-  }));
+        base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+        EXPECT_CALL(frame_receiver, OnPhotoError())
+            .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+        [captureDevice takePhoto];
+        // There is no risk that takePhoto() has successfully finishes before
+        // stopCapture() because the takePhoto() call involes a
+        // PostDelayedTask() that cannot run until RunLoop::Run() below.
+        [captureDevice stopCapture];
+        run_loop.Run();
+      },
+      this));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest, MultiplePendingTakePhotos) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+       MultiplePendingTakePhotos) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
-    base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
-        [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
 
-    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
-    size_t photos_taken_count = 0;
-    EXPECT_CALL(frame_receiver, OnPhotoTaken)
-        .WillRepeatedly([&photos_taken_count, &run_loop] {
-          ++photos_taken_count;
-          if (photos_taken_count == 3) {
-            run_loop.Quit();
-          }
-        });
-    [captureDevice takePhoto];
-    [captureDevice takePhoto];
-    [captureDevice takePhoto];
-    run_loop.Run();
-  }));
+        base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+        EXPECT_CALL(frame_receiver, OnPhotoTaken(Ne(nullptr), Gt(0u), _))
+            .WillOnce(Return())
+            .WillOnce(Return())
+            .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+        [captureDevice takePhoto];
+        [captureDevice takePhoto];
+        [captureDevice takePhoto];
+        run_loop.Run();
+      },
+      this));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest,
-     StopCaptureWhileMultiplePendingTakePhotos) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+       StopCaptureWhileMultiplePendingTakePhotos) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
-    base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
-        [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
 
-    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
-    size_t photo_error_count = 0;
-    EXPECT_CALL(frame_receiver, OnPhotoError)
-        .WillRepeatedly([&photo_error_count, &run_loop] {
-          ++photo_error_count;
-          if (photo_error_count == 3) {
-            run_loop.Quit();
-          }
-        });
-    [captureDevice takePhoto];
-    [captureDevice takePhoto];
-    [captureDevice takePhoto];
-    // There is no risk that takePhoto() has successfully finishes before
-    // stopCapture() because the takePhoto() calls involes a PostDelayedTask()
-    // that cannot run until RunLoop::Run() below.
-    [captureDevice stopCapture];
-    run_loop.Run();
-  }));
+        base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+        EXPECT_CALL(frame_receiver, OnPhotoError)
+            .WillOnce(Return())
+            .WillOnce(Return())
+            .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+        [captureDevice takePhoto];
+        [captureDevice takePhoto];
+        [captureDevice takePhoto];
+        // There is no risk that takePhoto() has successfully finishes before
+        // stopCapture() because the takePhoto() calls involes a
+        // PostDelayedTask() that cannot run until RunLoop::Run() below.
+        [captureDevice stopCapture];
+        run_loop.Run();
+      },
+      this));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest,
-     StopStillImageOutputWhenNoLongerTakingPhotos) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+       StopPhotoOutputWhenNoLongerTakingPhotos) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
-    base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
-        [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
 
-    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
-    [captureDevice
-        setOnStillImageOutputStoppedForTesting:run_loop.QuitClosure()];
-    base::TimeTicks start_time = base::TimeTicks::Now();
-    [captureDevice takePhoto];
-    // The RunLoop automatically advances mocked time when there are delayed
-    // tasks pending. This allows the test to run fast and still assert how much
-    // mocked time has elapsed.
-    run_loop.Run();
-    auto time_elapsed = base::TimeTicks::Now() - start_time;
-    // Still image output is not stopped until 60 seconds of inactivity, so the
-    // mocked time must have advanced at least this much.
-    EXPECT_GE(time_elapsed.InSeconds(), 60);
-  }));
+        base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+        [captureDevice
+            setOnPhotoOutputStoppedForTesting:run_loop.QuitClosure()];
+        base::TimeTicks start_time = base::TimeTicks::Now();
+        [captureDevice takePhoto];
+        // The RunLoop automatically advances mocked time when there are delayed
+        // tasks pending. This allows the test to run fast and still assert how
+        // much mocked time has elapsed.
+        run_loop.Run();
+        auto time_elapsed = base::TimeTicks::Now() - start_time;
+        // Still image output is not stopped until 60 seconds of inactivity, so
+        // the mocked time must have advanced at least this much.
+        EXPECT_GE(time_elapsed.InSeconds(), 60);
+      },
+      this));
 }
 
-TEST(VideoCaptureDeviceAVFoundationMacTest,
-     TakePhotoAndShutDownWithoutWaiting) {
-  RunTestCase(base::BindOnce([] {
-    NSString* deviceId = GetFirstDeviceId();
-    if (!deviceId) {
-      DVLOG(1) << "No camera available. Exiting test.";
-      return;
-    }
+TEST_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+       TakePhotoAndShutDownWithoutWaiting) {
+  RunTestCase(base::BindOnce(
+      [](VideoCaptureDeviceAVFoundationMacTakePhotoTest* thiz) {
+        NSString* deviceId = GetFirstDeviceId();
+        if (!deviceId) {
+          DVLOG(1) << "No camera available. Exiting test.";
+          return;
+        }
 
-    testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
-        frame_receiver;
-    base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice(
-        [[VideoCaptureDeviceAVFoundation alloc]
-            initWithFrameReceiver:&frame_receiver]);
+        testing::NiceMock<MockVideoCaptureDeviceAVFoundationFrameReceiver>
+            frame_receiver;
+        base::scoped_nsobject<VideoCaptureDeviceAVFoundation> captureDevice =
+            thiz->CreateCaptureDevice(&frame_receiver);
 
-    NSString* errorMessage = nil;
-    ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
-                                   errorMessage:&errorMessage]);
-    ASSERT_TRUE([captureDevice startCapture]);
+        NSString* errorMessage = nil;
+        ASSERT_TRUE([captureDevice setCaptureDevice:deviceId
+                                       errorMessage:&errorMessage]);
+        ASSERT_TRUE([captureDevice startCapture]);
 
-    [captureDevice takePhoto];
-  }));
+        [captureDevice takePhoto];
+      },
+      this));
 }
 
+// When not forcing legacy API, AVCapturePhotoOutput is used if available
+// (macOS 10.15+). Otherwise AVCaptureStillImageOutput is used.
+INSTANTIATE_TEST_SUITE_P(VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+                         VideoCaptureDeviceAVFoundationMacTakePhotoTest,
+                         // Force legacy API?
+                         testing::Values(false, true));
+
 TEST(VideoCaptureDeviceAVFoundationMacTest, ForwardsOddPixelBufferResolution) {
   // See crbug/1168112.
   RunTestCase(base::BindOnce([] {