Migrate deeplink json creation to public AGP api #173794

reidbaker · 2025-08-14T19:16:49Z

Reviewers please pay special attention to the tests that were added and tests that were removed. If you see a set of functionality that is not covered and should be please say something.

I read every test (and TBH also had to edit most of them) but I had gemini's agent mode help and I dont trust there is something I missed.

Related to #173651

Newly added tests can be run from packages/flutter_tools/gradle with ./gradlew test --tests com.flutter.gradle.tasks.DeepLinkJsonFromManifestTaskTest

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

…ut of utils into task

… and add unit tests

gemini-code-assist

Code Review

This pull request does a great job of migrating the deeplink JSON creation to the modern AGP Variant API. The refactoring into a dedicated task and helper class, along with the new unit tests, significantly improves the maintainability and robustness of this feature. However, I've identified a critical issue where the logic for copying Flutter assets appears to have been unintentionally commented out, which would likely break asset packaging. Please see the detailed comment.

packages/flutter_tools/gradle/src/main/kotlin/FlutterPlugin.kt

reidbaker · 2025-08-15T17:36:45Z

@gemini-code-assist[bot] please review this PR

reidbaker · 2025-08-15T17:37:40Z

/gemini review

gemini-code-assist

Code Review

This pull request successfully migrates the deeplink JSON creation to the modern public Android Gradle Plugin (AGP) APIs, which is a significant improvement for maintainability. The introduction of DeepLinkJsonFromManifestTask and its helper class provides a clean separation of concerns, and the new tests are thorough. My review includes suggestions to enhance the new helper class by addressing some TODOs for efficiency, ensuring deterministic build outputs, and adhering to Kotlin style conventions.

gemini-code-assist · 2025-08-15T17:38:21Z

packages/flutter_tools/gradle/src/main/kotlin/tasks/DeepLinkJsonFromManifestTaskHelper.kt

+                    schemes.forEach { scheme ->
+                        hosts.forEach { host ->
+                            paths.forEach { path ->
+                                appLinkSettings.deeplinks.add(
+                                    Deeplink(
+                                        scheme,
+                                        host,
+                                        path,
+                                        intentFilterCheck
+                                    )
+                                )
+                            }
+                        }
+                    }
+                }


The comment on line 164 correctly points out that iterating over unordered sets can be problematic. The MutableSet relies on insertion order, which can be non-deterministic when parsing XML attributes. This can lead to non-deterministic JSON output, affecting build caching and testability. To ensure deterministic builds, you should sort the collections before iterating over them.

schemes.sortedBy { it ?: "" }.forEach { scheme -> hosts.sortedBy { it ?: "" }.forEach { host -> paths.sortedBy { it ?: "" }.forEach { path -> appLinkSettings.deeplinks.add( Deeplink( scheme, host, path, intentFilterCheck ) ) } } }

gemini-code-assist · 2025-08-15T17:38:21Z

packages/flutter_tools/gradle/src/main/kotlin/tasks/DeepLinkJsonFromManifestTaskHelper.kt

+     *     ]
+     * }
+     */
+    public fun createAppLinkSettingsFile(


The public modifier is redundant here as it's the default visibility in Kotlin. Removing it aligns with the Android Kotlin Style Guide and makes the code cleaner.¹

Suggested change

public fun createAppLinkSettingsFile(

fun createAppLinkSettingsFile(

Style Guide References

Footnotes

The provided style guide specifies that Kotlin code should follow the Android Kotlin Style Guide, which recommends omitting the public modifier as it is the default visibility. (link) ↩

gemini-code-assist · 2025-08-15T17:38:21Z

packages/flutter_tools/gradle/src/main/kotlin/tasks/DeepLinkJsonFromManifestTaskHelper.kt

+                actionItems.forEach { action ->
+                    if (action.attribute(MANIFEST_NAME_KEY) == "android.intent.action.VIEW") {
+                        intentFilterCheck.hasActionView = true
+                    }
+                }


This forEach loop iterates through all actionItems even if the desired action is found early. Using any is more efficient because it short-circuits the iteration. It also makes the code more concise and idiomatic, and addresses the TODO comment.

intentFilterCheck.hasActionView = actionItems.any { action -> action.attribute(MANIFEST_NAME_KEY) == "android.intent.action.VIEW" }

gemini-code-assist · 2025-08-15T17:38:21Z

packages/flutter_tools/gradle/src/main/kotlin/tasks/DeepLinkJsonFromManifestTaskHelper.kt

+                categoryItems.forEach { category ->
+                    // TODO: Exit out early per intent filter default category.
+                    if (category.attribute(MANIFEST_NAME_KEY) == "android.intent.category.DEFAULT") {
+                        intentFilterCheck.hasDefaultCategory = true
+                    }
+                    // TODO: Exit out early per intent filter browsable category.
+                    if (category.attribute(MANIFEST_NAME_KEY) == "android.intent.category.BROWSABLE") {
+                        intentFilterCheck.hasBrowsableCategory =
+                            true
+                    }
+                }


This forEach loop can be made more efficient and readable. By using any for each category check, the code becomes more idiomatic and benefits from early exit once a match is found. This change addresses the TODO comments regarding early exits.

intentFilterCheck.hasDefaultCategory = categoryItems.any { it.attribute(MANIFEST_NAME_KEY) == "android.intent.category.DEFAULT" } intentFilterCheck.hasBrowsableCategory = categoryItems.any { it.attribute(MANIFEST_NAME_KEY) == "android.intent.category.BROWSABLE" }

gemini-code-assist

Code Review

This pull request is a great improvement, successfully migrating the deeplink JSON creation to use modern, public AGP APIs. The refactoring into a dedicated DeepLinkJsonFromManifestTask and a DeepLinkJsonFromManifestTaskHelper improves code organization and maintainability. The new tests in DeepLinkJsonFromManifestTaskTest.kt are thorough and cover various manifest scenarios.

I have one suggestion in DeepLinkJsonFromManifestTaskHelper.kt to address a pre-existing issue with non-deterministic output, which will improve the robustness of this new task.

gemini-code-assist · 2025-08-15T17:39:50Z

packages/flutter_tools/gradle/src/main/kotlin/tasks/DeepLinkJsonFromManifestTaskHelper.kt

+                    // Sets are not ordered this could produce a bug.
+                    schemes.forEach { scheme ->
+                        hosts.forEach { host ->
+                            paths.forEach { path ->
+                                appLinkSettings.deeplinks.add(
+                                    Deeplink(
+                                        scheme,
+                                        host,
+                                        path,
+                                        intentFilterCheck
+                                    )
+                                )
+                            }
+                        }
+                    }


The comment on line 164 correctly identifies a potential issue. The use of forEach on schemes, hosts, and paths can lead to non-deterministic output in the generated JSON file. This is because the insertion order into these sets depends on the attribute order in the AndroidManifest.xml, which is not guaranteed to be stable.

To ensure deterministic output, you should sort these collections before iterating over them. This will make the task's output predictable and prevent potential issues with caching or tests that rely on stable file content. You may need to add import kotlin.comparisons.naturalOrder and import kotlin.comparisons.nullsFirst.

Suggested change

// Sets are not ordered this could produce a bug.

schemes.forEach { scheme ->

hosts.forEach { host ->

paths.forEach { path ->

appLinkSettings.deeplinks.add(

Deeplink(

scheme,

host,

path,

intentFilterCheck

)

)

}

}

}

// Sort sets to ensure deterministic output, as attribute order in XML is not guaranteed.

schemes.sortedWith(nullsFirst(naturalOrder())).forEach { scheme ->

hosts.sortedWith(nullsFirst(naturalOrder())).forEach { host ->

paths.sortedWith(nullsFirst(naturalOrder())).forEach { path ->

appLinkSettings.deeplinks.add(

Deeplink(

scheme,

host,

path,

intentFilterCheck

)

)

}

}

}

gmackall

LGMT. The gemini comments about switching to any vs foreach seem reasonable to me BTW.

I tested that the deeplink cookbook from https://docs.flutter.dev/cookbook/navigation/set-up-app-links still works properly. Have you tested any more involved examples? Just asking because this seems

like a fragile area of the code
I don't know how well it is tested.

reidbaker · 2025-08-18T19:04:47Z

LGMT. The gemini comments about switching to any vs foreach seem reasonable to me BTW.

I tested that the deeplink cookbook from https://docs.flutter.dev/cookbook/navigation/set-up-app-links still works properly. Have you tested any more involved examples? Just asking because this seems

like a fragile area of the code

I don't know how well it is tested.

I authored integration tests here

flutter/packages/flutter_tools/test/integration.shard/android_gradle_outputs_app_link_settings_test.dart

Line 4 in 2265d94

during the intial conversion. Those tests use object based xml that is then written to a string so we dont have to worry about authoring valid xml.

These tests use strings because we are testing the file input and output and are intentionally not covering the full parsing.

As far as the parsing code If I put up a follow up pr with those changes after this lands would that be ok with you? Right now the parsing code is a straight move after the xml file is read.

gmackall · 2025-08-18T19:14:01Z

LGMT. The gemini comments about switching to any vs foreach seem reasonable to me BTW.
I tested that the deeplink cookbook from https://docs.flutter.dev/cookbook/navigation/set-up-app-links still works properly. Have you tested any more involved examples? Just asking because this seems

like a fragile area of the code

I don't know how well it is tested.

I authored integration tests here

flutter/packages/flutter_tools/test/integration.shard/android_gradle_outputs_app_link_settings_test.dart

Line 4 in 2265d94

during the intial conversion. Those tests use object based xml that is then written to a string so we dont have to worry about authoring valid xml.
These tests use strings because we are testing the file input and output and are intentionally not covering the full parsing.

As far as the parsing code If I put up a follow up pr with those changes after this lands would that be ok with you? Right now the parsing code is a straight move after the xml file is read.

Yes I think a follow up pr makes sense, just surfacing them in case you weren't paying attention to the gemini comments 👍

…lutter#173794)

flutter/flutter@2265d94...e65380a 2025-08-19 engine-flutter-autoroll@skia.org Roll Dart SDK from 502455ee300b to 9105d946af95 (3 revisions) (flutter/flutter#174002) 2025-08-18 matanlurey@users.noreply.github.com Add `open_jdk` to `Linux analyze` (flutter/flutter#173988) 2025-08-18 magder@google.com Add "team-ios" label to iOS team triage query (flutter/flutter#173997) 2025-08-18 1063596+reidbaker@users.noreply.github.com Migrate deeplink json creation to public AGP api (flutter/flutter#173794) 2025-08-18 bkonyi@google.com [ Widget Preview ] Don't crash when directory watcher restarts on Windows (flutter/flutter#173987) 2025-08-18 bkonyi@google.com [ Widget Preview ] Don't try to instantiate invalid `@Preview()` applications (flutter/flutter#173984) 2025-08-18 737941+loic-sharma@users.noreply.github.com Explain how to run Google Test tests directly (flutter/flutter#173978) 2025-08-18 58529443+srujzs@users.noreply.github.com [flutter_tools] Use DWDS 25.0.1 (flutter/flutter#173777) 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 Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

…#9850) flutter/flutter@2265d94...e65380a 2025-08-19 engine-flutter-autoroll@skia.org Roll Dart SDK from 502455ee300b to 9105d946af95 (3 revisions) (flutter/flutter#174002) 2025-08-18 matanlurey@users.noreply.github.com Add `open_jdk` to `Linux analyze` (flutter/flutter#173988) 2025-08-18 magder@google.com Add "team-ios" label to iOS team triage query (flutter/flutter#173997) 2025-08-18 1063596+reidbaker@users.noreply.github.com Migrate deeplink json creation to public AGP api (flutter/flutter#173794) 2025-08-18 bkonyi@google.com [ Widget Preview ] Don't crash when directory watcher restarts on Windows (flutter/flutter#173987) 2025-08-18 bkonyi@google.com [ Widget Preview ] Don't try to instantiate invalid `@Preview()` applications (flutter/flutter#173984) 2025-08-18 737941+loic-sharma@users.noreply.github.com Explain how to run Google Test tests directly (flutter/flutter#173978) 2025-08-18 58529443+srujzs@users.noreply.github.com [flutter_tools] Use DWDS 25.0.1 (flutter/flutter#173777) 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 Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

Reviewers please pay special attention to the tests that were added and tests that were removed. If you see a set of functionality that is not covered and should be please say something. I read every test (and TBH also had to edit most of them) but I had gemini's agent mode help and I dont trust there is something I missed. Related to flutter#173651 Newly added tests can be run from `packages/flutter_tools/gradle` with `./gradlew test --tests com.flutter.gradle.tasks.DeepLinkJsonFromManifestTaskTest` ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.

reidbaker added 11 commits August 11, 2025 13:41

Does the copy task actualy do anything

9300a5b

formatting trailing comma

cc18285

rename compileTask to make it look more specific when reading code

2dc957b

Modify annotations based on reading the code

57a605e

Working custom task using the variane api that can read the manifest

e4c0327

Extract out app links settings creation

858acef

use namespace in parsing

1d4bc02

Match existing behavior for json file location and name, move logic o…

18a58bd

…ut of utils into task

Migrate logic to helper, move to using copyto instead of copying text…

d31c48b

… and add unit tests

Add tests for helper class

36e4d9d

Add test for file creation

dd9fca9

reidbaker requested a review from a team as a code owner August 14, 2025 19:16

reidbaker marked this pull request as draft August 14, 2025 19:16

github-actions bot added platform-android Android applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Aug 14, 2025

gemini-code-assist bot reviewed Aug 14, 2025

View reviewed changes

packages/flutter_tools/gradle/src/main/kotlin/FlutterPlugin.kt Outdated Show resolved Hide resolved

reidbaker added 4 commits August 14, 2025 15:26

Flutter plugin test passes with new component mock

ef3bcf2

remove tests that are testing functionality that is removed

312b00d

revert changes to copy task

2928f62

Kotlin formatting and fixed missed compileTask rename

29bb1bf

reidbaker force-pushed the i166550-copy-task-part-1 branch from ce9dba9 to 29bb1bf Compare August 15, 2025 16:05

reidbaker requested review from gmackall and jesswrd August 15, 2025 17:21

reidbaker marked this pull request as ready for review August 15, 2025 17:33

gemini-code-assist bot reviewed Aug 15, 2025

View reviewed changes

gmackall approved these changes Aug 18, 2025

View reviewed changes

reidbaker requested a review from mboetger August 18, 2025 19:11

reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 18, 2025

auto-submit bot added this pull request to the merge queue Aug 18, 2025

Merged via the queue into flutter:master with commit 1e1908d Aug 18, 2025
144 of 145 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 18, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 19, 2025

1e1908dc2 Migrate deeplink json creation to public AGP api (flutter/f…

fa028b0

…lutter#173794)

engine-flutter-autoroll mentioned this pull request Aug 19, 2025

Roll Flutter from 2265d94c6b0b to e65380a22076 (8 revisions) flutter/packages#9850

Merged

reidbaker added a commit to reidbaker/flutter-work that referenced this pull request Aug 19, 2025

Fix gemini suggested fixes from flutter#173794

2ff4d13

reidbaker mentioned this pull request Aug 20, 2025

Increase testing coverage and maintainability of android manifest parsing logic #174070

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate deeplink json creation to public AGP api #173794

Migrate deeplink json creation to public AGP api #173794

reidbaker commented Aug 14, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

reidbaker commented Aug 15, 2025

Uh oh!

reidbaker commented Aug 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Uh oh!

gmackall left a comment

Uh oh!

reidbaker commented Aug 18, 2025

Uh oh!

gmackall commented Aug 18, 2025

Uh oh!

Uh oh!

Uh oh!

	public fun createAppLinkSettingsFile(
	fun createAppLinkSettingsFile(

Migrate deeplink json creation to public AGP api #173794

Migrate deeplink json creation to public AGP api #173794

Conversation

reidbaker commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

reidbaker commented Aug 15, 2025

Uh oh!

reidbaker commented Aug 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

gmackall left a comment

Choose a reason for hiding this comment

Uh oh!

reidbaker commented Aug 18, 2025

Uh oh!

gmackall commented Aug 18, 2025

Uh oh!

Uh oh!

Uh oh!

reidbaker commented Aug 14, 2025 •

edited

Loading