Dispose all disposables in the Memory screen (#8953)

Author: kenzieschmoll

packages/devtools_app/lib/src/screens/memory/framework/memory_controller.dart

@@ -23,25 +23,22 @@ import '../panes/profile/profile_pane_controller.dart';
 import '../panes/tracing/tracing_pane_controller.dart';
 import 'offline_data/offline_data.dart';
 
-/// This class contains the business logic for memory screen.
+/// Screen controller for the Memory screen.
+///
+/// This controller can be accessed from anywhere in DevTools, as long as it was
+/// first registered, by calling `screenControllers.lookup<MemoryController>()`.
+///
+/// The controller lifecycle is managed by the [ScreenControllers] class. The
+/// `init` method is called lazily upon the first controller access from
+/// `screenControllers`. The `dispose` method is called by `screenControllers`
+/// when DevTools is destroying a set of DevTools screen controllers.
 ///
 /// This class must not have direct dependencies on web-only libraries. This
 /// allows tests of the complicated logic in this class to run on the VM.
-///
-/// The controller should be recreated for every new connection.
 class MemoryController extends DevToolsScreenController
     with
         AutoDisposeControllerMixin,
         OfflineScreenControllerMixin<OfflineMemoryData> {
-  MemoryController({
-    @visibleForTesting DiffPaneController? connectedDiff,
-    @visibleForTesting ProfilePaneController? connectedProfile,
-  }) {
-    unawaited(
-      _init(connectedDiff: connectedDiff, connectedProfile: connectedProfile),
-    );
-  }
-
   Future<void> get initialized => _initialized.future;
   final _initialized = Completer<void>();
 
@@ -61,14 +58,26 @@ class MemoryController extends DevToolsScreenController
 
   late final TracePaneController? trace;
 
+  @override
+  void init({
+    @visibleForTesting DiffPaneController? connectedDiff,
+    @visibleForTesting ProfilePaneController? connectedProfile,
+  }) {
+    super.init();
+    unawaited(
+      _init(connectedDiff: connectedDiff, connectedProfile: connectedProfile),
+    );
+  }
+
   @override
   void dispose() {
-    super.dispose();
     HeapClassName.dispose();
-    chart.dispose();
-    trace?.dispose();
     diff.dispose();
     profile?.dispose();
+    chart.dispose();
+    trace?.dispose();
+    _gcing.dispose();
+    super.dispose();
   }
 
   static const _dataKey = 'data';
@@ -77,7 +86,7 @@ class MemoryController extends DevToolsScreenController
     @visibleForTesting DiffPaneController? connectedDiff,
     @visibleForTesting ProfilePaneController? connectedProfile,
   }) async {
-    assert(!_initialized.isCompleted);
+    if (_initialized.isCompleted) return;
     if (offlineDataController.showingOfflineData.value) {
       assert(connectedDiff == null && connectedProfile == null);
       await maybeLoadOfflineData(

packages/devtools_app/lib/src/screens/memory/panes/chart/controller/chart_connection.dart

@@ -112,6 +112,7 @@ class ChartVmConnection extends DisposableController
     _polling?.cancel();
     _polling?.dispose();
     _polling = null;
+    _memoryTracker.dispose();
     super.dispose();
   }
 }

packages/devtools_app/lib/src/screens/memory/panes/chart/controller/chart_data.dart

@@ -72,5 +72,6 @@ class ChartData with Serializable {
   void dispose() {
     _displayInterval.dispose();
     _isLegendVisible.dispose();
+    timeline.dispose();
   }
 }

packages/devtools_app/lib/src/screens/memory/panes/chart/controller/chart_pane_controller.dart

@@ -15,6 +15,24 @@ import 'charts/vm_chart_controller.dart';
 class MemoryChartPaneController extends DisposableController
     with AutoDisposeControllerMixin {
   MemoryChartPaneController({required this.data}) {
+    init();
+  }
+
+  late final ChartData data;
+
+  ChartVmConnection? _chartConnection;
+
+  late final event = EventChartController(data.timeline, paused: paused);
+  late final vm = VMChartController(data.timeline, paused: paused);
+  late final android = AndroidChartController(
+    data.timeline,
+    sharedLabels: vm.labelTimestamps,
+    paused: paused,
+  );
+
+  @override
+  void init() {
+    super.init();
     if (offlineDataController.showingOfflineData.value) {
       // Setting paused to false, because `recomputeChartData` is noop when it is true.
       _paused.value = false;
@@ -32,18 +50,6 @@ class MemoryChartPaneController extends DisposableController
     );
   }
 
-  late final ChartData data;
-
-  ChartVmConnection? _chartConnection;
-
-  late final event = EventChartController(data.timeline, paused: paused);
-  late final vm = VMChartController(data.timeline, paused: paused);
-  late final android = AndroidChartController(
-    data.timeline,
-    sharedLabels: vm.labelTimestamps,
-    paused: paused,
-  );
-
   void resetAll() {
     event.reset();
     vm.reset();
@@ -101,12 +107,13 @@ class MemoryChartPaneController extends DisposableController
 
   @override
   void dispose() {
-    super.dispose();
     data.dispose();
     event.dispose();
     vm.dispose();
     android.dispose();
+    _paused.dispose();
     isAndroidChartVisible.dispose();
     _chartConnection?.dispose();
+    super.dispose();
   }
 }

packages/devtools_app/lib/src/screens/memory/panes/chart/controller/memory_tracker.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:math' as math;
 
 import 'package:devtools_app_shared/service.dart' show FlutterEvent;
+import 'package:devtools_app_shared/utils.dart';
 import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/foundation.dart';
 import 'package:logging/logging.dart';
@@ -20,7 +21,7 @@ final _log = Logger('memory_protocol');
 
 enum _ContinuesState { none, stop, next }
 
-class MemoryTracker {
+class MemoryTracker extends Disposable {
   MemoryTracker(this.timeline, {required this.isAndroidChartVisible});
 
   final MemoryTimeline timeline;
@@ -291,4 +292,11 @@ class MemoryTracker {
 
     return null;
   }
+
+  @override
+  void dispose() {
+    _monitorContinues?.cancel();
+    _monitorContinues = null;
+    super.dispose();
+  }
 }

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

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
 
+import 'package:devtools_app_shared/utils.dart';
 import 'package:flutter/foundation.dart';
 
 import '../../../../../shared/memory/classes.dart';
@@ -11,12 +12,19 @@ import '../../../shared/heap/class_filter.dart';
 import '../../../shared/primitives/simple_elements.dart';
 import '../data/classes_diff.dart';
 
-class RetainingPathController {
+class RetainingPathController extends Disposable {
   final hideStandard = ValueNotifier<bool>(true);
   final invert = ValueNotifier<bool>(true);
+
+  @override
+  void dispose() {
+    hideStandard.dispose();
+    invert.dispose();
+    super.dispose();
+  }
 }
 
-class ClassesTableSingleData {
+class ClassesTableSingleData extends Disposable {
   ClassesTableSingleData({
     required this.heap,
     required this.totalHeapSize,
@@ -37,9 +45,15 @@ class ClassesTableSingleData {
 
   /// Selected class.
   final selection = ValueNotifier<SingleClassData?>(null);
+
+  @override
+  void dispose() {
+    selection.dispose();
+    super.dispose();
+  }
 }
 
-class ClassesTableDiffData {
+class ClassesTableDiffData extends Disposable {
   ClassesTableDiffData({
     required this.heapBefore,
     required this.heapAfter,
@@ -63,6 +77,13 @@ class ClassesTableDiffData {
 
   /// Selected class.
   final selection = ValueNotifier<DiffClassData?>(null);
+
+  @override
+  void dispose() {
+    selectedSizeType.dispose();
+    selection.dispose();
+    super.dispose();
+  }
 }
 
 /// Data for visualization of a path.

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

@@ -220,13 +220,21 @@ class DiffPaneController extends DisposableController with Serializable {
       ),
     );
   }
+
+  @override
+  void dispose() {
+    retainingPathController.dispose();
+    core.dispose();
+    derived.dispose();
+    super.dispose();
+  }
 }
 
 /// Values that define what data to show on diff screen.
 ///
 /// Widgets should not update the fields directly, they should use
 /// [DiffPaneController] or [DerivedData] for this.
-class CoreData {
+class CoreData extends Disposable {
   CoreData(this.rootPackage);
 
   final String? rootPackage;
@@ -263,44 +271,21 @@ class CoreData {
   /// This filter is applied to all snapshots.
   ValueListenable<ClassFilter> get classFilter => _classFilter;
   final _classFilter = ValueNotifier(ClassFilter.theDefault());
+
+  @override
+  void dispose() {
+    _snapshots.dispose();
+    _selectedSnapshotIndex.dispose();
+    _classFilter.dispose();
+    super.dispose();
+  }
 }
 
 /// Values that can be calculated from [CoreData] and notifiers that take signal
 /// from widgets.
 class DerivedData extends DisposableController with AutoDisposeControllerMixin {
   DerivedData(this._core) {
-    _selectedItem = ValueNotifier<SnapshotItem>(_core.selectedItem);
-
-    final classFilterData = ClassFilterData(
-      filter: _core.classFilter,
-      onChanged: applyFilter,
-      rootPackage: _core.rootPackage,
-    );
-
-    classesTableSingle = ClassesTableSingleData(
-      heap: () => (_core.selectedItem as SnapshotDataItem).heap!,
-      filterData: classFilterData,
-      totalHeapSize: () => (_core.selectedItem as SnapshotDataItem).totalSize!,
-    );
-
-    classesTableDiff = ClassesTableDiffData(
-      heapBefore: () => _currentDiff()!.before,
-      heapAfter: () => _currentDiff()!.after,
-      filterData: classFilterData,
-    );
-
-    addAutoDisposeListener(
-      classesTableSingle.selection,
-      () => _setClassIfNotNull(classesTableSingle.selection.value?.className),
-    );
-    addAutoDisposeListener(
-      classesTableDiff.selection,
-      () => _setClassIfNotNull(classesTableDiff.selection.value?.className),
-    );
-    addAutoDisposeListener(
-      selectedPath,
-      () => _setPathIfNotNull(selectedPath.value?.path),
-    );
+    init();
   }
 
   final CoreData _core;
@@ -337,6 +322,56 @@ class DerivedData extends DisposableController with AutoDisposeControllerMixin {
   /// Storage for already calculated diffs between snapshots.
   final _diffStore = HeapDiffStore();
 
+  @override
+  void init() {
+    super.init();
+    _selectedItem = ValueNotifier<SnapshotItem>(_core.selectedItem);
+
+    final classFilterData = ClassFilterData(
+      filter: _core.classFilter,
+      onChanged: applyFilter,
+      rootPackage: _core.rootPackage,
+    );
+
+    classesTableSingle = ClassesTableSingleData(
+      heap: () => (_core.selectedItem as SnapshotDataItem).heap!,
+      filterData: classFilterData,
+      totalHeapSize: () => (_core.selectedItem as SnapshotDataItem).totalSize!,
+    );
+
+    classesTableDiff = ClassesTableDiffData(
+      heapBefore: () => _currentDiff()!.before,
+      heapAfter: () => _currentDiff()!.after,
+      filterData: classFilterData,
+    );
+
+    addAutoDisposeListener(
+      classesTableSingle.selection,
+      () => _setClassIfNotNull(classesTableSingle.selection.value?.className),
+    );
+    addAutoDisposeListener(
+      classesTableDiff.selection,
+      () => _setClassIfNotNull(classesTableDiff.selection.value?.className),
+    );
+    addAutoDisposeListener(
+      selectedPath,
+      () => _setPathIfNotNull(selectedPath.value?.path),
+    );
+  }
+
+  @override
+  void dispose() {
+    _selectedItem.dispose();
+    classesBeforeFiltering.dispose();
+    _singleClassesToShow.dispose();
+    _diffClassesToShow.dispose();
+    classData.dispose();
+    selectedPath.dispose();
+    classesTableSingle.dispose();
+    classesTableDiff.dispose();
+    super.dispose();
+  }
+
   void applyFilter(ClassFilter filter) {
     if (filter == _core.classFilter.value) return;
     _core._classFilter.value = filter;

packages/devtools_app/lib/src/screens/memory/panes/profile/profile_pane_controller.dart

@@ -48,7 +48,9 @@ class ProfilePaneController extends DisposableController
   bool _initialized = false;
 
   /// Initializes the controller if it is not initialized yet.
-  void initialize() {
+  @override
+  void init() {
+    super.init();
     if (_initialized) return;
 
     if (!offlineDataController.showingOfflineData.value) {
@@ -202,4 +204,13 @@ class ProfilePaneController extends DisposableController
       type: ExportFileType.csv,
     );
   }
+
+  @override
+  void dispose() {
+    _currentAllocationProfile.dispose();
+    _classFilter.dispose();
+    _refreshOnGc.dispose();
+    selection.dispose();
+    super.dispose();
+  }
 }