[dds] Bump DDS to 5.0.0, use devtools_shared 11.0.0, and support a second DTD URI for web editors

devtools_shared 11.0.0 updates some signatures to use a new DTDInfo class to support both a local + exposed URI for DTD. A new CLI flag `--dtd-exposed-uri` for `devtools server` allows passing this second URI.

The DevTools server will use `--dtd-uri` to connect to DTD itself, but serve `--dtd-exposed-uri` to the frontend.

`--dtd-exposed-uri` is entirely optional and if not supplied, the value from `--dtd-uri` will be used in its place.

Change-Id: I5ab052ff9c4e7b2b186c1592f1ba2d63b7711113
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383400
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>

DEPS

@@ -102,7 +102,7 @@ vars = {
   "boringssl_rev": "2db0eb3f96a5756298dcd7f9319e56a98585bd10",
   "browser-compat-data_tag": "ac8cae697014da1ff7124fba33b0b4245cc6cd1b", # v1.0.22
   "cpu_features_rev": "936b9ab5515dead115606559502e3864958f7f6e",
-  "devtools_rev": "25053ae4af8f162188388c6f3786e03349652e51",
+  "devtools_rev": "f363ca0a9d233cbf238465185ab09d90cbcc75dd",
   "icu_rev": "43953f57b037778a1b8005564afabe214834f7bd",
   "jinja2_rev": "2222b31554f03e62600cd7e383376a7c187967a1",
   "libcxx_rev": "44079a4cc04cdeffb9cfe8067bfb3c276fb2bab0",

pkg/dds/CHANGELOG.md

@@ -1,5 +1,8 @@
+# 5.0.0
+- Updated the `devtools_shared` dependency to version `^11.0.0`.
+
 # 4.2.7
-- Added a new constant `RpcErrorCodes.kConnectionDisposed = -32010`) for requests
+- Added a new constant `RpcErrorCodes.kConnectionDisposed = -32010` for requests
   failing because the service connection was closed. This value is not currently
   used but is provided for clients to handle in preperation for a future release
   that will use it to avoid clients having to read error messages.

pkg/dds/lib/dds.dart

@@ -9,6 +9,8 @@ library;
 import 'dart:async';
 import 'dart:io';
 
+import 'package:devtools_shared/devtools_shared.dart' show DtdInfo;
+
 import 'src/dds_impl.dart';
 
 typedef UriConverter = String? Function(String uri);
@@ -154,7 +156,7 @@ abstract class DartDevelopmentService {
   ///
   /// This will be null if DTD was not started by the DevTools server. For
   /// example, it may have been started by an IDE.
-  ({String? uri, String? secret})? get hostedDartToolingDaemon;
+  DtdInfo? get hostedDartToolingDaemon;
 
   /// Set to `true` if this instance of [DartDevelopmentService] is accepting
   /// requests.

pkg/dds/lib/devtools_server.dart

@@ -41,6 +41,7 @@ class DevToolsServer {
   static const argDdsPort = 'dds-port';
   static const argDebugMode = 'debug';
   static const argDtdUri = 'dtd-uri';
+  static const argDtdExposedUri = 'dtd-exposed-uri';
   static const argPrintDtd = 'print-dtd';
   static const argLaunchBrowser = 'launch-browser';
   static const argMachine = 'machine';
@@ -107,6 +108,13 @@ class DevToolsServer {
         help: 'A URI pointing to a Dart Tooling Daemon that DevTools should '
             'interface with.',
       )
+      ..addOption(
+        argDtdExposedUri,
+        valueHelp: 'uri',
+        help: 'An optional URI for the DartTooling Daemon (--dtd-uri) that has '
+            'been exposed to the front-end to support environments split across '
+            'machines such as a web-based editor.',
+      )
       ..addFlag(
         argLaunchBrowser,
         help:
@@ -257,7 +265,7 @@ class DevToolsServer {
     String? profileFilename,
     String? appSizeBase,
     String? appSizeTest,
-    String? dtdUri,
+    DtdInfo? dtdInfo,
   }) async {
     hostname ??= 'localhost';
 
@@ -285,20 +293,15 @@ class DevToolsServer {
       requestNotificationPermissions: enableNotifications,
     );
 
-    String? dtdSecret;
-    if (dtdUri == null) {
-      final (:uri, :secret) = await startDtd(
-        machineMode: machineMode,
-        printDtdUri: printDtdUri,
-      );
-      dtdUri = uri;
-      dtdSecret = secret;
-    }
+    dtdInfo ??= await startDtd(
+      machineMode: machineMode,
+      printDtdUri: printDtdUri,
+    );
 
     handler ??= await defaultHandler(
       buildDir: customDevToolsPath!,
       clientManager: clientManager,
-      dtd: (uri: dtdUri, secret: dtdSecret),
+      dtd: dtdInfo,
       devtoolsExtensionsManager: ExtensionsManager(),
     );
 
@@ -493,19 +496,40 @@ class DevToolsServer {
     final bool verboseMode = args[argVerbose];
     final String? hostname = args[argHost];
 
-    String? dtdUri;
+    // A helper to print a message and usage information that can be used in
+    // a return statement.
+    Null printUsage(String message) {
+      print(message);
+      print('');
+      _printUsage(buildArgParser(verbose: verbose));
+    }
+
+    Uri? dtdUri;
     if (args.wasParsed(argDtdUri)) {
-      dtdUri = args[argDtdUri];
+      dtdUri = Uri.tryParse(args[argDtdUri]);
+      if (dtdUri == null || !dtdUri.hasScheme) {
+        return printUsage('--dtd-uri must be a valid URI');
+      }
+    }
+
+    Uri? dtdExposedUri;
+    if (args.wasParsed(argDtdExposedUri)) {
+      if (dtdUri == null) {
+        return printUsage(
+            '--dtd-exposed-uri can only be supplied with --dtd-uri');
+      }
+
+      dtdExposedUri = Uri.tryParse(args[argDtdExposedUri]);
+      if (dtdExposedUri == null || !dtdExposedUri.hasScheme) {
+        return printUsage('--dtd-exposed-uri must be a valid URI');
+      }
     }
 
     final printDtdUri = args.wasParsed(argPrintDtd);
 
     if (help) {
-      print(
+      return printUsage(
           'Dart DevTools version ${await DevToolsUtils.getVersion(customDevToolsPath ?? "")}');
-      print('');
-      _printUsage(buildArgParser(verbose: verbose));
-      return null;
     }
 
     if (version) {
@@ -566,7 +590,8 @@ class DevToolsServer {
       hostname: hostname,
       appSizeBase: appSizeBase,
       appSizeTest: appSizeTest,
-      dtdUri: dtdUri,
+      dtdInfo:
+          dtdUri != null ? DtdInfo(dtdUri, exposedUri: dtdExposedUri) : null,
       printDtdUri: printDtdUri,
     );
   }

pkg/dds/lib/src/dds_cli_entrypoint.dart

@@ -137,7 +137,9 @@ ${argParser.usage}
       if (dds.devToolsUri != null) 'devToolsUri': dds.devToolsUri.toString(),
       if (dtdInfo != null)
         'dtd': {
-          'uri': dtdInfo.uri,
+          // For DDS-hosted DTD, there's only ever a local URI since there
+          // is no mechanism for exposing URIs.
+          'uri': dtdInfo.localUri.toString(),
         },
     }));
   } catch (e, st) {

pkg/dds/lib/src/dds_impl.dart

@@ -10,7 +10,7 @@ import 'dart:math';
 import 'dart:typed_data';
 
 import 'package:devtools_shared/devtools_extensions_io.dart';
-import 'package:devtools_shared/devtools_shared.dart' show DTDConnectionInfo;
+import 'package:devtools_shared/devtools_shared.dart' show DtdInfo;
 import 'package:json_rpc_2/json_rpc_2.dart' as json_rpc;
 import 'package:meta/meta.dart';
 import 'package:shelf/shelf.dart';
@@ -379,10 +379,7 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
           dds: this,
           buildDir: buildDir,
           notFoundHandler: notFoundHandler,
-          dtd: (
-            uri: _hostedDartToolingDaemon?.uri,
-            secret: _hostedDartToolingDaemon?.secret
-          ),
+          dtd: _hostedDartToolingDaemon,
           devtoolsExtensionsManager: ExtensionsManager(),
         ) as FutureOr<Response> Function(Request);
       }
@@ -512,9 +509,9 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
   }
 
   @override
-  DTDConnectionInfo? get hostedDartToolingDaemon => _hostedDartToolingDaemon;
+  DtdInfo? get hostedDartToolingDaemon => _hostedDartToolingDaemon;
 
-  DTDConnectionInfo? _hostedDartToolingDaemon;
+  DtdInfo? _hostedDartToolingDaemon;
 
   final bool _ipv6;
 

pkg/dds/lib/src/devtools/client.dart

@@ -6,7 +6,6 @@
 
 import 'dart:async';
 
-import 'package:devtools_shared/devtools_server.dart';
 import 'package:json_rpc_2/src/peer.dart' as json_rpc;
 import 'package:meta/meta.dart';
 import 'package:sse/src/server/sse_handler.dart';
@@ -214,18 +213,6 @@ class DevToolsClient {
       _embedded = parameters['embedded'].asBool;
     });
 
-    _devToolsPeer.registerMethod('getPreferenceValue', (parameters) {
-      final key = parameters['key'].asString;
-      final value = ServerApi.devToolsPreferences.properties[key];
-      return value;
-    });
-
-    _devToolsPeer.registerMethod('setPreferenceValue', (parameters) {
-      final key = parameters['key'].asString;
-      final value = parameters['value'].value;
-      ServerApi.devToolsPreferences.properties[key] = value;
-    });
-
     _devToolsPeer.registerMethod('pingResponse', (parameters) {
       _nextPingResponse.complete();
       _nextPingResponse = Completer();

pkg/dds/lib/src/devtools/dtd.dart

@@ -7,12 +7,12 @@ import 'dart:convert';
 import 'dart:io';
 import 'dart:isolate';
 
-import 'package:devtools_shared/devtools_shared.dart' show DTDConnectionInfo;
+import 'package:devtools_shared/devtools_shared.dart' show DtdInfo;
 import 'package:path/path.dart' as path;
 
 import 'utils.dart';
 
-Future<DTDConnectionInfo> startDtd({
+Future<DtdInfo?> startDtd({
   required bool machineMode,
   required bool printDtdUri,
 }) async {
@@ -24,8 +24,8 @@ Future<DTDConnectionInfo> startDtd({
     'dart_tooling_daemon.dart.snapshot',
   );
 
-  final completer = Completer<DTDConnectionInfo>();
-  void completeForError() => completer.complete((uri: null, secret: null));
+  final completer = Completer<DtdInfo?>();
+  void completeForError() => completer.complete(null);
 
   final exitPort = ReceivePort()
     ..listen((_) {
@@ -57,7 +57,7 @@ Future<DTDConnectionInfo> startDtd({
               machineMode: machineMode,
             );
           }
-          completer.complete((uri: uri, secret: secret));
+          completer.complete(DtdInfo(Uri.parse(uri), secret: secret));
         }
       } catch (_) {
         completeForError();
@@ -78,7 +78,7 @@ Future<DTDConnectionInfo> startDtd({
 
   final result = await completer.future.timeout(
     const Duration(seconds: 5),
-    onTimeout: () => (uri: null, secret: null),
+    onTimeout: () => null,
   );
   receivePort.close();
   errorPort.close();

pkg/dds/lib/src/devtools/handler.dart

@@ -45,7 +45,7 @@ FutureOr<Handler> defaultHandler({
   required String buildDir,
   ClientManager? clientManager,
   Handler? notFoundHandler,
-  DTDConnectionInfo? dtd,
+  DtdInfo? dtd,
   required ExtensionsManager devtoolsExtensionsManager,
 }) {
   // When served through DDS, the app root is /devtools.

pkg/dds/lib/src/devtools/machine_mode_command_handler.dart

@@ -272,7 +272,7 @@ class MachineModeCommandHandler {
           _devToolsUsage = null;
         }
         break;
-      case apiSetActiveSurvey:
+      case SurveyApi.setActiveSurvey:
         _devToolsUsage!.activeSurvey = value;
         DevToolsUtils.printOutput(
           'DevTools Survey',
@@ -286,7 +286,7 @@ class MachineModeCommandHandler {
           machineMode: machineMode,
         );
         break;
-      case apiGetSurveyActionTaken:
+      case SurveyApi.getSurveyActionTaken:
         DevToolsUtils.printOutput(
           'DevTools Survey',
           {
@@ -299,7 +299,7 @@ class MachineModeCommandHandler {
           machineMode: machineMode,
         );
         break;
-      case apiSetSurveyActionTaken:
+      case SurveyApi.setSurveyActionTaken:
         _devToolsUsage!.surveyActionTaken = jsonDecode(value);
         DevToolsUtils.printOutput(
           'DevTools Survey',
@@ -313,7 +313,7 @@ class MachineModeCommandHandler {
           machineMode: machineMode,
         );
         break;
-      case apiGetSurveyShownCount:
+      case SurveyApi.getSurveyShownCount:
         DevToolsUtils.printOutput(
           'DevTools Survey',
           {
@@ -326,7 +326,7 @@ class MachineModeCommandHandler {
           machineMode: machineMode,
         );
         break;
-      case apiIncrementSurveyShownCount:
+      case SurveyApi.incrementSurveyShownCount:
         _devToolsUsage!.incrementSurveyShownCount();
         DevToolsUtils.printOutput(
           'DevTools Survey',

pkg/dds/pubspec.yaml

@@ -1,5 +1,5 @@
 name: dds
-version: 4.2.7
+version: 5.0.0
 description: >-
   A library used to spawn the Dart Developer Service, used to communicate with
   a Dart VM Service instance.
@@ -16,7 +16,7 @@ dependencies:
   dds_service_extensions: ^2.0.0
   dap: ^1.3.0
   extension_discovery: ^2.0.0
-  devtools_shared: ^10.0.2
+  devtools_shared: ^11.0.0
   http_multi_server: ^3.0.0
   json_rpc_2: ^3.0.0
   meta: ^1.1.8

pkg/dds/test/devtools_server/dtd_test.dart

@@ -8,20 +8,25 @@ import 'package:test/test.dart';
 import 'utils/server_driver.dart';
 
 void main() {
+  const dtdUriSwitch = '--${DevToolsServer.argDtdUri}';
+  const dtdExposedUriSwitch = '--${DevToolsServer.argDtdExposedUri}';
+
   group('Dart Tooling Daemon connection', () {
     test('does not start DTD when a DTD uri is passed as an argument',
         () async {
       final server = await DevToolsServerDriver.create(
-        additionalArgs: ['--${DevToolsServer.argDtdUri}=some_uri'],
+        additionalArgs: ['$dtdUriSwitch=ws://localhost:123/'],
       );
       try {
+        // Ensure the event does not arrive within some reasonable amount of
+        // time.
         final dtdStartedEvent = await server.stdout
             .firstWhere(
               (map) => map!['event'] == 'server.dtdStarted',
               orElse: () => null,
             )
             .timeout(
-              const Duration(seconds: 3),
+              Duration(seconds: 3),
               onTimeout: () => null,
             );
         expect(dtdStartedEvent, isNull);
@@ -42,5 +47,45 @@ void main() {
         server.kill();
       }
     });
+
+    test('rejects invalid URIs for --dtd-uri', () async {
+      final server = await DevToolsServerDriver.create(
+        additionalArgs: ['$dtdUriSwitch=some_uri'],
+      );
+      try {
+        final firstLine = await server.stdoutRaw.first;
+        expect(firstLine, '$dtdUriSwitch must be a valid URI');
+      } finally {
+        server.kill();
+      }
+    });
+
+    test('rejects invalid URIs for --dtd-exposed-uri', () async {
+      final server = await DevToolsServerDriver.create(
+        additionalArgs: [
+          '$dtdUriSwitch=ws://localhost:123/',
+          '$dtdExposedUriSwitch=some_uri'
+        ],
+      );
+      try {
+        final firstLine = await server.stdoutRaw.first;
+        expect(firstLine, '$dtdExposedUriSwitch must be a valid URI');
+      } finally {
+        server.kill();
+      }
+    });
+
+    test('rejects --dtd-exposed-uri without --dtd-uri', () async {
+      final server = await DevToolsServerDriver.create(
+        additionalArgs: ['$dtdExposedUriSwitch=some_uri'],
+      );
+      try {
+        final firstLine = await server.stdoutRaw.first;
+        expect(firstLine,
+            '$dtdExposedUriSwitch can only be supplied with $dtdUriSwitch');
+      } finally {
+        server.kill();
+      }
+    });
   });
 }

pkg/dds/test/devtools_server/utils/server_driver.dart

@@ -18,38 +18,51 @@ class DevToolsServerDriver {
     this._stdin,
     Stream<String> _stdout,
     Stream<String> _stderr,
-  )   : stdout = _convertToMapStream(_stdout),
-        stderr = _stderr.map((line) {
+  ) : stderr = _stderr.map((line) {
           _trace('<== STDERR $line');
           return line;
-        });
+        }) {
+    // Many tests verify JSON output in stdout but some verify usage output
+    // to stdout for invalid args, so split the process stdout into two
+    // streams, one as JSON and one raw strings.
+    var stdoutRawController = StreamController<String>();
+    stdoutRaw = stdoutRawController.stream;
+    var stdoutJsonController = StreamController<Map<String, Object?>?>();
+    stdout = stdoutJsonController.stream;
+    _stdout.listen((line) {
+      _trace('<== $line');
+
+      // Send to raw stdout stream.
+      stdoutRawController.add(line);
+
+      // If the output is JSON, also send a copy to stdoutJson.
+      try {
+        var json = jsonDecode(line) as Map<String, Object?>;
+        stdoutJsonController.add(json);
+      } catch (_) {}
+    }, onError: (e, s) {
+      stdoutRawController.addError(e, s);
+      stdoutJsonController.addError(e, s);
+    }, onDone: () {
+      stdoutRawController.close();
+      stdoutJsonController.close();
+    });
+  }
 
   final Process _process;
-  final Stream<Map<String, dynamic>?> stdout;
+  late final Stream<Map<String, dynamic>?> stdout;
+  late final Stream<String> stdoutRaw;
   final Stream<String> stderr;
   final StringSink _stdin;
 
+  Future<int> get exitCode => _process.exitCode;
+
   void write(Map<String, dynamic> request) {
     final line = jsonEncode(request);
     _trace('==> $line');
     _stdin.writeln(line);
   }
 
-  static Stream<Map<String, dynamic>?> _convertToMapStream(
-    Stream<String> stream,
-  ) {
-    return stream.map((line) {
-      _trace('<== $line');
-      return line;
-    }).map((line) {
-      try {
-        return jsonDecode(line) as Map<String, dynamic>;
-      } catch (e) {
-        return null;
-      }
-    }).where((item) => item != null);
-  }
-
   static void _trace(String message) {
     if (verbose) {
       print(message);