[image_picker] Fix exception when canceling pickMultipleMedia on iOS (

#4761) Makes the Pigeon interface for the iOS implementation consistent in returning non-nullable lists, in keeping with our strong preference in this repo for collections being non-nullable whenever reasonable (to avoid the issue of having two ways of expressing the same thing), and updates the native side never to return a `nil` list. Fixes flutter/flutter#133111
flutter · Aug 23, 2023 · 32460c7 · 32460c7
1 parent c0136f9
commit 32460c7
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 30 deletions.
diff --git a/packages/image_picker/image_picker_ios/CHANGELOG.md b/packages/image_picker/image_picker_ios/CHANGELOG.md
@@ -1,5 +1,6 @@
-## NEXT
+## 0.8.8+1
 
+* Fixes exception when canceling pickMultipleMedia.
 * Updates minimum supported SDK version to Flutter 3.7/Dart 2.19.
 
 ## 0.8.8

diff --git a/packages/image_picker/image_picker_ios/example/ios/RunnerTests/ImagePickerPluginTests.m b/packages/image_picker/image_picker_ios/example/ios/RunnerTests/ImagePickerPluginTests.m
@@ -360,6 +360,20 @@ - (void)testPluginMediaPathHasNoItem {
   [self waitForExpectationsWithTimeout:30 handler:nil];
 }
 
+- (void)testPluginMediaPathConvertsNilToEmptyList {
+  FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];
+
+  XCTestExpectation *resultExpectation = [self expectationWithDescription:@"result"];
+  plugin.callContext = [[FLTImagePickerMethodCallContext alloc]
+      initWithResult:^(NSArray<NSString *> *_Nullable result, FlutterError *_Nullable error) {
+        XCTAssertEqualObjects(result, @[]);
+        [resultExpectation fulfill];
+      }];
+  [plugin sendCallResultWithSavedPathList:nil];
+
+  [self waitForExpectationsWithTimeout:30 handler:nil];
+}
+
 - (void)testPluginMediaPathHasItem {
   FLTImagePickerPlugin *plugin = [[FLTImagePickerPlugin alloc] init];
   NSArray *pathList = @[ @"test" ];

diff --git a/packages/image_picker/image_picker_ios/ios/Classes/FLTImagePickerPlugin.m b/packages/image_picker/image_picker_ios/ios/Classes/FLTImagePickerPlugin.m
@@ -173,7 +173,7 @@ - (void)pickImageWithSource:(nonnull FLTSourceSpecification *)source
   [self cancelInProgressCall];
   FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc]
       initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) {
-        if (paths && paths.count != 1) {
+        if (paths.count > 1) {
           completion(nil, [FlutterError errorWithCode:@"invalid_result"
                                               message:@"Incorrect number of return paths provided"
                                               details:nil]);
@@ -246,7 +246,7 @@ - (void)pickVideoWithSource:(nonnull FLTSourceSpecification *)source
                      (nonnull void (^)(NSString *_Nullable, FlutterError *_Nullable))completion {
   FLTImagePickerMethodCallContext *context = [[FLTImagePickerMethodCallContext alloc]
       initWithResult:^void(NSArray<NSString *> *paths, FlutterError *error) {
-        if (paths && paths.count != 1) {
+        if (paths.count > 1) {
           completion(nil, [FlutterError errorWithCode:@"invalid_result"
                                               message:@"Incorrect number of return paths provided"
                                               details:nil]);
@@ -687,7 +687,7 @@ - (void)sendCallResultWithSavedPathList:(nullable NSArray *)pathList {
                                                      message:@"pathList's items should not be null"
                                                      details:nil]);
   } else {
-    self.callContext.result(pathList, nil);
+    self.callContext.result(pathList ?: [NSArray array], nil);
   }
   self.callContext = nil;
 }

diff --git a/packages/image_picker/image_picker_ios/lib/image_picker_ios.dart b/packages/image_picker/image_picker_ios/lib/image_picker_ios.dart
@@ -88,7 +88,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
     double? maxHeight,
     int? imageQuality,
   }) async {
-    final List<dynamic>? paths = await _pickMultiImageAsPath(
+    final List<dynamic> paths = await _pickMultiImageAsPath(
       options: MultiImagePickerOptions(
         imageOptions: ImageOptions(
           maxWidth: maxWidth,
@@ -97,7 +97,9 @@ class ImagePickerIOS extends ImagePickerPlatform {
         ),
       ),
     );
-    if (paths == null) {
+    // Convert an empty list to a null return since that was the legacy behavior
+    // of this method.
+    if (paths.isEmpty) {
       return null;
     }
 
@@ -108,15 +110,11 @@ class ImagePickerIOS extends ImagePickerPlatform {
   Future<List<XFile>> getMultiImageWithOptions({
     MultiImagePickerOptions options = const MultiImagePickerOptions(),
   }) async {
-    final List<String>? paths = await _pickMultiImageAsPath(options: options);
-    if (paths == null) {
-      return <XFile>[];
-    }
-
+    final List<String> paths = await _pickMultiImageAsPath(options: options);
     return paths.map((String path) => XFile(path)).toList();
   }
 
-  Future<List<String>?> _pickMultiImageAsPath({
+  Future<List<String>> _pickMultiImageAsPath({
     MultiImagePickerOptions options = const MultiImagePickerOptions(),
   }) async {
     final int? imageQuality = options.imageOptions.imageQuality;
@@ -141,7 +139,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
             MaxSize(width: maxWidth, height: maxHeight),
             imageQuality,
             options.imageOptions.requestFullMetadata))
-        ?.cast<String>();
+        .cast<String>();
   }
 
   Future<String?> _pickImageAsPath({
@@ -272,7 +270,7 @@ class ImagePickerIOS extends ImagePickerPlatform {
     double? maxHeight,
     int? imageQuality,
   }) async {
-    final List<String>? paths = await _pickMultiImageAsPath(
+    final List<String> paths = await _pickMultiImageAsPath(
       options: MultiImagePickerOptions(
         imageOptions: ImageOptions(
           maxWidth: maxWidth,
@@ -281,7 +279,9 @@ class ImagePickerIOS extends ImagePickerPlatform {
         ),
       ),
     );
-    if (paths == null) {
+    // Convert an empty list to a null return since that was the legacy behavior
+    // of this method.
+    if (paths.isEmpty) {
       return null;
     }
 

diff --git a/packages/image_picker/image_picker_ios/lib/src/messages.g.dart b/packages/image_picker/image_picker_ios/lib/src/messages.g.dart
@@ -179,7 +179,7 @@ class ImagePickerApi {
     }
   }
 
-  Future<List<String?>?> pickMultiImage(MaxSize arg_maxSize,
+  Future<List<String?>> pickMultiImage(MaxSize arg_maxSize,
       int? arg_imageQuality, bool arg_requestFullMetadata) async {
     final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
         'dev.flutter.pigeon.ImagePickerApi.pickMultiImage', codec,
@@ -198,8 +198,13 @@ class ImagePickerApi {
         message: replyList[1] as String?,
         details: replyList[2],
       );
+    } else if (replyList[0] == null) {
+      throw PlatformException(
+        code: 'null-error',
+        message: 'Host platform returned null value for non-null return value.',
+      );
     } else {
-      return (replyList[0] as List<Object?>?)?.cast<String?>();
+      return (replyList[0] as List<Object?>?)!.cast<String?>();
     }
   }
 

diff --git a/packages/image_picker/image_picker_ios/pigeons/messages.dart b/packages/image_picker/image_picker_ios/pigeons/messages.dart
@@ -54,7 +54,7 @@ abstract class ImagePickerApi {
       int? imageQuality, bool requestFullMetadata);
   @async
   @ObjCSelector('pickMultiImageWithMaxSize:quality:fullMetadata:')
-  List<String>? pickMultiImage(
+  List<String?> pickMultiImage(
       MaxSize maxSize, int? imageQuality, bool requestFullMetadata);
   @async
   @ObjCSelector('pickVideoWithSource:maxDuration:')

diff --git a/packages/image_picker/image_picker_ios/pubspec.yaml b/packages/image_picker/image_picker_ios/pubspec.yaml
@@ -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
+version: 0.8.8+1
 
 environment:
   sdk: ">=2.19.0 <4.0.0"

diff --git a/packages/image_picker/image_picker_ios/test/image_picker_ios_test.dart b/packages/image_picker/image_picker_ios/test/image_picker_ios_test.dart
@@ -57,7 +57,7 @@ class _ApiLogger implements TestHostImagePickerApi {
   }
 
   @override
-  Future<List<String?>?> pickMultiImage(
+  Future<List<String?>> pickMultiImage(
     MaxSize maxSize,
     int? imageQuality,
     bool requestFullMetadata,
@@ -68,7 +68,7 @@ class _ApiLogger implements TestHostImagePickerApi {
       'imageQuality': imageQuality,
       'requestFullMetadata': requestFullMetadata,
     }));
-    return returnValue as List<String?>?;
+    return returnValue as List<String?>;
   }
 
   @override
@@ -441,8 +441,8 @@ void main() {
       );
     });
 
-    test('handles a null image path response gracefully', () async {
-      log.returnValue = null;
+    test('returns null for an empty list', () async {
+      log.returnValue = <String>[];
 
       expect(await picker.pickMultiImage(), isNull);
     });
@@ -883,11 +883,10 @@ void main() {
       );
     });
 
-    test('handles a null image path response gracefully', () async {
-      log.returnValue = null;
+    test('returns null for an empty list', () async {
+      log.returnValue = <String>[];
 
       expect(await picker.getMultiImage(), isNull);
-      expect(await picker.getMultiImage(), isNull);
     });
   });
 
@@ -1643,8 +1642,8 @@ void main() {
       );
     });
 
-    test('handles a null image path response gracefully', () async {
-      log.returnValue = null;
+    test('handles an empty response', () async {
+      log.returnValue = <String>[];
 
       expect(await picker.getMultiImageWithOptions(), isEmpty);
     });

diff --git a/packages/image_picker/image_picker_ios/test/test_api.g.dart b/packages/image_picker/image_picker_ios/test/test_api.g.dart
@@ -54,7 +54,7 @@ abstract class TestHostImagePickerApi {
   Future<String?> pickImage(SourceSpecification source, MaxSize maxSize,
       int? imageQuality, bool requestFullMetadata);
 
-  Future<List<String?>?> pickMultiImage(
+  Future<List<String?>> pickMultiImage(
       MaxSize maxSize, int? imageQuality, bool requestFullMetadata);
 
   Future<String?> pickVideo(
@@ -117,7 +117,7 @@ abstract class TestHostImagePickerApi {
           final bool? arg_requestFullMetadata = (args[2] as bool?);
           assert(arg_requestFullMetadata != null,
               'Argument for dev.flutter.pigeon.ImagePickerApi.pickMultiImage was null, expected non-null bool.');
-          final List<String?>? output = await api.pickMultiImage(
+          final List<String?> output = await api.pickMultiImage(
               arg_maxSize!, arg_imageQuality, arg_requestFullMetadata!);
           return <Object?>[output];
         });