Launch DevTools from pub instead of devtools_server (#71737)

* Launch DevTools from pub instead of devtools_server

packages/flutter_tools/lib/executable.dart

@@ -5,6 +5,7 @@
 import 'package:meta/meta.dart';
 
 import 'runner.dart' as runner;
+import 'src/artifacts.dart';
 import 'src/base/context.dart';
 import 'src/base/file_system.dart';
 import 'src/base/io.dart';
@@ -44,10 +45,10 @@ import 'src/commands/symbolize.dart';
 import 'src/commands/test.dart';
 import 'src/commands/update_packages.dart';
 import 'src/commands/upgrade.dart';
+import 'src/devtools_launcher.dart';
 import 'src/features.dart';
 import 'src/globals.dart' as globals;
 // Files in `isolated` are intentionally excluded from google3 tooling.
-import 'src/isolated/devtools_launcher.dart';
 import 'src/isolated/mustache_template.dart';
 import 'src/isolated/resident_web_runner.dart';
 import 'src/resident_runner.dart';
@@ -150,7 +151,11 @@ Future<void> main(List<String> args) async {
        TemplateRenderer: () => const MustacheTemplateRenderer(),
        // The devtools launcher is not supported in google3 because it depends on
        // devtools source code.
-       DevtoolsLauncher: () => DevtoolsServerLauncher(logger: globals.logger),
+       DevtoolsLauncher: () => DevtoolsServerLauncher(
+         processManager: globals.processManager,
+         pubExecutable: globals.artifacts.getArtifactPath(Artifact.pubExecutable),
+         logger: globals.logger,
+       ),
        Logger: () {
         final LoggerFactory loggerFactory = LoggerFactory(
           outputPreferences: globals.outputPreferences,

packages/flutter_tools/lib/src/artifacts.dart

@@ -85,6 +85,9 @@ enum Artifact {
   /// Tools related to subsetting or icon font files.
   fontSubset,
   constFinder,
+
+  /// The pub or pub.bat executable
+  pubExecutable,
 }
 
 String _artifactToFileName(Artifact artifact, [ TargetPlatform platform, BuildMode mode ]) {
@@ -178,6 +181,11 @@ String _artifactToFileName(Artifact artifact, [ TargetPlatform platform, BuildMo
     case Artifact.webPrecompiledCanvaskitSoundSdkSourcemaps:
     case Artifact.webPrecompiledCanvaskitAndHtmlSoundSdkSourcemaps:
       return 'dart_sdk.js.map';
+    case Artifact.pubExecutable:
+      if (platform == TargetPlatform.windows_x64) {
+        return 'pub.bat';
+      }
+      return 'pub';
   }
   assert(false, 'Invalid artifact $artifact.');
   return null;
@@ -457,6 +465,8 @@ class CachedArtifacts implements Artifacts {
         return _fileSystem.path.join(_getFlutterWebSdkPath(), 'kernel', 'amd-canvaskit-html-sound', _artifactToFileName(artifact, platform, mode));
       case Artifact.webPrecompiledCanvaskitAndHtmlSoundSdkSourcemaps:
         return _fileSystem.path.join(_getFlutterWebSdkPath(), 'kernel', 'amd-canvaskit-html-sound', _artifactToFileName(artifact, platform, mode));
+      case Artifact.pubExecutable:
+        return _fileSystem.path.join(_dartSdkPath(_fileSystem), 'bin',  _artifactToFileName(artifact, platform, mode));
       default:
         assert(false, 'Artifact $artifact not available for platform $platform.');
         return null;
@@ -699,6 +709,8 @@ class LocalEngineArtifacts implements Artifacts {
         return _fileSystem.path.join(_getFlutterWebSdkPath(), 'kernel', 'amd-canvaskit-html-sound', artifactFileName);
       case Artifact.webPrecompiledCanvaskitAndHtmlSoundSdkSourcemaps:
         return _fileSystem.path.join(_getFlutterWebSdkPath(), 'kernel', 'amd-canvaskit-html-sound', artifactFileName);
+      case Artifact.pubExecutable:
+        return _fileSystem.path.join(_hostEngineOutPath, 'dart-sdk', 'bin', _artifactToFileName(artifact, platform, mode));
     }
     assert(false, 'Invalid artifact $artifact.');
     return null;

packages/flutter_tools/lib/src/commands/daemon.dart

@@ -872,8 +872,8 @@ class DevToolsDomain extends Domain {
 
   Future<Map<String, dynamic>> serve([ Map<String, dynamic> args ]) async {
     _devtoolsLauncher ??= DevtoolsLauncher.instance;
-    final DevToolsServerAddress server = await _devtoolsLauncher.serve();
-
+    final bool openInBrowser = args != null && (args['openInBrowser'] == 'true');
+    final DevToolsServerAddress server = await _devtoolsLauncher.serve(openInBrowser: openInBrowser);
     return<String, dynamic>{
       'host': server?.host,
       'port': server?.port,

packages/flutter_tools/lib/src/context_runner.dart

@@ -28,6 +28,7 @@ import 'cache.dart';
 import 'dart/pub.dart';
 import 'devfs.dart';
 import 'device.dart';
+import 'devtools_launcher.dart';
 import 'doctor.dart';
 import 'emulator.dart';
 import 'features.dart';
@@ -46,6 +47,7 @@ import 'macos/xcode.dart';
 import 'mdns_discovery.dart';
 import 'persistent_tool_state.dart';
 import 'reporting/reporting.dart';
+import 'resident_runner.dart';
 import 'run_hot.dart';
 import 'runner/local_engine.dart';
 import 'version.dart';
@@ -178,6 +180,11 @@ Future<T> runInContext<T>(
         operatingSystemUtils: globals.os,
         terminal: globals.terminal,
       ),
+      DevtoolsLauncher: () => DevtoolsServerLauncher(
+        processManager: globals.processManager,
+        pubExecutable: globals.artifacts.getArtifactPath(Artifact.pubExecutable),
+        logger: globals.logger,
+      ),
       Doctor: () => Doctor(logger: globals.logger),
       DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
       EmulatorManager: () => EmulatorManager(

packages/flutter_tools/lib/src/devtools_launcher.dart

+// Copyright 2014 The Flutter 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:browser_launcher/browser_launcher.dart';
+import 'package:meta/meta.dart';
+import 'package:process/process.dart';
+
+import 'base/io.dart' as io;
+import 'base/logger.dart';
+import 'convert.dart';
+import 'resident_runner.dart';
+
+/// An implementation of the devtools launcher that uses the server package.
+///
+/// This is implemented in isolated to prevent the flutter_tool from needing
+/// a devtools dep in google3.
+class DevtoolsServerLauncher extends DevtoolsLauncher {
+  DevtoolsServerLauncher({
+    @required ProcessManager processManager,
+    @required String pubExecutable,
+    @required Logger logger,
+  })  : _processManager = processManager,
+        _pubExecutable = pubExecutable,
+        _logger = logger;
+
+  final ProcessManager _processManager;
+  final String _pubExecutable;
+  final Logger _logger;
+
+  io.Process _devToolsProcess;
+  Uri _devToolsUri;
+
+  static final RegExp _serveDevToolsPattern =
+      RegExp(r'Serving DevTools at ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+)');
+
+  @override
+  Future<void> launch(Uri vmServiceUri, {bool openInBrowser = false}) async {
+    if (_devToolsProcess != null && _devToolsUri != null) {
+      // DevTools is already running.
+      if (openInBrowser) {
+        await Chrome.start(<String>[_devToolsUri.toString()]);
+      }
+      return;
+    }
+
+    final Status status = _logger.startProgress(
+      'Activating Dart DevTools...',
+    );
+    try {
+      // TODO(kenz): https://github.com/dart-lang/pub/issues/2791 - calling `pub
+      // global activate` adds ~ 4.5 seconds of latency.
+      final io.ProcessResult _devToolsActivateProcess = await _processManager.run(<String>[
+        _pubExecutable,
+        'global',
+        'activate',
+        'devtools'
+      ]);
+      if (_devToolsActivateProcess.exitCode != 0) {
+        status.cancel();
+        _logger.printError('Error running `pub global activate '
+            'devtools`:\n${_devToolsActivateProcess.stderr}');
+        return;
+      }
+      status.stop();
+
+      _devToolsProcess = await _processManager.start(<String>[
+        _pubExecutable,
+        'global',
+        'run',
+        'devtools',
+        if (vmServiceUri != null) '--vm-uri=$vmServiceUri',
+      ]);
+      final Completer<Uri> completer = Completer<Uri>();
+      _devToolsProcess.stdout
+          .transform(utf8.decoder)
+          .transform(const LineSplitter())
+          .listen((String line) {
+            final Match match = _serveDevToolsPattern.firstMatch(line);
+            if (match != null) {
+              // We are trying to pull "http://127.0.0.1:9101" from "Serving
+              // DevTools at http://127.0.0.1:9101.". `match[1]` will return
+              // "http://127.0.0.1:9101.", and we need to trim the trailing period
+              // so that we don't throw an exception from `Uri.parse`.
+              String uri = match[1];
+              if (uri.endsWith('.')) {
+                uri = uri.substring(0, uri.length - 1);
+              }
+              completer.complete(Uri.parse(uri));
+            }
+            _logger.printStatus(line);
+         });
+      _devToolsProcess.stderr
+          .transform(utf8.decoder)
+          .transform(const LineSplitter())
+          .listen(_logger.printError);
+      _devToolsUri = await completer.future;
+    } on Exception catch (e, st) {
+      status.cancel();
+      _logger.printError('Failed to launch DevTools: $e', stackTrace: st);
+    }
+  }
+
+  @override
+  Future<DevToolsServerAddress> serve({bool openInBrowser = false}) async {
+    await launch(null, openInBrowser: openInBrowser);
+    if (_devToolsUri == null) {
+      return null;
+    }
+    return DevToolsServerAddress(_devToolsUri.host, _devToolsUri.port);
+  }
+
+  @override
+  Future<void> close() async {
+    if (_devToolsProcess != null) {
+      _devToolsProcess.kill();
+      await _devToolsProcess.exitCode;
+    }
+  }
+}

packages/flutter_tools/lib/src/isolated/devtools_launcher.dart

-// Copyright 2014 The Flutter 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 'package:devtools_server/devtools_server.dart' as devtools_server;
-import 'package:meta/meta.dart';
-
-import '../base/io.dart' as io;
-import '../base/logger.dart';
-import '../resident_runner.dart';
-
-/// An implementation of the devtools launcher that uses the server package.
-///
-/// This is implemented in isolated to prevent the flutter_tool from needing
-/// a devtools dep in google3.
-class DevtoolsServerLauncher extends DevtoolsLauncher {
-  DevtoolsServerLauncher({
-    @required Logger logger,
-  }) : _logger = logger;
-
-  final Logger _logger;
-
-  io.HttpServer _devtoolsServer;
-
-  @override
-  Future<void> launch(Uri observatoryAddress) async {
-    try {
-      await serve();
-      await devtools_server.launchDevTools(
-        <String, dynamic>{
-          'reuseWindows': true,
-        },
-        observatoryAddress,
-        'http://${_devtoolsServer.address.host}:${_devtoolsServer.port}',
-        false,  // headless mode,
-        false,  // machine mode
-      );
-    } on Exception catch (e, st) {
-      _logger.printTrace('Failed to launch DevTools: $e\n$st');
-    }
-  }
-
-  @override
-  Future<DevToolsServerAddress> serve() async {
-    try {
-      _devtoolsServer ??= await devtools_server.serveDevTools(
-        enableStdinCommands: false,
-      );
-      return DevToolsServerAddress(_devtoolsServer.address.host, _devtoolsServer.port);
-    } on Exception catch (e, st) {
-      _logger.printTrace('Failed to serve DevTools: $e\n$st');
-      return null;
-    }
-  }
-
-  @override
-  Future<void> close() async {
-    await _devtoolsServer?.close();
-    _devtoolsServer = null;
-  }
-}

packages/flutter_tools/lib/src/resident_runner.dart

@@ -1250,13 +1250,16 @@ abstract class ResidentRunner {
     }
   }
 
-  Future<bool> launchDevTools() async {
+  Future<bool> launchDevTools({bool openInBrowser = false}) async {
     if (!supportsServiceProtocol) {
       return false;
     }
     assert(supportsServiceProtocol);
     _devtoolsLauncher ??= DevtoolsLauncher.instance;
-    await _devtoolsLauncher.launch(flutterDevices.first.vmService.httpAddress);
+    unawaited(_devtoolsLauncher.launch(
+      flutterDevices.first.vmService.httpAddress,
+      openInBrowser: openInBrowser,
+    ));
     return true;
   }
 
@@ -1549,7 +1552,7 @@ class TerminalHandler {
       case 'U':
         return residentRunner.debugDumpSemanticsTreeInInverseHitTestOrder();
       case 'v':
-        return residentRunner.launchDevTools();
+        return residentRunner.launchDevTools(openInBrowser: true);
       case 'w':
       case 'W':
         return residentRunner.debugDumpApp();
@@ -1642,9 +1645,12 @@ String nextPlatform(String currentPlatform, FeatureFlags featureFlags) {
 
 /// A launcher for the devtools debugger and analysis tool.
 abstract class DevtoolsLauncher {
-  Future<void> launch(Uri observatoryAddress);
+  /// Launch a Dart DevTools process, optionally targeting a specific VM Service
+  /// URI if [vmServiceUri] is non-null.
+  Future<void> launch(Uri vmServiceUri, {bool openInBrowser = false});
 
-  Future<DevToolsServerAddress> serve();
+  /// Serve Dart DevTools and return the host and port they are available on.
+  Future<DevToolsServerAddress> serve({bool openInBrowser = false});
 
   Future<void> close();
 

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

+// Copyright 2014 The Flutter 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/io.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/devtools_launcher.dart';
+import 'package:flutter_tools/src/resident_runner.dart';
+
+import '../src/common.dart';
+import '../src/context.dart';
+
+void main() {
+  testWithoutContext('DevtoolsLauncher launches DevTools through pub and saves the URI', () async {
+    final Completer<void> completer = Completer<void>();
+    final DevtoolsLauncher launcher = DevtoolsServerLauncher(
+      pubExecutable: 'pub',
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.list(<FakeCommand>[
+        const FakeCommand(
+          command: <String>[
+            'pub',
+            'global',
+            'activate',
+            'devtools',
+          ],
+          stdout: 'Activated DevTools 0.9.5',
+        ),
+        FakeCommand(
+          command: const <String>[
+            'pub',
+            'global',
+            'run',
+            'devtools',
+          ],
+          stdout: 'Serving DevTools at http://127.0.0.1:9100\n',
+          completer: completer,
+        ),
+      ]),
+    );
+
+    final DevToolsServerAddress address = await launcher.serve();
+    expect(address.host, '127.0.0.1');
+    expect(address.port, 9100);
+  });
+
+  testWithoutContext('DevtoolsLauncher prints error if exception is thrown during activate', () async {
+    final BufferLogger logger = BufferLogger.test();
+    final DevtoolsLauncher launcher = DevtoolsServerLauncher(
+      pubExecutable: 'pub',
+      logger: logger,
+      processManager: FakeProcessManager.list(<FakeCommand>[
+        const FakeCommand(
+          command: <String>[
+            'pub',
+            'global',
+            'activate',
+            'devtools',
+          ],
+          stderr: 'Error - could not activate devtools',
+          exitCode: 1,
+        ),
+        FakeCommand(
+            command: const <String>[
+              'pub',
+              'global',
+              'run',
+              'devtools',
+              '--vm-uri=http://127.0.0.1:1234/abcdefg',
+            ],
+            onRun: () {
+              throw const ProcessException('pub', <String>[]);
+            }
+        )
+      ]),
+    );
+
+    await launcher.launch(Uri.parse('http://127.0.0.1:1234/abcdefg'));
+
+    expect(logger.errorText, contains('Error running `pub global activate devtools`:\nError - could not activate devtools'));
+  });
+
+  testWithoutContext('DevtoolsLauncher prints error if exception is thrown during launch', () async {
+    final BufferLogger logger = BufferLogger.test();
+    final DevtoolsLauncher launcher = DevtoolsServerLauncher(
+      pubExecutable: 'pub',
+      logger: logger,
+      processManager: FakeProcessManager.list(<FakeCommand>[
+        const FakeCommand(
+          command: <String>[
+            'pub',
+            'global',
+            'activate',
+            'devtools',
+          ],
+          stdout: 'Activated DevTools 0.9.5',
+        ),
+        FakeCommand(
+            command: const <String>[
+              'pub',
+              'global',
+              'run',
+              'devtools',
+              '--vm-uri=http://127.0.0.1:1234/abcdefg',
+            ],
+            onRun: () {
+              throw const ProcessException('pub', <String>[]);
+            }
+        )
+      ]),
+    );
+
+    await launcher.launch(Uri.parse('http://127.0.0.1:1234/abcdefg'));
+
+    expect(logger.errorText, contains('Failed to launch DevTools: ProcessException'));
+  });
+}

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

@@ -289,7 +289,7 @@ void main() {
       when(mockResidentRunner.supportsServiceProtocol).thenReturn(true);
       await terminalHandler.processTerminalInput('v');
 
-      verify(mockResidentRunner.launchDevTools()).called(1);
+      verify(mockResidentRunner.launchDevTools(openInBrowser: true)).called(1);
     });
 
     testWithoutContext('w,W - debugDumpApp with service protocol', () async {

packages/flutter_tools/test/src/fake_process_manager.dart

@@ -118,16 +118,12 @@ class _FakeProcess implements Process {
   _FakeProcess(
     this._exitCode,
     Duration duration,
-    VoidCallback onRun,
     this.pid,
     this._stderr,
     this.stdin,
     this._stdout,
     this._completer,
   ) : exitCode = Future<void>.delayed(duration).then((void value) {
-        if (onRun != null) {
-          onRun();
-        }
         if (_completer != null) {
           return _completer.future.then((void _) => _exitCode);
         }
@@ -231,10 +227,12 @@ abstract class FakeProcessManager implements ProcessManager {
   ) {
     _pid += 1;
     final FakeCommand fakeCommand = findCommand(command, workingDirectory, environment, encoding);
+    if (fakeCommand.onRun != null) {
+      fakeCommand.onRun();
+    }
     return _FakeProcess(
       fakeCommand.exitCode,
       fakeCommand.duration,
-      fakeCommand.onRun,
       _pid,
       fakeCommand.stderr,
       fakeCommand.stdin,