Skip to content

Commit 168bb17

Browse files
authored
Move Inspector error tracking out of ErrorBadgeManager (#9923)
1 parent 1d105ed commit 168bb17

11 files changed

Lines changed: 349 additions & 192 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: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,15 @@ import '../../shared/console/primitives/simple_items.dart';
3131
import '../../shared/diagnostics/diagnostics_node.dart';
3232
import '../../shared/diagnostics/inspector_service.dart';
3333
import '../../shared/diagnostics/primitives/instance_ref.dart';
34+
import '../../shared/framework/screen_controllers.dart';
3435
import '../../shared/globals.dart';
3536
import '../../shared/managers/notifications.dart';
3637
import '../../shared/primitives/query_parameters.dart';
3738
import '../../shared/primitives/utils.dart';
3839
import '../../shared/utils/utils.dart';
3940
import 'inspector_data_models.dart';
4041
import 'inspector_screen.dart';
42+
import 'inspector_screen_controller.dart';
4143
import 'inspector_tree_controller.dart';
4244

4345
final _log = Logger('inspector_controller');
@@ -149,6 +151,21 @@ class InspectorController extends DisposableController
149151
}
150152
}
151153

154+
/// Returns the [InspectorScreenController] when it is registered.
155+
///
156+
/// [InspectorController] is sometimes constructed in unit tests without an
157+
/// [InspectorScreenController] registered, so callers that only need to clear
158+
/// errors on connect/reload should use this nullable accessor.
159+
InspectorScreenController? get _inspectorScreenControllerOrNull {
160+
final controllers = globals[ScreenControllers] as ScreenControllers?;
161+
if (controllers == null) return null;
162+
if (!controllers.isRegistered<InspectorScreenController>()) return null;
163+
return controllers.lookup<InspectorScreenController>();
164+
}
165+
166+
InspectorScreenController get _inspectorScreenController =>
167+
screenControllers.lookup<InspectorScreenController>();
168+
152169
void _handleConnectionStart() {
153170
// Clear any existing badge/errors for older errors that were collected.
154171
// Do this in a post frame callback so that we are not trying to clear the
@@ -157,7 +174,7 @@ class InspectorController extends DisposableController
157174
// TODO(kenz): When this method is called outside createState(), this post
158175
// frame callback can be removed.
159176
WidgetsBinding.instance.addPostFrameCallback((_) {
160-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
177+
_inspectorScreenControllerOrNull?.clearErrors();
161178
});
162179
}
163180

@@ -437,7 +454,7 @@ class InspectorController extends DisposableController
437454
}
438455

439456
if (event.kind == EventKind.kIsolateReload) {
440-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
457+
_inspectorScreenControllerOrNull?.clearErrors();
441458
_receivedIsolateReloadEvent = true;
442459
}
443460
}
@@ -870,9 +887,7 @@ class InspectorController extends DisposableController
870887
void _updateSelectedErrorFromNode(InspectorTreeNode? node) {
871888
final inspectorRef = node?.diagnostic?.valueRef.id;
872889

873-
final errors = serviceConnection.errorBadgeManager
874-
.erroredItemsForPage(InspectorScreen.id)
875-
.value;
890+
final errors = _inspectorScreenController.inspectorErrors.value;
876891

877892
// Check whether the node that was just selected has any errors associated
878893
// with it.
@@ -888,20 +903,15 @@ class InspectorController extends DisposableController
888903
if (errorIndex != null) {
889904
// Marking an error as read will automatically update the badge count to
890905
// reflect the remaining unread errors.
891-
serviceConnection.errorBadgeManager.markErrorAsRead(
892-
InspectorScreen.id,
893-
errors[inspectorRef!]!,
894-
);
906+
_inspectorScreenController.markErrorAsRead(errors[inspectorRef!]!);
895907
}
896908
}
897909

898910
/// Updates the index of the selected error and selects its node in the tree.
899911
void selectErrorByIndex(int index) {
900912
_selectedErrorIndex.value = index;
901913

902-
final errors = serviceConnection.errorBadgeManager
903-
.erroredItemsForPage(InspectorScreen.id)
904-
.value;
914+
final errors = _inspectorScreenController.inspectorErrors.value;
905915

906916
unawaited(
907917
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: 153 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,164 @@ 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(extensionData.data, null, false, null);
107+
108+
final errorSummaryNode = node.inlineProperties.firstWhereOrNull(
109+
(p) => p.type == 'ErrorSummary',
110+
);
111+
final errorMessage = errorSummaryNode?.description;
112+
if (errorMessage == null) {
113+
return null;
114+
}
115+
116+
final devToolsUrlNode = node.inlineProperties.firstWhereOrNull(
117+
(p) =>
118+
p.type == 'DevToolsDeepLinkProperty' &&
119+
p.getStringMember('value') != null,
120+
);
121+
if (devToolsUrlNode == null) {
122+
return null;
123+
}
124+
125+
final queryParams = DevToolsQueryParams.fromUrl(
126+
devToolsUrlNode.getStringMember('value')!,
127+
);
128+
final inspectorRef = queryParams.inspectorRef ?? '';
129+
130+
return InspectableWidgetError(errorMessage, inspectorRef);
131+
}
132+
133+
/// Appends an error to the inspector's active errors and updates the badge
134+
/// count.
135+
void appendError(DevToolsError error) {
136+
final errors = _activeInspectorErrors;
137+
final previousError = errors.value[error.id];
138+
139+
// Build a new map with the new error. Adding to the existing map
140+
// won't cause the ValueNotifier to fire (and it's not permitted to call
141+
// notifyListeners() directly).
142+
final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
143+
newValue[error.id] = error;
144+
errors.value = newValue;
145+
146+
if (previousError == null) {
147+
if (!error.read) {
148+
_incrementUnreadCount();
149+
}
150+
return;
151+
}
152+
153+
if (previousError.read && !error.read) {
154+
_incrementUnreadCount();
155+
} else if (!previousError.read && error.read) {
156+
_decrementUnreadCount();
157+
}
158+
}
159+
160+
/// Clears all inspector errors and resets the badge count.
161+
void clearErrors() {
162+
_activeInspectorErrors.value = LinkedHashMap<String, DevToolsError>();
163+
serviceConnection.errorBadgeManager.resetErrorCount(InspectorScreen.id);
164+
}
165+
166+
/// Marks an error as read and decrements the unread count.
167+
void markErrorAsRead(DevToolsError error) {
168+
final errors = _activeInspectorErrors;
169+
170+
// If this error doesn't exist anymore or is already read, nothing to do.
171+
final currentError = errors.value[error.id];
172+
if (currentError == null || currentError.read) {
173+
return;
174+
}
175+
176+
// Otherwise, replace the map with a new one that has the error marked
177+
// as read.
178+
final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
179+
newValue[error.id] = currentError.asRead();
180+
errors.value = newValue;
181+
_decrementUnreadCount();
182+
}
183+
184+
void _incrementUnreadCount() {
185+
serviceConnection.errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
186+
}
187+
188+
void _decrementUnreadCount() {
189+
serviceConnection.errorBadgeManager.decrementBadgeCount(InspectorScreen.id);
39190
}
40191

41192
@override
42193
void dispose() {
194+
_activeInspectorErrors.dispose();
43195
inspectorTreeController.dispose();
44196
inspectorController.dispose();
45197
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

‎packages/devtools_app/lib/src/shared/framework/screen_controllers.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ class ScreenControllers {
5151
controllers[T] = _LazyController<T>(creator: controllerCreator);
5252
}
5353

54+
/// Whether a controller of type [T] has been registered for the active mode.
55+
bool isRegistered<T>() {
56+
final controllers = offlineDataController.showingOfflineData.value
57+
? offlineControllers
58+
: this.controllers;
59+
return controllers.containsKey(T);
60+
}
61+
5462
/// Returns the active screen controller of type [T].
5563
///
5664
/// When DevTools is showing offline data, the offline screen controller will

0 commit comments

Comments
 (0)