Skip to content

[camera_avfoundation] ios saving path#10832

Merged
auto-submit[bot] merged 15 commits intoflutter:mainfrom
ivan-vanyusho:ios_saving_path
Feb 4, 2026
Merged

[camera_avfoundation] ios saving path#10832
auto-submit[bot] merged 15 commits intoflutter:mainfrom
ivan-vanyusho:ios_saving_path

Conversation

@ivan-vanyusho
Copy link
Contributor

@ivan-vanyusho ivan-vanyusho commented Jan 20, 2026

Description

Changed iOS file storage path from documentDirectory to temporaryDirectory to ensure consistency with Android where files are stored in cache directory.

Changes

  • iOS now uses temporaryDirectory instead of documentDirectory for consistency with Android

Code changes

// Before:
let documentDirectory = FileManager.default.urls(
      for: .documentDirectory,
      in: .userDomainMask)[0]

// After:
let documentDirectory = FileManager.default.temporaryDirectory

Issues

flutter/flutter#166933

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.

Footnotes

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

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly changes the iOS file storage path from documentDirectory to temporaryDirectory to align with Android's behavior, improving cross-platform consistency. I have one suggestion to improve code clarity by renaming a variable that became misleading due to this change.

@stuartmorgan-g
Copy link
Collaborator

Thanks for the submission! This will need the remaining items in the checklist addressed before it moves forward with review. If you have any questions about completing those steps that aren’t addressed in the linked documentation, please let us know.

@stuartmorgan-g stuartmorgan-g marked this pull request as draft January 20, 2026 12:54
@ivan-vanyusho ivan-vanyusho marked this pull request as ready for review January 22, 2026 14:08
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request updates the iOS camera plugin to store captured pictures and videos in the system's temporaryDirectory instead of the application's documentDirectory. This change aligns the iOS behavior with Android's cache directory usage. The CHANGELOG.md has been updated to reflect this as a breaking change, and the pubspec.yaml version has been incremented accordingly.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

This still needs tests before it can be reviewed, per the checklist.

@ivan-vanyusho ivan-vanyusho marked this pull request as ready for review January 26, 2026 14:13
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully updates the iOS file storage path for camera pictures and videos from the application's documentDirectory to the temporaryDirectory, aligning with Android's cache directory usage for consistency. The change is well-supported by a new test case that verifies the updated storage location. The CHANGELOG.md and pubspec.yaml files have been updated appropriately.

}

func testCaptureToFile_mustSavePhotoToCameraPicturesDirectory() {
let expectedPath = "/tmp/camera/pictures/"

Choose a reason for hiding this comment

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

medium

The expectedPath is hardcoded as "/tmp/camera/pictures/". While this is correct for the current implementation, it could be more robust to derive this path dynamically within the test, perhaps by calling the same utility function that constructs the path in DefaultCamera.swift (if exposed or mockable), or by inspecting FileManager.default.temporaryDirectory and appending the known subpaths. This would make the test less brittle if the subfolder structure changes in the future.

@stuartmorgan-g stuartmorgan-g added the triage-ios Should be looked at in iOS triage label Jan 27, 2026
Copy link
Contributor

@hellohuanlin hellohuanlin left a comment

Choose a reason for hiding this comment

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

iOS now uses temporaryDirectory instead of documentDirectory for consistency with Android

I don't see why we want this "consistency". Apple's recommendation is to use documentDirectory for user generated files (so that it could be exported when connected to mac).

To fix the broken file, we could either (1) save to temp directory first, and move to document directory once done, or (2) save to document directory, and delete if error happens.

@LouiseHsu
Copy link
Contributor

iOS now uses temporaryDirectory instead of documentDirectory for consistency with Android

I don't see why we want this "consistency". Apple's recommendation is to use documentDirectory for user generated files (so that it could be exported when connected to mac).

To fix the broken file, we could either (1) save to temp directory first, and move to document directory once done, or (2) save to document directory, and delete if error happens.

to be fair, the method the change is in is literally called getTemporaryFilePath. It does seem misleading for that method to originally return the Documents folder which is persistent instead of the tmp folder.

@hellohuanlin
Copy link
Contributor

to be fair, the method the change is in is literally called getTemporaryFilePath. It does seem misleading for that method to originally return the Documents folder which is persistent instead of the tmp folder.

Good catch @LouiseHsu. I missed this part. If it's intended to save to temp path, then we should use temp path.

Copy link
Contributor

@LouiseHsu LouiseHsu left a comment

Choose a reason for hiding this comment

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

LGTM. I worry a little that this may unintentionally break certain implementations for people who were relying on the videos being incorrectly saved to the documents folder, but its probably fine lol.

@hellohuanlin
Copy link
Contributor

I worry a little that this may unintentionally break certain implementations for people who were relying on the videos being incorrectly saved to the documents folder, but its probably fine lol.

The path (documentDirectory/camera/pictures/) isn't public API so unlikely developers would use it.

@LouiseHsu
Copy link
Contributor

sorry, looks like there are some conflicts. once you resolve those we can merge :)

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Feb 4, 2026

@LouiseHsu FYI, changelog and version conflicts are usually trivial for us to resolve in the GitHub UI.

@hellohuanlin If you are adding the final approval for a non-team PR, and there are no follow-up changes to make, please add autosubmit at that point.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 4, 2026
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 4, 2026
@auto-submit
Copy link
Contributor

auto-submit bot commented Feb 4, 2026

autosubmit label was removed for flutter/packages/10832, because This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan-g}, please make the needed changes and resubmit this PR.
The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 4, 2026
@auto-submit auto-submit bot merged commit 3bddf2c into flutter:main Feb 4, 2026
80 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 4, 2026
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 4, 2026
flutter/packages@5b1bea8...3bddf2c

2026-02-04 ivan-vanyusho@yandex-team.ru [camera_avfoundation] ios saving
path (flutter/packages#10832)
2026-02-04 stuartmorgan@google.com [video_player] Remove OCMock
(flutter/packages#10932)
2026-02-04 stuartmorgan@google.com [google_maps_flutter] Remove use of
OCMock (flutter/packages#10863)
2026-02-04 32538273+ValentinVignal@users.noreply.github.com
[go_router_builder] Add support for `@TypedGoRouteParameter` to
customize parameter names (flutter/packages#10793)
2026-02-03 engine-flutter-autoroll@skia.org Roll Flutter from
c305f1f to bf701fe (9 revisions) (flutter/packages#10957)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com 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:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LongCatIsLooong pushed a commit to LongCatIsLooong/flutter that referenced this pull request Feb 6, 2026
…r#181918)

flutter/packages@5b1bea8...3bddf2c

2026-02-04 ivan-vanyusho@yandex-team.ru [camera_avfoundation] ios saving
path (flutter/packages#10832)
2026-02-04 stuartmorgan@google.com [video_player] Remove OCMock
(flutter/packages#10932)
2026-02-04 stuartmorgan@google.com [google_maps_flutter] Remove use of
OCMock (flutter/packages#10863)
2026-02-04 32538273+ValentinVignal@users.noreply.github.com
[go_router_builder] Add support for `@TypedGoRouteParameter` to
customize parameter names (flutter/packages#10793)
2026-02-03 engine-flutter-autoroll@skia.org Roll Flutter from
c305f1f to bf701fe (9 revisions) (flutter/packages#10957)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com 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:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter-zl pushed a commit to flutter-zl/flutter that referenced this pull request Feb 10, 2026
…r#181918)

flutter/packages@5b1bea8...3bddf2c

2026-02-04 ivan-vanyusho@yandex-team.ru [camera_avfoundation] ios saving
path (flutter/packages#10832)
2026-02-04 stuartmorgan@google.com [video_player] Remove OCMock
(flutter/packages#10932)
2026-02-04 stuartmorgan@google.com [google_maps_flutter] Remove use of
OCMock (flutter/packages#10863)
2026-02-04 32538273+ValentinVignal@users.noreply.github.com
[go_router_builder] Add support for `@TypedGoRouteParameter` to
customize parameter names (flutter/packages#10793)
2026-02-03 engine-flutter-autoroll@skia.org Roll Flutter from
c305f1f to bf701fe (9 revisions) (flutter/packages#10957)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com 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:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
rickhohler pushed a commit to rickhohler/flutter that referenced this pull request Feb 19, 2026
…r#181918)

flutter/packages@5b1bea8...3bddf2c

2026-02-04 ivan-vanyusho@yandex-team.ru [camera_avfoundation] ios saving
path (flutter/packages#10832)
2026-02-04 stuartmorgan@google.com [video_player] Remove OCMock
(flutter/packages#10932)
2026-02-04 stuartmorgan@google.com [google_maps_flutter] Remove use of
OCMock (flutter/packages#10863)
2026-02-04 32538273+ValentinVignal@users.noreply.github.com
[go_router_builder] Add support for `@TypedGoRouteParameter` to
customize parameter names (flutter/packages#10793)
2026-02-03 engine-flutter-autoroll@skia.org Roll Flutter from
c305f1f to bf701fe (9 revisions) (flutter/packages#10957)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com 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:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-ios platform-macos triage-ios Should be looked at in iOS triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments