Reland "[ VM / DartDev ] Run all debugging sessions via 'dart run'"

This is a reland of 604a366065

Original change's description:
> [ VM / DartDev ] Run all debugging sessions via 'dart run'
> 
> Prior to this change, running `dart --observe foo.dart` would result in
> the DartDev flow being bypassed. While `dart foo.dart` will continue to
> bypass DartDev and be run directly, running either `dart --observe
> <vm-flags> foo.dart` or `dart --enable-vm-service <vm-flags> foo.dart`
> will result in the command being implicitly converted to `dart run
> --observe <vm-flags> foo.dart`. This is required for all standalone VM
> instances to run against DDS instead of the VM service directly.
> 
> As usual, the DartDev flow can always be bypassed by providing the
> --disable-dart-dev flag.
> 
> Change-Id: I211cd1ec4b1ec0e75ae0a568a66f1a0fc7b3852f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/147342
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>

Change-Id: I917d3056322d74766bdf376a3e28871ad5b66cfa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/147980
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/analysis_server/benchmark/perf/memory_tests.dart

@@ -98,11 +98,16 @@ class ServiceProtocol {
     socket.listen(_handleMessage);
   }
 
-  Future<Map> call(String method, [Map args]) {
+  Future<Map> call(String method, [Map args = const {}]) {
     var id = '${++_id}';
     var completer = Completer<Map>();
     _completers[id] = completer;
-    var m = <String, dynamic>{'id': id, 'method': method};
+    var m = <String, dynamic>{
+      'jsonrpc': '2.0',
+      'id': id,
+      'method': method,
+      'args': args
+    };
     if (args != null) m['params'] = args;
     var message = jsonEncode(m);
     socket.add(message);

pkg/analysis_server/test/integration/support/integration_tests.dart

@@ -587,7 +587,9 @@ class Server {
       serverPath = normalize(join(rootDir, 'bin', 'server.dart'));
     }
 
-    var arguments = <String>[];
+    var arguments = <String>[
+      '--disable-dart-dev',
+    ];
     //
     // Add VM arguments.
     //

pkg/dartdev/lib/src/commands/run.dart

@@ -60,7 +60,7 @@ Run a Dart file.''');
     // synchronization).
     if (args.any((element) => (element.startsWith('--observe') ||
         element.startsWith('--enable-vm-service')))) {
-      return await _DebuggingSession(args).start();
+      return await _DebuggingSession(this, args).start();
     }
 
     // Starting in ProcessStartMode.inheritStdio mode means the child process
@@ -73,7 +73,8 @@ Run a Dart file.''');
 }
 
 class _DebuggingSession {
-  _DebuggingSession(List<String> args) : _args = args.toList() {
+  _DebuggingSession(this._runCommand, List<String> args)
+      : _args = args.toList() {
     // Process flags that are meant to configure the VM service HTTP server or
     // dump VM service connection information to a file. Since the VM service
     // clients won't actually be connecting directly to the service, we'll make
@@ -84,23 +85,32 @@ class _DebuggingSession {
         if (isObserve) {
           _observe = true;
         }
-        // These flags can be provided by the embedder so we need to check for
-        // both `=` and `:` separators.
-        final observatoryBindInfo =
-            (arg.contains('=') ? arg.split('=') : arg.split(':'))[1].split('/');
-        _port = int.tryParse(observatoryBindInfo.first) ?? 0;
-        if (observatoryBindInfo.length > 1) {
-          try {
-            _bindAddress = Uri.http(observatoryBindInfo[1], '');
-          } on FormatException {
-            // TODO(bkonyi): log invalid parse? The VM service just ignores bad
-            // input flags.
-            // Ignore.
+        if (arg.contains('=') || arg.contains(':')) {
+          // These flags can be provided by the embedder so we need to check for
+          // both `=` and `:` separators.
+          final observatoryBindInfo =
+              (arg.contains('=') ? arg.split('=') : arg.split(':'))[1]
+                  .split('/');
+          _port = int.tryParse(observatoryBindInfo.first) ?? 0;
+          if (observatoryBindInfo.length > 1) {
+            try {
+              _bindAddress = Uri.http(observatoryBindInfo[1], '');
+            } on FormatException {
+              // TODO(bkonyi): log invalid parse? The VM service just ignores bad
+              // input flags.
+              // Ignore.
+            }
           }
         }
       } else if (arg.startsWith('--write-service-info=')) {
         try {
-          _serviceInfoUri = Uri.parse(arg.split('=')[1]);
+          final split = arg.split('=');
+          if (split[1].isNotEmpty) {
+            _serviceInfoUri = Uri.parse(split[1]);
+          } else {
+            _runCommand.usageException(
+                'Invalid URI argument to --write-service-info: "${split[1]}"');
+          }
         } on FormatException {
           // TODO(bkonyi): log invalid parse? The VM service just ignores bad
           // input flags.
@@ -129,7 +139,7 @@ class _DebuggingSession {
     // Start using ProcessStartMode.normal and forward stdio manually as we
     // need to filter the true VM service URI and replace it with the DDS URI.
     _process = await Process.start(
-      'dart',
+      sdk.dart,
       [
         '--disable-dart-dev',
         _observe
@@ -152,7 +162,7 @@ class _DebuggingSession {
       // Shutdown DDS if it was started and wait for the process' stdio streams
       // to close so we don't truncate program output.
       await Future.wait([
-        _dds?.shutdown(),
+        if (_dds != null) _dds.shutdown(),
         _stderrDone,
         _stdoutDone,
       ]);
@@ -194,8 +204,8 @@ class _DebuggingSession {
       if (_dds == null) {
         return msg;
       }
-      if (msg.startsWith('Observatory listening on') ||
-          msg.startsWith('Connect to Observatory at')) {
+      if (msg.contains('Observatory listening on') ||
+          msg.contains('Connect to Observatory at')) {
         // Search for the VM service URI in the message and replace it.
         msg = msg.replaceFirst(
           RegExp(r'https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.'
@@ -244,7 +254,7 @@ class _DebuggingSession {
   Uri _bindAddress = Uri.http('127.0.0.1', '');
   DartDevelopmentService _dds;
   bool _observe = false;
-  int _port;
+  int _port = 0;
   Process _process;
   Uri _serviceInfoUri;
   Future _stderrDone;
@@ -252,4 +262,5 @@ class _DebuggingSession {
 
   final List<String> _args;
   final Completer<void> _ddsCompleter = Completer();
+  final RunCommand _runCommand;
 }

pkg/dds/lib/src/dds_impl.dart

@@ -65,7 +65,7 @@ class _DartDevelopmentService implements DartDevelopmentService {
     }
     _shuttingDown = true;
     // Don't accept anymore HTTP requests.
-    await _server.close();
+    await _server?.close();
 
     // Close connections to clients.
     await clientManager.shutdown();

pkg/dds/test/smoke_test.dart

@@ -18,6 +18,7 @@ Future<Process> spawnDartProcess(String script) async {
   final serviceInfoFile = await File.fromUri(serviceInfoUri).create();
 
   final arguments = [
+    '--disable-dart-dev',
     '--observe=0',
     '--pause-isolates-on-start',
     '--write-service-info=$serviceInfoUri',

pkg/vm/test/incremental_compiler_test.dart

@@ -454,7 +454,7 @@ main() {
 
       const kObservatoryListening = 'Observatory listening on ';
       final RegExp observatoryPortRegExp =
-          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)/");
+          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)");
       int port;
       final splitter = new LineSplitter();
       Completer<String> portLineCompleter = new Completer<String>();
@@ -580,7 +580,7 @@ main() {
 
       const kObservatoryListening = 'Observatory listening on ';
       final RegExp observatoryPortRegExp =
-          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)/");
+          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)");
       int port;
       final splitter = new LineSplitter();
       Completer<String> portLineCompleter = new Completer<String>();
@@ -679,7 +679,7 @@ main() {
       String portLine = await portLineCompleter.future;
 
       final RegExp observatoryPortRegExp =
-          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)/");
+          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)");
       expect(observatoryPortRegExp.hasMatch(portLine), isTrue);
       final match = observatoryPortRegExp.firstMatch(portLine);
       final port = int.parse(match.group(1));
@@ -820,7 +820,7 @@ main() {
 
       const kObservatoryListening = 'Observatory listening on ';
       final RegExp observatoryPortRegExp =
-          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)/");
+          new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)");
       int port;
       final splitter = new LineSplitter();
       Completer<String> portLineCompleter = new Completer<String>();
@@ -1297,7 +1297,7 @@ class RemoteVm {
 
   /// Retrieves the ID of the main isolate using the service protocol.
   Future<String> _computeMainId() async {
-    var vm = await rpc.sendRequest('getVM');
+    var vm = await rpc.sendRequest('getVM', {});
     var isolates = vm['isolates'];
     for (var isolate in isolates) {
       if (isolate['name'].contains(r'$main')) {

pkg/vm_service/test/common/test_helper.dart

@@ -141,7 +141,9 @@ class _ServiceTesteeLauncher {
       List<String> extraArgs) {
     String dartExecutable = Platform.executable;
 
-    var fullArgs = <String>[];
+    var fullArgs = <String>[
+      '--disable-dart-dev',
+    ];
     if (pause_on_start) {
       fullArgs.add('--pause-isolates-on-start');
     }

runtime/bin/main.cc

@@ -999,7 +999,7 @@ void main(int argc, char** argv) {
   char* script_name;
   const int EXTRA_VM_ARGUMENTS = 10;
   CommandLineOptions vm_options(argc + EXTRA_VM_ARGUMENTS);
-  CommandLineOptions dart_options(argc);
+  CommandLineOptions dart_options(argc + EXTRA_VM_ARGUMENTS);
   bool print_flags_seen = false;
   bool verbose_debug_seen = false;
 

runtime/bin/main_options.cc

@@ -35,6 +35,7 @@ static const char* kSnapshotKindNames[] = {
 };
 
 SnapshotKind Options::gen_snapshot_kind_ = kNone;
+bool Options::enable_vm_service_ = false;
 
 #define OPTION_FIELD(variable) Options::variable##_
 
@@ -308,7 +309,8 @@ bool Options::ProcessEnableVmServiceOption(const char* arg,
   if (value == NULL) {
     return false;
   }
-  if (!ExtractPortAndAddress(
+  if (Options::disable_dart_dev() &&
+      !ExtractPortAndAddress(
           value, &vm_service_server_port_, &vm_service_server_ip_,
           DEFAULT_VM_SERVICE_SERVER_PORT, DEFAULT_VM_SERVICE_SERVER_IP)) {
     Syslog::PrintErr(
@@ -319,7 +321,7 @@ bool Options::ProcessEnableVmServiceOption(const char* arg,
 #if !defined(DART_PRECOMPILED_RUNTIME)
   dfe()->set_use_incremental_compiler(true);
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
-
+  enable_vm_service_ = true;
   return true;
 }
 
@@ -329,7 +331,8 @@ bool Options::ProcessObserveOption(const char* arg,
   if (value == NULL) {
     return false;
   }
-  if (!ExtractPortAndAddress(
+  if (Options::disable_dart_dev() &&
+      !ExtractPortAndAddress(
           value, &vm_service_server_port_, &vm_service_server_ip_,
           DEFAULT_VM_SERVICE_SERVER_PORT, DEFAULT_VM_SERVICE_SERVER_IP)) {
     Syslog::PrintErr(
@@ -346,6 +349,7 @@ bool Options::ProcessObserveOption(const char* arg,
 #if !defined(DART_PRECOMPILED_RUNTIME)
   dfe()->set_use_incremental_compiler(true);
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
+  enable_vm_service_ = true;
   return true;
 }
 
@@ -376,6 +380,10 @@ bool Options::ProcessAbiVersionOption(const char* arg,
   return true;
 }
 
+static bool IsOption(const char* actual, const char* expected) {
+  return (OptionProcessor::ProcessOption(actual, expected) != nullptr);
+}
+
 int Options::ParseArguments(int argc,
                             char** argv,
                             bool vm_run_app_snapshot,
@@ -397,7 +405,7 @@ int Options::ParseArguments(int argc,
 
   // Parse out the vm options.
   while (i < argc) {
-    if (OptionProcessor::TryProcess(argv[i], vm_options)) {
+    if (OptionProcessor::TryProcess(argv[i], &temp_vm_options)) {
       i++;
     } else {
       // Check if this flag is a potentially valid VM flag.
@@ -442,18 +450,21 @@ int Options::ParseArguments(int argc,
   // The arguments to the VM are at positions 1 through i-1 in argv.
   Platform::SetExecutableArguments(i, argv);
 
-  bool is_script = false;
+  bool implicitly_use_dart_dev = false;
+  bool run_script = false;
   int script_or_cmd_index = -1;
   // Get the script name.
   if (i < argc) {
-    // If the script name is a valid file or a URL, we'll run the script directly.
-    // Otherwise, this might be a DartDev command and we need to try to
-    // find the DartDev snapshot so we can forward the command and its arguments.
+    // If the script name is a valid file or a URL, we'll run the script
+    // directly. Otherwise, this might be a DartDev command and we need to try
+    // to find the DartDev snapshot so we can forward the command and its
+    // arguments.
+    bool is_potential_file_path = !DartDevUtils::ShouldParseCommand(argv[i]);
     script_or_cmd_index = i;
     if (Options::disable_dart_dev() ||
-        !DartDevUtils::ShouldParseCommand(argv[i])) {
+        (is_potential_file_path && !enable_vm_service_)) {
       *script_name = strdup(argv[i]);
-      is_script = true;
+      run_script = true;
       i++;
     } else if (!DartDevUtils::TryResolveDartDevSnapshotPath(script_name)) {
       Syslog::PrintErr(
@@ -461,6 +472,22 @@ int Options::ParseArguments(int argc,
           argv[i]);
       Platform::Exit(kErrorExitCode);
     }
+    // Handle the special case where the user is running a Dart program without
+    // using a DartDev command and wants to use the VM service. Here we'll run
+    // the program using DartDev as it's used to spawn a DDS instance
+    if (!Options::disable_dart_dev() && is_potential_file_path &&
+        enable_vm_service_) {
+      implicitly_use_dart_dev = true;
+      dart_options->AddArgument("run");
+      for (int j = 1; j < i; ++j) {
+        if (IsOption(argv[j], "--observe")) {
+          dart_options->AddArgument(argv[j]);
+        }
+        if (IsOption(argv[j], "--enable-vm-service")) {
+          dart_options->AddArgument(argv[j]);
+        }
+      }
+    }
   } else if (!Options::disable_dart_dev() &&
              ((Options::help_option() && !Options::verbose_option()) ||
               (argc == 1)) &&
@@ -472,11 +499,18 @@ int Options::ParseArguments(int argc,
     return -1;
   }
 
-  if (Options::disable_dart_dev() || is_script) {
-    // Only populate the VM options if we're not running with dartdev.
-    const char** vm_argv = temp_vm_options.arguments();
-    int vm_argc = temp_vm_options.count();
+  const char** vm_argv = temp_vm_options.arguments();
+  int vm_argc = temp_vm_options.count();
+
+  if (Options::disable_dart_dev() || run_script) {
+    // Only populate the VM options if we're not running with DartDev.
     vm_options->AddArguments(vm_argv, vm_argc);
+  } else if (implicitly_use_dart_dev) {
+    // If we're using DartDev implicitly (e.g., dart --observe foo.dart), we
+    // want to forward all the VM arguments to the spawned process to ensure
+    // the program behaves as the user expects even though we're running
+    // through DartDev without their knowledge.
+    dart_options->AddArguments(vm_argv, vm_argc);
   } else if (i > 1) {
     // If we're running with DartDev, we're going to ignore the VM options for
     // this VM instance and print a warning.

runtime/bin/main_options.h

@@ -168,6 +168,7 @@ class Options {
 
   // VM Service argument processing.
   static const char* vm_service_server_ip_;
+  static bool enable_vm_service_;
   static int vm_service_server_port_;
   static bool ExtractPortAndAddress(const char* option_value,
                                     int* out_port,

runtime/observatory/tests/service/test_helper.dart

@@ -159,7 +159,9 @@ class _ServiceTesteeLauncher {
 
     final String dartExecutable = Platform.executable;
 
-    final fullArgs = <String>[];
+    final fullArgs = <String>[
+      '--disable-dart-dev',
+    ];
     if (pause_on_start) {
       fullArgs.add('--pause-isolates-on-start');
     }
@@ -415,7 +417,7 @@ class _ServiceTesterRunner {
       () => ignoreLateException(
         () async {
           if (useDds) {
-            await dds.shutdown();
+            await dds?.shutdown();
           }
           process.requestExit();
         },