[camera]remove "selfRef" for SavePhotoDelegate and ensure thread safety

packages/camera/camera/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.9.4+13
+
+* Updates iOS camera's photo capture delegate reference on a background queue to prevent potential race conditions, and some related internal code cleanup.
+
 ## 0.9.4+12
 
 * Skips unnecessary AppDelegate setup for unit tests on iOS.

packages/camera/camera/example/ios/Runner.xcodeproj/project.pbxproj

@@ -3,7 +3,7 @@
 	archiveVersion = 1;
 	classes = {
 	};
-	objectVersion = 50;
+	objectVersion = 46;
 	objects = {
 
 /* Begin PBXBuildFile section */
@@ -23,11 +23,12 @@
 		E01EE4A82799F3A5008C1950 /* QueueHelperTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E01EE4A72799F3A5008C1950 /* QueueHelperTests.m */; };
 		E032F250279F5E94009E9028 /* CameraCaptureSessionQueueRaceConditionTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E032F24F279F5E94009E9028 /* CameraCaptureSessionQueueRaceConditionTests.m */; };
 		E04F108627A87CA600573D0C /* FLTSavePhotoDelegateTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E04F108527A87CA600573D0C /* FLTSavePhotoDelegateTests.m */; };
+		E071CF7227B3061B006EF3BA /* FLTCamPhotoCaptureTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E071CF7127B3061B006EF3BA /* FLTCamPhotoCaptureTests.m */; };
+		E071CF7427B31DE4006EF3BA /* FLTCamSampleBufferTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E071CF7327B31DE4006EF3BA /* FLTCamSampleBufferTests.m */; };
 		E0C6E2002770F01A00EA6AA3 /* ThreadSafeMethodChannelTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E0C6E1FD2770F01A00EA6AA3 /* ThreadSafeMethodChannelTests.m */; };
 		E0C6E2012770F01A00EA6AA3 /* ThreadSafeTextureRegistryTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E0C6E1FE2770F01A00EA6AA3 /* ThreadSafeTextureRegistryTests.m */; };
 		E0C6E2022770F01A00EA6AA3 /* ThreadSafeEventChannelTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E0C6E1FF2770F01A00EA6AA3 /* ThreadSafeEventChannelTests.m */; };
 		E0F95E3D27A32AB900699390 /* CameraPropertiesTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E0F95E3C27A32AB900699390 /* CameraPropertiesTests.m */; };
-		E0F95E4427A36B9200699390 /* SampleBufferQueueTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E0F95E4327A36B9200699390 /* SampleBufferQueueTests.m */; };
 		E487C86026D686A10034AC92 /* CameraPreviewPauseTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E487C85F26D686A10034AC92 /* CameraPreviewPauseTests.m */; };
 		F6EE622F2710A6FC00905E4A /* MockFLTThreadSafeFlutterResult.m in Sources */ = {isa = PBXBuildFile; fileRef = F6EE622E2710A6FC00905E4A /* MockFLTThreadSafeFlutterResult.m */; };
 /* End PBXBuildFile section */
@@ -85,11 +86,12 @@
 		E01EE4A72799F3A5008C1950 /* QueueHelperTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = QueueHelperTests.m; sourceTree = "<group>"; };
 		E032F24F279F5E94009E9028 /* CameraCaptureSessionQueueRaceConditionTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = CameraCaptureSessionQueueRaceConditionTests.m; sourceTree = "<group>"; };
 		E04F108527A87CA600573D0C /* FLTSavePhotoDelegateTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FLTSavePhotoDelegateTests.m; sourceTree = "<group>"; };
+		E071CF7127B3061B006EF3BA /* FLTCamPhotoCaptureTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FLTCamPhotoCaptureTests.m; sourceTree = "<group>"; };
+		E071CF7327B31DE4006EF3BA /* FLTCamSampleBufferTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FLTCamSampleBufferTests.m; sourceTree = "<group>"; };
 		E0C6E1FD2770F01A00EA6AA3 /* ThreadSafeMethodChannelTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ThreadSafeMethodChannelTests.m; sourceTree = "<group>"; };
 		E0C6E1FE2770F01A00EA6AA3 /* ThreadSafeTextureRegistryTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ThreadSafeTextureRegistryTests.m; sourceTree = "<group>"; };
 		E0C6E1FF2770F01A00EA6AA3 /* ThreadSafeEventChannelTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ThreadSafeEventChannelTests.m; sourceTree = "<group>"; };
 		E0F95E3C27A32AB900699390 /* CameraPropertiesTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CameraPropertiesTests.m; sourceTree = "<group>"; };
-		E0F95E4327A36B9200699390 /* SampleBufferQueueTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SampleBufferQueueTests.m; sourceTree = "<group>"; };
 		E487C85F26D686A10034AC92 /* CameraPreviewPauseTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CameraPreviewPauseTests.m; sourceTree = "<group>"; };
 		F63F9EED27143B19002479BF /* MockFLTThreadSafeFlutterResult.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MockFLTThreadSafeFlutterResult.h; sourceTree = "<group>"; };
 		F6EE622E2710A6FC00905E4A /* MockFLTThreadSafeFlutterResult.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MockFLTThreadSafeFlutterResult.m; sourceTree = "<group>"; };
@@ -126,8 +128,9 @@
 				E0C6E1FF2770F01A00EA6AA3 /* ThreadSafeEventChannelTests.m */,
 				E0C6E1FD2770F01A00EA6AA3 /* ThreadSafeMethodChannelTests.m */,
 				E0C6E1FE2770F01A00EA6AA3 /* ThreadSafeTextureRegistryTests.m */,
-				E0F95E4327A36B9200699390 /* SampleBufferQueueTests.m */,
 				E04F108527A87CA600573D0C /* FLTSavePhotoDelegateTests.m */,
+				E071CF7127B3061B006EF3BA /* FLTCamPhotoCaptureTests.m */,
+				E071CF7327B31DE4006EF3BA /* FLTCamSampleBufferTests.m */,
 				E01EE4A72799F3A5008C1950 /* QueueHelperTests.m */,
 				E487C85F26D686A10034AC92 /* CameraPreviewPauseTests.m */,
 				F6EE622E2710A6FC00905E4A /* MockFLTThreadSafeFlutterResult.m */,
@@ -396,12 +399,13 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
-				E0F95E4427A36B9200699390 /* SampleBufferQueueTests.m in Sources */,
 				03F6F8B226CBB4670024B8D3 /* ThreadSafeFlutterResultTests.m in Sources */,
 				033B94BE269C40A200B4DF97 /* CameraMethodChannelTests.m in Sources */,
+				E071CF7227B3061B006EF3BA /* FLTCamPhotoCaptureTests.m in Sources */,
 				E0F95E3D27A32AB900699390 /* CameraPropertiesTests.m in Sources */,
 				03BB766B2665316900CE5A93 /* CameraFocusTests.m in Sources */,
 				E487C86026D686A10034AC92 /* CameraPreviewPauseTests.m in Sources */,
+				E071CF7427B31DE4006EF3BA /* FLTCamSampleBufferTests.m in Sources */,
 				E04F108627A87CA600573D0C /* FLTSavePhotoDelegateTests.m in Sources */,
 				F6EE622F2710A6FC00905E4A /* MockFLTThreadSafeFlutterResult.m in Sources */,
 				334733EA2668111C00DCC49E /* CameraOrientationTests.m in Sources */,

packages/camera/camera/example/ios/RunnerTests/FLTCamPhotoCaptureTests.m

@@ -0,0 +1,114 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+@import camera;
+@import camera.Test;
+@import AVFoundation;
+@import XCTest;
+#import <OCMock/OCMock.h>
+
+@interface FLTCamPhotoCaptureTests : XCTestCase
+
+@end
+
+@implementation FLTCamPhotoCaptureTests
+
+- (void)testCaptureToFile_mustReportErrorToResultIfSavePhotoDelegateCompletionsWithError {
+  XCTestExpectation *errorExpectation =
+      [self expectationWithDescription:
+                @"Must send error to result if save photo delegate completes with error."];
+
+  dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
+  dispatch_queue_set_specific(captureSessionQueue, FLTCaptureSessionQueueSpecific,
+                              (void *)FLTCaptureSessionQueueSpecific, NULL);
+  FLTCam *cam = [self createFLTCamWithCaptureSessionQueue:captureSessionQueue];
+  AVCapturePhotoSettings *settings = [AVCapturePhotoSettings photoSettings];
+  id mockSettings = OCMClassMock([AVCapturePhotoSettings class]);
+  OCMStub([mockSettings photoSettings]).andReturn(settings);
+
+  NSError *error = [NSError errorWithDomain:@"test" code:0 userInfo:nil];
+  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
+  OCMStub([mockResult sendError:error]).andDo(^(NSInvocation *invocation) {
+    [errorExpectation fulfill];
+  });
+
+  id mockOutput = OCMClassMock([AVCapturePhotoOutput class]);
+  OCMStub([mockOutput capturePhotoWithSettings:OCMOCK_ANY delegate:OCMOCK_ANY])
+      .andDo(^(NSInvocation *invocation) {
+        FLTSavePhotoDelegate *delegate = cam.inProgressSavePhotoDelegates[@(settings.uniqueID)];
+        // Completion runs on IO queue.
+        dispatch_queue_t ioQueue = dispatch_queue_create("io_queue", NULL);
+        dispatch_async(ioQueue, ^{
+          delegate.completionHandler(nil, error);
+        });
+      });
+  cam.capturePhotoOutput = mockOutput;
+
+  // `FLTCam::captureToFile` runs on capture session queue.
+  dispatch_async(captureSessionQueue, ^{
+    [cam captureToFile:mockResult];
+  });
+
+  [self waitForExpectationsWithTimeout:1 handler:nil];
+}
+
+- (void)testCaptureToFile_mustReportPathToResultIfSavePhotoDelegateCompletionsWithPath {
+  XCTestExpectation *pathExpectation =
+      [self expectationWithDescription:
+                @"Must send file path to result if save photo delegate completes with file path."];
+
+  dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
+  dispatch_queue_set_specific(captureSessionQueue, FLTCaptureSessionQueueSpecific,
+                              (void *)FLTCaptureSessionQueueSpecific, NULL);
+  FLTCam *cam = [self createFLTCamWithCaptureSessionQueue:captureSessionQueue];
+
+  AVCapturePhotoSettings *settings = [AVCapturePhotoSettings photoSettings];
+  id mockSettings = OCMClassMock([AVCapturePhotoSettings class]);
+  OCMStub([mockSettings photoSettings]).andReturn(settings);
+
+  NSString *filePath = @"test";
+  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
+  OCMStub([mockResult sendSuccessWithData:filePath]).andDo(^(NSInvocation *invocation) {
+    [pathExpectation fulfill];
+  });
+
+  id mockOutput = OCMClassMock([AVCapturePhotoOutput class]);
+  OCMStub([mockOutput capturePhotoWithSettings:OCMOCK_ANY delegate:OCMOCK_ANY])
+      .andDo(^(NSInvocation *invocation) {
+        FLTSavePhotoDelegate *delegate = cam.inProgressSavePhotoDelegates[@(settings.uniqueID)];
+        // Completion runs on IO queue.
+        dispatch_queue_t ioQueue = dispatch_queue_create("io_queue", NULL);
+        dispatch_async(ioQueue, ^{
+          delegate.completionHandler(filePath, nil);
+        });
+      });
+  cam.capturePhotoOutput = mockOutput;
+
+  // `FLTCam::captureToFile` runs on capture session queue.
+  dispatch_async(captureSessionQueue, ^{
+    [cam captureToFile:mockResult];
+  });
+  [self waitForExpectationsWithTimeout:1 handler:nil];
+}
+
+/// Creates an `FLTCam` that runs its operations on a given capture session queue.
+- (FLTCam *)createFLTCamWithCaptureSessionQueue:(dispatch_queue_t)captureSessionQueue {
+  id inputMock = OCMClassMock([AVCaptureDeviceInput class]);
+  OCMStub([inputMock deviceInputWithDevice:[OCMArg any] error:[OCMArg setTo:nil]])
+      .andReturn(inputMock);
+
+  id sessionMock = OCMClassMock([AVCaptureSession class]);
+  OCMStub([sessionMock alloc]).andReturn(sessionMock);
+  OCMStub([sessionMock addInputWithNoConnections:[OCMArg any]]);  // no-op
+  OCMStub([sessionMock canSetSessionPreset:[OCMArg any]]).andReturn(YES);
+
+  return [[FLTCam alloc] initWithCameraName:@"camera"
+                           resolutionPreset:@"medium"
+                                enableAudio:true
+                                orientation:UIDeviceOrientationPortrait
+                        captureSessionQueue:captureSessionQueue
+                                      error:nil];
+}
+
+@end

packages/camera/camera/example/ios/RunnerTests/SampleBufferQueueTests.m → packages/camera/camera/example/ios/RunnerTests/FLTCamSampleBufferTests.m

@@ -8,11 +8,11 @@
 @import XCTest;
 #import <OCMock/OCMock.h>
 
-@interface SampleBufferQueueTests : XCTestCase
+@interface FLTCamSampleBufferTests : XCTestCase
 
 @end
 
-@implementation SampleBufferQueueTests
+@implementation FLTCamSampleBufferTests
 
 - (void)testSampleBufferCallbackQueueMustBeCaptureSessionQueue {
   id inputMock = OCMClassMock([AVCaptureDeviceInput class]);

packages/camera/camera/example/ios/RunnerTests/FLTSavePhotoDelegateTests.m

@@ -14,40 +14,44 @@ @interface FLTSavePhotoDelegateTests : XCTestCase
 
 @implementation FLTSavePhotoDelegateTests
 
-- (void)testHandlePhotoCaptureResult_mustSendErrorIfFailedToCapture {
-  NSError *error = [NSError errorWithDomain:@"test" code:0 userInfo:nil];
-  dispatch_queue_t ioQueue = dispatch_queue_create("test", NULL);
-  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
-  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc] initWithPath:@"test"
-                                                                       result:mockResult
-                                                                      ioQueue:ioQueue];
+- (void)testHandlePhotoCaptureResult_mustCompleteWithErrorIfFailedToCapture {
+  XCTestExpectation *completionExpectation =
+      [self expectationWithDescription:@"Must complete with error if failed to capture photo."];
 
-  [delegate handlePhotoCaptureResultWithError:error
+  NSError *captureError = [NSError errorWithDomain:@"test" code:0 userInfo:nil];
+  dispatch_queue_t ioQueue = dispatch_queue_create("test", NULL);
+  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc]
+           initWithPath:@"test"
+                ioQueue:ioQueue
+      completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
+        XCTAssertEqualObjects(captureError, error);
+        XCTAssertNil(path);
+        [completionExpectation fulfill];
+      }];
+
+  [delegate handlePhotoCaptureResultWithError:captureError
                             photoDataProvider:^NSData * {
                               return nil;
                             }];
-  OCMVerify([mockResult sendError:error]);
+  [self waitForExpectationsWithTimeout:1 handler:nil];
 }
 
-- (void)testHandlePhotoCaptureResult_mustSendErrorIfFailedToWrite {
-  XCTestExpectation *resultExpectation =
-      [self expectationWithDescription:@"Must send IOError to the result if failed to write file."];
+- (void)testHandlePhotoCaptureResult_mustCompleteWithErrorIfFailedToWrite {
+  XCTestExpectation *completionExpectation =
+      [self expectationWithDescription:@"Must complete with error if failed to write file."];
   dispatch_queue_t ioQueue = dispatch_queue_create("test", NULL);
-  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
 
   NSError *ioError = [NSError errorWithDomain:@"IOError"
                                          code:0
                                      userInfo:@{NSLocalizedDescriptionKey : @"Localized IO Error"}];
-
-  OCMStub([mockResult sendErrorWithCode:@"IOError"
-                                message:@"Unable to write file"
-                                details:ioError.localizedDescription])
-      .andDo(^(NSInvocation *invocation) {
-        [resultExpectation fulfill];
-      });
-  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc] initWithPath:@"test"
-                                                                       result:mockResult
-                                                                      ioQueue:ioQueue];
+  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc]
+           initWithPath:@"test"
+                ioQueue:ioQueue
+      completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
+        XCTAssertEqualObjects(ioError, error);
+        XCTAssertNil(path);
+        [completionExpectation fulfill];
+      }];
 
   // We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
   // `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
@@ -63,23 +67,25 @@ - (void)testHandlePhotoCaptureResult_mustSendErrorIfFailedToWrite {
   [self waitForExpectationsWithTimeout:1 handler:nil];
 }
 
-- (void)testHandlePhotoCaptureResult_mustSendSuccessIfSuccessToWrite {
-  XCTestExpectation *resultExpectation = [self
-      expectationWithDescription:@"Must send file path to the result if success to write file."];
+- (void)testHandlePhotoCaptureResult_mustCompleteWithFilePathIfSuccessToWrite {
+  XCTestExpectation *completionExpectation =
+      [self expectationWithDescription:@"Must complete with file path if success to write file."];
 
   dispatch_queue_t ioQueue = dispatch_queue_create("test", NULL);
-  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
-  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc] initWithPath:@"test"
-                                                                       result:mockResult
-                                                                      ioQueue:ioQueue];
-  OCMStub([mockResult sendSuccessWithData:delegate.path]).andDo(^(NSInvocation *invocation) {
-    [resultExpectation fulfill];
-  });
+  NSString *filePath = @"test";
+  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc]
+           initWithPath:filePath
+                ioQueue:ioQueue
+      completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
+        XCTAssertNil(error);
+        XCTAssertEqualObjects(filePath, path);
+        [completionExpectation fulfill];
+      }];
 
   // We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
   // `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
   id mockData = OCMPartialMock([NSData data]);
-  OCMStub([mockData writeToFile:OCMOCK_ANY options:NSDataWritingAtomic error:[OCMArg setTo:nil]])
+  OCMStub([mockData writeToFile:filePath options:NSDataWritingAtomic error:[OCMArg setTo:nil]])
       .andReturn(YES);
 
   [delegate handlePhotoCaptureResultWithError:nil
@@ -94,16 +100,12 @@ - (void)testHandlePhotoCaptureResult_bothProvideDataAndSaveFileMustRunOnIOQueue
       [self expectationWithDescription:@"Data provider must run on io queue."];
   XCTestExpectation *writeFileQueueExpectation =
       [self expectationWithDescription:@"File writing must run on io queue"];
-  XCTestExpectation *resultExpectation = [self
-      expectationWithDescription:@"Must send file path to the result if success to write file."];
+  XCTestExpectation *completionExpectation =
+      [self expectationWithDescription:@"Must complete with file path if success to write file."];
 
   dispatch_queue_t ioQueue = dispatch_queue_create("test", NULL);
   const char *ioQueueSpecific = "io_queue_specific";
   dispatch_queue_set_specific(ioQueue, ioQueueSpecific, (void *)ioQueueSpecific, NULL);
-  id mockResult = OCMClassMock([FLTThreadSafeFlutterResult class]);
-  OCMStub([mockResult sendSuccessWithData:OCMOCK_ANY]).andDo(^(NSInvocation *invocation) {
-    [resultExpectation fulfill];
-  });
 
   // We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
   // `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
@@ -116,9 +118,14 @@ - (void)testHandlePhotoCaptureResult_bothProvideDataAndSaveFileMustRunOnIOQueue
       })
       .andReturn(YES);
 
-  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc] initWithPath:@"test"
-                                                                       result:mockResult
-                                                                      ioQueue:ioQueue];
+  NSString *filePath = @"test";
+  FLTSavePhotoDelegate *delegate = [[FLTSavePhotoDelegate alloc]
+           initWithPath:filePath
+                ioQueue:ioQueue
+      completionHandler:^(NSString *_Nullable path, NSError *_Nullable error) {
+        [completionExpectation fulfill];
+      }];
+
   [delegate handlePhotoCaptureResultWithError:nil
                             photoDataProvider:^NSData * {
                               if (dispatch_get_specific(ioQueueSpecific)) {