Skip to content

Commit ad6f03f

Browse files
committed
Move Inspector error tracking out of ErrorBadgeManager
1 parent 1681401 commit ad6f03f

10 files changed

Lines changed: 349 additions & 205 deletions

File tree

packages/devtools_app/lib/devtools_app.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export 'src/screens/deep_link_validation/deep_links_screen.dart';
2727
export 'src/screens/dtd/dtd_tools_controller.dart';
2828
export 'src/screens/dtd/dtd_tools_screen.dart';
2929
export 'src/screens/inspector/inspector_controller.dart';
30+
export 'src/screens/inspector/inspector_errors.dart';
3031
export 'src/screens/inspector/inspector_screen.dart';
3132
export 'src/screens/inspector/inspector_screen_body.dart';
3233
export 'src/screens/inspector/inspector_screen_controller.dart';

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import '../../shared/primitives/utils.dart';
3838
import '../../shared/utils/utils.dart';
3939
import 'inspector_data_models.dart';
4040
import 'inspector_screen.dart';
41+
import 'inspector_screen_controller.dart';
4142
import 'inspector_tree_controller.dart';
4243

4344
final _log = Logger('inspector_controller');
@@ -149,6 +150,9 @@ class InspectorController extends DisposableController
149150
}
150151
}
151152

153+
InspectorScreenController get _inspectorScreenController =>
154+
screenControllers.lookup<InspectorScreenController>();
155+
152156
void _handleConnectionStart() {
153157
// Clear any existing badge/errors for older errors that were collected.
154158
// Do this in a post frame callback so that we are not trying to clear the
@@ -157,7 +161,7 @@ class InspectorController extends DisposableController
157161
// TODO(kenz): When this method is called outside createState(), this post
158162
// frame callback can be removed.
159163
WidgetsBinding.instance.addPostFrameCallback((_) {
160-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
164+
_inspectorScreenController.clearErrors();
161165
});
162166
}
163167

@@ -467,7 +471,7 @@ class InspectorController extends DisposableController
467471
}
468472

469473
if (event.kind == EventKind.kIsolateReload) {
470-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
474+
_inspectorScreenController.clearErrors();
471475
_receivedIsolateReloadEvent = true;
472476
}
473477
}
@@ -952,9 +956,7 @@ class InspectorController extends DisposableController
952956
void _updateSelectedErrorFromNode(InspectorTreeNode? node) {
953957
final inspectorRef = node?.diagnostic?.valueRef.id;
954958

955-
final errors = serviceConnection.errorBadgeManager
956-
.erroredItemsForPage(InspectorScreen.id)
957-
.value;
959+
final errors = _inspectorScreenController.inspectorErrors.value;
958960

959961
// Check whether the node that was just selected has any errors associated
960962
// with it.
@@ -970,20 +972,15 @@ class InspectorController extends DisposableController
970972
if (errorIndex != null) {
971973
// Marking an error as read will automatically update the badge count to
972974
// reflect the remaining unread errors.
973-
serviceConnection.errorBadgeManager.markErrorAsRead(
974-
InspectorScreen.id,
975-
errors[inspectorRef!]!,
976-
);
975+
_inspectorScreenController.markErrorAsRead(errors[inspectorRef!]!);
977976
}
978977
}
979978

980979
/// Updates the index of the selected error and selects its node in the tree.
981980
void selectErrorByIndex(int index) {
982981
_selectedErrorIndex.value = index;
983982

984-
final errors = serviceConnection.errorBadgeManager
985-
.erroredItemsForPage(InspectorScreen.id)
986-
.value;
983+
final errors = _inspectorScreenController.inspectorErrors.value;
987984

988985
unawaited(
989986
updateSelectionFromService(inspectorRef: errors.keys.elementAt(index)),
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2024 The Flutter Authors
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
4+
5+
import '../../shared/managers/error_badge_manager.dart';
6+
7+
/// An error associated with a specific widget that can be inspected in the
8+
/// Inspector screen.
9+
class InspectableWidgetError extends DevToolsError {
10+
InspectableWidgetError(super.errorMessage, super.id, {super.read});
11+
12+
@override
13+
InspectableWidgetError asRead() =>
14+
InspectableWidgetError(errorMessage, id, read: true);
15+
}

packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import '../../shared/ui/search.dart';
2121
import '../../shared/utils/utils.dart';
2222
import 'inspector_controller.dart';
2323
import 'inspector_controls.dart';
24+
import 'inspector_errors.dart';
2425
import 'inspector_screen.dart';
26+
import 'inspector_screen_controller.dart';
2527
import 'inspector_tree_controller.dart';
2628
import 'widget_details.dart';
2729

@@ -149,8 +151,9 @@ class InspectorScreenBodyState extends State<InspectorScreenBody>
149151
),
150152
Expanded(
151153
child: ValueListenableBuilder(
152-
valueListenable: serviceConnection.errorBadgeManager
153-
.erroredItemsForPage(InspectorScreen.id),
154+
valueListenable: screenControllers
155+
.lookup<InspectorScreenController>()
156+
.inspectorErrors,
154157
builder: (_, LinkedHashMap<String, DevToolsError> errors, _) {
155158
final inspectableErrors =
156159
errors.map(

packages/devtools_app/lib/src/screens/inspector/inspector_screen_controller.dart

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,26 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
44

5+
import 'dart:collection';
6+
7+
import 'package:collection/collection.dart' show IterableExtension;
8+
import 'package:devtools_app_shared/service.dart';
9+
import 'package:devtools_app_shared/utils.dart';
10+
import 'package:flutter/foundation.dart';
11+
import 'package:vm_service/vm_service.dart';
12+
13+
import '../../service/vm_service_wrapper.dart';
514
import '../../shared/analytics/metrics.dart';
615
import '../../shared/console/primitives/simple_items.dart';
16+
import '../../shared/diagnostics/diagnostics_node.dart';
717
import '../../shared/framework/screen.dart';
818
import '../../shared/framework/screen_controllers.dart';
19+
import '../../shared/globals.dart';
20+
import '../../shared/managers/error_badge_manager.dart';
21+
import '../../shared/primitives/query_parameters.dart';
922
import 'inspector_controller.dart';
23+
import 'inspector_errors.dart';
24+
import 'inspector_screen.dart';
1025
import 'inspector_tree_controller.dart';
1126

1227
/// Screen controller for the Inspector screen.
@@ -19,27 +34,169 @@ import 'inspector_tree_controller.dart';
1934
/// `init` method is called lazily upon the first controller access from
2035
/// `screenControllers`. The `dispose` method is called by `screenControllers`
2136
/// when DevTools is destroying a set of DevTools screen controllers.
22-
class InspectorScreenController extends DevToolsScreenController {
37+
class InspectorScreenController extends DevToolsScreenController
38+
with AutoDisposeControllerMixin {
2339
@override
2440
final screenId = ScreenMetaData.inspector.id;
2541

2642
late InspectorController inspectorController;
2743
late InspectorTreeController inspectorTreeController;
2844

45+
/// Stores the inspector-specific errors keyed by inspector reference ID.
46+
final _activeInspectorErrors =
47+
ValueNotifier<LinkedHashMap<String, DevToolsError>>(
48+
LinkedHashMap<String, DevToolsError>(),
49+
);
50+
51+
/// The errors currently tracked for the inspector screen.
52+
ValueListenable<LinkedHashMap<String, DevToolsError>> get inspectorErrors =>
53+
_activeInspectorErrors;
54+
55+
/// The count of unread inspector errors (used for the badge).
56+
ValueListenable<int> get inspectorErrorCount => serviceConnection
57+
.errorBadgeManager
58+
.errorCountNotifier(InspectorScreen.id);
59+
2960
@override
3061
void init() {
3162
super.init();
63+
// Inspector owns unread state for its badge; scaffold tab switches must not
64+
// clear it. See https://github.com/flutter/devtools/pull/9805.
65+
serviceConnection.errorBadgeManager.manageErrorCount(InspectorScreen.id);
66+
3267
inspectorTreeController = InspectorTreeController(
3368
gaId: InspectorScreenMetrics.summaryTreeGaId,
3469
);
3570
inspectorController = InspectorController(
3671
inspectorTree: inspectorTreeController,
3772
treeType: FlutterTreeType.widget,
3873
);
74+
75+
// Listen for Flutter extension events to extract inspector-specific errors.
76+
// Match other screen controllers: attach now if connected, and on connect.
77+
addAutoDisposeListener(serviceConnection.serviceManager.connectedState, () {
78+
if (serviceConnection.serviceManager.connectedState.value.connected) {
79+
_handleConnectionStart(serviceConnection.serviceManager.service!);
80+
}
81+
});
82+
if (serviceConnection.serviceManager.connectedAppInitialized) {
83+
_handleConnectionStart(serviceConnection.serviceManager.service!);
84+
}
85+
}
86+
87+
void _handleConnectionStart(VmServiceWrapper service) {
88+
autoDisposeStreamSubscription(
89+
service.onExtensionEventWithHistorySafe.listen(_handleExtensionEvent),
90+
);
91+
}
92+
93+
void _handleExtensionEvent(Event e) {
94+
if (e.extensionKind == FlutterEvent.error) {
95+
final inspectableError = _extractInspectableError(e);
96+
if (inspectableError != null) {
97+
appendError(inspectableError);
98+
}
99+
}
100+
}
101+
102+
InspectableWidgetError? _extractInspectableError(Event error) {
103+
final extensionData = error.extensionData;
104+
if (extensionData == null) return null;
105+
106+
final node = RemoteDiagnosticsNode(
107+
extensionData.data,
108+
null,
109+
false,
110+
null,
111+
);
112+
113+
final errorSummaryNode = node.inlineProperties.firstWhereOrNull(
114+
(p) => p.type == 'ErrorSummary',
115+
);
116+
final errorMessage = errorSummaryNode?.description;
117+
if (errorMessage == null) {
118+
return null;
119+
}
120+
121+
final devToolsUrlNode = node.inlineProperties.firstWhereOrNull(
122+
(p) =>
123+
p.type == 'DevToolsDeepLinkProperty' &&
124+
p.getStringMember('value') != null,
125+
);
126+
if (devToolsUrlNode == null) {
127+
return null;
128+
}
129+
130+
final queryParams = DevToolsQueryParams.fromUrl(
131+
devToolsUrlNode.getStringMember('value')!,
132+
);
133+
final inspectorRef = queryParams.inspectorRef ?? '';
134+
135+
return InspectableWidgetError(errorMessage, inspectorRef);
136+
}
137+
138+
/// Appends an error to the inspector's active errors and updates the badge
139+
/// count.
140+
void appendError(DevToolsError error) {
141+
final errors = _activeInspectorErrors;
142+
final previousError = errors.value[error.id];
143+
144+
// Build a new map with the new error. Adding to the existing map
145+
// won't cause the ValueNotifier to fire (and it's not permitted to call
146+
// notifyListeners() directly).
147+
final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
148+
newValue[error.id] = error;
149+
errors.value = newValue;
150+
151+
if (previousError == null) {
152+
if (!error.read) {
153+
_incrementUnreadCount();
154+
}
155+
return;
156+
}
157+
158+
if (previousError.read && !error.read) {
159+
_incrementUnreadCount();
160+
} else if (!previousError.read && error.read) {
161+
_decrementUnreadCount();
162+
}
163+
}
164+
165+
/// Clears all inspector errors and resets the badge count.
166+
void clearErrors() {
167+
_activeInspectorErrors.value = LinkedHashMap<String, DevToolsError>();
168+
serviceConnection.errorBadgeManager.resetErrorCount(InspectorScreen.id);
169+
}
170+
171+
/// Marks an error as read and decrements the unread count.
172+
void markErrorAsRead(DevToolsError error) {
173+
final errors = _activeInspectorErrors;
174+
175+
// If this error doesn't exist anymore or is already read, nothing to do.
176+
final currentError = errors.value[error.id];
177+
if (currentError == null || currentError.read) {
178+
return;
179+
}
180+
181+
// Otherwise, replace the map with a new one that has the error marked
182+
// as read.
183+
final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
184+
newValue[error.id] = currentError.asRead();
185+
errors.value = newValue;
186+
_decrementUnreadCount();
187+
}
188+
189+
void _incrementUnreadCount() {
190+
serviceConnection.errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
191+
}
192+
193+
void _decrementUnreadCount() {
194+
serviceConnection.errorBadgeManager.decrementBadgeCount(InspectorScreen.id);
39195
}
40196

41197
@override
42198
void dispose() {
199+
_activeInspectorErrors.dispose();
43200
inspectorTreeController.dispose();
44201
inspectorController.dispose();
45202
super.dispose();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import '../../shared/ui/search.dart';
3030
import '../../shared/ui/utils.dart';
3131
import '../../shared/utils/utils.dart';
3232
import 'inspector_controller.dart';
33+
import 'inspector_errors.dart';
3334

3435
final _log = Logger('inspector_tree_controller');
3536

0 commit comments

Comments
 (0)