[image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing by victogomez-cs · Pull Request #13079 · flutter/packages

victogomez-cs · 2026-09-30T16:57:56Z

Adds an injectable FIPImageDataRequesting seam so plugin tests can supply image data without OCMock. The production implementation, FIPDefaultImageDataRequester, forwards to PHImageManager. Migrates ImagePickerPluginTests to Swift Testing and removes OCMock.

Plugin behavior is unchanged. Bumps image_picker_ios to 0.8.13+10.

PR 2/5 of the Obj-C → Swift migration. Part of flutter/flutter#119107

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

gemini-code-assist

Code Review

This pull request migrates the iOS image picker plugin's unit tests from Objective-C (using OCMock) to Swift (using the Swift Testing framework), deleting ImagePickerPluginTests.m and ImagePickerTestImages.m in favor of ImagePickerPluginTests.swift and ImagePickerTestImages.swift. To support this migration, a new FIPImageDataRequesting protocol and its default implementation FIPDefaultImageDataRequester were introduced to allow injecting image data requests during testing. Feedback on these changes highlights a critical compatibility issue where calling requestImageDataAndOrientationForAsset:options:resultHandler: directly in FIPPickerSeams.m will crash on iOS 12 or older, requiring an @available check. Additionally, it is recommended to avoid force-casting UIWindowScene in the tests to prevent potential crashes in headless environments.

gemini-code-assist · 2026-09-30T16:59:16Z

+- (void)requestImageDataAndOrientationForAsset:(PHAsset *)asset
+                                       options:(nullable PHImageRequestOptions *)options
+                                 resultHandler:
+                                     (void (^)(NSData *_Nullable imageData,
+                                               NSString *_Nullable dataUTI,
+                                               CGImagePropertyOrientation orientation,
+                                               NSDictionary *_Nullable info))resultHandler {
+  [[PHImageManager defaultManager] requestImageDataAndOrientationForAsset:asset
+                                                                  options:options
+                                                            resultHandler:resultHandler];
+}


The method requestImageDataAndOrientationForAsset:options:resultHandler: is only available on iOS 13.0 and later. Calling it directly on iOS 12 or older will result in a runtime crash (unrecognized selector sent to instance). Since the plugin still supports iOS 12, we must wrap this call in an @available(iOS 13.0, *) check and provide a fallback using the deprecated requestImageDataForAsset:options:resultHandler: method.

- (void)requestImageDataAndOrientationForAsset:(PHAsset *)asset options:(nullable PHImageRequestOptions *)options resultHandler: (void (^)(NSData *_Nullable imageData, NSString *_Nullable dataUTI, CGImagePropertyOrientation orientation, NSDictionary *_Nullable info))resultHandler { if (@available(iOS 13.0, *)) { [[PHImageManager defaultManager] requestImageDataAndOrientationForAsset:asset options:options resultHandler:resultHandler]; } else { #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" [[PHImageManager defaultManager] requestImageDataForAsset:asset options:options resultHandler:^(NSData *_Nullable imageData, NSString *_Nullable dataUTI, UIImageOrientation orientation, NSDictionary *_Nullable info) { resultHandler(imageData, dataUTI, kCGImagePropertyOrientationUp, info); }]; #pragma clang diagnostic pop } }

The plugin minimum is iOS 13 (image_picker_ios.podspec and Package.swift), and requestImageDataAndOrientationForAsset: is available starting then. The iOS 12 fallback is not needed, and it would also report kCGImagePropertyOrientationUp instead of the asset orientation

gemini-code-assist · 2026-09-30T16:59:16Z

+  @Test func cameraPickerInteractionBlockerWindowIsAddedAndRemoved() {
+    let (_, camera, permissions) = pluginWithAuthorizedCamera()
+    let scene = UIApplication.shared.connectedScenes.first as! UIWindowScene


Force-casting UIApplication.shared.connectedScenes.first as UIWindowScene using as! can cause a crash if the test is run in a headless environment or if no scenes are connected. Using Swift Testing's #require with an optional cast as? is much safer and will fail the test gracefully instead of crashing the test runner process.

Suggested change

@Test func cameraPickerInteractionBlockerWindowIsAddedAndRemoved() {

let (_, camera, permissions) = pluginWithAuthorizedCamera()

let scene = UIApplication.shared.connectedScenes.first as! UIWindowScene

@Test func cameraPickerInteractionBlockerWindowIsAddedAndRemoved() throws {

let (_, camera, permissions) = pluginWithAuthorizedCamera()

let scene = try #require(UIApplication.shared.connectedScenes.first as? UIWindowScene)

Done. The scene is now try #require(UIApplication.shared.connectedScenes.first as? UIWindowScene), so a missing scene fails the test instead of crashing the runner

LongCatIsLooong

LGTM overall but I have a few questions regarding some test implementations.

LongCatIsLooong · 2026-10-08T17:46:14Z

+
+  @Test func pluginMultiImagePathHasNullItem() async {
+    let plugin = FLTImagePickerPlugin(viewProvider: StubViewProvider())
+    await confirmation("result") { confirmed in


This works because the callback is actually invoked synchronously?

Nothing wrong with the approach, but it might be easier for readers to understand by introducing a helper so this becomes synchronous.

Yes. sendCallResultWithSavedPathList: invokes the context callback on the calling thread. Those five tests now go through captureSendResult, which returns the callback arguments directly

LongCatIsLooong · 2026-10-08T17:51:28Z

+    _ items: [any PickerItem], using plugin: FLTImagePickerPlugin
+  ) async -> (paths: [String]?, error: FlutterError?, onMainThread: Bool) {
+    let picker = PHPickerViewController(configuration: PHPickerConfiguration())
+    nonisolated(unsafe) var received: [String]?


nit: both confirmation and withCheckedContinuation allow you to return a value no? You can just return those values via continuation.resume.

Or you can mark the FLTImagePickerMethodCallContext callback @MainActor then this helper function can just be sync.

The callback now resumes the continuation with (paths, error, onMainThread). Marking the callback @MainActor would not make the helper synchronous: processPickerItems saves on a background queue and delivers the result later on the main queue

LongCatIsLooong · 2026-10-08T17:51:53Z

+    nonisolated(unsafe) var received: [String]?
+    nonisolated(unsafe) var receivedError: FlutterError?
+    nonisolated(unsafe) var onMainThread = false
+    await confirmation("result") { confirmed in


Why is this confirmation needed? There's already withCheckedContinuation?

Removed. The continuation is what waits for that later callback

LongCatIsLooong · 2026-10-08T18:02:09Z

+    await confirmation("result") { confirmed in
+      await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
+        plugin.callContext = FLTImagePickerMethodCallContext { result, error in
+          onMainThread = Thread.isMainThread


I looked at the callsite of this helper, this should always be true. Is it possible to just #expect this.

Agree. #expect(onMainThread) is now in the helper, after the continuation returns. The three callers no longer check it. #expect inside the callback is not recorded on this test, because that block runs on the main queue outside the test task

LongCatIsLooong · 2026-10-08T18:11:24Z

+  @Test func pickImageInvalidResultWhenMultiplePathsReturned() async {
+    let plugin = FLTImagePickerPlugin(viewProvider: StubViewProvider())
+    var completionCount = 0
+    await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in


Why checkedContinuation here instead of confirmation?

Both completions run synchronously inside sendCallResult, so this is now confirmation(expectedCount: 2). The video test uses the same pattern

LongCatIsLooong · 2026-10-08T18:28:35Z

+      fullMetadata: true
+    ) { _, _ in }
+    await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
+      DispatchQueue.main.async {


Why? Is this for introducing a delay?

This waits for the main-queue hop in checkCameraAuthorizationWithImagePicker:. The fake requestAccess handler runs on the calling thread, then the plugin dispatch_asyncs showCamera: onto the main queue. The continuation resumes from that block so the source-type check runs after the picker is configured. confirmation returns as soon as its body returns, which is before that block runs

Ah do you mean the completion handler isn't invoked in that path? It sounds like a bug but the original test seems to be doing that too.

The completion is not called when access is granted and the camera is presented. showCamera:withImagePicker: only configures and presents the picker. The Dart call stays open until the picker delegate runs, which is the same as the other presentation paths. The denied path is the one that completes immediately. The original test left this completion empty and checked sourceType on the next main-queue turn, so I kept that

LongCatIsLooong · 2026-10-08T18:31:36Z

+    {
+      return loaded
+    }
+    return Data(base64Encoded: fallback) ?? Data()


nit: prefer test utilities to fail as fast as possible so maybe force unwrap here?

Done. A bad fixture now traps on Data(base64Encoded: fallback)!

…rop OCMock.

…ol seams and update tests. This update introduces an injectable protocol for image data requests, allowing for better testability and flexibility in the `FLTImagePickerPlugin`. The `FIPDefaultImageDataRequester` is implemented to forward requests to `PHImageManager`. Additionally, the test suite has been expanded to include new tests for image data handling, ensuring robust coverage for various scenarios, including error handling and metadata retrieval.

…ol seam addition. This commit updates the CHANGELOG to reflect the addition of an injectable protocol seam for PHImageManager image data requests in version 0.8.13+10. The version number in pubspec.yaml has also been incremented accordingly.

…dowScene.

… reduce async complexity.

…xecution.

LongCatIsLooong

Still LGTM (free feel to put the autosubmit label on)!

LongCatIsLooong · 2026-10-12T22:53:37Z

+
+// These tests share UIApplication, windows, and the main queue. Running them
+// together deadlocks a test that is waiting for a main-queue callback.
+@Suite(.serialized)


Was this added because some tests were async?

No. The suite is @MainActor, and several tests wait for a callback on the main queue. Running those together deadlocked the runner in CI: the iOS shard restarted and then recorded 0 tests. .serialized keeps them from overlapping. Async by itself was not the problem

LongCatIsLooong · 2026-10-12T22:56:04Z

+  }
+
+  @Test func pickMultiImageShouldUseUIImagePickerControllerOnPreiOS14() {
+    if #available(iOS 14, *) {


nit: would it be possible to use https://developer.apple.com/documentation/testing/definingtests#Limit-the-availability-of-a-test instead?

Done. Both pre-iOS 14 tests are now @available(iOS, obsoleted: 14, message: "PHPicker is used on iOS 14 and later."). Swift Testing skips them on iOS 14 and later instead of returning at the start of the body. pickMediaShouldUseUIImagePickerControllerOnPreiOS14 uses the same attribute

That form failed to compile in CI. RunnerTests is built for iOS 15, and the @Test macro still calls the function, so a function obsoleted in iOS 14 is unavailable. Both tests now use .enabled(if: ProcessInfo.processInfo.operatingSystemVersion.majorVersion < 14, "PHPicker is used on iOS 14 and later."). Swift Testing skips them on iOS 14 and later

LongCatIsLooong · 2026-10-12T23:40:38Z

+      fullMetadata: true
+    ) { _, _ in }
+    await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
+      DispatchQueue.main.async {


Ah do you mean the completion handler isn't invoked in that path? It sounds like a bug but the original test seems to be doing that too.

…4 and later.

…tests for pre-iOS 14 compatibility.

Resolves conflicts with flutter#12267, flutter#13079, and flutter#13224: - Moves this change to 0.8.13+13, above main's 0.8.13+12 entry. - Keeps main's removal of the kUTTypeGIF fallback in the GIF writer and this change's NULL destination check. - Ports the two UIImagePickerController save-failure tests to the Swift plugin tests, using the PHImageManager seam instead of OCMock. - Replaces the undecodable-data PHPicker test, which flutter#12267 now rejects earlier, with one that reaches the save failure through maxWidth 0. - Drops the test bridging header, which main removed: the test subclass now replaces +temporaryFilePath: by selector, and the GIF destination test goes through the public save method.

Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 [email protected] [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 [email protected] [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 [email protected] [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 [email protected] [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 [email protected] [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 [email protected] [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 [email protected] [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 [email protected] [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 [email protected] [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 [email protected] Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 [email protected] Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 [email protected] [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 [email protected] Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 [email protected] [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 [email protected] [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 [email protected] Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 [email protected] [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 [email protected] [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 [email protected] [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 [email protected] [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 [email protected] Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 [email protected] [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 [email protected] [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 [email protected] [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 [email protected] [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 [email protected] [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 [email protected] FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 [email protected] [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 [email protected] [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 [email protected] [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 [email protected] [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 [email protected] [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 [email protected] [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

flutter-dashboard Bot added the CICD Run CI/CD label Sep 30, 2026

gemini-code-assist Bot reviewed Sep 30, 2026

View reviewed changes

victogomez-cs added the triage-ios Should be looked at in iOS triage label Sep 30, 2026

github-actions Bot added p: image_picker platform-ios labels Sep 30, 2026

cbracken requested review from LongCatIsLooong and okorohelijah October 1, 2026 21:56

LongCatIsLooong approved these changes Oct 8, 2026

View reviewed changes

victogomez-cs added 5 commits October 9, 2026 09:46

[image_picker_ios] Migrate plugin native tests to Swift Testing and d…

8c4d878

…rop OCMock.

[image_picker_ios] Update camera picker test to handle optional UIWin…

cac47b9

…dowScene.

[image_picker_ios] Refactor image picker tests to improve clarity and…

96f5a69

… reduce async complexity.

victogomez-cs force-pushed the pr2/image-picker-ios-plugin-tests branch from cc90433 to 96f5a69 Compare October 9, 2026 15:55

[image_picker_ios] Serialize test suite to prevent deadlocks during e…

3ea92e1

…xecution.

jmagman mentioned this pull request Oct 12, 2026

[ci] Increase deprecation check minimum to iOS 16 #13211

Merged

11 tasks done

LongCatIsLooong approved these changes Oct 12, 2026

View reviewed changes

victogomez-cs added 2 commits October 13, 2026 09:44

[image_picker_ios] Update image picker tests to use PHPicker on iOS 1…

8db1642

…4 and later.

[image_picker_ios] Update image picker tests to conditionally enable …

64880b4

…tests for pre-iOS 14 compatibility.

victogomez-cs added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 13, 2026

auto-submit Bot merged commit 312bdb5 into flutter:main Oct 13, 2026
13 checks passed

victogomez-cs deleted the pr2/image-picker-ios-plugin-tests branch October 13, 2026 16:32

engine-flutter-autoroll mentioned this pull request Oct 14, 2026

Roll Packages from a1ed23c8f760 to ac2db9cfeaf6 (12 revisions) flutter/flutter#194387

Closed

technobibi mentioned this pull request Oct 15, 2026

[image_picker_ios] Return an error instead of an empty file when an image cannot be saved #12998

Open

11 tasks done

engine-flutter-autoroll mentioned this pull request Oct 15, 2026

Roll Packages from a1ed23c8f760 to da40eaf4548a (33 revisions) flutter/flutter#194459

Merged

Conversation

victogomez-cs commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

victogomez-cs commented Sep 30, 2026 •

edited

Loading