-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[image_picker_ios] infinite await in picking methods when native picker is closed quickly #10460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,40 @@ - (instancetype)initWithResult:(nonnull FlutterResultAdapter)result { | |
| } | ||
| @end | ||
|
|
||
| /** | ||
| * a callback function what the PickerViewController remove from window. | ||
| */ | ||
| typedef void (^FLTImagePickerRemoveCallback)(void); | ||
|
|
||
| /** | ||
| * Add the view to the PickerViewController's view, observing its window to observe the window of PickerViewController. | ||
| * This is to prevent PickerViewController from being removed from the screen without receiving callback information under other circumstances, | ||
| * such as being interactively dismissed before PickerViewController has fully popped up. | ||
| */ | ||
|
Comment on lines
30
to
40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The documentation for /**
* A callback that is invoked when the picker view controller is removed from the window.
*/
typedef void (^FLTImagePickerRemoveCallback)(void);
/**
* A view that uses `didMoveToWindow` to observe when it has been removed from a window.
*
* This is added to the picker's view hierarchy to detect when the picker has been dismissed
* in cases where the standard delegate methods are not called (e.g., a quick interactive
* dismissal).
*/ |
||
| @interface FLTImagePickerRemoveObserverView : UIView | ||
|
|
||
| @property(nonatomic, copy, nonnull) FLTImagePickerRemoveCallback removeCallback; | ||
|
|
||
| -(instancetype)initWithRemoveCallback:(FLTImagePickerRemoveCallback)callback; | ||
|
|
||
| @end | ||
|
|
||
| @implementation FLTImagePickerRemoveObserverView | ||
|
|
||
| - (instancetype)initWithRemoveCallback:(FLTImagePickerRemoveCallback)callback{ | ||
|
||
| if (self = [super init]) { | ||
| self.removeCallback = callback; | ||
| } | ||
| return self; | ||
| } | ||
| - (void)didMoveToWindow { | ||
| if (!self.window) { | ||
| [self removeFromSuperview]; | ||
| [[NSOperationQueue mainQueue]addOperationWithBlock:self.removeCallback]; | ||
| } | ||
| } | ||
| @end | ||
|
|
||
| #pragma mark - | ||
|
|
||
| @interface FLTImagePickerPlugin () | ||
|
|
@@ -114,6 +148,7 @@ - (void)launchPHPickerWithContext:(nonnull FLTImagePickerMethodCallContext *)con | |
| pickerViewController.presentationController.delegate = self; | ||
| self.callContext = context; | ||
|
|
||
| [self bindRemoveObserver:pickerViewController context:context]; | ||
| [self showPhotoLibraryWithPHPicker:pickerViewController]; | ||
| } | ||
|
|
||
|
|
@@ -137,6 +172,7 @@ - (void)launchUIImagePickerWithSource:(nonnull FLTSourceSpecification *)source | |
|
|
||
| self.callContext = context; | ||
|
|
||
| [self bindRemoveObserver:imagePickerController context:context]; | ||
| switch (source.type) { | ||
| case FLTSourceTypeCamera: | ||
| [self checkCameraAuthorizationWithImagePicker:imagePickerController | ||
|
|
@@ -157,6 +193,24 @@ - (void)launchUIImagePickerWithSource:(nonnull FLTSourceSpecification *)source | |
| } | ||
| } | ||
|
|
||
| - (void)bindRemoveObserver:(nonnull UIViewController *)controller | ||
| context:(nonnull FLTImagePickerMethodCallContext *)context { | ||
| __weak typeof(self) weakSelf = self; | ||
| FLTImagePickerRemoveObserverView *removeObserverView = | ||
| [[FLTImagePickerRemoveObserverView alloc]initWithRemoveCallback:^{ | ||
| __strong typeof(weakSelf) strongSelf = weakSelf; | ||
| // Add a small delay to ensure delegate methods have a chance to run first | ||
| // This prevents the observer from firing during normal selection flow | ||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ | ||
| if(strongSelf && strongSelf.callContext == context && !strongSelf.isProcessingSelection) { | ||
|
||
| // Only send result if context is still active and we're not processing a selection | ||
| [strongSelf sendCallResultWithSavedPathList:nil]; | ||
| } | ||
| }); | ||
| }]; | ||
| [controller.view addSubview:removeObserverView]; | ||
| } | ||
|
|
||
| #pragma mark - FLTImagePickerApi | ||
|
|
||
| - (void)pickImageWithSource:(nonnull FLTSourceSpecification *)source | ||
|
|
@@ -475,6 +529,8 @@ - (void)picker:(PHPickerViewController *)picker | |
| [self sendCallResultWithSavedPathList:nil]; | ||
| return; | ||
| } | ||
| // Mark that we're processing a selection to prevent observer from interfering | ||
| self.isProcessingSelection = YES; | ||
| __block NSOperationQueue *saveQueue = [[NSOperationQueue alloc] init]; | ||
| saveQueue.name = @"Flutter Save Image Queue"; | ||
| saveQueue.qualityOfService = NSQualityOfServiceUserInitiated; | ||
|
|
@@ -496,6 +552,8 @@ - (void)picker:(PHPickerViewController *)picker | |
| } else { | ||
| [weakSelf sendCallResultWithSavedPathList:pathList]; | ||
| } | ||
| // Clear the processing flag after sending result | ||
| weakSelf.isProcessingSelection = NO; | ||
| // Retain queue until here. | ||
| saveQueue = nil; | ||
| }]; | ||
|
|
@@ -659,6 +717,8 @@ - (void)sendCallResultWithSavedPathList:(nullable NSArray *)pathList { | |
| self.callContext.result(pathList ?: [NSArray array], nil); | ||
| } | ||
| self.callContext = nil; | ||
| // Reset processing flag | ||
| self.isProcessingSelection = NO; | ||
| } | ||
|
|
||
| /// Sends the given error via `callContext.result` as the result of the original platform channel | ||
|
|
@@ -671,6 +731,8 @@ - (void)sendCallResultWithError:(FlutterError *)error { | |
| } | ||
| self.callContext.result(nil, error); | ||
| self.callContext = nil; | ||
| // Reset processing flag | ||
| self.isProcessingSelection = NO; | ||
| } | ||
|
|
||
| @end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test manually creates a
removeCallbackthat duplicates the implementation logic frombindRemoveObserver:. This makes the test brittle; if the implementation changes, this test would need to be updated manually and might not fail correctly. It also doesn't test the newFLTImagePickerRemoveObserverViewclass and itsdidMoveToWindowtrigger.A better approach would be to write this as an integration test that calls one of the
pick...methods to set up the real observer, and then simulates the dismissal to trigger it. This would provide more robust testing of the actual code path.