Migrated video_player_android to Built-in Kotlin

[jesswrd]

Migrating one plugin video_player_android to built-in kotlin as a test-run. Once this PR has been merged, another PR with all affected plugins will be made + validation of changes to the build files.

Followed these Flutter docs: https://docs.flutter.dev/release/breaking-changes/migrate-to-built-in-kotlin/for-plugin-authors to migrate plugins to built-in kotlin.

Partially Adresses flutter/flutter#187261

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the AI contribution guidelines and understand my responsibilities, or I am not using AI tools.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I followed the version and CHANGELOG instructions, using semantic versioning and the repository CHANGELOG style, or I have commented below to indicate which documented exception this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

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. ↩ ↩²

[gemini-code-assist]

Code Review

This pull request migrates the video_player_android plugin to built-in Kotlin to support AGP 9, updates the Gradle wrapper to version 9.1.0, and bumps the minimum supported SDK versions to Flutter 3.41 and Dart 3.11. Feedback on the changes points out that the android.builtInKotlin and android.newDsl flags in gradle.properties should be set to true to prevent build failures, and recommends using the simple name JvmTarget.JVM_17 in build.gradle.kts since it is already imported.

[gemini-code-assist]

Since this pull request is migrating the plugin to built-in Kotlin and the new Kotlin DSL, these flags should be set to true (or removed entirely to use the defaults in newer Flutter versions). Keeping them as false disables the built-in Kotlin and new DSL features. Furthermore, since id("kotlin-android") was removed from example/android/app/build.gradle.kts, setting android.builtInKotlin=false will result in Kotlin not being applied at all for the example app, which will break the build if there is any Kotlin code.

# This builtInKotlin flag was added automatically by Flutter migrator
android.builtInKotlin=true
# This newDsl flag was added automatically by Flutter migrator
android.newDsl=true

[stuartmorgan-g]

What's the test matrix plan? IIUC we now have three configurations we are supporting:

1. AGP < 9 (tested by the legacy build-all project, I believe)
2. AGP 9 in an app with built-in kotlin disabled (this config)
3. AGP 9 in an app with built-in kotlin enabled

Shouldn't we be testing 3 somewhere? Will be doing that in non-legacy build-all once all the plugin are migrated?

[jesswrd]

Yes 3 should be tested somewhere. The non-legacy build-all seems like a good place. Added an issue for that here.

[stuartmorgan-g]

So that I understand the plan here, is the plan to land this as test run, then if all goes well do a second PR that does everything else and also adds repo tooling validation of the new entries?

[stuartmorgan-g]

What's the test matrix plan? IIUC we now have three configurations we are supporting:

1. AGP < 9 (tested by the legacy build-all project, I believe)
2. AGP 9 in an app with built-in kotlin disabled (this config)
3. AGP 9 in an app with built-in kotlin enabled

Shouldn't we be testing 3 somewhere? Will be doing that in non-legacy build-all once all the plugin are migrated?

[stuartmorgan-g]

Could we clearly capture somewhere easily discoverable (maybe in a comment on the tracking issue) the reasoning for why we are doing this version, rather than the unconditional version that requires 3.44? (To be clear, I'm not arguing that we shouldn't, I just want to have a record of why we chose that option over the other one.)

We should also file a tracking issue (with a pointer to the doc) to simplify all of this logic once we no longer support <3.44 in plugins, so that we don't forget and end up with this as cruft indefinitely. It would be great to include a TODO comment referencing that in this code even.

[jesswrd]

Talked offline. Since the resolver will prevent developers from upgrading to a package version where Flutter 3.44 is the minimum, we decided it is okay to bump the Flutter minimum to 3.44. Because of that, we do not need to implement the workaround or clean up that workaround in the future.

[flutter-dashboard]

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.

[jesswrd]

So that I understand the plan here, is the plan to land this as test run, then if all goes well do a second PR that does everything else and also adds repo tooling validation of the new entries?

Yes. I just added that to the PR description. The next PR will contain the rest of the affected plugins and I will add validation for the gradle files.

[stuartmorgan-g]

LGTM