[path_provider] Remove dependency on engine PathUtils#11467
Conversation
Removes the calls to the Flutter engine's `PathUtils`, and replaces them with direct `Context` calls that have the same logic as the expected codepath for those methods so that there will be continuity with previous releases of `path_provider`. This avoids reliance on `PathUtils` not being removed by R8, and also means that engine changes can't unexpectedly change the paths returned by `path_provider`, since we want to make sure any such changes in this package's return values follow semver. This does not include the fallback logic from flutter/engine#30367 because unlike the engine callers, `path_provider` doesn't try to assert a non-null return value, and if the paths aren't available (e.g., due to the device being full) we should return null rather than silently return a different directory than was previously returned. Fixes flutter/flutter#184750
|
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. |
|
test-exempt: code refactor with no semantic change |
There was a problem hiding this comment.
Code Review
This pull request updates path_provider_android to version 2.3.1, removing the dependency on PathUtils to prevent potential ClassNotFoundException issues in release mode. The implementation of getApplicationSupportPath and getApplicationDocumentsPath now uses direct JNI calls to Context methods instead of the utility class, including explicit memory management for the returned JNI objects. I have no feedback to provide.
Removes the calls to the Flutter engine's
PathUtils, and replaces them with directContextcalls that have the same logic as the expected codepath for those methods so that there will be continuity with previous releases ofpath_provider. This avoids reliance onPathUtilsnot being removed by R8, and also means that engine changes can't unexpectedly change the paths returned bypath_provider, since we want to make sure any such changes in this package's return values follow semver.This does not include the fallback logic from
flutter/engine#30367 because unlike the engine callers,
path_providerdoesn't try to assert a non-null return value, and if the paths aren't available (e.g., due to the device being full) we should return null rather than silently return a different directory than was previously returned.Fixes flutter/flutter#184750
Pre-Review Checklist
[shared_preferences]///).Footnotes
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