Add a warning to stop launching on pub for DevTools version 2.8.0 (flutter#3464)

Author: kenzieschmoll

packages/devtools_app/lib/src/app.dart

@@ -47,6 +47,7 @@ import 'screen.dart';
 import 'snapshot_screen.dart';
 import 'theme.dart';
 import 'ui/service_extension_widgets.dart';
+import 'utils.dart';
 import 'vm_developer/vm_developer_tools_controller.dart';
 import 'vm_developer/vm_developer_tools_screen.dart';
 
@@ -60,6 +61,13 @@ const showVmDeveloperMode = false;
 /// Whether this DevTools build is external.
 bool isExternalBuild = true;
 
+// TODO(kenz): remove the pub warning code after devtools version 2.8.0 ships
+/// Whether DevTools should warn users to stop launching DevTools from Pub.
+///
+/// This flag will be turned on for the final release of DevTools on pub, but
+/// should remain off at HEAD.
+const showPubWarning = false;
+
 /// Top-level configuration for the app.
 @immutable
 class DevToolsApp extends StatefulWidget {
@@ -487,7 +495,6 @@ class DevToolsAboutDialog extends StatelessWidget {
   @override
   Widget build(BuildContext context) {
     final theme = Theme.of(context);
-
     return DevToolsDialog(
       title: dialogTitleText(theme, 'About DevTools'),
       content: Column(
@@ -496,6 +503,10 @@ class DevToolsAboutDialog extends StatelessWidget {
         children: [
           _aboutDevTools(context),
           const SizedBox(height: defaultSpacing),
+          if (shouldShowPubWarning()) ...[
+            const PubWarningText(),
+            const SizedBox(height: defaultSpacing),
+          ],
           ...dialogSubHeader(theme, 'Feedback'),
           Wrap(
             children: [

packages/devtools_app/lib/src/common_widgets.dart

@@ -8,6 +8,7 @@ import 'dart:math';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 
+import 'globals.dart';
 import 'scaffold.dart';
 import 'theme.dart';
 import 'ui/icons.dart';
@@ -1531,3 +1532,40 @@ class CheckboxSetting extends StatelessWidget {
     return content;
   }
 }
+
+class PubWarningText extends StatelessWidget {
+  const PubWarningText({Key key}) : super(key: key);
+
+  @override
+  Widget build(BuildContext context) {
+    final theme = Theme.of(context);
+    final isFlutterApp = serviceManager.connectedApp.isFlutterAppNow;
+    final sdkName = isFlutterApp ? 'Flutter' : 'Dart';
+    final minSdkVersion = isFlutterApp ? '2.8.0' : '2.15.0';
+    return SelectableText.rich(
+      TextSpan(
+        text: 'Warning: you should no longer be launching DevTools from'
+            ' pub.\n\n',
+        style: theme.subtleTextStyle
+            .copyWith(color: theme.colorScheme.errorTextColor),
+        children: [
+          TextSpan(
+            text: 'DevTools version 2.8.0 will be the last version to '
+                'be shipped on pub. As of $sdkName\nversion >= '
+                '$minSdkVersion, DevTools should be launched by running '
+                'the ',
+            style: theme.subtleTextStyle,
+          ),
+          TextSpan(
+            text: '`dart devtools`',
+            style: theme.subtleFixedFontStyle,
+          ),
+          TextSpan(
+            text: '\ncommand.',
+            style: theme.subtleTextStyle,
+          ),
+        ],
+      ),
+    );
+  }
+}

packages/devtools_app/lib/src/initializer.dart

@@ -150,8 +150,7 @@ class _InitializerState extends State<Initializer>
     final theme = Theme.of(context);
     currentDisconnectedOverlay = OverlayEntry(
       builder: (context) => Container(
-        // TODO(dantup): Change this to a theme colour and ensure it works in both dart/light themes
-        color: const Color.fromRGBO(128, 128, 128, 0.5),
+        color: theme.colorScheme.overlayShadowColor,
         child: Center(
           child: Column(
             children: [

packages/devtools_app/lib/src/performance/performance_screen.dart

@@ -21,7 +21,6 @@ import '../screen.dart';
 import '../service_extensions.dart';
 import '../split.dart';
 import '../theme.dart';
-import '../ui/colors.dart';
 import '../ui/icons.dart';
 import '../ui/service_extension_widgets.dart';
 import '../ui/vm_flag_widgets.dart';
@@ -367,7 +366,8 @@ class EnhanceTracingButton extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    final textStyle = Theme.of(context).subtleTextStyle;
+    final theme = Theme.of(context);
+    final textStyle = theme.subtleTextStyle;
     return ServiceExtensionCheckboxGroupButton(
       title: 'Enhance Tracing',
       icon: Icons.auto_awesome,
@@ -387,7 +387,8 @@ class EnhanceTracingButton extends StatelessWidget {
           children: [
             TextSpan(
               text: 'frame times may be negatively affected',
-              style: textStyle.copyWith(color: rasterJankColor),
+              style:
+                  textStyle.copyWith(color: theme.colorScheme.errorTextColor),
             ),
             TextSpan(
               text: '.',

packages/devtools_app/lib/src/scaffold.dart

@@ -137,6 +137,49 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
         frameworkController.onShowPageId.listen(_showPageById);
 
     _initTitle();
+    _maybeShowPubWarning();
+  }
+
+  bool _pubWarningShown = false;
+
+  // TODO(kenz): remove the pub warning code after devtools version 2.8.0 ships
+  void _maybeShowPubWarning() {
+    if (!_pubWarningShown) {
+      serviceManager.onConnectionAvailable?.listen((event) {
+        if (shouldShowPubWarning()) {
+          final colorScheme = Theme.of(context).colorScheme;
+          OverlayEntry _entry;
+          Overlay.of(context).insert(
+            _entry = OverlayEntry(
+              maintainState: true,
+              builder: (context) {
+                return Material(
+                  color: colorScheme.overlayShadowColor,
+                  child: Center(
+                    child: Container(
+                      padding: const EdgeInsets.all(defaultSpacing),
+                      color: colorScheme.overlayBackgroundColor,
+                      child: Column(
+                        mainAxisSize: MainAxisSize.min,
+                        children: [
+                          const PubWarningText(),
+                          const SizedBox(height: defaultSpacing),
+                          ElevatedButton(
+                            child: const Text('Got it'),
+                            onPressed: () => _entry.remove(),
+                          ),
+                        ],
+                      ),
+                    ),
+                  ),
+                );
+              },
+            ),
+          );
+          _pubWarningShown = true;
+        }
+      });
+    }
   }
 
   @override

packages/devtools_app/lib/src/theme.dart

@@ -234,6 +234,13 @@ extension DevToolsColorScheme on ColorScheme {
   Color get contrastForeground =>
       isLight ? Colors.black : _contrastForegroundWhite;
 
+  Color get overlayShadowColor => const Color.fromRGBO(0, 0, 0, 0.5);
+
+  Color get overlayBackgroundColor =>
+      isLight ? Colors.white : const Color(0xFF424242);
+
+  Color get errorTextColor => const Color(0xFFC3595A);
+
   Color get toggleButtonsTitle =>
       isLight ? const Color(0xFF464646) : const Color(0xFFAEAEB1);
 

packages/devtools_app/lib/src/utils.dart

@@ -16,9 +16,11 @@ import 'package:flutter/widgets.dart';
 import 'package:intl/intl.dart';
 import 'package:vm_service/vm_service.dart';
 
+import 'app.dart';
 import 'config_specific/logger/logger.dart' as logger;
 import 'globals.dart';
 import 'notifications.dart';
+import 'version.dart';
 
 bool isPrivate(String member) => member.startsWith('_');
 
@@ -1425,3 +1427,12 @@ const connectToNewAppText = 'Connect to a new app';
 /// Exception thrown when a request to process data has been cancelled in
 /// favor of a new request.
 class ProcessCancelledException implements Exception {}
+
+bool shouldShowPubWarning() =>
+    showPubWarning &&
+    (serviceManager.connectedApp?.isFlutterAppNow != null &&
+            serviceManager.connectedApp.flutterVersionNow >=
+                SemanticVersion(major: 2, minor: 8) ||
+        (serviceManager.vm != null &&
+            SemanticVersion.parse(serviceManager.vm.version) >=
+                SemanticVersion(major: 2, minor: 15)));

packages/devtools_app/lib/src/version.dart

@@ -16,33 +16,12 @@ class FlutterVersion extends SemanticVersion {
     @required this.engineRevision,
     @required this.dartSdkVersion,
   }) {
-    // Flutter versions are expected in the format '2.3.1-16.1-pre', so we split
-    // on the dash char to separate the main semantic version from the pre
-    // release version.
-    final splitOnDash = version.split('-');
-    assert(splitOnDash.length <= 2);
-
-    final semVersion = splitOnDash.first;
-    final _versionParts = semVersion.split('.');
-    major =
-        _versionParts.isNotEmpty ? int.tryParse(_versionParts.first) ?? 0 : 0;
-    minor = _versionParts.length > 1 ? int.tryParse(_versionParts[1]) ?? 0 : 0;
-    patch = _versionParts.length > 2 ? int.tryParse(_versionParts[2]) ?? 0 : 0;
-
-    if (splitOnDash.length == 2) {
-      final preRelease = splitOnDash.last;
-      final preReleaseParts = preRelease
-          .split('.')
-          .map((part) => RegExp(r'\d+').stringMatch(part) ?? '')
-          .toList()
-        ..removeWhere((part) => part.isEmpty);
-      preReleaseMajor = preReleaseParts.isNotEmpty
-          ? int.tryParse(preReleaseParts.first) ?? 0
-          : 0;
-      preReleaseMinor = preReleaseParts.length > 1
-          ? int.tryParse(preReleaseParts[1]) ?? 0
-          : 0;
-    }
+    final semVer = SemanticVersion.parse(version);
+    major = semVer.major;
+    minor = semVer.minor;
+    patch = semVer.patch;
+    preReleaseMajor = semVer.preReleaseMajor;
+    preReleaseMinor = semVer.preReleaseMinor;
   }
 
   factory FlutterVersion.parse(Map<String, dynamic> json) {
@@ -53,7 +32,7 @@ class FlutterVersion extends SemanticVersion {
       frameworkRevision: json['frameworkRevisionShort'],
       frameworkCommitDate: json['frameworkCommitDate'],
       engineRevision: json['engineRevisionShort'],
-      dartSdkVersion: json['dartSdkVersion'],
+      dartSdkVersion: _parseDartVersion(json['dartSdkVersion']),
     );
   }
 
@@ -69,7 +48,7 @@ class FlutterVersion extends SemanticVersion {
 
   final String engineRevision;
 
-  final String dartSdkVersion;
+  final SemanticVersion dartSdkVersion;
 
   String get flutterVersionSummary => [
         if (version != 'unknown') version,
@@ -103,6 +82,19 @@ class FlutterVersion extends SemanticVersion {
         engineRevision,
         dartSdkVersion,
       );
+
+  static SemanticVersion _parseDartVersion(String versionString) {
+    if (versionString == null) return null;
+
+    // Example Dart version string: "2.15.0 (build 2.15.0-178.1.beta)"
+    const prefix = '(build ';
+    final startIndex = versionString.indexOf(prefix) + prefix.length;
+    final rawVersion = versionString.substring(
+      startIndex,
+      versionString.length - 1,
+    );
+    return SemanticVersion.parse(rawVersion);
+  }
 }
 
 class SemanticVersion with CompareMixin {
@@ -114,6 +106,54 @@ class SemanticVersion with CompareMixin {
     this.preReleaseMinor,
   });
 
+  factory SemanticVersion.parse(String versionString) {
+    // [versionString] is expected to be of the form for VM.version, Dart, and
+    // Flutter, respectively:
+    // 2.15.0-233.0.dev (dev) (Mon Oct 18 14:06:26 2021 -0700) on "ios_x64"
+    // 2.15.0-178.1.beta
+    // 2.6.0-12.0.pre.443
+    //
+    // So split on the spaces to the version, and then on the dash char to
+    // separate the main semantic version from the pre release version.
+    final splitOnSpaces = versionString.split(' ');
+    final version = splitOnSpaces.first;
+    final splitOnDash = version.split('-');
+    assert(splitOnDash.length <= 2, 'version: $version');
+
+    final semVersion = splitOnDash.first;
+    final _versionParts = semVersion.split('.');
+    final major =
+        _versionParts.isNotEmpty ? int.tryParse(_versionParts.first) ?? 0 : 0;
+    final minor =
+        _versionParts.length > 1 ? int.tryParse(_versionParts[1]) ?? 0 : 0;
+    final patch =
+        _versionParts.length > 2 ? int.tryParse(_versionParts[2]) ?? 0 : 0;
+
+    int preReleaseMajor;
+    int preReleaseMinor;
+    if (splitOnDash.length == 2) {
+      final preRelease = splitOnDash.last;
+      final preReleaseParts = preRelease
+          .split('.')
+          .map((part) => RegExp(r'\d+').stringMatch(part) ?? '')
+          .toList()
+        ..removeWhere((part) => part.isEmpty);
+      preReleaseMajor = preReleaseParts.isNotEmpty
+          ? int.tryParse(preReleaseParts.first) ?? 0
+          : 0;
+      preReleaseMinor = preReleaseParts.length > 1
+          ? int.tryParse(preReleaseParts[1]) ?? 0
+          : 0;
+    }
+    return SemanticVersion(
+      major: major,
+      minor: minor,
+      patch: patch,
+      preReleaseMajor: preReleaseMajor,
+      preReleaseMinor: preReleaseMinor,
+    );
+  }
+
   int major;
 
   int minor;
@@ -134,8 +174,8 @@ class SemanticVersion with CompareMixin {
     if (major == other.major &&
         minor == other.minor &&
         patch == other.patch &&
-        preReleaseMajor == other.preReleaseMajor &&
-        preReleaseMinor == other.preReleaseMinor) {
+        (preReleaseMajor ?? 0) == (other.preReleaseMajor ?? 0) &&
+        (preReleaseMinor ?? 0) == (other.preReleaseMinor ?? 0)) {
       return 0;
     }
     if (major > other.major ||
@@ -145,7 +185,7 @@ class SemanticVersion with CompareMixin {
     }
     if (major == other.major && minor == other.minor && patch == other.patch) {
       if (isPreRelease != other.isPreRelease) {
-        return isPreRelease ? 1 : -1;
+        return isPreRelease ? -1 : 1;
       }
       if (preReleaseMajor > other.preReleaseMajor ||
           (preReleaseMajor == other.preReleaseMajor &&

packages/devtools_app/test/version_test.dart

@@ -121,11 +121,21 @@ void main() {
         version.compareTo(SemanticVersion(major: 1, minor: 1, patch: 1)),
         equals(0),
       );
+      expect(
+        version.compareTo(SemanticVersion(
+          major: 1,
+          minor: 1,
+          patch: 1,
+          preReleaseMajor: 0,
+          preReleaseMinor: 0,
+        )),
+        equals(0),
+      );
 
       expect(
         version.compareTo(
             SemanticVersion(major: 1, minor: 1, patch: 1, preReleaseMajor: 1)),
-        equals(-1),
+        equals(1),
       );
 
       version = SemanticVersion(