Reland "Enable dump-skp-on-shader-compilation in drive (#43022)" (#43455)

This reverts commit b9eb3f71.

We fixed the issue by updating the constructors.

packages/flutter_tools/lib/src/commands/drive.dart

@@ -276,6 +276,7 @@ Future<LaunchResult> _startApp(DriveCommand command) async {
       observatoryPort: command.observatoryPort,
       verboseSystemLogs: command.verboseSystemLogs,
       cacheSkSL: command.cacheSkSL,
+      dumpSkpOnShaderCompilation: command.dumpSkpOnShaderCompilation,
     ),
     platformArgs: platformArgs,
     prebuiltApplication: !command.shouldBuild,

packages/flutter_tools/lib/src/commands/run.dart

@@ -43,7 +43,15 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
       )
       ..addFlag('cache-sksl',
         negatable: false,
-        help: 'Only cache the shader in SkSL instead of binary or GLSL.',)
+        help: 'Only cache the shader in SkSL instead of binary or GLSL.',
+      )
+      ..addFlag('dump-skp-on-shader-compilation',
+        negatable: false,
+        help: 'Automatically dump the skp that triggers new shader compilations. '
+            'This is useful for wrting custom ShaderWarmUp to reduce jank. '
+            'By default, this is not enabled to reduce the overhead. '
+            'This is only available in profile or debug build. ',
+      )
       ..addOption('route',
         help: 'Which route to load when running the app.',
       )
@@ -63,6 +71,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
 
   bool get traceStartup => argResults['trace-startup'];
   bool get cacheSkSL => argResults['cache-sksl'];
+  bool get dumpSkpOnShaderCompilation => argResults['dump-skp-on-shader-compilation'];
 
   String get route => argResults['route'];
 }
@@ -98,13 +107,6 @@ class RunCommand extends RunCommandBase {
         help: 'Enable tracing to the system tracer. This is only useful on '
               'platforms where such a tracer is available (Android and Fuchsia).',
       )
-      ..addFlag('dump-skp-on-shader-compilation',
-        negatable: false,
-        help: 'Automatically dump the skp that triggers new shader compilations. '
-              'This is useful for wrting custom ShaderWarmUp to reduce jank. '
-              'By default, this is not enabled to reduce the overhead. '
-              'This is only available in profile or debug build. ',
-      )
       ..addFlag('await-first-frame-when-tracing',
         defaultsTo: true,
         help: 'Whether to wait for the first frame when tracing startup ("--trace-startup"), '
@@ -309,7 +311,7 @@ class RunCommand extends RunCommandBase {
         skiaDeterministicRendering: argResults['skia-deterministic-rendering'],
         traceSkia: argResults['trace-skia'],
         traceSystrace: argResults['trace-systrace'],
-        dumpSkpOnShaderCompilation: argResults['dump-skp-on-shader-compilation'],
+        dumpSkpOnShaderCompilation: dumpSkpOnShaderCompilation,
         cacheSkSL: cacheSkSL,
         observatoryPort: observatoryPort,
         verboseSystemLogs: argResults['verbose-system-logs'],

packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart

@@ -459,6 +459,114 @@ void main() {
         ProcessManager: () => FakeProcessManager.any(),
       });
     });
+
+    group('debugging options', () {
+      DebuggingOptions debuggingOptions;
+
+      String testApp, testFile;
+
+      setUp(() {
+        restoreAppStarter();
+      });
+
+      Future<void> appStarterSetup() async {
+        mockDevice = MockDevice();
+        testDeviceManager.addDevice(mockDevice);
+
+        final MockDeviceLogReader mockDeviceLogReader = MockDeviceLogReader();
+        when(mockDevice.getLogReader()).thenReturn(mockDeviceLogReader);
+        final MockLaunchResult mockLaunchResult = MockLaunchResult();
+        when(mockLaunchResult.started).thenReturn(true);
+        when(mockDevice.startApp(
+          null,
+          mainPath: anyNamed('mainPath'),
+          route: anyNamed('route'),
+          debuggingOptions: anyNamed('debuggingOptions'),
+          platformArgs: anyNamed('platformArgs'),
+          prebuiltApplication: anyNamed('prebuiltApplication'),
+        )).thenAnswer((Invocation invocation) async {
+          debuggingOptions = invocation.namedArguments[#debuggingOptions];
+          return mockLaunchResult;
+        });
+        when(mockDevice.isAppInstalled(any))
+            .thenAnswer((_) => Future<bool>.value(false));
+
+        testApp = fs.path.join(tempDir.path, 'test', 'e2e.dart');
+        testFile = fs.path.join(tempDir.path, 'test_driver', 'e2e_test.dart');
+
+        testRunner = (List<String> testArgs, String observatoryUri) async {
+          throwToolExit(null, exitCode: 123);
+        };
+        appStopper = expectAsync1(
+          (DriveCommand command) async {
+            return true;
+          },
+          count: 2,
+        );
+
+        final MemoryFileSystem memFs = fs;
+        await memFs.file(testApp).writeAsString('main() {}');
+        await memFs.file(testFile).writeAsString('main() {}');
+      }
+
+      void _testOptionThatDefaultsToFalse(
+        String optionName,
+        bool setToTrue,
+        bool optionValue(),
+      ) {
+        testUsingContext('$optionName ${setToTrue ? 'works' : 'defaults to false'}', () async {
+          await appStarterSetup();
+
+          final List<String> args = <String>[
+            'drive',
+            '--target=$testApp',
+            if (setToTrue) optionName,
+            '--no-pub',
+          ];
+          try {
+            await createTestCommandRunner(command).run(args);
+          } on ToolExit catch (e) {
+            expect(e.exitCode, 123);
+            expect(e.message, null);
+          }
+          verify(mockDevice.startApp(
+            null,
+            mainPath: anyNamed('mainPath'),
+            route: anyNamed('route'),
+            debuggingOptions: anyNamed('debuggingOptions'),
+            platformArgs: anyNamed('platformArgs'),
+            prebuiltApplication: false,
+          ));
+          expect(optionValue(), setToTrue ? isTrue : isFalse);
+        }, overrides: <Type, Generator>{
+          FileSystem: () => fs,
+          ProcessManager: () => FakeProcessManager.any(),
+        });
+      }
+
+      void testOptionThatDefaultsToFalse(
+        String optionName,
+        bool optionValue(),
+      ) {
+        _testOptionThatDefaultsToFalse(optionName, true, optionValue);
+        _testOptionThatDefaultsToFalse(optionName, false, optionValue);
+      }
+
+      testOptionThatDefaultsToFalse(
+        '--dump-skp-on-shader-compilation',
+        () => debuggingOptions.dumpSkpOnShaderCompilation,
+      );
+
+      testOptionThatDefaultsToFalse(
+        '--verbose-system-logs',
+        () => debuggingOptions.verboseSystemLogs,
+      );
+
+      testOptionThatDefaultsToFalse(
+        '--cache-sksl',
+        () => debuggingOptions.cacheSkSL,
+      );
+    });
   });
 }