Chrome fixes for VS code (#342)

- As suggested by the Chrome docs, always use a tmp dir when passing `--remote-debugging-port`
- Add graceful exit handler for daemon command
  - This ensures that Chrome is properly closed
- Temporary pin build_daemon version to fix integration tests
- Fix concurrent modification issue

Closes #319
Closes #300

This reverts #322 & #316

Author: grouma

dwds/pubspec.yaml

@@ -24,6 +24,7 @@ dependencies:
 
 dev_dependencies:
   args: ^1.0.0
+  build_daemon: ^0.5.0
   build_runner: ^1.0.0
   build_web_compilers: '>=1.0.0 <3.0.0'
   test: ^1.6.0

dwds/test/test_context.dart

@@ -48,7 +48,7 @@ class TestContext {
       }
       printOnFailure(line);
     });
-    await assetReadyCompleter.future;
+    await assetReadyCompleter.future.timeout(Duration(seconds: 60));
     appUrl = 'http://localhost:$port/hello_world/';
     var debugPort = await findUnusedPort();
     webDriver = await createDriver(desired: {

example/pubspec.yaml

@@ -7,3 +7,4 @@ environment:
 dev_dependencies:
   build_runner: '>=1.3.0 <2.0.0'
   build_web_compilers: '>=1.0.0 <3.0.0'
+  build_daemon: ^0.5.0

webdev/lib/src/command/daemon_command.dart

@@ -7,6 +7,7 @@ import 'dart:convert';
 import 'dart:io';
 
 import 'package:args/command_runner.dart';
+import 'package:async/async.dart';
 import 'package:pedantic/pedantic.dart';
 
 import '../daemon/app_domain.dart';
@@ -52,6 +53,14 @@ class DaemonCommand extends Command<int> {
   Future<int> run() async {
     Daemon daemon;
     DevWorkflow workflow;
+    var cancelCount = 0;
+    var cancelSub = StreamGroup.merge(
+            [ProcessSignal.sigint.watch(), ProcessSignal.sigterm.watch()])
+        .listen((signal) async {
+      cancelCount++;
+      daemon?.shutdown();
+      if (cancelCount > 1) exit(1);
+    });
     try {
       daemon = Daemon(_stdinCommandStream, _stdoutCommandResponse);
       var daemonDomain = DaemonDomain(daemon);
@@ -76,6 +85,7 @@ class DaemonCommand extends Command<int> {
       daemon?.shutdown();
       rethrow;
     } finally {
+      unawaited(cancelSub.cancel());
       unawaited(workflow?.shutDown());
     }
   }

webdev/lib/src/serve/chrome.dart

@@ -46,20 +46,33 @@ var _currentCompleter = Completer<Chrome>();
 class Chrome {
   final int debugPort;
   final Process _process;
+  final Directory _dataDir;
 
   final ChromeConnection chromeConnection;
 
   Chrome._(
     this.debugPort,
     this.chromeConnection, {
     Process process,
-  }) : _process = process;
+    Directory dataDir,
+  })  : _process = process,
+        _dataDir = dataDir;
 
   Future<void> close() async {
     if (_currentCompleter.isCompleted) _currentCompleter = Completer<Chrome>();
     chromeConnection.close();
-    _process?.kill();
+    _process?.kill(ProcessSignal.sigkill);
     await _process?.exitCode;
+    try {
+      // Chrome starts another process as soon as it dies that modifies the
+      // profile information. Give it some time before attempting to delete
+      // the directory.
+      await Future.delayed(Duration(milliseconds: 500));
+      await _dataDir?.delete(recursive: true);
+    } catch (_) {
+      // Silently fail if we can't clean up the profile information.
+      // It is a system tmp directory so it should get cleaned up eventually.
+    }
   }
 
   /// Connects to an instance of Chrome with an open debug port.
@@ -72,9 +85,7 @@ class Chrome {
   ///
   /// Each url in [urls] will be loaded in a separate tab.
   static Future<Chrome> start(List<String> urls, {int port}) async {
-    var dataDir = Directory(p.joinAll(
-        [Directory.current.path, '.dart_tool', 'webdev', 'chrome_profile']))
-      ..createSync(recursive: true);
+    var dataDir = Directory.systemTemp.createTempSync();
     port = port == null || port == 0 ? await findUnusedPort() : port;
     var args = [
       // Using a tmp directory ensures that a new instance of chrome launches
@@ -109,6 +120,7 @@ class Chrome {
       port,
       ChromeConnection('localhost', port),
       process: process,
+      dataDir: dataDir,
     ));
   }
 

webdev/lib/src/serve/handlers/dev_handler.dart

@@ -50,9 +50,11 @@ class DevHandler {
 
   Future<void> close() async {
     await _sub.cancel();
-    for (var connection in _connections) {
-      await connection.sink.close();
-    }
+    // We listen for connections to close and remove them from the connections
+    // set. Therefore we shouldn't asynchronously iterate through the
+    // connections.
+    await Future.wait(
+        _connections.map((connection) => connection.sink.close()));
     await Future.wait(_servicesByAppId.values.map((futureServices) async {
       await (await futureServices).close();
     }));