Skip to content

Commit 568ccc0

Browse files
kenzieschmollkenzieschmoll
authored
Dispose all disposables in the Inspector screen (#8970)
1 parent 547bfe7 commit 568ccc0

8 files changed

Lines changed: 107 additions & 68 deletions

File tree

‎packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,6 @@ class InspectorController extends DisposableController
194194
InspectorTreeController inspectorTree;
195195
final FlutterTreeType treeType;
196196

197-
bool _disposed = false;
198-
199197
late RateLimiter _refreshRateLimiter;
200198

201199
InspectorServiceBase get inspectorService =>
@@ -246,9 +244,8 @@ class InspectorController extends DisposableController
246244
RemoteDiagnosticsNode? get selectedDiagnostic =>
247245
selectedNode.value?.diagnostic;
248246

249-
final _selectedErrorIndex = ValueNotifier<int?>(null);
250-
251247
ValueListenable<int?> get selectedErrorIndex => _selectedErrorIndex;
248+
final _selectedErrorIndex = ValueNotifier<int?>(null);
252249

253250
/// Tracks whether the first load of the inspector tree has been completed.
254251
///
@@ -365,12 +362,12 @@ class InspectorController extends DisposableController
365362

366363
@override
367364
Future<void> onForceRefresh() async {
368-
assert(!_disposed);
369-
if (!visibleToUser || _disposed) {
365+
assert(!disposed);
366+
if (!visibleToUser || disposed) {
370367
return;
371368
}
372369
await _recomputeTreeRoot(null, null, false);
373-
if (_disposed) {
370+
if (disposed) {
374371
return;
375372
}
376373

@@ -414,7 +411,7 @@ class InspectorController extends DisposableController
414411
}
415412

416413
if (flutterAppFrameReady) {
417-
if (_disposed) return;
414+
if (disposed) return;
418415
// We need to start by querying the inspector service to find out the
419416
// current state of the UI.
420417
final inspectorRef = DevToolsQueryParams.load().inspectorRef;
@@ -423,7 +420,7 @@ class InspectorController extends DisposableController
423420
inspectorRef: inspectorRef,
424421
);
425422
} else {
426-
if (_disposed) return;
423+
if (disposed) return;
427424
if (inspectorService is InspectorService) {
428425
final widgetTreeReady =
429426
await (inspectorService as InspectorService).isWidgetTreeReady();
@@ -441,9 +438,9 @@ class InspectorController extends DisposableController
441438
bool setSubtreeRoot, {
442439
int subtreeDepth = 2,
443440
}) async {
444-
assert(!_disposed);
441+
assert(!disposed);
445442
final treeGroups = _treeGroups;
446-
if (_disposed || treeGroups == null) {
443+
if (disposed || treeGroups == null) {
447444
return;
448445
}
449446

@@ -454,7 +451,7 @@ class InspectorController extends DisposableController
454451
await (detailsSubtree
455452
? group.getDetailsSubtree(subtreeRoot, subtreeDepth: subtreeDepth)
456453
: group.getRoot(treeType, isSummaryTree: true));
457-
if (node == null || group.disposed || _disposed) {
454+
if (node == null || group.disposed || disposed) {
458455
return;
459456
}
460457
// TODO(jacobr): as a performance optimization we should check if the
@@ -636,7 +633,7 @@ class InspectorController extends DisposableController
636633
InspectorInstanceRef(inspectorRef),
637634
false,
638635
);
639-
if (_disposed) return;
636+
if (disposed) return;
640637
}
641638
final pendingSelectionFuture = group.getSelection(
642639
selectedDiagnostic,
@@ -649,12 +646,12 @@ class InspectorController extends DisposableController
649646

650647
try {
651648
final newSelection = await pendingSelectionFuture;
652-
if (_disposed || group.disposed) return;
649+
if (disposed || group.disposed) return;
653650
RemoteDiagnosticsNode? detailsSelection;
654651

655652
if (pendingDetailsFuture != null) {
656653
detailsSelection = await pendingDetailsFuture;
657-
if (_disposed || group.disposed) return;
654+
if (disposed || group.disposed) return;
658655
}
659656

660657
if (!firstFrame &&
@@ -794,7 +791,7 @@ class InspectorController extends DisposableController
794791
final isolateRef = inspectorService.isolateRef;
795792
final instanceRef = await node.diagnostic!.objectGroupApi
796793
?.toObservatoryInstanceRef(valueRef);
797-
if (_disposed) return;
794+
if (disposed) return;
798795

799796
if (instanceRef != null) {
800797
await serviceConnection.consoleService.appendInstanceRef(
@@ -908,8 +905,7 @@ class InspectorController extends DisposableController
908905

909906
@override
910907
void dispose() {
911-
assert(!_disposed);
912-
_disposed = true;
908+
assert(!disposed);
913909
if (serviceConnection.inspectorService != null) {
914910
shutdownTree(false);
915911
}
@@ -918,6 +914,10 @@ class InspectorController extends DisposableController
918914
_selectionGroups?.clear(false);
919915
_selectionGroups = null;
920916
details?.dispose();
917+
918+
_refreshRateLimiter.dispose();
919+
_selectedNode.dispose();
920+
_selectedErrorIndex.dispose();
921921
super.dispose();
922922
}
923923

‎packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,7 @@ class _InspectorTreeRowState extends State<_InspectorTreeRowWidget>
105105
class InspectorTreeController extends DisposableController
106106
with SearchControllerMixin<InspectorTreeRow> {
107107
InspectorTreeController({this.gaId}) {
108-
ga.select(
109-
gac.inspector,
110-
gac.inspectorTreeControllerInitialized,
111-
nonInteraction: true,
112-
screenMetricsProvider:
113-
() => InspectorScreenMetrics.legacy(
114-
inspectorTreeControllerId: gaId,
115-
rootSetCount: _rootSetCount,
116-
rowCount: _root?.subtreeSize,
117-
),
118-
);
108+
init();
119109
}
120110

121111
/// Clients the controller notifies to trigger changes to the UI.
@@ -130,6 +120,22 @@ class InspectorTreeController extends DisposableController
130120
SearchTargetType _searchTarget = SearchTargetType.widget;
131121
int _rootSetCount = 0;
132122

123+
@override
124+
void init() {
125+
super.init();
126+
ga.select(
127+
gac.inspector,
128+
gac.inspectorTreeControllerInitialized,
129+
nonInteraction: true,
130+
screenMetricsProvider:
131+
() => InspectorScreenMetrics.legacy(
132+
inspectorTreeControllerId: gaId,
133+
rootSetCount: _rootSetCount,
134+
rowCount: _root?.subtreeSize,
135+
),
136+
);
137+
}
138+
133139
void addClient(InspectorControllerClient value) {
134140
final firstClient = _clients.isEmpty;
135141
_clients.add(value);
@@ -163,6 +169,8 @@ class InspectorTreeController extends DisposableController
163169
InspectorTreeNode? _root;
164170

165171
set root(InspectorTreeNode? node) {
172+
if (disposed) return;
173+
166174
setState(() {
167175
_root = node;
168176
_populateSearchableCachedRows();
@@ -826,11 +834,11 @@ class _InspectorTreeState extends State<InspectorTree>
826834

827835
@override
828836
void dispose() {
829-
super.dispose();
830837
treeController?.removeClient(this);
831838
_scrollControllerX.dispose();
832839
_scrollControllerY.dispose();
833840
_constraintDisplayController?.dispose();
841+
super.dispose();
834842
}
835843

836844
@override

‎packages/devtools_app/lib/src/screens/inspector_shared/inspector_screen_controller.dart‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,27 @@ import '../inspector/inspector_tree_controller.dart' as legacy;
1010
import '../inspector_v2/inspector_controller.dart' as v2;
1111
import '../inspector_v2/inspector_tree_controller.dart' as v2;
1212

13+
/// Screen controller for the Inspector screen.
14+
///
15+
/// This controller can be accessed from anywhere in DevTools, as long as it was
16+
/// first registered, by
17+
/// calling `screenControllers.lookup<InspectorScreenController>()`.
18+
///
19+
/// The controller lifecycle is managed by the [ScreenControllers] class. The
20+
/// `init` method is called lazily upon the first controller access from
21+
/// `screenControllers`. The `dispose` method is called by `screenControllers`
22+
/// when DevTools is destroying a set of DevTools screen controllers.
1323
class InspectorScreenController extends DevToolsScreenController {
14-
InspectorScreenController() {
15-
_init();
16-
}
17-
1824
late v2.InspectorController v2InspectorController;
1925
late v2.InspectorTreeController v2InspectorTreeController;
2026

2127
late legacy.InspectorController legacyInspectorController;
2228
late legacy.InspectorTreeController legacyInspectorTreeController;
2329
late legacy.InspectorTreeController legacyDetailsTreeController;
2430

25-
void _init() {
31+
@override
32+
void init() {
33+
super.init();
2634
v2InspectorTreeController = v2.InspectorTreeController(
2735
gaId: InspectorScreenMetrics.summaryTreeGaId,
2836
);

‎packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ class InspectorController extends DisposableController
6161
with AutoDisposeControllerMixin
6262
implements InspectorServiceClient {
6363
InspectorController({required this.inspectorTree, required this.treeType}) {
64-
unawaited(_init());
64+
unawaited(init());
6565
}
6666

67-
Future<void> _init() async {
67+
@override
68+
Future<void> init() async {
69+
super.init();
6870
_refreshRateLimiter = RateLimiter(refreshFramesPerSecond, refresh);
6971

7072
inspectorTree.config = InspectorTreeConfig(
@@ -200,8 +202,6 @@ class InspectorController extends DisposableController
200202
InspectorTreeController inspectorTree;
201203
final FlutterTreeType treeType;
202204

203-
bool _disposed = false;
204-
205205
late RateLimiter _refreshRateLimiter;
206206

207207
InspectorServiceBase get inspectorService =>
@@ -265,9 +265,8 @@ class InspectorController extends DisposableController
265265
RemoteDiagnosticsNode? get selectedDiagnostic =>
266266
selectedNode.value?.diagnostic;
267267

268-
final _selectedErrorIndex = ValueNotifier<int?>(null);
269-
270268
ValueListenable<int?> get selectedErrorIndex => _selectedErrorIndex;
269+
final _selectedErrorIndex = ValueNotifier<int?>(null);
271270

272271
/// Tracks whether the first load of the inspector tree has been completed.
273272
///
@@ -371,12 +370,12 @@ class InspectorController extends DisposableController
371370

372371
@override
373372
Future<void> onForceRefresh() async {
374-
assert(!_disposed);
375-
if (!visibleToUser || _disposed) {
373+
assert(!disposed);
374+
if (!visibleToUser || disposed) {
376375
return;
377376
}
378377
await _recomputeTreeRoot(null);
379-
if (_disposed) {
378+
if (disposed) {
380379
return;
381380
}
382381

@@ -432,13 +431,13 @@ class InspectorController extends DisposableController
432431
}
433432

434433
if (flutterAppFrameReady) {
435-
if (_disposed) return;
434+
if (disposed) return;
436435
// We need to start by querying the inspector service to find out the
437436
// current state of the UI.
438437
final inspectorRef = DevToolsQueryParams.load().inspectorRef;
439438
await updateSelectionFromService(inspectorRef: inspectorRef);
440439
} else {
441-
if (_disposed) return;
440+
if (disposed) return;
442441
if (inspectorService is InspectorService) {
443442
final widgetTreeReady =
444443
await (inspectorService as InspectorService).isWidgetTreeReady();
@@ -483,10 +482,10 @@ class InspectorController extends DisposableController
483482
RemoteDiagnosticsNode? newSelection, {
484483
bool? hideImplementationWidgets,
485484
}) async {
486-
assert(!_disposed);
485+
assert(!disposed);
487486
hideImplementationWidgets ??= _implementationWidgetsHidden.value;
488487
final treeGroups = _treeGroups;
489-
if (_disposed || treeGroups == null) {
488+
if (disposed || treeGroups == null) {
490489
return;
491490
}
492491

@@ -498,7 +497,7 @@ class InspectorController extends DisposableController
498497
isSummaryTree: hideImplementationWidgets,
499498
includeFullDetails: false,
500499
);
501-
if (node == null || group.disposed || _disposed) {
500+
if (node == null || group.disposed || disposed) {
502501
return;
503502
}
504503
// TODO(jacobr): as a performance optimization we should check if the
@@ -746,7 +745,7 @@ class InspectorController extends DisposableController
746745
InspectorInstanceRef(inspectorRef),
747746
false,
748747
);
749-
if (_disposed) return;
748+
if (disposed) return;
750749
}
751750
final pendingSelectionFuture = group.getSelection(
752751
selectedDiagnostic,
@@ -759,7 +758,7 @@ class InspectorController extends DisposableController
759758
try {
760759
final newSelection = await pendingSelectionFuture;
761760

762-
if (_disposed || group.disposed) return;
761+
if (disposed || group.disposed) return;
763762

764763
selectionGroups.promoteNext();
765764

@@ -1017,7 +1016,7 @@ class InspectorController extends DisposableController
10171016
final isolateRef = inspectorService.isolateRef;
10181017
final instanceRef = await node.diagnostic!.objectGroupApi
10191018
?.toObservatoryInstanceRef(valueRef);
1020-
if (_disposed) return;
1019+
if (disposed) return;
10211020

10221021
if (instanceRef != null) {
10231022
await serviceConnection.consoleService.appendInstanceRef(
@@ -1088,15 +1087,23 @@ class InspectorController extends DisposableController
10881087

10891088
@override
10901089
void dispose() {
1091-
assert(!_disposed);
1092-
_disposed = true;
1090+
assert(!disposed);
10931091
if (serviceConnection.inspectorService != null) {
10941092
shutdownTree(false);
10951093
}
1094+
10961095
_treeGroups?.clear(false);
10971096
_treeGroups = null;
10981097
_selectionGroups?.clear(false);
10991098
_selectionGroups = null;
1099+
_layoutGroups?.clear(false);
1100+
_layoutGroups = null;
1101+
1102+
_refreshRateLimiter.dispose();
1103+
_selectedNode.dispose();
1104+
_selectedNodeProperties.dispose();
1105+
_implementationWidgetsHidden.dispose();
1106+
_selectedErrorIndex.dispose();
11001107
super.dispose();
11011108
}
11021109

0 commit comments

Comments
 (0)