From e6abda7f00697dd2964019063a0cb46c9e352ee7 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 16 Apr 2020 15:48:21 -0700 Subject: [PATCH] [flutter_tools] remove Isolate implementations of vm_service methods (#54920) --- .../lib/src/resident_runner.dart | 13 +- packages/flutter_tools/lib/src/run_hot.dart | 32 ++--- packages/flutter_tools/lib/src/vmservice.dart | 128 +++--------------- .../general.shard/resident_runner_test.dart | 11 +- 4 files changed, 48 insertions(+), 136 deletions(-) diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index f551ddf0904..0f240d56398 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:vm_service/vm_service.dart' as vm_service; import 'package:devtools_server/devtools_server.dart' as devtools_server; import 'package:meta/meta.dart'; @@ -286,17 +287,19 @@ class FlutterDevice { return devFS.create(); } - List>> reloadSources( + List> reloadSources( String entryPath, { bool pause = false, }) { - final Uri deviceEntryUri = devFS.baseUri.resolveUri(globals.fs.path.toUri(entryPath)); - return >>[ + final String deviceEntryUri = devFS.baseUri + .resolveUri(globals.fs.path.toUri(entryPath)).toString(); + return >[ for (final Isolate isolate in vmService.vm.isolates) - isolate.reloadSources( + vmService.reloadSources( + isolate.id, pause: pause, rootLibUri: deviceEntryUri, - ), + ) ]; } diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 3042f8229a8..82d1fc19f17 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -58,7 +58,7 @@ class DeviceReloadReport { DeviceReloadReport(this.device, this.reports); FlutterDevice device; - List> reports; // List has one report per Flutter view. + List reports; // List has one report per Flutter view. } // TODO(mklim): Test this, flutter/flutter#23031. @@ -179,12 +179,12 @@ class HotRunner extends ResidentRunner { from: projectRootPath, ); for (final FlutterDevice device in flutterDevices) { - final List>> reportFutures = device.reloadSources( + final List> reportFutures = device.reloadSources( entryPath, pause: false, ); - final List> reports = await Future.wait(reportFutures); - final Map firstReport = reports.first; - await device.updateReloadStatus(validateReloadReport(firstReport, printErrors: false)); + final List reports = await Future.wait(reportFutures); + final vm_service.ReloadReport firstReport = reports.first; + await device.updateReloadStatus(validateReloadReport(firstReport.json, printErrors: false)); } } on Exception catch (error) { return OperationResult(1, error.toString()); @@ -539,7 +539,7 @@ class HotRunner extends ResidentRunner { final ServiceEvent pauseEvent = view.uiIsolate.pauseEvent; if ((pauseEvent != null) && pauseEvent.isPauseEvent) { // Resume the isolate so that it can be killed by the embedder. - return view.uiIsolate.resume(); + return device.vmService.resume(view.uiIsolate.id); } return null; })); @@ -834,18 +834,18 @@ class HotRunner extends ResidentRunner { await device.resetAssetDirectory(); _shouldResetAssetDirectory = false; } - final List>> reportFutures = device.reloadSources( + final List> reportFutures = device.reloadSources( entryPath, pause: pause, ); allReportsFutures.add(Future.wait(reportFutures).then( - (List> reports) async { + (List reports) async { // TODO(aam): Investigate why we are validating only first reload report, // which seems to be current behavior - final Map firstReport = reports.first; + final vm_service.ReloadReport firstReport = reports.first; // Don't print errors because they will be printed further down when // `validateReloadReport` is called again. await device.updateReloadStatus( - validateReloadReport(firstReport, printErrors: false), + validateReloadReport(firstReport.json, printErrors: false), ); return DeviceReloadReport(device, reports); }, @@ -853,8 +853,8 @@ class HotRunner extends ResidentRunner { } final List reports = await Future.wait(allReportsFutures); for (final DeviceReloadReport report in reports) { - final Map reloadReport = report.reports[0]; - if (!validateReloadReport(reloadReport)) { + final vm_service.ReloadReport reloadReport = report.reports[0]; + if (!validateReloadReport(reloadReport.json)) { // Reload failed. HotEvent('reload-reject', targetPlatform: targetPlatform, @@ -868,9 +868,9 @@ class HotRunner extends ResidentRunner { // Collect stats only from the first device. If/when run -d all is // refactored, we'll probably need to send one hot reload/restart event // per device to analytics. - firstReloadDetails ??= castStringKeyedMap(reloadReport['details']); - final int loadedLibraryCount = reloadReport['details']['loadedLibraryCount'] as int; - final int finalLibraryCount = reloadReport['details']['finalLibraryCount'] as int; + firstReloadDetails ??= castStringKeyedMap(reloadReport.json['details']); + final int loadedLibraryCount = reloadReport.json['details']['loadedLibraryCount'] as int; + final int finalLibraryCount = reloadReport.json['details']['finalLibraryCount'] as int; globals.printTrace('reloaded $loadedLibraryCount of $finalLibraryCount libraries'); reloadMessage = 'Reloaded $loadedLibraryCount of $finalLibraryCount libraries'; } @@ -878,7 +878,7 @@ class HotRunner extends ResidentRunner { globals.printTrace('Hot reload failed: $error\n$stackTrace'); final int errorCode = error['code'] as int; String errorMessage = error['message'] as String; - if (errorCode == Isolate.kIsolateReloadBarred) { + if (errorCode == kIsolateReloadBarred) { errorMessage = 'Unable to hot reload application due to an unrecoverable error in ' 'the source code. Please address the error and then use "R" to ' 'restart the app.\n' diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart index f6ea3f79cc7..3de223858ad 100644 --- a/packages/flutter_tools/lib/src/vmservice.dart +++ b/packages/flutter_tools/lib/src/vmservice.dart @@ -8,7 +8,6 @@ import 'dart:math' as math; import 'package:meta/meta.dart' show required; import 'package:vm_service/vm_service.dart' as vm_service; -import 'base/common.dart'; import 'base/context.dart'; import 'base/io.dart' as io; import 'base/utils.dart'; @@ -22,6 +21,9 @@ const String kSetAssetBundlePathMethod = '_flutter.setAssetBundlePath'; const String kFlushUIThreadTasksMethod = '_flutter.flushUIThreadTasks'; const String kRunInViewMethod = '_flutter.runInView'; +/// The error response code from an unrecoverable compilation failure. +const int kIsolateReloadBarred = 1005; + /// Override `WebSocketConnector` in [context] to use a different constructor /// for [WebSocket]s (used by tests). typedef WebSocketConnector = Future Function(String url, {io.CompressionOptions compression}); @@ -525,6 +527,23 @@ class VMService implements vm_service.VmService { _delegateService?.dispose(); } + @override + Future reloadSources( + String isolateId, { + bool force, + bool pause, + String rootLibUri, + String packagesUri, + }) { + return _delegateService.reloadSources( + isolateId, + force: force, + pause: pause, + rootLibUri: rootLibUri, + packagesUri: packagesUri, + ); + } + // To enable a gradual migration to package:vm_service @override dynamic noSuchMethod(Invocation invocation) { @@ -1047,46 +1066,6 @@ class VM extends ServiceObjectOwner { return invokeRpcRaw('_createDevFS', params: {'fsName': fsName}); } - /// List the development file system son the device. - Future> listDevFS() async { - return (await invokeRpcRaw('_listDevFS'))['fsNames'] as List; - } - - // Write one file into a file system. - Future> writeDevFSFile( - String fsName, { - @required String path, - @required List fileContents, - }) { - assert(path != null); - assert(fileContents != null); - return invokeRpcRaw( - '_writeDevFSFile', - params: { - 'fsName': fsName, - 'path': path, - 'fileContents': base64.encode(fileContents), - }, - ); - } - - // Read one file from a file system. - Future> readDevFSFile(String fsName, String path) async { - final Map response = await invokeRpcRaw( - '_readDevFSFile', - params: { - 'fsName': fsName, - 'path': path, - }, - ); - return base64.decode(response['fileContents'] as String); - } - - /// The complete list of a file system. - Future> listDevFSFiles(String fsName) async { - return (await invokeRpcRaw('_listDevFSFiles', params: {'fsName': fsName}))['files'] as List; - } - /// Delete an existing file system. Future> deleteDevFS(String fsName) { return invokeRpcRaw('_deleteDevFS', params: {'fsName': fsName}); @@ -1212,12 +1191,6 @@ class Isolate extends ServiceObjectOwner { final Map _cache = {}; - HeapSpace _newSpace; - HeapSpace _oldSpace; - - HeapSpace get newSpace => _newSpace; - HeapSpace get oldSpace => _oldSpace; - @override ServiceObject getFromMap(Map map) { if (map == null) { @@ -1262,18 +1235,6 @@ class Isolate extends ServiceObjectOwner { return vm.invokeRpcRaw(method, params: params); } - /// Invoke the RPC and return a ServiceObject response. - Future invokeRpc(String method, Map params) async { - return getFromMap(await invokeRpcRaw(method, params: params)); - } - - void _updateHeaps(Map map, bool mapIsRef) { - _newSpace ??= HeapSpace._empty(this); - _newSpace._update(castStringKeyedMap(map['new']), mapIsRef); - _oldSpace ??= HeapSpace._empty(this); - _oldSpace._update(castStringKeyedMap(map['old']), mapIsRef); - } - @override void _update(Map map, bool mapIsRef) { if (mapIsRef) { @@ -1287,46 +1248,6 @@ class Isolate extends ServiceObjectOwner { _upgradeCollection(map, this); pauseEvent = map['pauseEvent'] as ServiceEvent; - - _updateHeaps(castStringKeyedMap(map['_heaps']), mapIsRef); - } - - static const int kIsolateReloadBarred = 1005; - - Future> reloadSources({ - bool pause = false, - Uri rootLibUri, - }) async { - try { - final Map arguments = { - 'pause': pause, - }; - if (rootLibUri != null) { - arguments['rootLibUri'] = rootLibUri.toString(); - } - final Map response = await invokeRpcRaw('_reloadSources', params: arguments); - return response; - } on vm_service.RPCError catch (e) { - return Future>.value({ - 'code': e.code, - 'message': e.message, - 'data': e.data, - }); - } on vm_service.SentinelException catch (e) { - throwToolExit('Unexpected Sentinel while reloading sources: $e'); - } - assert(false); - return null; - } - - Future> getObject(Map objectRef) { - return invokeRpcRaw('getObject', - params: {'objectId': objectRef['id']}); - } - - /// Resumes the isolate. - Future> resume() { - return invokeRpcRaw('resume'); } // Flutter extension methods. @@ -1425,15 +1346,6 @@ class Isolate extends ServiceObjectOwner { ); } - Future> flutterDebugSaveCompilationTrace() async { - final Map result = - await invokeFlutterExtensionRpcRaw('ext.flutter.saveCompilationTrace'); - if (result != null && result['value'] is List) { - return (result['value'] as List).cast(); - } - return null; - } - // Application control extension methods. Future> flutterExit() { return invokeFlutterExtensionRpcRaw('ext.flutter.exit'); diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index 1bdec09158a..6065f64d055 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:vm_service/vm_service.dart' as vm_service; import 'package:file/memory.dart'; import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/artifacts.dart'; @@ -25,7 +26,6 @@ import 'package:flutter_tools/src/run_cold.dart'; import 'package:flutter_tools/src/run_hot.dart'; import 'package:flutter_tools/src/vmservice.dart'; import 'package:mockito/mockito.dart'; -import 'package:vm_service/vm_service.dart' as vm_service; import '../src/common.dart'; import '../src/context.dart'; @@ -127,8 +127,8 @@ void main() { when(mockFlutterDevice.vmService).thenReturn(mockVMService); when(mockFlutterDevice.refreshViews()).thenAnswer((Invocation invocation) async { }); when(mockFlutterDevice.getVMs()).thenAnswer((Invocation invocation) async { }); - when(mockFlutterDevice.reloadSources(any, pause: anyNamed('pause'))).thenReturn(>>[ - Future>.value({ + when(mockFlutterDevice.reloadSources(any, pause: anyNamed('pause'))).thenReturn(>[ + Future.value(vm_service.ReloadReport.parse({ 'type': 'ReloadReport', 'success': true, 'details': { @@ -138,7 +138,7 @@ void main() { 'receivedClassesCount': 1, 'receivedProceduresCount': 1, }, - }), + })), ]); // VMService mocks. when(mockVMService.wsAddress).thenReturn(testUri); @@ -146,9 +146,6 @@ void main() { final Completer result = Completer.sync(); return result.future; }); - when(mockIsolate.resume()).thenAnswer((Invocation invocation) { - return Future>.value(null); - }); when(mockIsolate.flutterExit()).thenAnswer((Invocation invocation) { return Future>.value(null); });