Collect impeller analytics for appbundles#184146
Conversation
There was a problem hiding this comment.
Code Review
This pull request extends Impeller analytics reporting to build_appbundle.dart and refines existing reporting in build_aar.dart and build_apk.dart by removing explanatory comments. It also introduces new tests for build_appbundle.dart to validate Impeller settings from AndroidManifest.xml and corrects minor grammatical errors in test descriptions. Feedback highlights opportunities to reduce code duplication by extracting the Impeller analytics reporting logic into a shared helper method and by consolidating common test utility functions and boilerplate code within the new build_appbundle_test.dart.
| final bool impellerEnabled = project.android.computeImpellerEnabled(); | ||
| final buildLabel = impellerEnabled ? 'manifest-impeller-enabled' : 'manifest-impeller-disabled'; | ||
| globals.analytics.send(Event.flutterBuildInfo(label: buildLabel, buildType: 'android')); |
There was a problem hiding this comment.
This logic for sending Impeller analytics is now duplicated across build_aar.dart, build_apk.dart, and this file. To improve maintainability and reduce code duplication, consider extracting this logic into a shared helper method. A good place for this could be on the AndroidProject class, which would also encapsulate the computeImpellerEnabled logic.
For example, you could add a method to AndroidProject like this:
// In packages/flutter_tools/lib/src/project.dart within AndroidProject class
void reportImpellerAnalytics({String? buildKey}) {
final bool impellerEnabled = computeImpellerEnabled();
final String keyPart = buildKey == null ? '' : '-$buildKey';
final String status = impellerEnabled ? 'enabled' : 'disabled';
final String buildLabel = 'manifest$keyPart-impeller-$status';
globals.analytics.send(Event.flutterBuildInfo(label: buildLabel, buildType: 'android'));
}Then, this file and others could call project.android.reportImpellerAnalytics(). While modifying project.dart might be out of scope for this PR, it's a good follow-up to consider.
| }, | ||
| ); | ||
|
|
||
| group('Impeller AndroidManifest.xml setting', () { |
There was a problem hiding this comment.
There are a couple of opportunities to reduce code duplication in this new test group:
-
The
writeManifestMetadatahelper function is also present inbuild_aar_test.dartandbuild_apk_test.dart. Consider extracting it to a shared test utility file (e.g.,test/src/android_common.dart) to promote code reuse. -
The
try/finallyblock for changing the current working directory is repeated in all three new tests. This could be extracted into a local helper function to reduce boilerplate. For example:Future<void> runBuildInProject(String projectPath) async { final Directory oldCwd = globals.localFileSystem.currentDirectory; try { globals.localFileSystem.currentDirectory = globals.localFileSystem.directory( projectPath, ); await runBuildAppBundleCommand(projectPath); } finally { globals.localFileSystem.currentDirectory = oldCwd; } }
Each test could then simply call
await runBuildInProject(projectPath);.
| buildNumber: buildNumber, | ||
| ); | ||
|
|
||
| // When an aar is successfully built, record to analytics whether Impeller |
There was a problem hiding this comment.
I also deleted these comments in the two places they existed. The first half is obvious, and the second half was no longer true (and more generally is a type of comment I dislike, as it has a tendency of becoming un-true)
While looking in to flutter#184127 I noticed we don't collect analytics for impeller when building appbundles. I assume this is a simple oversight, hence this PR. --------- Co-authored-by: Gray Mackall <[email protected]>
…11372) Manual roll Flutter from 82d96ef98a33 to e79bf6cbc140 (32 revisions) Manual roll requested by [email protected] flutter/flutter@82d96ef...e79bf6c 2026-03-26 [email protected] Roll Dart SDK from a3af8949863e to 7587a31814c6 (1 revision) (flutter/flutter#184213) 2026-03-26 [email protected] Adds rockchip series to block list for vulkan. (flutter/flutter#184207) 2026-03-26 [email protected] [ios]add ci/cd label to ios triage (flutter/flutter#184214) 2026-03-26 [email protected] Roll Skia from 10c97361d8f3 to bee5a06ef578 (1 revision) (flutter/flutter#184203) 2026-03-26 [email protected] Roll Fuchsia Linux SDK from rS5ezRgehkw26fKRX... to BIlBJNOlKjQeRFoFy... (flutter/flutter#184197) 2026-03-26 [email protected] Roll Packages from 5909bdd to 0dd2410 (3 revisions) (flutter/flutter#184198) 2026-03-26 [email protected] fix: use atomic write for engine.stamp to prevent race conditions (flutter/flutter#184131) 2026-03-26 [email protected] Roll Skia from 77dfb68002cd to 10c97361d8f3 (1 revision) (flutter/flutter#184195) 2026-03-26 [email protected] Roll Dart SDK from 349dbbbdba99 to a3af8949863e (1 revision) (flutter/flutter#184194) 2026-03-26 [email protected] Roll Skia from 81ef2238cb09 to 77dfb68002cd (12 revisions) (flutter/flutter#184186) 2026-03-26 [email protected] Revert "Keep glyphs for variable fonts (#183857)" (flutter/flutter#184147) 2026-03-26 [email protected] Expand simple shape path optimization logic and move it from dl_dispatcher to dl_builder (flutter/flutter#184096) 2026-03-26 [email protected] Fix merge changelog workflow. (flutter/flutter#184145) 2026-03-26 [email protected] Add notes on HCPP to `Android-Platform-Views.md` (flutter/flutter#183859) 2026-03-26 [email protected] Adds explicit name to the cicd label job. (flutter/flutter#184070) 2026-03-26 [email protected] Roll Dart SDK from 26f80b9403f5 to 349dbbbdba99 (3 revisions) (flutter/flutter#184176) 2026-03-26 [email protected] Collect impeller analytics for appbundles (flutter/flutter#184146) 2026-03-26 [email protected] [Dot shorthands] Migrate examples/api/lib/material (flutter/flutter#183963) 2026-03-26 [email protected] Roll Dart DevTools to a version with correct CIPD tags (flutter/flutter#184172) 2026-03-25 [email protected] Pipes ScrollCacheExtent through more scroll views (flutter/flutter#184078) 2026-03-25 [email protected] Add widget of the week link in SensitiveContent documentation (flutter/flutter#183972) 2026-03-25 [email protected] Roll Packages from 8dcfd11 to 5909bdd (13 revisions) (flutter/flutter#184123) 2026-03-25 [email protected] Roll Dart SDK from c48f0c954d86 to 26f80b9403f5 (1 revision) (flutter/flutter#184117) 2026-03-25 [email protected] Roll Skia from 51725ae49ef5 to 81ef2238cb09 (3 revisions) (flutter/flutter#184115) 2026-03-25 [email protected] Roll Fuchsia Linux SDK from M3sjWggTQmP2AD4bS... to rS5ezRgehkw26fKRX... (flutter/flutter#184114) 2026-03-25 [email protected] Add SensitiveContent widget sample code (flutter/flutter#183846) 2026-03-25 [email protected] SelectableRegion should passthrough constraints to child unmodified (flutter/flutter#184083) 2026-03-25 [email protected] Roll Dart SDK from ce171d5026ff to c48f0c954d86 (2 revisions) (flutter/flutter#184105) 2026-03-25 [email protected] Roll Skia from 789ad6b12776 to 51725ae49ef5 (3 revisions) (flutter/flutter#184106) 2026-03-25 [email protected] Roll Skia from f4e59f82dc69 to 789ad6b12776 (7 revisions) (flutter/flutter#184098) 2026-03-25 [email protected] Roll Dart SDK from b08bd0a3ee39 to ce171d5026ff (4 revisions) (flutter/flutter#184095) 2026-03-24 [email protected] [flutter_goldens] Remove dead check on null being in a list of non-nullables (flutter/flutter#183938) 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 [email protected],[email protected] 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 ...
While looking in to flutter#184127 I noticed we don't collect analytics for impeller when building appbundles. I assume this is a simple oversight, hence this PR. --------- Co-authored-by: Gray Mackall <[email protected]>
While looking in to #184127 I noticed we don't collect analytics for impeller when building appbundles. I assume this is a simple oversight, hence this PR.