[flutter_tools] refactor devtools handler to expose single method for...

[flutter_tools] refactor devtools handler to expose single method for run/attach and restart (#75807)

packages/flutter_tools/lib/src/resident_devtools_handler.dart

@@ -13,10 +13,27 @@ import 'base/logger.dart';
 import 'resident_runner.dart';
 import 'vmservice.dart';
 
+typedef ResidentDevtoolsHandlerFactory = ResidentDevtoolsHandler Function(DevtoolsLauncher, ResidentRunner, Logger);
+
+ResidentDevtoolsHandler createDefaultHandler(DevtoolsLauncher launcher, ResidentRunner runner, Logger logger) {
+  return FlutterResidentDevtoolsHandler(launcher, runner, logger);
+}
+
 /// Helper class to manage the life-cycle of devtools and its interaction with
 /// the resident runner.
-class ResidentDevtoolsHandler {
-  ResidentDevtoolsHandler(this._devToolsLauncher, this._residentRunner, this._logger);
+abstract class ResidentDevtoolsHandler {
+  /// The current devtools server, or null if one is not running.
+  DevToolsServerAddress get activeDevToolsServer;
+
+  Future<void> hotRestart(List<FlutterDevice> flutterDevices);
+
+  Future<void> serveAndAnnounceDevTools({Uri devToolsServerAddress, List<FlutterDevice> flutterDevices});
+
+  Future<void> shutdown();
+}
+
+class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
+  FlutterResidentDevtoolsHandler(this._devToolsLauncher, this._residentRunner, this._logger);
 
   final DevtoolsLauncher _devToolsLauncher;
   final ResidentRunner _residentRunner;
@@ -24,10 +41,11 @@ class ResidentDevtoolsHandler {
   bool _shutdown = false;
   bool _served = false;
 
-  /// The current devtools server, or null if one is not running.
+  @override
   DevToolsServerAddress get activeDevToolsServer =>  _devToolsLauncher?.activeDevToolsServer;
 
   // This must be guaranteed not to return a Future that fails.
+  @override
   Future<void> serveAndAnnounceDevTools({
     Uri devToolsServerAddress,
     @required List<FlutterDevice> flutterDevices,
@@ -48,12 +66,16 @@ class ResidentDevtoolsHandler {
       // report their URLs yet. Do so now.
       _residentRunner.printDebuggerList(includeObservatory: false);
     }
-    await maybeCallDevToolsUriServiceExtension(
+    await _waitForExtensions(flutterDevices);
+    await _maybeCallDevToolsUriServiceExtension(
+      flutterDevices,
+    );
+    await _callConnectedVmServiceUriExtension(
       flutterDevices,
     );
   }
 
-   Future<void> maybeCallDevToolsUriServiceExtension(
+   Future<void> _maybeCallDevToolsUriServiceExtension(
      List<FlutterDevice> flutterDevices,
    ) async {
      if (_devToolsLauncher?.activeDevToolsServer == null) {
@@ -69,7 +91,6 @@ class ResidentDevtoolsHandler {
   Future<void> _callDevToolsUriExtension(
     FlutterDevice device,
   ) async {
-    await waitForExtension(device.vmService, 'ext.flutter.activeDevToolsServerAddress');
     try {
       await _invokeRpcOnFirstView(
         'ext.flutter.activeDevToolsServerAddress',
@@ -86,7 +107,15 @@ class ResidentDevtoolsHandler {
     }
   }
 
-  Future<void> callConnectedVmServiceUriExtension(List<FlutterDevice> flutterDevices) async {
+  Future<void> _waitForExtensions(List<FlutterDevice> flutterDevices) async {
+    await Future.wait(<Future<void>>[
+      for (final FlutterDevice device in flutterDevices)
+        if (device.vmService != null)
+          waitForExtension(device.vmService, 'ext.flutter.connectedVmServiceUri'),
+    ]);
+  }
+
+  Future<void> _callConnectedVmServiceUriExtension(List<FlutterDevice> flutterDevices) async {
     await Future.wait(<Future<void>>[
       for (final FlutterDevice device in flutterDevices)
         if (device.vmService != null)
@@ -95,24 +124,24 @@ class ResidentDevtoolsHandler {
   }
 
   Future<void> _callConnectedVmServiceExtension(FlutterDevice device) async {
-    if (device.vmService.httpAddress != null || device.vmService.wsAddress != null) {
-      final Uri uri = device.vmService.httpAddress ?? device.vmService.wsAddress;
-      await waitForExtension(device.vmService, 'ext.flutter.connectedVmServiceUri');
-      try {
-        await _invokeRpcOnFirstView(
-          'ext.flutter.connectedVmServiceUri',
-          device: device,
-          params: <String, dynamic>{
-            'value': uri.toString(),
-          },
-        );
-      } on Exception catch (e) {
-        _logger.printError(e.toString());
-        _logger.printError(
-          'Failed to set vm service URI: ${e.toString()}. Deep links to DevTools'
-          ' will not show in Flutter errors.',
-        );
-      }
+    final Uri uri = device.vmService.httpAddress ?? device.vmService.wsAddress;
+    if (uri == null) {
+      return;
+    }
+    try {
+      await _invokeRpcOnFirstView(
+        'ext.flutter.connectedVmServiceUri',
+        device: device,
+        params: <String, dynamic>{
+          'value': uri.toString(),
+        },
+      );
+    } on Exception catch (e) {
+      _logger.printError(e.toString());
+      _logger.printError(
+        'Failed to set vm service URI: ${e.toString()}. Deep links to DevTools'
+        ' will not show in Flutter errors.',
+      );
     }
   }
 
@@ -121,6 +150,9 @@ class ResidentDevtoolsHandler {
     @required Map<String, dynamic> params,
   }) async {
     final List<FlutterView> views = await device.vmService.getFlutterViews();
+    if (views.isEmpty) {
+      return;
+    }
     await device.vmService
       .invokeFlutterExtensionRpcRaw(
         method,
@@ -130,13 +162,16 @@ class ResidentDevtoolsHandler {
       );
   }
 
+  @override
   Future<void> hotRestart(List<FlutterDevice> flutterDevices) async {
+    await _waitForExtensions(flutterDevices);
     await Future.wait(<Future<void>>[
-      maybeCallDevToolsUriServiceExtension(flutterDevices),
-      callConnectedVmServiceUriExtension(flutterDevices),
+      _maybeCallDevToolsUriServiceExtension(flutterDevices),
+      _callConnectedVmServiceUriExtension(flutterDevices),
     ]);
   }
 
+  @override
   Future<void> shutdown() async {
     if (_devToolsLauncher == null || _shutdown || !_served) {
       return;
@@ -176,3 +211,29 @@ Future<void> waitForExtension(vm_service.VmService vmService, String extension)
   }
   await completer.future;
 }
+
+@visibleForTesting
+NoOpDevtoolsHandler createNoOpHandler(DevtoolsLauncher launcher, ResidentRunner runner, Logger logger) {
+  return NoOpDevtoolsHandler();
+}
+
+@visibleForTesting
+class NoOpDevtoolsHandler implements ResidentDevtoolsHandler {
+  @override
+  DevToolsServerAddress get activeDevToolsServer => null;
+
+  @override
+  Future<void> hotRestart(List<FlutterDevice> flutterDevices) async {
+    return;
+  }
+
+  @override
+  Future<void> serveAndAnnounceDevTools({Uri devToolsServerAddress, List<FlutterDevice> flutterDevices}) async {
+    return;
+  }
+
+  @override
+  Future<void> shutdown() async {
+    return;
+  }
+}

packages/flutter_tools/lib/src/resident_runner.dart

@@ -758,6 +758,7 @@ abstract class ResidentRunner {
     this.hotMode = true,
     String dillOutputPath,
     this.machine = false,
+    ResidentDevtoolsHandlerFactory devtoolsHandler = createDefaultHandler,
   }) : mainPath = globals.fs.path.absolute(target),
        packagesFilePath = debuggingOptions.buildInfo.packagesPath,
        projectRootPath = projectRootPath ?? globals.fs.currentDirectory.path,
@@ -775,7 +776,7 @@ abstract class ResidentRunner {
     if (!artifactDirectory.existsSync()) {
       artifactDirectory.createSync(recursive: true);
     }
-    _residentDevtoolsHandler = ResidentDevtoolsHandler(DevtoolsLauncher.instance, this, globals.logger);
+    _residentDevtoolsHandler = devtoolsHandler(DevtoolsLauncher.instance, this, globals.logger);
   }
 
   @protected

packages/flutter_tools/lib/src/run_cold.dart

@@ -13,6 +13,7 @@ import 'base/file_system.dart';
 import 'build_info.dart';
 import 'device.dart';
 import 'globals.dart' as globals;
+import 'resident_devtools_handler.dart';
 import 'resident_runner.dart';
 import 'tracing.dart';
 import 'vmservice.dart';
@@ -28,6 +29,7 @@ class ColdRunner extends ResidentRunner {
     bool ipv6 = false,
     bool stayResident = true,
     bool machine = false,
+    ResidentDevtoolsHandlerFactory devtoolsHandler = createDefaultHandler,
   }) : super(
           devices,
           target: target,
@@ -36,6 +38,7 @@ class ColdRunner extends ResidentRunner {
           stayResident: stayResident,
           ipv6: ipv6,
           machine: machine,
+          devtoolsHandler: devtoolsHandler,
         );
 
   final bool traceStartup;
@@ -73,14 +76,6 @@ class ColdRunner extends ResidentRunner {
       return 1;
     }
 
-    if (enableDevTools) {
-      // The method below is guaranteed never to return a failing future.
-      unawaited(residentDevtoolsHandler.serveAndAnnounceDevTools(
-        devToolsServerAddress: debuggingOptions.devToolsServerAddress,
-        flutterDevices: flutterDevices,
-      ));
-    }
-
     // Connect to observatory.
     if (debuggingEnabled) {
       try {
@@ -92,6 +87,14 @@ 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 (flutterDevices.first.observatoryUris != null) {
       // For now, only support one debugger connection.
       connectionInfoCompleter?.complete(DebugConnectionInfo(
@@ -125,12 +128,6 @@ class ColdRunner extends ResidentRunner {
       appFinished();
     }
 
-    if (debuggingEnabled) {
-      unawaited(residentDevtoolsHandler.callConnectedVmServiceUriExtension(
-        flutterDevices,
-      ));
-    }
-
     appStartedCompleter?.complete();
 
     writeVmServiceFile();
@@ -160,14 +157,6 @@ class ColdRunner extends ResidentRunner {
       return 2;
     }
 
-    if (enableDevTools) {
-      // The method below is guaranteed never to return a failing future.
-      unawaited(residentDevtoolsHandler.serveAndAnnounceDevTools(
-        devToolsServerAddress: debuggingOptions.devToolsServerAddress,
-        flutterDevices: flutterDevices,
-      ));
-    }
-
     for (final FlutterDevice device in flutterDevices) {
       await device.initLogReader();
     }
@@ -178,9 +167,13 @@ class ColdRunner extends ResidentRunner {
       }
     }
 
-    unawaited(residentDevtoolsHandler.callConnectedVmServiceUriExtension(
-      flutterDevices,
-    ));
+    if (enableDevTools && debuggingEnabled) {
+      // The method below is guaranteed never to return a failing future.
+      unawaited(residentDevtoolsHandler.serveAndAnnounceDevTools(
+        devToolsServerAddress: debuggingOptions.devToolsServerAddress,
+        flutterDevices: flutterDevices,
+      ));
+    }
 
     appStartedCompleter?.complete();
     if (stayResident) {

packages/flutter_tools/lib/src/run_hot.dart

@@ -5,10 +5,11 @@
 // @dart = 2.8
 
 import 'dart:async';
-import 'package:package_config/package_config.dart';
-import 'package:vm_service/vm_service.dart' as vm_service;
+
 import 'package:meta/meta.dart';
+import 'package:package_config/package_config.dart';
 import 'package:pool/pool.dart';
+import 'package:vm_service/vm_service.dart' as vm_service;
 
 import 'base/common.dart';
 import 'base/context.dart';
@@ -26,6 +27,7 @@ import 'device.dart';
 import 'features.dart';
 import 'globals.dart' as globals;
 import 'reporting/reporting.dart';
+import 'resident_devtools_handler.dart';
 import 'resident_runner.dart';
 import 'vmservice.dart';
 
@@ -78,6 +80,7 @@ class HotRunner extends ResidentRunner {
     bool stayResident = true,
     bool ipv6 = false,
     bool machine = false,
+    ResidentDevtoolsHandlerFactory devtoolsHandler = createDefaultHandler,
   }) : super(
           devices,
           target: target,
@@ -88,6 +91,7 @@ class HotRunner extends ResidentRunner {
           dillOutputPath: dillOutputPath,
           ipv6: ipv6,
           machine: machine,
+          devtoolsHandler: devtoolsHandler,
         );
 
   final bool benchmarkMode;
@@ -222,10 +226,6 @@ class HotRunner extends ResidentRunner {
       return 3;
     }
 
-    unawaited(residentDevtoolsHandler.callConnectedVmServiceUriExtension(
-      flutterDevices,
-    ));
-
     final Stopwatch initialUpdateDevFSsTimer = Stopwatch()..start();
     final UpdateFSReport devfsResult = await _updateDevFS(fullRestart: true);
     _addBenchmarkData(

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

@@ -7,6 +7,7 @@
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/resident_devtools_handler.dart';
 import 'package:vm_service/vm_service.dart' as vm_service;
 import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/base/io.dart';
@@ -187,6 +188,7 @@ void main() {
         devices,
         debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
         target: 'main.dart',
+        devtoolsHandler: createNoOpHandler,
       ).restart(fullRestart: true);
       // Expect hot restart failed.
       expect(result.isOk, false);
@@ -219,6 +221,7 @@ void main() {
         devices,
         debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
         target: 'main.dart',
+        devtoolsHandler: createNoOpHandler,
       ).restart(fullRestart: true);
       // Expect hot restart failed.
       expect(result.isOk, false);
@@ -322,6 +325,7 @@ void main() {
         devices,
         debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
         target: 'main.dart',
+        devtoolsHandler: createNoOpHandler,
       );
       final OperationResult result = await hotRunner.restart(fullRestart: true);
       // Expect hot restart was successful.
@@ -351,6 +355,7 @@ void main() {
         devices,
         debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
         target: 'main.dart',
+        devtoolsHandler: createNoOpHandler,
       ).restart(fullRestart: true);
       expect(result.isOk, false);
       expect(result.message, 'setupHotRestart failed');
@@ -417,6 +422,7 @@ void main() {
         devices,
         debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
         target: 'main.dart',
+        devtoolsHandler: createNoOpHandler,
       );
       final OperationResult result = await hotRunner.restart(fullRestart: true);
       // Expect hot restart successful.

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

@@ -6,6 +6,7 @@
 
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/devtools_launcher.dart';
+import 'package:flutter_tools/src/vmservice.dart';
 import 'package:vm_service/vm_service.dart' as vm_service;
 
 import 'package:flutter_tools/src/base/logger.dart';
@@ -16,6 +17,31 @@ import 'package:test/fake.dart';
 import '../src/common.dart';
 import '../src/context.dart';
 
+ final vm_service.Isolate isolate = vm_service.Isolate(
+  id: '1',
+  pauseEvent: vm_service.Event(
+    kind: vm_service.EventKind.kResume,
+    timestamp: 0
+  ),
+  breakpoints: <vm_service.Breakpoint>[],
+  exceptionPauseMode: null,
+  libraries: <vm_service.LibraryRef>[
+    vm_service.LibraryRef(
+      id: '1',
+      uri: 'file:///hello_world/main.dart',
+      name: '',
+    ),
+  ],
+  livePorts: 0,
+  name: 'test',
+  number: '1',
+  pauseOnExit: false,
+  runnable: true,
+  startTime: 0,
+  isSystemIsolate: false,
+  isolateFlags: <vm_service.IsolateFlag>[],
+  extensionRPCs: <String>['foo']
+);
 
 final vm_service.Isolate fakeUnpausedIsolate = vm_service.Isolate(
   id: '1',
@@ -58,9 +84,23 @@ final vm_service.VM fakeVM = vm_service.VM(
   systemIsolates: <vm_service.IsolateRef>[],
 );
 
+final FlutterView fakeFlutterView = FlutterView(
+  id: 'a',
+  uiIsolate: fakeUnpausedIsolate,
+);
+
+final FakeVmServiceRequest listViews = FakeVmServiceRequest(
+  method: kListViewsMethod,
+  jsonResponse: <String, Object>{
+    'views': <Object>[
+      fakeFlutterView.toJson(),
+    ],
+  },
+);
+
 void main() {
   testWithoutContext('Does not serve devtools if launcher is null', () async {
-    final ResidentDevtoolsHandler handler = ResidentDevtoolsHandler(
+    final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
       null,
       FakeResidentRunner(),
       BufferLogger.test(),
@@ -72,7 +112,7 @@ void main() {
   });
 
   testWithoutContext('Does not serve devtools if ResidentRunner does not support the service protocol', () async {
-    final ResidentDevtoolsHandler handler = ResidentDevtoolsHandler(
+    final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
       FakeDevtoolsLauncher(),
       FakeResidentRunner()..supportsServiceProtocol = false,
       BufferLogger.test(),
@@ -91,7 +131,7 @@ void main() {
       platform: FakePlatform(),
       persistentToolState: null,
     );
-    final ResidentDevtoolsHandler handler = ResidentDevtoolsHandler(
+    final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
       // Uses real devtools instance which should be a no-op if
       // URI is already set.
       launcher,
@@ -108,8 +148,8 @@ void main() {
     expect(handler.activeDevToolsServer.port, 8181);
   });
 
-  testWithoutContext('can serveAndAnnounceDevTools with attached device does not fail on null vm service', () async {
-    final ResidentDevtoolsHandler handler = ResidentDevtoolsHandler(
+  testWithoutContext('serveAndAnnounceDevTools with attached device does not fail on null vm service', () async {
+    final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
       FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
       FakeResidentRunner(),
       BufferLogger.test(),
@@ -123,33 +163,63 @@ void main() {
     );
   });
 
-  testWithoutContext('wait for extension handles an immediate extension', () {
-    final vm_service.Isolate isolate = vm_service.Isolate(
-      id: '1',
-      pauseEvent: vm_service.Event(
-        kind: vm_service.EventKind.kResume,
-        timestamp: 0
+  testWithoutContext('serveAndAnnounceDevTools with invokes devtools and vm_service setter', () async {
+    final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
+      FakeDevtoolsLauncher()..activeDevToolsServer = DevToolsServerAddress('localhost', 8080),
+      FakeResidentRunner(),
+      BufferLogger.test(),
+    );
+    final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
+      const FakeVmServiceRequest(
+        method: 'streamListen',
+        args: <String, Object>{
+          'streamId': 'Extension',
+        }
+      ),
+      FakeVmServiceRequest(method: 'getVM', jsonResponse: fakeVM.toJson()),
+      FakeVmServiceRequest(
+        method: 'getIsolate',
+        jsonResponse: isolate.toJson(),
+        args: <String, Object>{
+          'isolateId': '1',
+        },
       ),
-      breakpoints: <vm_service.Breakpoint>[],
-      exceptionPauseMode: null,
-      libraries: <vm_service.LibraryRef>[
-        vm_service.LibraryRef(
-          id: '1',
-          uri: 'file:///hello_world/main.dart',
-          name: '',
+      FakeVmServiceStreamResponse(
+        streamId: 'Extension',
+        event: vm_service.Event(
+          timestamp: 0,
+          extensionKind: 'Flutter.FrameworkInitialization',
+          kind: 'test',
         ),
-      ],
-      livePorts: 0,
-      name: 'test',
-      number: '1',
-      pauseOnExit: false,
-      runnable: true,
-      startTime: 0,
-      isSystemIsolate: false,
-      isolateFlags: <vm_service.IsolateFlag>[],
-      extensionRPCs: <String>['foo']
+      ),
+      listViews,
+      const FakeVmServiceRequest(
+        method: 'ext.flutter.activeDevToolsServerAddress',
+        args: <String, Object>{
+          'isolateId': '1',
+          'value': 'http://localhost:8080',
+        },
+      ),
+      listViews,
+      const FakeVmServiceRequest(
+        method: 'ext.flutter.connectedVmServiceUri',
+        args: <String, Object>{
+          'isolateId': '1',
+          'value': 'http://localhost:1234',
+        },
+      ),
+    ]);
+
+    final FakeFlutterDevice device = FakeFlutterDevice()
+      ..vmService = fakeVmServiceHost.vmService;
+    setHttpAddress(Uri.parse('http://localhost:1234'), fakeVmServiceHost.vmService);
+
+    await handler.serveAndAnnounceDevTools(
+      flutterDevices: <FlutterDevice>[device],
     );
+  });
 
+  testWithoutContext('wait for extension handles an immediate extension', () {
     final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
       const FakeVmServiceRequest(
         method: 'streamListen',