Merge remote-tracking branch 'origin/master' into simple-release-notes

Author: CoderDake

.github/PULL_REQUEST_TEMPLATE.md

@@ -20,12 +20,12 @@ RELEASE_NOTE_EXCEPTION=[REASON GOES HERE]
 
 ![build.yaml badge]
 
-If you need help, consider asking for advice on the #hackers-devexp-📐 channel on [Discord].
+If you need help, consider asking for help on [Discord].
 
 <!-- Links -->
 [Contributor Guide]: https://github.com/flutter/devtools/blob/master/CONTRIBUTING.md
 [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
 [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
 [CLA]: https://cla.developers.google.com/
-[Discord]: https://discord.com/channels/608014603317936148/608020249119555594
+[Discord]: https://github.com/flutter/flutter/wiki/Chat
 [build.yaml badge]: https://github.com/flutter/devtools/actions/workflows/build.yaml/badge.svg

packages/devtools_app/lib/src/app.dart

@@ -229,17 +229,17 @@ class DevToolsAppState extends State<DevToolsApp> with AutoDisposeMixin {
         return ValueListenableBuilder<bool>(
           valueListenable: preferences.vmDeveloperModeEnabled,
           builder: (_, __, child) {
-            final tabs = _visibleScreens()
+            final screens = _visibleScreens()
                 .where((p) => embed && page != null ? p.screenId == page : true)
                 .where((p) => !hide.contains(p.screenId))
                 .toList();
-            if (tabs.isEmpty) return child ?? const SizedBox.shrink();
+            if (screens.isEmpty) return child ?? const SizedBox.shrink();
             return _providedControllers(
               child: DevToolsScaffold(
                 embed: embed,
                 ideTheme: ideTheme,
                 page: page,
-                tabs: tabs,
+                screens: screens,
                 actions: [
                   // TODO(https://github.com/flutter/devtools/issues/1941)
                   if (serviceManager.connectedApp!.isFlutterAppNow!) ...[

packages/devtools_app/lib/src/framework/scaffold.dart

@@ -37,7 +37,7 @@ import 'status_line.dart';
 class DevToolsScaffold extends StatefulWidget {
   const DevToolsScaffold({
     Key? key,
-    required this.tabs,
+    required this.screens,
     this.page,
     this.actions,
     this.embed = false,
@@ -51,7 +51,7 @@ class DevToolsScaffold extends StatefulWidget {
     List<Widget>? actions,
   }) : this(
           key: key,
-          tabs: [SimpleScreen(child)],
+          screens: [SimpleScreen(child)],
           ideTheme: ideTheme,
           actions: actions,
         );
@@ -77,7 +77,7 @@ class DevToolsScaffold extends StatefulWidget {
       EdgeInsets.symmetric(horizontal: isEmbedded() ? 2.0 : 16.0);
 
   /// All of the [Screen]s that it's possible to navigate to from this Scaffold.
-  final List<Screen> tabs;
+  final List<Screen> screens;
 
   /// The page being rendered.
   final String? page;
@@ -134,22 +134,23 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
   void didUpdateWidget(DevToolsScaffold oldWidget) {
     super.didUpdateWidget(oldWidget);
 
-    if (widget.tabs.length != oldWidget.tabs.length) {
+    if (widget.screens.length != oldWidget.screens.length) {
       var newIndex = 0;
       // Stay on the current tab if possible when the collection of tabs changes.
       if (_tabController != null &&
-          widget.tabs.contains(oldWidget.tabs[_tabController!.index])) {
-        newIndex = widget.tabs.indexOf(oldWidget.tabs[_tabController!.index]);
+          widget.screens.contains(oldWidget.screens[_tabController!.index])) {
+        newIndex =
+            widget.screens.indexOf(oldWidget.screens[_tabController!.index]);
       }
       // Create a new tab controller to reflect the changed tabs.
       _setupTabController();
       _tabController!.index = newIndex;
-    } else if (widget.tabs[_tabController!.index].screenId != widget.page) {
+    } else if (widget.screens[_tabController!.index].screenId != widget.page) {
       // If the page changed (eg. the route was modified by pressing back in the
       // browser), animate to the new one.
       final newIndex = widget.page == null
           ? 0 // When there's no supplied page, we show the first one.
-          : widget.tabs.indexWhere((t) => t.screenId == widget.page);
+          : widget.screens.indexWhere((t) => t.screenId == widget.page);
       _tabController!.animateTo(newIndex);
     }
   }
@@ -183,19 +184,19 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
 
   void _setupTabController() {
     _tabController?.dispose();
-    _tabController = TabController(length: widget.tabs.length, vsync: this);
+    _tabController = TabController(length: widget.screens.length, vsync: this);
 
     if (widget.page != null) {
       final initialIndex =
-          widget.tabs.indexWhere((screen) => screen.screenId == widget.page);
+          widget.screens.indexWhere((screen) => screen.screenId == widget.page);
       if (initialIndex != -1) {
         _tabController!.index = initialIndex;
       }
     }
 
-    _currentScreen = widget.tabs[_tabController!.index];
+    _currentScreen = widget.screens[_tabController!.index];
     _tabController!.addListener(() {
-      final screen = widget.tabs[_tabController!.index];
+      final screen = widget.screens[_tabController!.index];
 
       if (_currentScreen != screen) {
         setState(() {
@@ -246,7 +247,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
     final existingTabIndex = _tabController!.index;
 
     final newIndex =
-        widget.tabs.indexWhere((screen) => screen.screenId == pageId);
+        widget.screens.indexWhere((screen) => screen.screenId == pageId);
 
     if (newIndex != -1 && newIndex != existingTabIndex) {
       DevToolsRouterDelegate.of(context).navigateIfNotCurrent(pageId);
@@ -278,7 +279,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
   Widget build(BuildContext context) {
     // Build the screens for each tab and wrap them in the appropriate styling.
     final tabBodies = [
-      for (var screen in widget.tabs)
+      for (var screen in widget.screens)
         Container(
           // TODO(kenz): this padding creates a flash when dragging and dropping
           // into the app size screen because it creates space that is outside
@@ -380,17 +381,17 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
 
     // Add a leading [BulletSpacer] to the actions if the screen is not narrow.
     final actions = List<Widget>.from(widget.actions ?? []);
-    if (!isNarrow && actions.isNotEmpty && widget.tabs.length > 1) {
+    if (!isNarrow && actions.isNotEmpty && widget.screens.length > 1) {
       actions.insert(0, const BulletSpacer(useAccentColor: true));
     }
 
-    final bool hasMultipleTabs = widget.tabs.length > 1;
+    final bool hasMultipleTabs = widget.screens.length > 1;
 
     if (hasMultipleTabs) {
       tabBar = TabBar(
         controller: _tabController,
         isScrollable: true,
-        tabs: [for (var screen in widget.tabs) screen.buildTab(context)],
+        tabs: [for (var screen in widget.screens) screen.buildTab(context)],
       );
       preferredSize = isNarrow
           ? Size.fromHeight(
@@ -461,7 +462,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
     // Approximate size of the title. Add [defaultSpacing] to account for
     // title's leading padding.
     double wideWidth = painter.width + defaultSpacing;
-    for (var tab in widget.tabs) {
+    for (var tab in widget.screens) {
       wideWidth += tab.approximateWidth(textTheme);
     }
     final actionsLength = widget.actions?.length ?? 0;

packages/devtools_app/lib/src/screens/memory/panes/diff/controller/heap_diff.dart

@@ -166,13 +166,17 @@ class ObjectSetDiff {
       final object = before.objectsByCodes[code] ?? after.objectsByCodes[code]!;
 
       if (inBefore) {
-        deleted.countInstance(object);
-        delta.uncountInstance(object);
+        final excludeFromRetained =
+            before.notCountedInRetained.contains(object.code);
+        deleted.countInstance(object, excludeFromRetained: excludeFromRetained);
+        delta.uncountInstance(object, excludeFromRetained: excludeFromRetained);
         continue;
       }
       if (inAfter) {
-        created.countInstance(object);
-        delta.countInstance(object);
+        final excludeFromRetained =
+            after.notCountedInRetained.contains(object.code);
+        created.countInstance(object, excludeFromRetained: excludeFromRetained);
+        delta.countInstance(object, excludeFromRetained: excludeFromRetained);
         continue;
       }
       assert(false);

packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_filter_dialog.dart

@@ -135,5 +135,5 @@ const _helpText = 'Choose and customize the filter.\n'
     '  MyClass\n'
     '  package:myPackage/src/\n\n'
     'Specify:\n'
-    '  - ${ClassFilter.noPackageLibrariesAlias} for classes without package prefix\n'
+    '  - ${ClassFilter.dartInternalAlias} for dart internal objects, not assigned to any package\n'
     '  - ${ClassFilter.dartAndFlutterLibrariesAlias} for most "dart:" and "package:" libraries published by Dart and Flutter orgs.';

packages/devtools_app/lib/src/screens/memory/shared/heap/class_filter.dart

@@ -36,15 +36,15 @@ class ClassFilter {
   ClassFilter.empty()
       : this(
           filterType: ClassFilterType.showAll,
-          except: '$noPackageLibrariesAlias\n$dartAndFlutterLibrariesAlias',
+          except: '$dartInternalAlias\n$dartAndFlutterLibrariesAlias',
           only: null,
         );
 
   static String _trimByLine(String value) =>
       value.split('\n').map((e) => e.trim()).join('\n');
 
   static const String dartAndFlutterLibrariesAlias = '\$dart-flutter-libraries';
-  static const String noPackageLibrariesAlias = '\$no-package-libraries';
+  static const String dartInternalAlias = '\$dart-internal-objects';
 
   final ClassFilterType filterType;
   final String except;
@@ -140,8 +140,7 @@ class ClassFilter {
     if (className.fullName.contains(filter)) return true;
     if (filter == dartAndFlutterLibrariesAlias && className.isDartOrFlutter)
       return true;
-    if (filter == noPackageLibrariesAlias && className.isPackageless)
-      return true;
+    if (filter == dartInternalAlias && className.isPackageless) return true;
     return false;
   }
 }

packages/devtools_app/lib/src/screens/memory/shared/heap/heap.dart

@@ -133,15 +133,19 @@ class SingleClassStats extends ClassStats {
     assert(!isSealed);
     final object = data.objects[objectIndex];
     assert(object.heapClass.fullName == heapClass.fullName);
-    objects.countInstance(object);
 
     final path = data.retainingPath(objectIndex);
+    objects.countInstance(
+      object,
+      excludeFromRetained: path?.isRetainedBySameClass ?? false,
+    );
+
     if (path == null) return;
     final objectsForPath = statsByPath.putIfAbsent(
       ClassOnlyHeapPath(path),
       () => ObjectSet(),
     );
-    objectsForPath.countInstance(object);
+    objectsForPath.countInstance(object, excludeFromRetained: false);
   }
 
   bool get isZero => objects.isZero;
@@ -174,16 +178,22 @@ class ObjectSetStats with Sealable {
   bool get isZero =>
       shallowSize == 0 && retainedSize == 0 && instanceCount == 0;
 
-  void countInstance(AdaptedHeapObject object) {
+  void countInstance(
+    AdaptedHeapObject object, {
+    required bool excludeFromRetained,
+  }) {
     assert(!isSealed);
-    retainedSize += object.retainedSize!;
+    if (!excludeFromRetained) retainedSize += object.retainedSize!;
     shallowSize += object.shallowSize;
     instanceCount++;
   }
 
-  void uncountInstance(AdaptedHeapObject object) {
+  void uncountInstance(
+    AdaptedHeapObject object, {
+    required bool excludeFromRetained,
+  }) {
     assert(!isSealed);
-    retainedSize -= object.retainedSize!;
+    if (!excludeFromRetained) retainedSize -= object.retainedSize!;
     shallowSize -= object.shallowSize;
     instanceCount--;
   }
@@ -194,19 +204,27 @@ class ObjectSet extends ObjectSetStats {
   static ObjectSet empty = ObjectSet()..seal();
 
   final objectsByCodes = <IdentityHashCode, AdaptedHeapObject>{};
+  final notCountedInRetained = <IdentityHashCode>{};
 
   @override
   bool get isZero => objectsByCodes.isEmpty;
 
   @override
-  void countInstance(AdaptedHeapObject object) {
+  void countInstance(
+    AdaptedHeapObject object, {
+    required bool excludeFromRetained,
+  }) {
     if (objectsByCodes.containsKey(object.code)) return;
-    super.countInstance(object);
+    super.countInstance(object, excludeFromRetained: excludeFromRetained);
     objectsByCodes[object.code] = object;
+    if (excludeFromRetained) notCountedInRetained.add(object.code);
   }
 
   @override
-  void uncountInstance(AdaptedHeapObject object) {
+  void uncountInstance(
+    AdaptedHeapObject object, {
+    required bool excludeFromRetained,
+  }) {
     throw AssertionError('uncountInstance is not valid for $ObjectSet');
   }
 }

packages/devtools_app/lib/src/screens/memory/shared/heap/model.dart

@@ -108,21 +108,26 @@ class AdaptedHeapData {
 typedef IdentityHashCode = int;
 
 /// Sequence of ids of objects in the heap.
-///
-/// TODO(polina-c): maybe we do not need to store path by objects.
-/// It can be that only classes are interesting, and we can save some
-/// performance on this object. It will become clear when the leak tracking
-/// feature stabilizes.
 class HeapPath {
   HeapPath(this.objects);
 
   final List<AdaptedHeapObject> objects;
 
+  late final bool isRetainedBySameClass = () {
+    if (objects.length < 2) return false;
+
+    final theClass = objects.last.heapClass;
+
+    return objects
+        .take(objects.length - 1)
+        .any((object) => object.heapClass == theClass);
+  }();
+
   /// Retaining path for the object in string format.
-  String? shortPath() => '/${objects.map((o) => o.shortName).join('/')}/';
+  String shortPath() => '/${objects.map((o) => o.shortName).join('/')}/';
 
   /// Retaining path for the object as an array of the retaining objects.
-  List<String>? detailedPath() =>
+  List<String> detailedPath() =>
       objects.map((o) => o.name).toList(growable: false);
 }