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

bkonyi · web-flow · commit 6cd4fa45e268 · 2023-01-25T12:46:08.000-05:00
This flag will allow for Observatory to be served by the VM service once it is disabled by default in the Dart SDK.
diff --git a/packages/flutter_tools/lib/src/base/dds.dart b/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);
+  }
 }
diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/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
diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/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,
       );
     }
diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/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,
diff --git a/packages/flutter_tools/lib/src/device.dart b/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,
     );
 }
diff --git a/packages/flutter_tools/lib/src/resident_devtools_handler.dart b/packages/flutter_tools/lib/src/resident_devtools_handler.dart
@@ -91,6 +91,16 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
     final List<FlutterDevice?> devicesWithExtension = await _devicesWithExtensions(flutterDevices);
     await _maybeCallDevToolsUriServiceExtension(devicesWithExtension);
     await _callConnectedVmServiceUriExtension(devicesWithExtension);
+
+    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(_devToolsLauncher!.devToolsUrl!);
+    }
+
     if (_shutdown) {
       // If we're shutting down, no point reporting the debugger list.
       return;
diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/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;
diff --git a/packages/flutter_tools/lib/src/run_cold.dart b/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();
diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/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(
diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/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) {
diff --git a/packages/flutter_tools/lib/src/test/flutter_tester_device.dart b/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);
       },
     );
diff --git a/packages/flutter_tools/test/general.shard/resident_devtools_handler_test.dart b/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) {}
+}
diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart
diff --git a/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart b/packages/flutter_tools/test/integration.shard/flutter_attach_test.dart
diff --git a/packages/flutter_tools/test/integration.shard/test_driver.dart b/packages/flutter_tools/test/integration.shard/test_driver.dart
diff --git a/packages/flutter_tools/test/integration.shard/test_test.dart b/packages/flutter_tools/test/integration.shard/test_test.dart

Original file line number	Diff line number	Diff line change
`@@ -96,4 +96,8 @@ class DartDevelopmentService {`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`Future<void> shutdown() async => _ddsInstance?.shutdown();`
	`99`	`+`
	`100`	`+ void setExternalDevToolsUri(Uri uri) {`
	`101`	`+ _ddsInstance?.setExternalDevToolsUri(uri);`
	`102`	`+ }`
`99`	`103`	`}`