Reland "Add --serve-observatory flag to run, attach, and test (#118402)" (#119529)

* Reland "Add --serve-observatory flag to run, attach, and test (#118402)"

This reverts commit 86ab01d2.

* Fix flaky failures

* Fix VM service disappearing failure

packages/flutter_tools/lib/src/base/dds.dart

@@ -96,4 +96,8 @@ class DartDevelopmentService {
   }
 
   Future<void> shutdown() async => _ddsInstance?.shutdown();
+
+  void setExternalDevToolsUri(Uri uri) {
+    _ddsInstance?.setExternalDevToolsUri(uri);
+  }
 }

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

@@ -139,6 +139,7 @@ class AttachCommand extends FlutterCommand {
     usesTrackWidgetCreation(verboseHelp: verboseHelp);
     addDdsOptions(verboseHelp: verboseHelp);
     addDevToolsOptions(verboseHelp: verboseHelp);
+    addServeObservatoryOptions(verboseHelp: verboseHelp);
     usesDeviceTimeoutOption();
   }
 
@@ -200,6 +201,8 @@ known, it can be explicitly provided to attach via the command-line, e.g.
     return uri;
   }
 
+  bool get serveObservatory => boolArg('serve-observatory') ?? false;
+
   String? get appId {
     return stringArgDeprecated('app-id');
   }
@@ -514,6 +517,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
       enableDds: enableDds,
       ddsPort: ddsPort,
       devToolsServerAddress: devToolsServerAddress,
+      serveObservatory: serveObservatory,
     );
 
     return buildInfo.isDebug

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

@@ -179,6 +179,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
     usesDeviceTimeoutOption();
     addDdsOptions(verboseHelp: verboseHelp);
     addDevToolsOptions(verboseHelp: verboseHelp);
+    addServeObservatoryOptions(verboseHelp: verboseHelp);
     addAndroidSpecificBuildOptions(hide: !verboseHelp);
     usesFatalWarningsOption(verboseHelp: verboseHelp);
     addEnableImpellerFlag(verboseHelp: verboseHelp);
@@ -279,6 +280,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
         nativeNullAssertions: boolArgDeprecated('native-null-assertions'),
         enableImpeller: enableImpeller,
         uninstallFirst: uninstallFirst,
+        serveObservatory: boolArgDeprecated('serve-observatory'),
         enableDartProfiling: enableDartProfiling,
       );
     }

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

@@ -215,6 +215,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
               'or as the string "none" to disable the timeout entirely.',
       );
     addDdsOptions(verboseHelp: verboseHelp);
+    addServeObservatoryOptions(verboseHelp: verboseHelp);
     usesFatalWarningsOption(verboseHelp: verboseHelp);
   }
 
@@ -404,6 +405,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
       buildInfo,
       startPaused: startPaused,
       disableServiceAuthCodes: boolArgDeprecated('disable-service-auth-codes'),
+      serveObservatory: boolArgDeprecated('serve-observatory'),
       // On iOS >=14, keeping this enabled will leave a prompt on the screen.
       disablePortPublication: true,
       enableDds: enableDds,

packages/flutter_tools/lib/src/device.dart

@@ -754,6 +754,7 @@ class DebuggingOptions {
     this.nativeNullAssertions = false,
     this.enableImpeller = false,
     this.uninstallFirst = false,
+    this.serveObservatory = true,
     this.enableDartProfiling = true,
    }) : debuggingEnabled = true;
 
@@ -799,7 +800,8 @@ class DebuggingOptions {
       fastStart = false,
       webEnableExpressionEvaluation = false,
       nullAssertions = false,
-      nativeNullAssertions = false;
+      nativeNullAssertions = false,
+      serveObservatory = false;
 
   DebuggingOptions._({
     required this.buildInfo,
@@ -844,6 +846,7 @@ class DebuggingOptions {
     required this.nativeNullAssertions,
     required this.enableImpeller,
     required this.uninstallFirst,
+    required this.serveObservatory,
     required this.enableDartProfiling,
   });
 
@@ -880,6 +883,7 @@ class DebuggingOptions {
   final bool webUseSseForDebugBackend;
   final bool webUseSseForInjectedClient;
   final bool enableImpeller;
+  final bool serveObservatory;
   final bool enableDartProfiling;
 
   /// Whether the tool should try to uninstall a previously installed version of the app.
@@ -1008,6 +1012,7 @@ class DebuggingOptions {
     'nullAssertions': nullAssertions,
     'nativeNullAssertions': nativeNullAssertions,
     'enableImpeller': enableImpeller,
+    'serveObservatory': serveObservatory,
     'enableDartProfiling': enableDartProfiling,
   };
 
@@ -1055,6 +1060,7 @@ class DebuggingOptions {
       nativeNullAssertions: json['nativeNullAssertions']! as bool,
       enableImpeller: (json['enableImpeller'] as bool?) ?? false,
       uninstallFirst: (json['uninstallFirst'] as bool?) ?? false,
+      serveObservatory: (json['serveObservatory'] as bool?) ?? false,
       enableDartProfiling: (json['enableDartProfiling'] as bool?) ?? true,
     );
 }

packages/flutter_tools/lib/src/resident_devtools_handler.dart

@@ -91,12 +91,26 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
     final List<FlutterDevice?> devicesWithExtension = await _devicesWithExtensions(flutterDevices);
     await _maybeCallDevToolsUriServiceExtension(devicesWithExtension);
     await _callConnectedVmServiceUriExtension(devicesWithExtension);
+
     if (_shutdown) {
       // If we're shutting down, no point reporting the debugger list.
       return;
     }
     _readyToAnnounce = true;
     assert(_devToolsLauncher!.activeDevToolsServer != null);
+
+    final Uri? devToolsUrl = _devToolsLauncher!.devToolsUrl;
+    if (devToolsUrl != null) {
+      for (final FlutterDevice? device in devicesWithExtension) {
+        if (device == null) {
+          continue;
+        }
+        // Notify the DDS instances that there's a DevTools instance available so they can correctly
+        // redirect DevTools related requests.
+        device.device?.dds.setExternalDevToolsUri(devToolsUrl);
+      }
+    }
+
     if (_residentRunner.reportedDebuggers) {
       // Since the DevTools only just became available, we haven't had a chance to
       // report their URLs yet. Do so now.

packages/flutter_tools/lib/src/resident_runner.dart

@@ -1402,6 +1402,26 @@ abstract class ResidentRunner extends ResidentHandlers {
     _finished.complete(0);
   }
 
+  Future<void> enableObservatory() async {
+    assert(debuggingOptions.serveObservatory);
+    final List<Future<vm_service.Response?>> serveObservatoryRequests = <Future<vm_service.Response?>>[];
+    for (final FlutterDevice? device in flutterDevices) {
+      if (device == null) {
+        continue;
+      }
+      // Notify the VM service if the user wants Observatory to be served.
+      serveObservatoryRequests.add(
+        device.vmService?.callMethodWrapper('_serveObservatory') ??
+          Future<vm_service.Response?>.value(),
+      );
+    }
+    try {
+      await Future.wait(serveObservatoryRequests);
+    } on vm_service.RPCError catch(e) {
+      globals.printWarning('Unable to enable Observatory: $e');
+    }
+  }
+
   void appFinished() {
     if (_finished.isCompleted) {
       return;

packages/flutter_tools/lib/src/run_cold.dart

@@ -80,12 +80,17 @@ class ColdRunner extends ResidentRunner {
       }
     }
 
-    if (enableDevTools && debuggingEnabled) {
-      // The method below is guaranteed never to return a failing future.
-      unawaited(residentDevtoolsHandler!.serveAndAnnounceDevTools(
-        devToolsServerAddress: debuggingOptions.devToolsServerAddress,
-        flutterDevices: flutterDevices,
-      ));
+    if (debuggingEnabled) {
+      if (enableDevTools) {
+        // The method below is guaranteed never to return a failing future.
+        unawaited(residentDevtoolsHandler!.serveAndAnnounceDevTools(
+          devToolsServerAddress: debuggingOptions.devToolsServerAddress,
+          flutterDevices: flutterDevices,
+        ));
+      }
+      if (debuggingOptions.serveObservatory) {
+        await enableObservatory();
+      }
     }
 
     if (flutterDevices.first.observatoryUris != null) {
@@ -162,12 +167,17 @@ class ColdRunner extends ResidentRunner {
       }
     }
 
-    if (enableDevTools && debuggingEnabled) {
-      // The method below is guaranteed never to return a failing future.
-      unawaited(residentDevtoolsHandler!.serveAndAnnounceDevTools(
-        devToolsServerAddress: debuggingOptions.devToolsServerAddress,
-        flutterDevices: flutterDevices,
-      ));
+    if (debuggingEnabled) {
+      if (enableDevTools) {
+        // The method below is guaranteed never to return a failing future.
+        unawaited(residentDevtoolsHandler!.serveAndAnnounceDevTools(
+          devToolsServerAddress: debuggingOptions.devToolsServerAddress,
+          flutterDevices: flutterDevices,
+        ));
+      }
+      if (debuggingOptions.serveObservatory) {
+        await enableObservatory();
+      }
     }
 
     appStartedCompleter?.complete();

packages/flutter_tools/lib/src/run_hot.dart

@@ -237,6 +237,10 @@ class HotRunner extends ResidentRunner {
       return 2;
     }
 
+    if (debuggingOptions.serveObservatory) {
+      await enableObservatory();
+    }
+
     if (enableDevTools) {
       // The method below is guaranteed never to return a failing future.
       unawaited(residentDevtoolsHandler!.serveAndAnnounceDevTools(

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -458,6 +458,14 @@ abstract class FlutterCommand extends Command<void> {
     );
   }
 
+  void addServeObservatoryOptions({required bool verboseHelp}) {
+    argParser.addFlag('serve-observatory',
+      hide: !verboseHelp,
+      defaultsTo: true,
+      help: 'Serve the legacy Observatory developer tooling through the VM service.',
+    );
+  }
+
   late final bool enableDds = () {
     bool ddsEnabled = false;
     if (argResults?.wasParsed('disable-dds') ?? false) {

packages/flutter_tools/lib/src/test/flutter_tester_device.dart

@@ -11,6 +11,7 @@ import 'package:dds/dds.dart';
 import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 import 'package:stream_channel/stream_channel.dart';
+import 'package:vm_service/vm_service.dart' as vm_service;
 
 import '../base/file_system.dart';
 import '../base/io.dart';
@@ -180,8 +181,15 @@ class FlutterTesterTestDevice extends TestDevice {
           compileExpression: compileExpression,
           logger: logger,
         );
-        unawaited(localVmService.then((FlutterVmService vmservice) {
+        unawaited(localVmService.then((FlutterVmService vmservice) async {
           logger.printTrace('test $id: Successfully connected to service protocol: $forwardingUri');
+          if (debuggingOptions.serveObservatory) {
+            try {
+              await vmservice.callMethodWrapper('_serveObservatory');
+            } on vm_service.RPCError {
+              logger.printWarning('Unable to enable Observatory');
+            }
+          }
         }));
 
         if (debuggingOptions.startPaused && !machine!) {
@@ -190,6 +198,7 @@ class FlutterTesterTestDevice extends TestDevice {
           logger.printStatus('  $forwardingUri');
           logger.printStatus('You should first set appropriate breakpoints, then resume the test in the debugger.');
         }
+
         _gotProcessObservatoryUri.complete(forwardingUri);
       },
     );

packages/flutter_tools/test/general.shard/resident_devtools_handler_test.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:flutter_tools/src/base/dds.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
@@ -110,7 +111,9 @@ void main() {
 
   testWithoutContext('serveAndAnnounceDevTools with attached device does not fail on null vm service', () async {
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
-      FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
+      FakeDevtoolsLauncher()
+        ..activeDevToolsServer = DevToolsServerAddress('localhost', 8080)
+        ..devToolsUrl = Uri.parse('http://localhost:8080'),
       FakeResidentRunner(),
       BufferLogger.test(),
     );
@@ -125,7 +128,9 @@ void main() {
 
   testWithoutContext('serveAndAnnounceDevTools with invokes devtools and vm_service setter', () async {
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
-      FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
+      FakeDevtoolsLauncher()
+        ..activeDevToolsServer = DevToolsServerAddress('localhost', 8080)
+        ..devToolsUrl = Uri.parse('http://localhost:8080'),
       FakeResidentRunner(),
       BufferLogger.test(),
     );
@@ -194,7 +199,9 @@ void main() {
 
   testWithoutContext('serveAndAnnounceDevTools with web device', () async {
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
-      FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
+      FakeDevtoolsLauncher()
+        ..activeDevToolsServer = DevToolsServerAddress('localhost', 8080)
+        ..devToolsUrl = Uri.parse('http://localhost:8080'),
       FakeResidentRunner(),
       BufferLogger.test(),
     );
@@ -278,7 +285,9 @@ void main() {
 
   testWithoutContext('serveAndAnnounceDevTools with multiple devices and VM service disappears on one', () async {
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
-      FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
+      FakeDevtoolsLauncher()
+        ..activeDevToolsServer = DevToolsServerAddress('localhost', 8080)
+        ..devToolsUrl = Uri.parse('http://localhost:8080'),
       FakeResidentRunner(),
       BufferLogger.test(),
     );
@@ -442,6 +451,9 @@ class FakeResidentRunner extends Fake implements ResidentRunner {
 
   @override
   bool reportedDebuggers = false;
+
+  @override
+  DebuggingOptions debuggingOptions = DebuggingOptions.disabled(BuildInfo.debug);
 }
 
 class FakeFlutterDevice extends Fake implements FlutterDevice {
@@ -458,4 +470,35 @@ class FakeFlutterDevice extends Fake implements FlutterDevice {
 // Unfortunately Device, despite not being immutable, has an `operator ==`.
 // Until we fix that, we have to also ignore related lints here.
 // ignore: avoid_implementing_value_types
-class FakeDevice extends Fake implements Device { }
+class FakeDevice extends Fake implements Device {
+  @override
+  DartDevelopmentService get dds => FakeDartDevelopmentService();
+}
+
+class FakeDartDevelopmentService extends Fake implements DartDevelopmentService {
+  bool started = false;
+  bool disposed = false;
+
+  @override
+  final Uri uri = Uri.parse('http://127.0.0.1:1234/');
+
+  @override
+  Future<void> startDartDevelopmentService(
+    Uri observatoryUri, {
+    required Logger logger,
+    int? hostPort,
+    bool? ipv6,
+    bool? disableServiceAuthCodes,
+    bool cacheStartupProfile = false,
+  }) async {
+    started = true;
+  }
+
+  @override
+  Future<void> shutdown() async {
+    disposed = true;
+  }
+
+  @override
+  void setExternalDevToolsUri(Uri uri) {}
+}

packages/flutter_tools/test/integration.shard/flutter_attach_test.dart

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:convert';
+
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/base/io.dart';
 import 'package:vm_service/vm_service.dart';
@@ -157,4 +159,64 @@ void main() {
       expect(vmServiceUri.port, equals(ddsPort));
     });
   });
+
+  group('--serve-observatory', () {
+    late FlutterRunTestDriver flutterRun, flutterAttach;
+
+    setUp(() async {
+      flutterRun = FlutterRunTestDriver(tempDir,    logPrefix: '   RUN  ');
+      flutterAttach = FlutterRunTestDriver(
+        tempDir,
+        logPrefix: 'ATTACH  ',
+        // Only one DDS instance can be connected to the VM service at a time.
+        // DDS can also only initialize if the VM service doesn't have any existing
+        // clients, so we'll just let _flutterRun be responsible for spawning DDS.
+        spawnDdsInstance: false,
+      );
+    });
+
+    tearDown(() async {
+      await flutterAttach.detach();
+      await flutterRun.stop();
+    });
+
+    Future<bool> isObservatoryAvailable() async {
+      final HttpClient client = HttpClient();
+      final Uri vmServiceUri = Uri(
+        scheme: 'http',
+        host: flutterRun.vmServiceWsUri!.host,
+        port: flutterRun.vmServicePort,
+      );
+
+      final HttpClientRequest request = await client.getUrl(vmServiceUri);
+      final HttpClientResponse response = await request.close();
+      final String content = await response.transform(utf8.decoder).join();
+      return content.contains('Dart VM Observatory');
+    }
+
+    testWithoutContext('enables Observatory on run', () async {
+        await flutterRun.run(
+          withDebugger: true,
+          // TODO(bkonyi): uncomment once Observatory is disabled by default
+          // See https://github.com/dart-lang/sdk/issues/50233
+          // serveObservatory: true,
+        );
+        expect(await isObservatoryAvailable(), true);
+    });
+
+    testWithoutContext('enables Observatory on attach', () async {
+      await flutterRun.run(withDebugger: true, serveObservatory: false);
+      // Bail out if Observatory is still served by default in the VM.
+      if (await isObservatoryAvailable()) {
+        return;
+      }
+      await flutterAttach.attach(
+        flutterRun.vmServicePort!,
+        // TODO(bkonyi): uncomment once Observatory is disabled by default
+        // See https://github.com/dart-lang/sdk/issues/50233
+        // serveObservatory: true,
+      );
+      expect(await isObservatoryAvailable(), true);
+    });
+  });
 }

packages/flutter_tools/test/integration.shard/test_driver.dart

@@ -499,6 +499,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
     bool expressionEvaluation = true,
     bool structuredErrors = false,
     bool singleWidgetReloads = false,
+    bool serveObservatory = true,
     String? script,
     List<String>? additionalCommandArgs,
   }) async {
@@ -509,6 +510,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
           '--disable-service-auth-codes',
         '--machine',
         if (!spawnDdsInstance) '--no-dds',
+        '--${serveObservatory ? '' : 'no-'}serve-observatory',
         ...getLocalEngineArguments(),
         '-d',
         if (chrome)
@@ -537,6 +539,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
     bool startPaused = false,
     bool pauseOnExceptions = false,
     bool singleWidgetReloads = false,
+    bool serveObservatory = true,
     List<String>? additionalCommandArgs,
   }) async {
     _attachPort = port;
@@ -547,6 +550,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
         '--machine',
         if (!spawnDdsInstance)
           '--no-dds',
+        '--${serveObservatory ? '' : 'no-'}serve-observatory',
         '-d',
         'flutter-tester',
         '--debug-port',

packages/flutter_tools/test/integration.shard/test_test.dart

@@ -198,6 +198,31 @@ void main() {
   testWithoutContext('flutter gold skips tests where the expectations are missing', () async {
     return _testFile('flutter_gold', automatedTestsDirectory, flutterTestDirectory, exitCode: isZero);
   });
+
+  testWithoutContext('flutter test should respect --serve-observatory', () async {
+    late final Process process;
+    try {
+      process = await _runFlutterTestConcurrent('trivial', automatedTestsDirectory, flutterTestDirectory,
+        extraArguments: const <String>['--start-paused', '--serve-observatory']);
+      final Completer<Uri> completer = Completer<Uri>();
+      final RegExp vmServiceUriRegExp = RegExp(r'((http)?:\/\/)[^\s]+');
+      late final StreamSubscription<String> sub;
+      sub = process.stdout.transform(utf8.decoder).listen((String e) {
+        if (vmServiceUriRegExp.hasMatch(e)) {
+          completer.complete(Uri.parse(vmServiceUriRegExp.firstMatch(e)!.group(0)!));
+          sub.cancel();
+        }
+      });
+      final Uri vmServiceUri = await completer.future;
+      final HttpClient client = HttpClient();
+      final HttpClientRequest request = await client.getUrl(vmServiceUri);
+      final HttpClientResponse response = await request.close();
+      final String content = await response.transform(utf8.decoder).join();
+      expect(content.contains('Dart VM Observatory'), true);
+    } finally {
+      process.kill();
+    }
+  });
 }
 
 Future<void> _testFile(
@@ -328,3 +353,45 @@ Future<ProcessResult> _runFlutterTest(
     stderrEncoding: utf8,
   );
 }
+
+Future<Process> _runFlutterTestConcurrent(
+  String? testName,
+  String workingDirectory,
+  String testDirectory, {
+  List<String> extraArguments = const <String>[],
+}) async {
+
+  String testPath;
+  if (testName == null) {
+    // Test everything in the directory.
+    testPath = testDirectory;
+    final Directory directoryToTest = fileSystem.directory(testPath);
+    if (!directoryToTest.existsSync()) {
+      fail('missing test directory: $directoryToTest');
+    }
+  } else {
+    // Test just a specific test file.
+    testPath = fileSystem.path.join(testDirectory, '${testName}_test.dart');
+    final File testFile = fileSystem.file(testPath);
+    if (!testFile.existsSync()) {
+      fail('missing test file: $testFile');
+    }
+  }
+
+  final List<String> args = <String>[
+    'test',
+    '--no-color',
+    '--no-version-check',
+    '--no-pub',
+    '--reporter',
+    'compact',
+    ...extraArguments,
+    testPath,
+  ];
+
+  return Process.start(
+    flutterBin, // Uses the precompiled flutter tool for faster tests,
+    args,
+    workingDirectory: workingDirectory,
+  );
+}