Remove run in shell and add unit test for chrome launching (#39462)

packages/flutter_tools/lib/src/web/chrome.dart

@@ -14,7 +14,6 @@ import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/process_manager.dart';
 import '../convert.dart';
-import '../globals.dart';
 
 /// The [ChromeLauncher] instance.
 ChromeLauncher get chromeLauncher => context.get<ChromeLauncher>();
@@ -76,9 +75,15 @@ class ChromeLauncher {
   ///
   /// `headless` defaults to false, and controls whether we open a headless or
   /// a `headfull` browser.
-  Future<Chrome> launch(String url, { bool headless = false }) async {
+  ///
+  /// `skipCheck` does not attempt to make a devtools connection before returning.
+  Future<Chrome> launch(String url, { bool headless = false, bool skipCheck = false }) async {
     final String chromeExecutable = findChromeExecutable();
-    final Directory dataDir = fs.systemTempDirectory.createTempSync();
+    final Directory dataDir = fs.directory('.dart_tool')
+      .childDirectory('chrome_profile');
+    if (!dataDir.existsSync()) {
+      dataDir.createSync(recursive: true);
+    }
     final int port = await os.findFreePort();
     final List<String> args = <String>[
       chromeExecutable,
@@ -102,7 +107,7 @@ class ChromeLauncher {
       url,
     ];
 
-    final Process process = await processManager.start(args, runInShell: true);
+    final Process process = await processManager.start(args);
 
     // Wait until the DevTools are listening before trying to connect.
     await process.stderr
@@ -116,28 +121,29 @@ class ChromeLauncher {
           return null;
         });
     final Uri remoteDebuggerUri = await _getRemoteDebuggerUrl(Uri.parse('http://localhost:$port'));
-
     return _connect(Chrome._(
       port,
       ChromeConnection('localhost', port),
       process: process,
-      dataDir: dataDir,
       remoteDebuggerUri: remoteDebuggerUri,
-    ));
+    ), skipCheck);
   }
 
-  static Future<Chrome> _connect(Chrome chrome) async {
+  static Future<Chrome> _connect(Chrome chrome, bool skipCheck) async {
     if (_currentCompleter.isCompleted) {
       throwToolExit('Only one instance of chrome can be started.');
     }
     // The connection is lazy. Try a simple call to make sure the provided
     // connection is valid.
-    try {
-      await chrome.chromeConnection.getTabs();
-    } catch (e) {
-      await chrome.close();
-      throwToolExit(
-          'Unable to connect to Chrome debug port: ${chrome.debugPort}\n $e');
+    if (!skipCheck) {
+      try {
+        await chrome.chromeConnection.getTabs();
+      } catch (e) {
+        await chrome.close();
+        print('here');
+        throwToolExit(
+            'Unable to connect to Chrome debug port: ${chrome.debugPort}\n $e');
+      }
     }
     _currentCompleter.complete(chrome);
     return chrome;
@@ -145,7 +151,7 @@ class ChromeLauncher {
 
   /// Connects to an instance of Chrome with an open debug port.
   static Future<Chrome> fromExisting(int port) async =>
-      _connect(Chrome._(port, ChromeConnection('localhost', port)));
+      _connect(Chrome._(port, ChromeConnection('localhost', port)), false);
 
   static Future<Chrome> get connectedInstance => _currentCompleter.future;
 
@@ -176,14 +182,11 @@ class Chrome {
     this.debugPort,
     this.chromeConnection, {
     Process process,
-    Directory dataDir,
     this.remoteDebuggerUri,
-  })  : _process = process,
-        _dataDir = dataDir;
+  })  : _process = process;
 
   final int debugPort;
   final Process _process;
-  final Directory _dataDir;
   final ChromeConnection chromeConnection;
   final Uri remoteDebuggerUri;
 
@@ -198,19 +201,5 @@ class Chrome {
     chromeConnection.close();
     _process?.kill();
     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<void>.delayed(const Duration(milliseconds: 500));
-    } catch (_) {
-      // Silently fail if we can't clean up the profile information.
-    } finally {
-      try {
-        await _dataDir?.delete(recursive: true);
-      } on FileSystemException {
-        printError('failed to delete temporary profile at ${_dataDir.path}');
-      }
-    }
   }
 }

packages/flutter_tools/test/general.shard/web/chrome_test.dart

@@ -0,0 +1,68 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:flutter_tools/src/base/os.dart';
+import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/process_manager.dart';
+import 'package:flutter_tools/src/convert.dart';
+import 'package:flutter_tools/src/web/chrome.dart';
+import 'package:mockito/mockito.dart';
+import 'package:process/process.dart';
+
+import '../../src/common.dart';
+import '../../src/mocks.dart';
+import '../../src/testbed.dart';
+
+void main() {
+  Testbed testbed;
+
+  setUp(() {
+    final MockPlatform platform = MockPlatform();
+    when(platform.isWindows).thenReturn(false);
+    testbed = Testbed(overrides: <Type, Generator>{
+      ProcessManager: () => MockProcessManager(),
+      Platform: () => platform,
+      OperatingSystemUtils: () => MockOperatingSystemUtils(),
+    });
+  });
+
+  test('can launch chrome and connect to the devtools', () => testbed.run(() async {
+    when(os.findFreePort()).thenAnswer((Invocation invocation) async {
+      return 1234;
+    });
+    when(platform.environment).thenReturn(<String, String>{
+      kChromeEnvironment: 'example_chrome'
+    });
+    when(processManager.start(<String>[
+      'example_chrome',
+      '--user-data-dir=.dart_tool/chrome_profile',
+      '--remote-debugging-port=1234',
+      '--disable-background-timer-throttling',
+      '--disable-extensions',
+      '--disable-popup-blocking',
+      '--bwsi',
+      '--no-first-run',
+      '--no-default-browser-check',
+      '--disable-default-apps',
+      '--disable-translate',
+      'example_url'
+    ])).thenAnswer((Invocation invocation) async {
+      return FakeProcess(
+        exitCode: Completer<int>().future,
+        stdout: const Stream<List<int>>.empty(),
+        stderr: Stream<List<int>>.fromIterable(<List<int>>[
+          utf8.encode('\n\nDevTools listening\n\n')
+        ]),
+      );
+    });
+
+    await chromeLauncher.launch('example_url', skipCheck: true);
+  }));
+}
+
+class MockProcessManager extends Mock implements ProcessManager {}
+class MockPlatform extends Mock implements Platform {}
+class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}