Always activate DevTools if it's not installed (#81242)

packages/flutter_tools/lib/src/devtools_launcher.dart

@@ -90,11 +90,14 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
         // couple scripts. See https://github.com/flutter/devtools/issues/2420.
         return;
       } else {
-        final bool didActivateDevTools = await _activateDevTools();
-        final bool devToolsActive = await _checkForActiveDevTools();
-        if (!didActivateDevTools && !devToolsActive) {
-          // At this point, we failed to activate the DevTools package and the
-          // package is not already active.
+        bool devToolsActive = await _checkForActiveDevTools();
+        await _activateDevTools(throttleUpdates: devToolsActive);
+        if (!devToolsActive) {
+          devToolsActive = await _checkForActiveDevTools();
+        }
+        if (!devToolsActive) {
+          // We don't have devtools installed and installing it failed;
+          // _activateDevTools will have reported the error already.
           return;
         }
       }
@@ -135,51 +138,51 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
     }
   }
 
+  static final RegExp _devToolsInstalledPattern = RegExp(r'^devtools ', multiLine: true);
+
+  /// Check if the DevTools package is already active by running "pub global list".
   Future<bool> _checkForActiveDevTools() async {
-    // We are offline, and cannot activate DevTools, so check if the DevTools
-    // package is already active.
-    final io.ProcessResult _pubGlobalListProcess = await _processManager.run(<String>[
-      _pubExecutable,
-      'global',
-      'list',
-    ]);
-    if (_pubGlobalListProcess.stdout.toString().contains('devtools ')) {
-      return true;
-    }
-    return false;
+    final io.ProcessResult _pubGlobalListProcess = await _processManager.run(
+      <String>[ _pubExecutable, 'global', 'list' ],
+    );
+    return _pubGlobalListProcess.stdout.toString().contains(_devToolsInstalledPattern);
   }
 
   /// Helper method to activate the DevTools pub package.
   ///
-  /// Returns a bool indicating whether or not the package was successfully
-  /// activated from pub.
-  Future<bool> _activateDevTools() async {
-    final DateTime now = DateTime.now();
-    // Only attempt to activate DevTools twice a day.
-    final bool shouldActivate =
-        _persistentToolState.lastDevToolsActivationTime == null ||
-        now.difference(_persistentToolState.lastDevToolsActivationTime).inHours >= 12;
-    if (!shouldActivate) {
-      return false;
-    }
-    final Status status = _logger.startProgress(
-      'Activating Dart DevTools...',
-    );
+  /// If throttleUpdates is true, then this is a no-op if it was run in
+  /// the last twelve hours. It should be set to true if devtools is known
+  /// to already be installed.
+  ///
+  /// Return value indicates if DevTools was installed or updated.
+  Future<bool> _activateDevTools({@required bool throttleUpdates}) async {
+    assert(throttleUpdates != null);
+    const Duration _throttleDuration = Duration(hours: 12);
+    if (throttleUpdates) {
+      if (_persistentToolState.lastDevToolsActivationTime != null &&
+          DateTime.now().difference(_persistentToolState.lastDevToolsActivationTime) < _throttleDuration) {
+        _logger.printTrace('DevTools activation throttled until ${_persistentToolState.lastDevToolsActivationTime.add(_throttleDuration).toLocal()}.');
+        return false; // Throttled.
+      }
+    }
+    final Status status = _logger.startProgress('Activating Dart DevTools...');
     try {
       final io.ProcessResult _devToolsActivateProcess = await _processManager
           .run(<String>[
         _pubExecutable,
         'global',
         'activate',
-        'devtools'
+        'devtools',
       ]);
       if (_devToolsActivateProcess.exitCode != 0) {
-        _logger.printError('Error running `pub global activate '
-            'devtools`:\n${_devToolsActivateProcess.stderr}');
-        return false;
+        _logger.printError(
+          'Error running `pub global activate devtools`:\n'
+          '${_devToolsActivateProcess.stderr}'
+        );
+        return false; // Failed to activate.
       }
       _persistentToolState.lastDevToolsActivation = DateTime.now();
-      return true;
+      return true; // Activation succeeded!
     } on Exception catch (e, _) {
       _logger.printError('Error running `pub global activate devtools`: $e');
       return false;

packages/flutter_tools/lib/src/resident_devtools_handler.dart

@@ -25,6 +25,12 @@ abstract class ResidentDevtoolsHandler {
   /// The current devtools server, or null if one is not running.
   DevToolsServerAddress get activeDevToolsServer;
 
+  /// Whether it's ok to announce the [activeDevToolsServer].
+  ///
+  /// This should only return true once all the devices have been notified
+  /// of the DevTools.
+  bool get readyToAnnounce;
+
   Future<void> hotRestart(List<FlutterDevice> flutterDevices);
 
   Future<void> serveAndAnnounceDevTools({Uri devToolsServerAddress, List<FlutterDevice> flutterDevices});
@@ -44,6 +50,10 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
   @override
   DevToolsServerAddress get activeDevToolsServer => _devToolsLauncher?.activeDevToolsServer;
 
+  @override
+  bool get readyToAnnounce => _readyToAnnounce;
+  bool _readyToAnnounce = false;
+
   // This must be guaranteed not to return a Future that fails.
   @override
   Future<void> serveAndAnnounceDevTools({
@@ -60,18 +70,15 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
       await _devToolsLauncher.serve();
     }
     await _devToolsLauncher.ready;
+    final List<FlutterDevice> devicesWithExtension = await _devicesWithExtensions(flutterDevices);
+    await _maybeCallDevToolsUriServiceExtension(devicesWithExtension);
+    await _callConnectedVmServiceUriExtension(devicesWithExtension);
+    _readyToAnnounce = true;
     if (_residentRunner.reportedDebuggers) {
       // Since the DevTools only just became available, we haven't had a chance to
       // report their URLs yet. Do so now.
       _residentRunner.printDebuggerList(includeObservatory: false);
     }
-    final List<FlutterDevice> devicesWithExtension = await _devicesWithExtensions(flutterDevices);
-    await _maybeCallDevToolsUriServiceExtension(
-      devicesWithExtension,
-    );
-    await _callConnectedVmServiceUriExtension(
-      devicesWithExtension,
-    );
   }
 
   Future<void> _maybeCallDevToolsUriServiceExtension(
@@ -213,6 +220,9 @@ class NoOpDevtoolsHandler implements ResidentDevtoolsHandler {
   @override
   DevToolsServerAddress get activeDevToolsServer => null;
 
+  @override
+  bool get readyToAnnounce => false;
+
   @override
   Future<void> hotRestart(List<FlutterDevice> flutterDevices) async {
     return;

packages/flutter_tools/lib/src/resident_runner.dart

@@ -1423,9 +1423,10 @@ abstract class ResidentRunner extends ResidentHandlers {
 
   void printDebuggerList({ bool includeObservatory = true, bool includeDevtools = true }) {
     final DevToolsServerAddress devToolsServerAddress = residentDevtoolsHandler.activeDevToolsServer;
-    if (devToolsServerAddress == null) {
+    if (!residentDevtoolsHandler.readyToAnnounce) {
       includeDevtools = false;
     }
+    assert(!includeDevtools || devToolsServerAddress != null);
     for (final FlutterDevice device in flutterDevices) {
       if (device.vmService == null) {
         continue;

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

@@ -107,18 +107,18 @@ void main() {
           command: <String>[
             'pub',
             'global',
-            'activate',
-            'devtools',
+            'list',
           ],
-          stdout: 'Activated DevTools 0.9.5',
+          stdout: 'devtools 0.9.6',
         ),
         const FakeCommand(
           command: <String>[
             'pub',
             'global',
-            'list',
+            'activate',
+            'devtools',
           ],
-          stdout: 'devtools 0.9.6',
+          stdout: 'Activated DevTools 0.9.6',
         ),
         FakeCommand(
           command: const <String>[
@@ -148,6 +148,14 @@ void main() {
       persistentToolState: persistentToolState,
       httpClient: FakeHttpClient.any(),
       processManager: FakeProcessManager.list(<FakeCommand>[
+        const FakeCommand(
+          command: <String>[
+            'pub',
+            'global',
+            'list',
+          ],
+          stdout: '',
+        ),
         const FakeCommand(
           command: <String>[
             'pub',
@@ -163,7 +171,7 @@ void main() {
             'global',
             'list',
           ],
-          stdout: 'devtools 0.9.6',
+          stdout: 'devtools 0.9.5',
         ),
         FakeCommand(
           command: const <String>[
@@ -197,18 +205,18 @@ void main() {
           command: <String>[
             'pub',
             'global',
-            'activate',
-            'devtools',
+            'list',
           ],
-          stdout: 'Activated DevTools 0.9.5',
+          stdout: 'devtools 0.9.5',
         ),
         const FakeCommand(
           command: <String>[
             'pub',
             'global',
-            'list',
+            'activate',
+            'devtools',
           ],
-          stdout: 'devtools 0.9.6',
+          stdout: 'Activated DevTools 0.9.5',
         ),
         FakeCommand(
           command: const <String>[
@@ -279,19 +287,19 @@ void main() {
           command: <String>[
             'pub',
             'global',
-            'activate',
-            'devtools',
+            'list',
           ],
-          stderr: 'Error - could not activate devtools',
-          exitCode: 1,
+          stdout: 'devtools 0.9.6',
         ),
         const FakeCommand(
           command: <String>[
             'pub',
             'global',
-            'list',
+            'activate',
+            'devtools',
           ],
-          stdout: 'devtools 0.9.6',
+          stderr: 'Error - could not activate devtools',
+          exitCode: 1,
         ),
         const FakeCommand(
           command: <String>[
@@ -324,18 +332,18 @@ void main() {
           command: <String>[
             'pub',
             'global',
-            'activate',
-            'devtools',
+            'list',
           ],
-          stdout: 'Activated DevTools 0.9.5',
+          stdout: 'devtools 0.9.5',
         ),
         const FakeCommand(
           command: <String>[
             'pub',
             'global',
-            'list',
+            'activate',
+            'devtools',
           ],
-          stdout: 'devtools 0.9.6',
+          stdout: 'Activated DevTools 0.9.6',
         ),
         const FakeCommand(
           command: <String>[

packages/flutter_tools/test/integration.shard/overall_experience_test.dart

@@ -514,7 +514,7 @@ void main() {
     } finally {
       tryToDelete(fileSystem.directory(tempDirectory));
     }
-  }, skip: 'DevTools does not reliably launch on bots currently.'); // TODO(ianh): fix and re-enable test.
+  });
 
   testWithoutContext('flutter run help output', () async {
     // This test enables all logging so that it checks the exact text of starting up an application.