Remove default for stripped option in engine/src/flutter/tools/gn, …

…don't strip by default on android (flutter#161546)

Remake of flutter/engine#52852. 

Makes it so that `stripped` defaults to false for android in `gn`
arguments, i.e. we don't strip the Android engine builds. AGP does this
by default when the NDK is installed (and we download it automatically
now after flutter#159756).

In testing, the step that AGP does to strip symbols adds ~1-2 seconds to
the build.

Adds a tool verification for release app bundle builds that checks to
make sure we have stripped the debug symbols and placed them in the
[`BUNDLE-METADATA`
directory](https://developer.android.com/guide/app-bundle/app-bundle-format).
The check is done by invoking `apkanalyzer`, which takes ~`0.5` seconds,
which is why the check is only enabled for release builds.

BEFORE LANDING: I need to follow up on
flutter/engine#50443 (comment), to
ensure we start stripping symbols internally as well.

## 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].
- [ ] 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.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Gray Mackall <mackall@google.com>

engine/src/flutter/tools/gn

@@ -666,7 +666,12 @@ def to_gn_args(args):
   if args.build_embedder_examples is not None:
     gn_args['build_embedder_examples'] = args.build_embedder_examples
 
-  gn_args['stripped_symbols'] = args.stripped
+  if args.stripped is True:
+    gn_args['stripped_symbols'] = True
+  elif args.stripped is False:
+    gn_args['stripped_symbols'] = False
+  else:
+    gn_args['stripped_symbols'] = args.target_os != 'android'
 
   if args.msan:
     gn_args['is_msan'] = True
@@ -1113,7 +1118,6 @@ def parse_args(args):
 
   parser.add_argument(
       '--stripped',
-      default=True,
       action='store_true',
       help='Strip debug symbols from the output. This defaults to true and has no effect on iOS.'
   )

packages/flutter_tools/lib/src/android/gradle.dart

@@ -52,6 +52,11 @@ import 'migrations/top_level_gradle_build_file_migration.dart';
 final RegExp _kBuildVariantRegex = RegExp('^BuildVariant: (?<$_kBuildVariantRegexGroupName>.*)\$');
 const String _kBuildVariantRegexGroupName = 'variant';
 const String _kBuildVariantTaskName = 'printBuildVariants';
+@visibleForTesting
+const String failedToStripDebugSymbolsErrorMessage = r'''
+Release app bundle failed to strip debug symbols from native libraries. Please run flutter doctor and ensure that the Android toolchain does not report any issues.
+
+Otherwise, file an issue at https://github.com/flutter/flutter/issues.''';
 
 typedef _OutputParser = void Function(String line);
 
@@ -86,6 +91,9 @@ Directory getBundleDirectory(FlutterProject project) {
           .childDirectory('bundle');
 }
 
+@visibleForTesting
+final String apkAnalyzerBinaryName = globals.platform.isWindows ? 'apkanalyzer.bat' : 'apkanalyzer';
+
 /// The directory where the repo is generated.
 /// Only applicable to AARs.
 Directory getRepoDirectory(Directory buildDirectory) {
@@ -577,6 +585,16 @@ class AndroidGradleBuilder implements AndroidBuilder {
 
     if (isBuildingBundle) {
       final File bundleFile = findBundleFile(project, buildInfo, _logger, _analytics);
+
+      if ((buildInfo.mode == BuildMode.release) &&
+          !(await _isAabStrippedOfDebugSymbols(
+            project,
+            bundleFile.path,
+            androidBuildInfo.targetArchs,
+          ))) {
+        throwToolExit(failedToStripDebugSymbolsErrorMessage);
+      }
+
       final String appSize =
           (buildInfo.mode == BuildMode.debug)
               ? '' // Don't display the size when building a debug variant.
@@ -632,6 +650,64 @@ class AndroidGradleBuilder implements AndroidBuilder {
     }
   }
 
+  // Checks whether AGP has successfully stripped debug symbols from native libraries
+  // - libflutter.so, aka the engine
+  // - lib_app.so, aka the framework dart code
+  // and moved them to the BUNDLE-METADATA directory. Block the build if this
+  // isn't successful, as it means that debug symbols are getting included in
+  // the final app that would be delivered to users.
+  Future<bool> _isAabStrippedOfDebugSymbols(
+    FlutterProject project,
+    String aabPath,
+    Iterable<AndroidArch> targetArchs,
+  ) async {
+    if (globals.androidSdk == null) {
+      _logger.printTrace(
+        'Failed to find android sdk when checking final appbundle for debug symbols.',
+      );
+      return false;
+    }
+    if (!globals.androidSdk!.cmdlineToolsAvailable) {
+      _logger.printTrace(
+        'Failed to find cmdline-tools when checking final appbundle for debug symbols.',
+      );
+      return false;
+    }
+    final String? apkAnalyzerPath = globals.androidSdk!.getCmdlineToolsPath(apkAnalyzerBinaryName);
+    if (apkAnalyzerPath == null) {
+      _logger.printTrace(
+        'Failed to find apkanalyzer when checking final appbundle for debug symbols.',
+      );
+      return false;
+    }
+
+    final RunResult result = await _processUtils.run(
+      <String>[apkAnalyzerPath, 'files', 'list', aabPath],
+      workingDirectory: project.android.hostAppGradleRoot.path,
+      environment: _java?.environment,
+    );
+
+    if (result.exitCode != 0) {
+      _logger.printTrace(
+        'apkanalyzer finished with exit code 0 when checking final appbundle for debug symbols.\n'
+        'stderr was: ${result.stderr}\n'
+        'and stdout was: ${result.stdout}',
+      );
+      return false;
+    }
+
+    // As long as libflutter.so.sym is present for at least one architecture,
+    // assume AGP succeeded in stripping.
+    if (result.stdout.contains('libflutter.so.sym')) {
+      return true;
+    }
+
+    _logger.printTrace(
+      'libflutter.so.sym not present when checking final appbundle for debug symbols.',
+    );
+    return false;
+  }
+
   Future<void> _performCodeSizeAnalysis(
     String kind,
     File zipFile,