[dwds] callServiceExtension should check all extensions and return kMethodNotFound when extension not found and remove reassemble from reload

dart-lang#2584

Currently, callServiceExtension checks the extensionRpcs field in the VM
service. This list is populated when DWDS starts and checks the extensions
currently registered in the runtime. This list is prone to becoming stale,
however, as services may be registered or removed through the runtime
(via dart:developer) later. This is true for ext.flutter.reassemble, where DWDS
can not find the service because we computed the list too early.

Instead, we should seek to always check the runtime for the current registered
extensions and then update this list whenever we do. It’s still prone to becoming
stale, but it is less stale. Ideally, we would override extensionRpcs and fetch the
current registered extensions, but that isn’t possible since it requires awaiting a
future. We also can’t ignore this field altogether because it looks like clients, like
Dart DevTools, uses this field.

callServiceExtension should also return kMethodNotFound instead of its current
JS evaluation error when an extension is not present.

Lastly, the reassemble call is removed from reload as it can now be safely called in
Flutter tools instead.

Author: srujzs

dwds/CHANGELOG.md

@@ -1,5 +1,12 @@
 ## 24.3.7-wip
 
+- `callServiceExtension` now checks the runtime for the list of registered
+  service extensions. It also now throws a `RPCError` with
+  `RPCErrorKind.kMethodNotFound` when a service extension is not found instead
+  of throwing a JS evaluation error.
+- The registered extension `reassemble` is now no longer called when calling
+  `reloadSources`. Users should call `reassemble` using `callServiceExtension`.
+
 ## 24.3.6
 
 - Bump minimum sdk version to 3.7.0

dwds/lib/src/debugging/inspector.dart

@@ -115,8 +115,6 @@ class AppInspector implements AppInspectorInterface {
     await DartUri.initialize();
     DartUri.recordAbsoluteUris(libraries.map((lib) => lib.uri).nonNulls);
     DartUri.recordAbsoluteUris(scripts.map((script) => script.uri).nonNulls);
-
-    isolate.extensionRPCs?.addAll(await _getExtensionRpcs());
   }
 
   static IsolateRef _toIsolateRef(Isolate isolate) => IsolateRef(
@@ -760,11 +758,16 @@ class AppInspector implements AppInspectorInterface {
       scriptId == null ? null : _scriptRefsById[scriptId];
 
   /// Runs an eval on the page to compute all existing registered extensions.
-  Future<List<String>> _getExtensionRpcs() async {
+  ///
+  /// Combines this with the RPCs registered in the [isolate]. Use this over
+  /// [Isolate.extensionRPCs] as this computes a live set.
+  ///
+  /// Updates [Isolate.extensionRPCs] to this set.
+  Future<Set<String>> getExtensionRpcs() async {
     final expression =
         globalToolConfiguration.loadStrategy.dartRuntimeDebugger
             .getDartDeveloperExtensionNamesJsExpression();
-    final extensionRpcs = <String>[];
+    final extensionRpcs = <String>{};
     final params = {
       'expression': expression,
       'returnByValue': true,
@@ -784,6 +787,7 @@ class AppInspector implements AppInspectorInterface {
         s,
       );
     }
+    isolate.extensionRPCs = List<String>.from(extensionRpcs);
     return extensionRpcs;
   }
 

dwds/lib/src/injected/client.js

@@ -343,7 +343,7 @@ class ChromeProxyService implements VmServiceInterface {
     // TODO: We shouldn't need to fire these events since they exist on the
     // isolate, but devtools doesn't recognize extensions after a page refresh
     // otherwise.
-    for (final extensionRpc in inspector.isolate.extensionRPCs ?? []) {
+    for (final extensionRpc in await inspector.getExtensionRpcs()) {
       _streamNotify(
         'Isolate',
         Event(
@@ -509,6 +509,13 @@ class ChromeProxyService implements VmServiceInterface {
         v is String ? v : jsonEncode(v),
       ),
     );
+    if ((await inspector.getExtensionRpcs()).contains(method) != true) {
+      throw RPCError(
+        method,
+        RPCErrorKind.kMethodNotFound.code,
+        'Unknown service method: $method',
+      );
+    }
     final expression = globalToolConfiguration.loadStrategy.dartRuntimeDebugger
         .invokeExtensionJsExpression(method, jsonEncode(stringArgs));
     final result = await inspector.jsEvaluate(expression, awaitPromise: true);

dwds/lib/src/services/chrome_proxy_service.dart

@@ -30,13 +30,6 @@ extension type _Debugger._(JSObject _) implements JSObject {
       await invokeExtension(method, '{}').toDart;
     }
   }
-
-  Future<void> maybeInvokeFlutterReassemble() async {
-    final method = 'ext.flutter.reassemble';
-    if (extensionNames.toDart.contains(method.toJS)) {
-      await invokeExtension(method, '{}').toDart;
-    }
-  }
 }
 
 @JS('XMLHttpRequest')
@@ -92,9 +85,5 @@ class DdcLibraryBundleRestarter implements Restarter {
       }
     }
     await _dartDevEmbedder.hotReload(filesToLoad, librariesToReload).toDart;
-    // TODO(srujzs): Reassembling is slow. It's roughly almost the time it takes
-    // to recompile and do a hot reload. We should do some better profiling and
-    // see if we can improve this.
-    await _dartDevEmbedder.debugger.maybeInvokeFlutterReassemble();
   }
 }