Skip to content

Commit

Permalink
[image_picker] Prevent multiple active calls on iOS (flutter#5272)
Browse files Browse the repository at this point in the history
The image picker plugin's implementation doesn't currently handle multiple calls correctly due to the use of an ivar to track the response object; the original entry point handles that by cancelling earlier requests when new ones come in, but as we added more entry points we didn't replicate that logic. This adds it to all picker entry points. (Longer term, we should instead handle multiple concurrent calls, but this is consistent with historical behavior, and is better than having some `await`s on the Dart side never return as can happen now.)

The newer PHPicker code path not only didn't cancel, but used an ivar for the picker view controller, which in some cases could result in the same controller being presented multiple times, crashing the app (see referenced issue). While the new cancel calls will prevent that case from happening, to prevent anything similar from happening in the future this removes the ivar entirely, since we can just pass the controller to the necessary methods (as is already being done with the `UIImagePickerController` paths).

Fixes flutter#108900
  • Loading branch information
stuartmorgan authored Oct 31, 2023
1 parent b236d83 commit c9fec61
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 20 deletions.
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.8+3

* Fixes a possible crash when calling a picker method while another is waiting on permissions.

## 0.8.8+2

* Adds pub topics to package metadata.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,4 +531,83 @@ - (void)testPickImageAuthorizationDenied API_AVAILABLE(ios(14)) {
[self waitForExpectationsWithTimeout:30 handler:nil];
}

- (void)testPickMultiImageDuplicateCallCancels API_AVAILABLE(ios(14)) {
id mockPhotoLibrary = OCMClassMock([PHPhotoLibrary class]);
OCMStub([mockPhotoLibrary authorizationStatusForAccessLevel:PHAccessLevelReadWrite])
.andReturn(PHAuthorizationStatusNotDetermined);
OCMExpect([mockPhotoLibrary requestAuthorizationForAccessLevel:PHAccessLevelReadWrite
handler:OCMOCK_ANY]);

FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];

XCTestExpectation *firstCallExpectation = [self expectationWithDescription:@"first call"];
[plugin pickMultiImageWithMaxSize:[FLTMaxSize makeWithWidth:@100 height:@100]
quality:nil
fullMetadata:@YES
completion:^(NSArray<NSString *> *result, FlutterError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.code, @"multiple_request");
[firstCallExpectation fulfill];
}];
[plugin pickMultiImageWithMaxSize:[FLTMaxSize makeWithWidth:@100 height:@100]
quality:nil
fullMetadata:@YES
completion:^(NSArray<NSString *> *result, FlutterError *error){
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}

- (void)testPickMediaDuplicateCallCancels API_AVAILABLE(ios(14)) {
id mockPhotoLibrary = OCMClassMock([PHPhotoLibrary class]);
OCMStub([mockPhotoLibrary authorizationStatusForAccessLevel:PHAccessLevelReadWrite])
.andReturn(PHAuthorizationStatusNotDetermined);
OCMExpect([mockPhotoLibrary requestAuthorizationForAccessLevel:PHAccessLevelReadWrite
handler:OCMOCK_ANY]);

FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];

FLTMediaSelectionOptions *options =
[FLTMediaSelectionOptions makeWithMaxSize:[FLTMaxSize makeWithWidth:@(100) height:@(200)]
imageQuality:@(50)
requestFullMetadata:@YES
allowMultiple:@YES];
XCTestExpectation *firstCallExpectation = [self expectationWithDescription:@"first call"];
[plugin pickMediaWithMediaSelectionOptions:options
completion:^(NSArray<NSString *> *result, FlutterError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.code, @"multiple_request");
[firstCallExpectation fulfill];
}];
[plugin pickMediaWithMediaSelectionOptions:options
completion:^(NSArray<NSString *> *result, FlutterError *error){
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}

- (void)testPickVideoDuplicateCallCancels API_AVAILABLE(ios(14)) {
id mockPhotoLibrary = OCMClassMock([PHPhotoLibrary class]);
OCMStub([mockPhotoLibrary authorizationStatusForAccessLevel:PHAccessLevelReadWrite])
.andReturn(PHAuthorizationStatusNotDetermined);
OCMExpect([mockPhotoLibrary requestAuthorizationForAccessLevel:PHAccessLevelReadWrite
handler:OCMOCK_ANY]);

FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];

FLTSourceSpecification *source = [FLTSourceSpecification makeWithType:FLTSourceTypeCamera
camera:FLTSourceCameraRear];
XCTestExpectation *firstCallExpectation = [self expectationWithDescription:@"first call"];
[plugin pickVideoWithSource:source
maxDuration:nil
completion:^(NSString *result, FlutterError *error) {
XCTAssertNotNil(error);
XCTAssertEqualObjects(error.code, @"multiple_request");
[firstCallExpectation fulfill];
}];
[plugin pickVideoWithSource:source
maxDuration:nil
completion:^(NSString *result, FlutterError *error){
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ - (instancetype)initWithResult:(nonnull FlutterResultAdapter)result {

@interface FLTImagePickerPlugin ()

/**
* The PHPickerViewController instance used to pick multiple
* images.
*/
@property(strong, nonatomic) PHPickerViewController *pickerViewController API_AVAILABLE(ios(14));

/**
* The UIImagePickerController instances that will be used when a new
* controller would normally be created. Each call to
Expand Down Expand Up @@ -117,15 +111,16 @@ - (void)launchPHPickerWithContext:(nonnull FLTImagePickerMethodCallContext *)con
config.filter = [PHPickerFilter imagesFilter];
}

_pickerViewController = [[PHPickerViewController alloc] initWithConfiguration:config];
_pickerViewController.delegate = self;
_pickerViewController.presentationController.delegate = self;
PHPickerViewController *pickerViewController =
[[PHPickerViewController alloc] initWithConfiguration:config];
pickerViewController.delegate = self;
pickerViewController.presentationController.delegate = self;
self.callContext = context;

if (context.requestFullMetadata) {
[self checkPhotoAuthorizationForAccessLevel];
[self checkPhotoAuthorizationWithPHPicker:pickerViewController];
} else {
[self showPhotoLibraryWithPHPicker:_pickerViewController];
[self showPhotoLibraryWithPHPicker:pickerViewController];
}
}

Expand Down Expand Up @@ -201,6 +196,7 @@ - (void)pickMultiImageWithMaxSize:(nonnull FLTMaxSize *)maxSize
fullMetadata:(NSNumber *)fullMetadata
completion:(nonnull void (^)(NSArray<NSString *> *_Nullable,
FlutterError *_Nullable))completion {
[self cancelInProgressCall];
FLTImagePickerMethodCallContext *context =
[[FLTImagePickerMethodCallContext alloc] initWithResult:completion];
context.maxSize = maxSize;
Expand All @@ -220,6 +216,7 @@ - (void)pickMultiImageWithMaxSize:(nonnull FLTMaxSize *)maxSize
- (void)pickMediaWithMediaSelectionOptions:(nonnull FLTMediaSelectionOptions *)mediaSelectionOptions
completion:(nonnull void (^)(NSArray<NSString *> *_Nullable,
FlutterError *_Nullable))completion {
[self cancelInProgressCall];
FLTImagePickerMethodCallContext *context =
[[FLTImagePickerMethodCallContext alloc] initWithResult:completion];
context.maxSize = [mediaSelectionOptions maxSize];
Expand All @@ -244,6 +241,7 @@ - (void)pickVideoWithSource:(nonnull FLTSourceSpecification *)source
maxDuration:(nullable NSNumber *)maxDurationSeconds
completion:
(nonnull void (^)(NSString *_Nullable, FlutterError *_Nullable))completion {
[self cancelInProgressCall];
FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc]
initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) {
if (paths.count > 1) {
Expand Down Expand Up @@ -393,7 +391,8 @@ - (void)checkPhotoAuthorizationWithImagePicker:(UIImagePickerController *)imageP
}
}

- (void)checkPhotoAuthorizationForAccessLevel API_AVAILABLE(ios(14)) {
- (void)checkPhotoAuthorizationWithPHPicker:(PHPickerViewController *)pickerViewController
API_AVAILABLE(ios(14)) {
PHAccessLevel requestedAccessLevel = PHAccessLevelReadWrite;
PHAuthorizationStatus status =
[PHPhotoLibrary authorizationStatusForAccessLevel:requestedAccessLevel];
Expand All @@ -404,13 +403,9 @@ - (void)checkPhotoAuthorizationForAccessLevel API_AVAILABLE(ios(14)) {
handler:^(PHAuthorizationStatus status) {
dispatch_async(dispatch_get_main_queue(), ^{
if (status == PHAuthorizationStatusAuthorized) {
[self
showPhotoLibraryWithPHPicker:self->
_pickerViewController];
[self showPhotoLibraryWithPHPicker:pickerViewController];
} else if (status == PHAuthorizationStatusLimited) {
[self
showPhotoLibraryWithPHPicker:self->
_pickerViewController];
[self showPhotoLibraryWithPHPicker:pickerViewController];
} else {
[self errorNoPhotoAccess:status];
}
Expand All @@ -420,7 +415,7 @@ - (void)checkPhotoAuthorizationForAccessLevel API_AVAILABLE(ios(14)) {
}
case PHAuthorizationStatusAuthorized:
case PHAuthorizationStatusLimited:
[self showPhotoLibraryWithPHPicker:_pickerViewController];
[self showPhotoLibraryWithPHPicker:pickerViewController];
break;
case PHAuthorizationStatusDenied:
case PHAuthorizationStatusRestricted:
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.8+2
version: 0.8.8+3

environment:
sdk: ">=2.19.0 <4.0.0"
Expand Down

0 comments on commit c9fec61

Please sign in to comment.