From 2c1d12d4f77e4d82fc47c15a773a6c9903f4274c Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 5 Sep 2018 18:03:43 -0700 Subject: [PATCH] Revert "Add a detach command to detach without terminating" (#21464) --- packages/flutter_tools/doc/daemon.md | 12 ++----- .../lib/src/commands/daemon.dart | 21 +------------ packages/flutter_tools/lib/src/run_hot.dart | 15 ++------- .../test/integration/flutter_attach_test.dart | 27 +++++----------- .../test/integration/test_driver.dart | 31 ++----------------- 5 files changed, 16 insertions(+), 90 deletions(-) diff --git a/packages/flutter_tools/doc/daemon.md b/packages/flutter_tools/doc/daemon.md index 5e2c70ad543..a407c8afb67 100644 --- a/packages/flutter_tools/doc/daemon.md +++ b/packages/flutter_tools/doc/daemon.md @@ -92,12 +92,6 @@ The `callServiceExtension()` allows clients to make arbitrary calls to service p - `methodName`: the name of the service protocol extension to invoke; this is required. - `params`: an optional Map of parameters to pass to the service protocol extension. -#### app.detach - -The `detach()` command takes one parameter, `appId`. It returns a `bool` to indicate success or failure in detaching from an app without stopping it. - -- `appId`: the id of a previously started app; this is required. - #### app.stop The `stop()` command takes one parameter, `appId`. It returns a `bool` to indicate success or failure in stopping an app. @@ -116,7 +110,7 @@ This is sent when an observatory port is available for a started app. The `param #### app.started -This is sent once the application launch process is complete and the app is either paused before main() (if `startPaused` is true) or main() has begun running. When attaching, this even will be fired once attached. The `params` field will be a map containing the field `appId`. +This is sent once the application launch process is complete and the app is either paused before main() (if `startPaused` is true) or main() has begun running. The `params` field will be a map containing the field `appId`. #### app.log @@ -128,7 +122,7 @@ This is sent when an operation starts and again when it stops. When an operation #### app.stop -This is sent when an app is stopped or detached from. The `params` field will be a map with the field `appId`. +This is sent when an app is stopped. The `params` field will be a map with the field `appId`. ### device domain @@ -210,7 +204,6 @@ The following subset of the app domain is available in `flutter run --machine`. - Commands - [`restart`](#apprestart) - [`callServiceExtension`](#appcallserviceextension) - - [`detach`](#appdetach) - [`stop`](#appstop) - Events - [`start`](#appstart) @@ -226,7 +219,6 @@ See the [source](https://github.com/flutter/flutter/blob/master/packages/flutter ## Changelog -- 0.4.2: Added `app.detach` command - 0.4.1: Added `flutter attach --machine` - 0.4.0: Added `emulator.create` command - 0.3.0: Added `daemon.connected` event at startup diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 9fd00a4c968..274f45b713c 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -28,7 +28,7 @@ import '../runner/flutter_command.dart'; import '../tester/flutter_tester.dart'; import '../vmservice.dart'; -const String protocolVersion = '0.4.2'; +const String protocolVersion = '0.4.1'; /// A server process command. This command will start up a long-lived server. /// It reads JSON-RPC based commands from stdin, executes them, and returns @@ -316,7 +316,6 @@ class AppDomain extends Domain { registerHandler('restart', restart); registerHandler('callServiceExtension', callServiceExtension); registerHandler('stop', stop); - registerHandler('detach', detach); } static final Uuid _uuidGenerator = new Uuid(); @@ -517,23 +516,6 @@ class AppDomain extends Domain { }); } - Future detach(Map args) async { - final String appId = _getStringArg(args, 'appId', required: true); - - final AppInstance app = _getApp(appId); - if (app == null) - throw "app '$appId' not found"; - - return app.detach().timeout(const Duration(seconds: 5)).then((_) { - return true; - }).catchError((dynamic error) { - _sendAppEvent(app, 'log', { 'log': '$error', 'error': true }); - app.closeLogger(); - _apps.remove(app); - return false; - }); - } - AppInstance _getApp(String id) { return _apps.firstWhere((AppInstance app) => app.id == id, orElse: () => null); } @@ -787,7 +769,6 @@ class AppInstance { } Future stop() => runner.stop(); - Future detach() => runner.detach(); void closeLogger() { _logger.close(); diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 40a33402854..395d26c679b 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -66,7 +66,6 @@ class HotRunner extends ResidentRunner { final bool benchmarkMode; final File applicationBinary; final bool hostIsIde; - bool _didAttach = false; Set _dartDependencies; final String dillOutputPath; @@ -153,7 +152,6 @@ class HotRunner extends ResidentRunner { Completer appStartedCompleter, String viewFilter, }) async { - _didAttach = true; try { await connectToServiceProtocol(viewFilter: viewFilter, reloadSources: _reloadSourcesService, @@ -753,25 +751,18 @@ class HotRunner extends ResidentRunner { for (Uri uri in device.observatoryUris) printStatus('An Observatory debugger and profiler on $dname is available at: $uri'); } - final String quitMessage = _didAttach - ? 'To detach, press "d"; to quit, press "q".' - : 'To quit, press "q".'; if (details) { printHelpDetails(); - printStatus('To repeat this help message, press "h". $quitMessage'); + printStatus('To repeat this help message, press "h". To quit, press "q".'); } else { - printStatus('For a more detailed help message, press "h". $quitMessage'); + printStatus('For a more detailed help message, press "h". To quit, press "q".'); } } @override Future cleanupAfterSignal() async { await stopEchoingDeviceLog(); - if (_didAttach) { - appFinished(); - } else { - await stopApp(); - } + await stopApp(); } @override diff --git a/packages/flutter_tools/test/integration/flutter_attach_test.dart b/packages/flutter_tools/test/integration/flutter_attach_test.dart index b31d5f896f3..2a6e104cb5d 100644 --- a/packages/flutter_tools/test/integration/flutter_attach_test.dart +++ b/packages/flutter_tools/test/integration/flutter_attach_test.dart @@ -6,6 +6,7 @@ import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import '../src/common.dart'; +import '../src/context.dart'; import 'test_data/basic_project.dart'; import 'test_driver.dart'; @@ -17,36 +18,24 @@ void main() { setUp(() async { tempDir = fs.systemTempDirectory.createTempSync('flutter_attach_test.'); await _project.setUpIn(tempDir); - _flutterRun = new FlutterTestDriver(tempDir, logPrefix: 'RUN'); - _flutterAttach = new FlutterTestDriver(tempDir, logPrefix: 'ATTACH'); + _flutterRun = new FlutterTestDriver(tempDir); + _flutterAttach = new FlutterTestDriver(tempDir); }); tearDown(() async { - await _flutterAttach.detach(); + // We can't call stop() on both of these because they'll both try to stop the + // same app. Just quit the attach process and then send a stop to the original + // process. await _flutterRun.stop(); + await _flutterAttach.quit(); tryToDelete(tempDir); }); group('attached process', () { - test('can hot reload', () async { + testUsingContext('can hot reload', () async { await _flutterRun.run(withDebugger: true); await _flutterAttach.attach(_flutterRun.vmServicePort); await _flutterAttach.hotReload(); }); - test('can detach, reattach, hot reload', () async { - await _flutterRun.run(withDebugger: true); - await _flutterAttach.attach(_flutterRun.vmServicePort); - await _flutterAttach.detach(); - await _flutterAttach.attach(_flutterRun.vmServicePort); - await _flutterAttach.hotReload(); - }); - test('killing process behaves the same as detach ', () async { - await _flutterRun.run(withDebugger: true); - await _flutterAttach.attach(_flutterRun.vmServicePort); - await _flutterAttach.quit(); - _flutterAttach = new FlutterTestDriver(tempDir, logPrefix: 'ATTACH-2'); - await _flutterAttach.attach(_flutterRun.vmServicePort); - await _flutterAttach.hotReload(); - }); }, timeout: const Timeout.factor(6)); } diff --git a/packages/flutter_tools/test/integration/test_driver.dart b/packages/flutter_tools/test/integration/test_driver.dart index fb350f4348b..892c275b451 100644 --- a/packages/flutter_tools/test/integration/test_driver.dart +++ b/packages/flutter_tools/test/integration/test_driver.dart @@ -23,11 +23,9 @@ const Duration appStartTimeout = Duration(seconds: 120); const Duration quitTimeout = Duration(seconds: 10); class FlutterTestDriver { - FlutterTestDriver(this._projectFolder, {String logPrefix}): - this._logPrefix = logPrefix != null ? '$logPrefix: ' : ''; + FlutterTestDriver(this._projectFolder); final Directory _projectFolder; - final String _logPrefix; Process _proc; int _procPid; final StreamController _stdout = new StreamController.broadcast(); @@ -51,7 +49,7 @@ class FlutterTestDriver { msg.length > maxLength ? msg.substring(0, maxLength) + '...' : msg; _allMessages.add(truncatedMsg); if (_printJsonAndStderr) { - print('$_logPrefix$truncatedMsg'); + print(truncatedMsg); } return msg; } @@ -164,31 +162,6 @@ class FlutterTestDriver { _throwErrorResponse('Hot ${fullRestart ? 'restart' : 'reload'} request failed'); } - Future detach() async { - if (vmService != null) { - _debugPrint('Closing VM service'); - await vmService.close() - .timeout(quitTimeout, - onTimeout: () { _debugPrint('VM Service did not quit within $quitTimeout'); }); - } - if (_currentRunningAppId != null) { - _debugPrint('Detaching from app'); - await Future.any(>[ - _proc.exitCode, - _sendRequest( - 'app.detach', - {'appId': _currentRunningAppId} - ), - ]).timeout( - quitTimeout, - onTimeout: () { _debugPrint('app.detach did not return within $quitTimeout'); } - ); - _currentRunningAppId = null; - } - _debugPrint('Waiting for process to end'); - return _proc.exitCode.timeout(quitTimeout, onTimeout: _killGracefully); - } - Future stop() async { if (vmService != null) { _debugPrint('Closing VM service');