Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker] Migrate iOS to Pigeon and improve state handling #5285

Merged
fluttergithubbot merged 10 commits into from
Apr 22, 2022
Merged

[image_picker] Migrate iOS to Pigeon and improve state handling #5285

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8961c9b
Switch to internal copy of method channel implementation
stuartmorgan-g Apr 12, 2022
761d926
Remove Android-only functions
stuartmorgan-g Apr 13, 2022
99f75e0
Temporarily point to local pigeon
stuartmorgan-g Apr 14, 2022
9950255
Add Pigeon API, and update Dart to use it
stuartmorgan-g Apr 14, 2022
ebb4d84
Update ObjC implementation to use Pigeon
stuartmorgan-g Apr 15, 2022
8fd936a
Autoformat generated Pigeon code
stuartmorgan-g Apr 15, 2022
c4e2963
Reference Pigeon 3.0.2, not yet published
stuartmorgan-g Apr 15, 2022
f429c60
Version bump
stuartmorgan-g Apr 15, 2022
49e175c
Standardize Dart helper names
stuartmorgan-g Apr 18, 2022
fe9980c
Review comments
stuartmorgan-g Apr 22, 2022
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
Prev Previous commit
Review comments
  • Loading branch information
@stuartmorgan-g
stuartmorgan-g committed Apr 22, 2022
commit fe9980c680e9eba2cafbf3fe4d699a0c2f018f17
4 changes: 2 additions & 2 deletions packages/image_picker/image_picker_ios/example/ios/RunnerTests/ImagePickerPluginTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ - (void)testPluginMultiImagePathHasNullItem {
pickImageResult = error;
dispatch_semaphore_signal(resultSemaphore);
}];
[plugin returnSavedPathList:@[ [NSNull null] ]];
[plugin sendCallResultWithSavedPathList:@[ [NSNull null] ]];

dispatch_semaphore_wait(resultSemaphore, DISPATCH_TIME_FOREVER);

Expand All @@ -256,7 +256,7 @@ - (void)testPluginMultiImagePathHasItem {
pickImageResult = result;
dispatch_semaphore_signal(resultSemaphore);
}];
[plugin returnSavedPathList:pathList];
[plugin sendCallResultWithSavedPathList:pathList];

dispatch_semaphore_wait(resultSemaphore, DISPATCH_TIME_FOREVER);

Expand Down
83 changes: 41 additions & 42 deletions packages/image_picker/image_picker_ios/ios/Classes/FLTImagePickerPlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ - (void)launchUIImagePickerWithSource:(nonnull FLTSourceSpecification *)source
[self checkPhotoAuthorizationWithImagePicker:imagePickerController];
break;
default:
[self returnError:[FlutterError errorWithCode:@"invalid_source"
message:@"Invalid image source."
details:nil]];
[self sendCallResultWithError:[FlutterError errorWithCode:@"invalid_source"
message:@"Invalid image source."
details:nil]];
break;
}
}
Expand Down Expand Up @@ -237,9 +237,9 @@ - (void)pickVideoWithSource:(nonnull FLTSourceSpecification *)source
[self checkPhotoAuthorizationWithImagePicker:imagePickerController];
break;
default:
[self returnError:[FlutterError errorWithCode:@"invalid_source"
message:@"Invalid video source."
details:nil]];
[self sendCallResultWithError:[FlutterError errorWithCode:@"invalid_source"
message:@"Invalid video source."
details:nil]];
break;
}
}
Expand All @@ -254,9 +254,9 @@ - (void)pickVideoWithSource:(nonnull FLTSourceSpecification *)source
*/
- (void)cancelInProgressCall {
if (self.callContext) {
[self returnError:[FlutterError errorWithCode:@"multiple_request"
message:@"Cancelled by a second request"
details:nil]];
[self sendCallResultWithError:[FlutterError errorWithCode:@"multiple_request"
message:@"Cancelled by a second request"
details:nil]];
self.callContext = nil;
}
}
Expand Down Expand Up @@ -291,7 +291,7 @@ - (void)showCamera:(UIImagePickerControllerCameraDevice)device
[[self viewControllerWithWindow:nil] presentViewController:cameraErrorAlert
animated:YES
completion:nil];
[self returnSavedPathList:nil];
[self sendCallResultWithSavedPathList:nil];
}
}

Expand Down Expand Up @@ -388,31 +388,35 @@ - (void)checkPhotoAuthorizationForAccessLevel API_AVAILABLE(ios(14)) {
- (void)errorNoCameraAccess:(AVAuthorizationStatus)status {
switch (status) {
case AVAuthorizationStatusRestricted:
[self returnError:[FlutterError errorWithCode:@"camera_access_restricted"
message:@"The user is not allowed to use the camera."
details:nil]];
[self sendCallResultWithError:[FlutterError
errorWithCode:@"camera_access_restricted"
message:@"The user is not allowed to use the camera."
details:nil]];
break;
case AVAuthorizationStatusDenied:
default:
[self returnError:[FlutterError errorWithCode:@"camera_access_denied"
message:@"The user did not allow camera access."
details:nil]];
[self sendCallResultWithError:[FlutterError
errorWithCode:@"camera_access_denied"
message:@"The user did not allow camera access."
details:nil]];
break;
}
}

- (void)errorNoPhotoAccess:(PHAuthorizationStatus)status {
switch (status) {
case PHAuthorizationStatusRestricted:
[self returnError:[FlutterError errorWithCode:@"photo_access_restricted"
message:@"The user is not allowed to use the photo."
details:nil]];
[self sendCallResultWithError:[FlutterError
errorWithCode:@"photo_access_restricted"
message:@"The user is not allowed to use the photo."
details:nil]];
break;
case PHAuthorizationStatusDenied:
default:
[self returnError:[FlutterError errorWithCode:@"photo_access_denied"
message:@"The user did not allow photo access."
details:nil]];
[self sendCallResultWithError:[FlutterError
errorWithCode:@"photo_access_denied"
message:@"The user did not allow photo access."
details:nil]];
break;
}
}
Expand Down Expand Up @@ -445,7 +449,7 @@ - (NSNumber *)getDesiredImageQuality:(NSNumber *)imageQuality {
#pragma mark - UIAdaptivePresentationControllerDelegate

- (void)presentationControllerDidDismiss:(UIPresentationController *)presentationController {
[self returnSavedPathList:nil];
[self sendCallResultWithSavedPathList:nil];
}

#pragma mark - PHPickerViewControllerDelegate
Expand All @@ -454,7 +458,7 @@ - (void)picker:(PHPickerViewController *)picker
didFinishPicking:(NSArray<PHPickerResult *> *)results API_AVAILABLE(ios(14)) {
[picker dismissViewControllerAnimated:YES completion:nil];
if (results.count == 0) {
[self returnSavedPathList:nil];
[self sendCallResultWithSavedPathList:nil];
return;
}
dispatch_queue_t backgroundQueue =
Expand All @@ -481,7 +485,7 @@ - (void)picker:(PHPickerViewController *)picker
}
[operationQueue waitUntilAllOperationsAreFinished];
dispatch_async(dispatch_get_main_queue(), ^{
[self returnSavedPathList:pathList];
[self sendCallResultWithSavedPathList:pathList];
});
});
}
Expand Down Expand Up @@ -529,16 +533,17 @@ - (void)imagePickerController:(UIImagePickerController *)picker
[[NSFileManager defaultManager] copyItemAtURL:videoURL toURL:destination error:&error];

if (error) {
[self returnError:[FlutterError errorWithCode:@"flutter_image_picker_copy_video_error"
message:@"Could not cache the video file."
details:nil]];
[self sendCallResultWithError:[FlutterError
errorWithCode:@"flutter_image_picker_copy_video_error"
message:@"Could not cache the video file."
details:nil]];
return;
}
}
videoURL = destination;
}
}
[self returnSavedPathList:@[ videoURL.path ]];
[self sendCallResultWithSavedPathList:@[ videoURL.path ]];
} else {
UIImage *image = info[UIImagePickerControllerEditedImage];
if (image == nil) {
Expand Down Expand Up @@ -580,7 +585,7 @@ - (void)imagePickerController:(UIImagePickerController *)picker

- (void)imagePickerControllerDidCancel:(UIImagePickerController *)picker {
[picker dismissViewControllerAnimated:YES completion:nil];
[self returnSavedPathList:nil];
[self sendCallResultWithSavedPathList:nil];
}

#pragma mark -
Expand All @@ -596,7 +601,7 @@ - (void)saveImageWithOriginalImageData:(NSData *)originalImageData
maxWidth:maxWidth
maxHeight:maxHeight
imageQuality:imageQuality];
[self returnSavedPathList:@[ savedPath ]];
[self sendCallResultWithSavedPathList:@[ savedPath ]];
}

- (void)saveImageWithPickerInfo:(NSDictionary *)info
Expand All @@ -605,16 +610,10 @@ - (void)saveImageWithPickerInfo:(NSDictionary *)info
NSString *savedPath = [FLTImagePickerPhotoAssetUtil saveImageWithPickerInfo:info
image:image
imageQuality:imageQuality];
[self returnSavedPathList:@[ savedPath ]];
[self sendCallResultWithSavedPathList:@[ savedPath ]];
}

/**
* Validates the provided paths list, then returns it as the result of the original method call,
* clearing the in-progress call state.
*
* @param pathList The paths to return. nil indicates a cancelled operation.
*/
- (void)returnSavedPathList:(nullable NSArray *)pathList {
- (void)sendCallResultWithSavedPathList:(nullable NSArray *)pathList {
if (!self.callContext) {
Copy link
Member

@gaaclarke gaaclarke Apr 18, 2022

Choose a reason for hiding this comment

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

Should we silently fail in these cases? This seems like a logical error if we get here, no?

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Apr 18, 2022

Choose a reason for hiding this comment

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

Probably; this falls under the umbrella of leaving most of the code flows the same. I didn't want to risk introducing a new client-visible error here if there's a latent logic bug in the unchanged overall flow that this has been masking.

Fixing the use of ivars will eliminate the need for this method in the first place, so it'll get fixed then.

return;
}
Expand All @@ -630,12 +629,12 @@ - (void)returnSavedPathList:(nullable NSArray *)pathList {
}

/**
* Returns the given error as the result of the original method call, clearing the in-progress
* call state.
* Sends the given error via `callContext.result` as the result of the original platform channel
* method call, clearing the in-progress call state.
*
* @param error The error to return.
*/
- (void)returnError:(FlutterError *)error {
- (void)sendCallResultWithError:(FlutterError *)error {
if (!self.callContext) {
return;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/image_picker/image_picker_ios/ios/Classes/FLTImagePickerPlugin_Test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ typedef void (^FlutterResultAdapter)(NSArray<NSString *> *_Nullable, FlutterErro
@property(strong, nonatomic, nullable) FLTImagePickerMethodCallContext *callContext;

/**
* Validates the provided paths list, then returns it as the result of the original method call,
* clearing the in-progress call state.
* Validates the provided paths list, then sends it via `callContext.result` as the result of the
* original platform channel method call, clearing the in-progress call state.
*
* @param pathList The paths to return. Nil is allowed to indicated a cancelled operation.
* @param pathList The paths to return. nil indicates a cancelled operation.
*/
- (void)returnSavedPathList:(nullable NSArray *)pathList;
- (void)sendCallResultWithSavedPathList:(nullable NSArray *)pathList;

/**
* Tells the delegate that the user cancelled the pick operation.
Expand Down