Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/image_picker/image_picker_ios/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.8.13+7

* Fixes early dismissal of image picker.

## 0.8.13+6

* Replaces deprecated `kUTTypeGIF` with `UTTypeGIF` to fix iOS 15+ deprecation warnings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,67 @@ - (void)testPluginPickImageDeviceCancelClickMultipleTimes {
[plugin imagePickerControllerDidCancel:controller];
}

- (void)testCancelResultIsNotSentUntilDismissalCompletes {
FLTImagePickerPlugin *plugin =
[[FLTImagePickerPlugin alloc] initWithViewProvider:[[StubViewProvider alloc] init]];
__block NSArray<NSString *> *pickResult = nil;
plugin.callContext = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^(NSArray<NSString *> *_Nullable result, FlutterError *_Nullable error) {
pickResult = result;
}];

__block void (^dismissCompletion)(void) = nil;
id mockPicker = OCMClassMock([UIImagePickerController class]);
OCMStub([mockPicker
dismissViewControllerAnimated:YES
completion:[OCMArg checkWithBlock:^BOOL(void (^completion)(void)) {
dismissCompletion = [completion copy];
return YES;
}]]);

[plugin imagePickerControllerDidCancel:mockPicker];

XCTAssertNotNil(dismissCompletion);
XCTAssertNil(pickResult, @"The cancel result should not be sent before dismissal completes.");

dismissCompletion();

XCTAssertEqualObjects(pickResult, @[]);
}

- (void)testPickResultIsNotSentUntilDismissalCompletes {
FLTImagePickerPlugin *plugin =
[[FLTImagePickerPlugin alloc] initWithViewProvider:[[StubViewProvider alloc] init]];
__block NSArray<NSString *> *pickResult = nil;
plugin.callContext = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^(NSArray<NSString *> *_Nullable result, FlutterError *_Nullable error) {
pickResult = result;
}];

// Capture the dismissal completion instead of running it, so the test controls
// when dismissal "finishes".
__block void (^dismissCompletion)(void) = nil;
id mockPicker = OCMClassMock([UIImagePickerController class]);
OCMStub([mockPicker
dismissViewControllerAnimated:YES
completion:[OCMArg checkWithBlock:^BOOL(void (^completion)(void)) {
dismissCompletion = [completion copy];
return YES;
}]]);

UIImage *image = [UIImage imageWithData:ImagePickerTestImages.JPGTestData];
[plugin imagePickerController:mockPicker
didFinishPickingMediaWithInfo:@{UIImagePickerControllerOriginalImage : image}];

XCTAssertNotNil(dismissCompletion);
XCTAssertNil(pickResult, @"The picked media should not be sent before dismissal completes.");

dismissCompletion();

XCTAssertEqual(pickResult.count, 1);
XCTAssertGreaterThan(pickResult.firstObject.length, 0);
}

- (void)testCameraPickerInteractionBlockerWindowIsAddedAndRemoved {
id mockUIImagePicker = OCMClassMock([UIImagePickerController class]);
id mockAVCaptureDevice = OCMClassMock([AVCaptureDevice class]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,19 +533,26 @@ - (void)picker:(PHPickerViewController *)picker

- (void)imagePickerController:(UIImagePickerController *)picker
didFinishPickingMediaWithInfo:(NSDictionary<NSString *, id> *)info {
NSURL *videoURL = info[UIImagePickerControllerMediaURL];
FLTImagePickerMethodCallContext *context = self.callContext;
__weak typeof(self) weakSelf = self;
[picker dismissViewControllerAnimated:YES
completion:^{
[weakSelf removeInteractionBlocker];
if (weakSelf.callContext == context) {
[weakSelf sendCallResultWithPickerInfo:info];
}
}];
}
Comment on lines 534 to +545

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Deferring the call to sendCallResultWithPickerInfo: to the dismissal completion block introduces a race condition. During the dismissal animation (which takes about 300ms), a new platform channel request can be initiated. If a second request is made, cancelInProgressCall will cancel the first request and overwrite self.callContext with the new request's context. When the first request's dismissal completion block finally runs, it will deliver the first request's image to the second request's context.

To prevent this, capture the callContext at the moment the picker finishes, and only deliver the result if the captured context matches the current active context.

- (void)imagePickerController:(UIImagePickerController *)picker
    didFinishPickingMediaWithInfo:(NSDictionary<NSString *, id> *)info {
  __weak typeof(self) weakSelf = self;
  FLTImagePickerMethodCallContext *context = self.callContext;
  [picker dismissViewControllerAnimated:YES
                             completion:^{
                               [weakSelf removeInteractionBlocker];
                               if (context == weakSelf.callContext) {
                                 [weakSelf sendCallResultWithPickerInfo:info];
                               }
                             }];
}

Comment on lines 534 to +545

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The temporary file pointed to by UIImagePickerControllerMediaURL is deleted by the system as soon as the delegate method imagePickerController:didFinishPickingMediaWithInfo: returns.

Because this PR defers the execution of sendCallResultWithPickerInfo: (which copies the video file) to the asynchronous completion block of dismissViewControllerAnimated:completion:, the delegate method will have already returned, and the temporary video file will have been deleted by the time the completion block runs. This will cause video picking to consistently fail with a Could not cache the video file error.

To fix this, we should copy the video file immediately within the delegate method before it returns, and then pass the copied destination path to the completion block. We should also use the strong-weak dance (strongSelf) inside the completion block to ensure the plugin instance is not deallocated mid-execution.

- (void)imagePickerController:(UIImagePickerController *)picker
    didFinishPickingMediaWithInfo:(NSDictionary<NSString *, id> *)info {
  FLTImagePickerMethodCallContext *context = self.callContext;
  __weak typeof(self) weakSelf = self;

  NSURL *videoURL = info[UIImagePickerControllerMediaURL];
  if (videoURL != nil) {
    NSURL *destination = nil;
    if (@available(iOS 13.0, *)) {
      destination = [FLTImagePickerPhotoAssetUtil saveVideoFromURL:videoURL];
    }
    [picker dismissViewControllerAnimated:YES
                               completion:^{
                                 FLTImagePickerPlugin *strongSelf = weakSelf;
                                 if (!strongSelf) {
                                   return;
                                 }
                                 [strongSelf removeInteractionBlocker];
                                 if (strongSelf.callContext == context) {
                                   if (destination == nil) {
                                     [strongSelf sendCallResultWithError:[FlutterError
                                                                           errorWithCode:"flutter_image_picker_copy_video_error"
                                                                                 message:"Could not cache the video file."
                                                                                 details:nil]];
                                   } else {
                                     [strongSelf sendCallResultWithSavedPathList:@[ destination.path ]];
                                   }
                                 }
                               }];
  } else {
    [picker dismissViewControllerAnimated:YES
                               completion:^{
                                 FLTImagePickerPlugin *strongSelf = weakSelf;
                                 if (!strongSelf) {
                                   return;
                                 }
                                 [strongSelf removeInteractionBlocker];
                                 if (strongSelf.callContext == context) {
                                   [strongSelf sendCallResultWithPickerInfo:info];
                                 }
                               }];
  }
}


- (void)sendCallResultWithPickerInfo:(NSDictionary<NSString *, id> *)info {
// The method dismissViewControllerAnimated does not immediately prevent
// further didFinishPickingMediaWithInfo invocations. A nil check is necessary
// to prevent below code to be unwantly executed multiple times and cause a
// crash.
if (!self.callContext) {
return;
}
NSURL *videoURL = info[UIImagePickerControllerMediaURL];
if (videoURL != nil) {
if (@available(iOS 13.0, *)) {
NSURL *destination = [FLTImagePickerPhotoAssetUtil saveVideoFromURL:videoURL];
Expand Down Expand Up @@ -624,12 +631,15 @@ - (void)imagePickerController:(UIImagePickerController *)picker
}

- (void)imagePickerControllerDidCancel:(UIImagePickerController *)picker {
FLTImagePickerMethodCallContext *context = self.callContext;
__weak typeof(self) weakSelf = self;
[picker dismissViewControllerAnimated:YES
completion:^{
[weakSelf removeInteractionBlocker];
if (weakSelf.callContext == context) {
[weakSelf sendCallResultWithSavedPathList:nil];
}
}];
[self sendCallResultWithSavedPathList:nil];
}
Comment on lines 636 to 643

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similarly to the selection callback, deferring the cancellation result to the dismissal completion block introduces a race condition where a subsequent request's context could be incorrectly cancelled if it is initiated during the dismissal animation.

Capture the callContext before dismissal and verify it matches the active context in the completion block.

Suggested change
[picker dismissViewControllerAnimated:YES
completion:^{
[weakSelf removeInteractionBlocker];
[weakSelf sendCallResultWithSavedPathList:nil];
}];
[self sendCallResultWithSavedPathList:nil];
}
FLTImagePickerMethodCallContext *context = self.callContext;
[picker dismissViewControllerAnimated:YES
completion:^{
[weakSelf removeInteractionBlocker];
if (context == weakSelf.callContext) {
[weakSelf sendCallResultWithSavedPathList:nil];
}
}];
}

Comment on lines 633 to 643

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use the strong-weak dance (strongSelf) inside the completion block to ensure the plugin instance is not deallocated mid-execution.

- (void)imagePickerControllerDidCancel:(UIImagePickerController *)picker {
  FLTImagePickerMethodCallContext *context = self.callContext;
  __weak typeof(self) weakSelf = self;
  [picker dismissViewControllerAnimated:YES
                             completion:^{
                               FLTImagePickerPlugin *strongSelf = weakSelf;
                               if (!strongSelf) {
                                 return;
                               }
                               [strongSelf removeInteractionBlocker];
                               if (strongSelf.callContext == context) {
                                 [strongSelf sendCallResultWithSavedPathList:nil];
                               }
                             }];
}


#pragma mark -
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker_ios/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: image_picker_ios
description: iOS implementation of the image_picker plugin.
repository: https://github.com/flutter/packages/tree/main/packages/image_picker/image_picker_ios
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22
version: 0.8.13+6
version: 0.8.13+7

environment:
sdk: ^3.10.0
Expand Down
Loading