Move Inspector error tracking out of ErrorBadgeManager (#9923)
diff --git a/packages/devtools_app/lib/devtools_app.dart b/packages/devtools_app/lib/devtools_app.dart
index fb2637e..8088054 100644
--- a/packages/devtools_app/lib/devtools_app.dart
+++ b/packages/devtools_app/lib/devtools_app.dart
@@ -27,6 +27,7 @@
export 'src/screens/dtd/dtd_tools_controller.dart';
export 'src/screens/dtd/dtd_tools_screen.dart';
export 'src/screens/inspector/inspector_controller.dart';
+export 'src/screens/inspector/inspector_errors.dart';
export 'src/screens/inspector/inspector_screen.dart';
export 'src/screens/inspector/inspector_screen_body.dart';
export 'src/screens/inspector/inspector_screen_controller.dart';
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
index b9049b0..171231e 100644
--- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
@@ -31,6 +31,7 @@
import '../../shared/diagnostics/diagnostics_node.dart';
import '../../shared/diagnostics/inspector_service.dart';
import '../../shared/diagnostics/primitives/instance_ref.dart';
+import '../../shared/framework/screen_controllers.dart';
import '../../shared/globals.dart';
import '../../shared/managers/notifications.dart';
import '../../shared/primitives/query_parameters.dart';
@@ -38,6 +39,7 @@
import '../../shared/utils/utils.dart';
import 'inspector_data_models.dart';
import 'inspector_screen.dart';
+import 'inspector_screen_controller.dart';
import 'inspector_tree_controller.dart';
final _log = Logger('inspector_controller');
@@ -149,6 +151,21 @@
}
}
+ /// Returns the [InspectorScreenController] when it is registered.
+ ///
+ /// [InspectorController] is sometimes constructed in unit tests without an
+ /// [InspectorScreenController] registered, so callers that only need to clear
+ /// errors on connect/reload should use this nullable accessor.
+ InspectorScreenController? get _inspectorScreenControllerOrNull {
+ final controllers = globals[ScreenControllers] as ScreenControllers?;
+ if (controllers == null) return null;
+ if (!controllers.isRegistered<InspectorScreenController>()) return null;
+ return controllers.lookup<InspectorScreenController>();
+ }
+
+ InspectorScreenController get _inspectorScreenController =>
+ screenControllers.lookup<InspectorScreenController>();
+
void _handleConnectionStart() {
// Clear any existing badge/errors for older errors that were collected.
// Do this in a post frame callback so that we are not trying to clear the
@@ -157,7 +174,7 @@
// TODO(kenz): When this method is called outside createState(), this post
// frame callback can be removed.
WidgetsBinding.instance.addPostFrameCallback((_) {
- serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
+ _inspectorScreenControllerOrNull?.clearErrors();
});
}
@@ -437,7 +454,7 @@
}
if (event.kind == EventKind.kIsolateReload) {
- serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
+ _inspectorScreenControllerOrNull?.clearErrors();
_receivedIsolateReloadEvent = true;
}
}
@@ -870,9 +887,7 @@
void _updateSelectedErrorFromNode(InspectorTreeNode? node) {
final inspectorRef = node?.diagnostic?.valueRef.id;
- final errors = serviceConnection.errorBadgeManager
- .erroredItemsForPage(InspectorScreen.id)
- .value;
+ final errors = _inspectorScreenController.inspectorErrors.value;
// Check whether the node that was just selected has any errors associated
// with it.
@@ -888,10 +903,7 @@
if (errorIndex != null) {
// Marking an error as read will automatically update the badge count to
// reflect the remaining unread errors.
- serviceConnection.errorBadgeManager.markErrorAsRead(
- InspectorScreen.id,
- errors[inspectorRef!]!,
- );
+ _inspectorScreenController.markErrorAsRead(errors[inspectorRef!]!);
}
}
@@ -899,9 +911,7 @@
void selectErrorByIndex(int index) {
_selectedErrorIndex.value = index;
- final errors = serviceConnection.errorBadgeManager
- .erroredItemsForPage(InspectorScreen.id)
- .value;
+ final errors = _inspectorScreenController.inspectorErrors.value;
unawaited(
updateSelectionFromService(inspectorRef: errors.keys.elementAt(index)),
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_errors.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_errors.dart
new file mode 100644
index 0000000..eb12e8e
--- /dev/null
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_errors.dart
@@ -0,0 +1,15 @@
+// Copyright 2024 The Flutter Authors
+// 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 '../../shared/managers/error_badge_manager.dart';
+
+/// An error associated with a specific widget that can be inspected in the
+/// Inspector screen.
+class InspectableWidgetError extends DevToolsError {
+ InspectableWidgetError(super.errorMessage, super.id, {super.read});
+
+ @override
+ InspectableWidgetError asRead() =>
+ InspectableWidgetError(errorMessage, id, read: true);
+}
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart
index f61c22f..67403c5 100644
--- a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart
@@ -21,7 +21,9 @@
import '../../shared/utils/utils.dart';
import 'inspector_controller.dart';
import 'inspector_controls.dart';
+import 'inspector_errors.dart';
import 'inspector_screen.dart';
+import 'inspector_screen_controller.dart';
import 'inspector_tree_controller.dart';
import 'widget_details.dart';
@@ -149,8 +151,9 @@
),
Expanded(
child: ValueListenableBuilder(
- valueListenable: serviceConnection.errorBadgeManager
- .erroredItemsForPage(InspectorScreen.id),
+ valueListenable: screenControllers
+ .lookup<InspectorScreenController>()
+ .inspectorErrors,
builder: (_, LinkedHashMap<String, DevToolsError> errors, _) {
final inspectableErrors =
errors.map(
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_controller.dart
index 5a403db..4d33da8 100644
--- a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_controller.dart
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_controller.dart
@@ -2,11 +2,26 @@
// 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 'dart:collection';
+
+import 'package:collection/collection.dart' show IterableExtension;
+import 'package:devtools_app_shared/service.dart';
+import 'package:devtools_app_shared/utils.dart';
+import 'package:flutter/foundation.dart';
+import 'package:vm_service/vm_service.dart';
+
+import '../../service/vm_service_wrapper.dart';
import '../../shared/analytics/metrics.dart';
import '../../shared/console/primitives/simple_items.dart';
+import '../../shared/diagnostics/diagnostics_node.dart';
import '../../shared/framework/screen.dart';
import '../../shared/framework/screen_controllers.dart';
+import '../../shared/globals.dart';
+import '../../shared/managers/error_badge_manager.dart';
+import '../../shared/primitives/query_parameters.dart';
import 'inspector_controller.dart';
+import 'inspector_errors.dart';
+import 'inspector_screen.dart';
import 'inspector_tree_controller.dart';
/// Screen controller for the Inspector screen.
@@ -19,16 +34,36 @@
/// `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.
-class InspectorScreenController extends DevToolsScreenController {
+class InspectorScreenController extends DevToolsScreenController
+ with AutoDisposeControllerMixin {
@override
final screenId = ScreenMetaData.inspector.id;
late InspectorController inspectorController;
late InspectorTreeController inspectorTreeController;
+ /// Stores the inspector-specific errors keyed by inspector reference ID.
+ final _activeInspectorErrors =
+ ValueNotifier<LinkedHashMap<String, DevToolsError>>(
+ LinkedHashMap<String, DevToolsError>(),
+ );
+
+ /// The errors currently tracked for the inspector screen.
+ ValueListenable<LinkedHashMap<String, DevToolsError>> get inspectorErrors =>
+ _activeInspectorErrors;
+
+ /// The count of unread inspector errors (used for the badge).
+ ValueListenable<int> get inspectorErrorCount => serviceConnection
+ .errorBadgeManager
+ .errorCountNotifier(InspectorScreen.id);
+
@override
void init() {
super.init();
+ // Inspector owns unread state for its badge; scaffold tab switches must not
+ // clear it. See https://github.com/flutter/devtools/pull/9805.
+ serviceConnection.errorBadgeManager.manageErrorCount(InspectorScreen.id);
+
inspectorTreeController = InspectorTreeController(
gaId: InspectorScreenMetrics.summaryTreeGaId,
);
@@ -36,10 +71,127 @@
inspectorTree: inspectorTreeController,
treeType: FlutterTreeType.widget,
);
+
+ // Listen for Flutter extension events to extract inspector-specific errors.
+ // Match other screen controllers: attach now if connected, and on connect.
+ addAutoDisposeListener(serviceConnection.serviceManager.connectedState, () {
+ if (serviceConnection.serviceManager.connectedState.value.connected) {
+ _handleConnectionStart(serviceConnection.serviceManager.service!);
+ }
+ });
+ if (serviceConnection.serviceManager.connectedAppInitialized) {
+ _handleConnectionStart(serviceConnection.serviceManager.service!);
+ }
+ }
+
+ void _handleConnectionStart(VmServiceWrapper service) {
+ autoDisposeStreamSubscription(
+ service.onExtensionEventWithHistorySafe.listen(_handleExtensionEvent),
+ );
+ }
+
+ void _handleExtensionEvent(Event e) {
+ if (e.extensionKind == FlutterEvent.error) {
+ final inspectableError = _extractInspectableError(e);
+ if (inspectableError != null) {
+ appendError(inspectableError);
+ }
+ }
+ }
+
+ InspectableWidgetError? _extractInspectableError(Event error) {
+ final extensionData = error.extensionData;
+ if (extensionData == null) return null;
+
+ final node = RemoteDiagnosticsNode(extensionData.data, null, false, null);
+
+ final errorSummaryNode = node.inlineProperties.firstWhereOrNull(
+ (p) => p.type == 'ErrorSummary',
+ );
+ final errorMessage = errorSummaryNode?.description;
+ if (errorMessage == null) {
+ return null;
+ }
+
+ final devToolsUrlNode = node.inlineProperties.firstWhereOrNull(
+ (p) =>
+ p.type == 'DevToolsDeepLinkProperty' &&
+ p.getStringMember('value') != null,
+ );
+ if (devToolsUrlNode == null) {
+ return null;
+ }
+
+ final queryParams = DevToolsQueryParams.fromUrl(
+ devToolsUrlNode.getStringMember('value')!,
+ );
+ final inspectorRef = queryParams.inspectorRef ?? '';
+
+ return InspectableWidgetError(errorMessage, inspectorRef);
+ }
+
+ /// Appends an error to the inspector's active errors and updates the badge
+ /// count.
+ void appendError(DevToolsError error) {
+ final errors = _activeInspectorErrors;
+ final previousError = errors.value[error.id];
+
+ // Build a new map with the new error. Adding to the existing map
+ // won't cause the ValueNotifier to fire (and it's not permitted to call
+ // notifyListeners() directly).
+ final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
+ newValue[error.id] = error;
+ errors.value = newValue;
+
+ if (previousError == null) {
+ if (!error.read) {
+ _incrementUnreadCount();
+ }
+ return;
+ }
+
+ if (previousError.read && !error.read) {
+ _incrementUnreadCount();
+ } else if (!previousError.read && error.read) {
+ _decrementUnreadCount();
+ }
+ }
+
+ /// Clears all inspector errors and resets the badge count.
+ void clearErrors() {
+ _activeInspectorErrors.value = LinkedHashMap<String, DevToolsError>();
+ serviceConnection.errorBadgeManager.resetErrorCount(InspectorScreen.id);
+ }
+
+ /// Marks an error as read and decrements the unread count.
+ void markErrorAsRead(DevToolsError error) {
+ final errors = _activeInspectorErrors;
+
+ // If this error doesn't exist anymore or is already read, nothing to do.
+ final currentError = errors.value[error.id];
+ if (currentError == null || currentError.read) {
+ return;
+ }
+
+ // Otherwise, replace the map with a new one that has the error marked
+ // as read.
+ final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
+ newValue[error.id] = currentError.asRead();
+ errors.value = newValue;
+ _decrementUnreadCount();
+ }
+
+ void _incrementUnreadCount() {
+ serviceConnection.errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
+ }
+
+ void _decrementUnreadCount() {
+ serviceConnection.errorBadgeManager.decrementBadgeCount(InspectorScreen.id);
}
@override
void dispose() {
+ _activeInspectorErrors.dispose();
inspectorTreeController.dispose();
inspectorController.dispose();
super.dispose();
diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart
index 957364a..fee0b94 100644
--- a/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart
@@ -30,6 +30,7 @@
import '../../shared/ui/utils.dart';
import '../../shared/utils/utils.dart';
import 'inspector_controller.dart';
+import 'inspector_errors.dart';
final _log = Logger('inspector_tree_controller');
diff --git a/packages/devtools_app/lib/src/shared/framework/screen_controllers.dart b/packages/devtools_app/lib/src/shared/framework/screen_controllers.dart
index 427254c..7baade8 100644
--- a/packages/devtools_app/lib/src/shared/framework/screen_controllers.dart
+++ b/packages/devtools_app/lib/src/shared/framework/screen_controllers.dart
@@ -51,6 +51,14 @@
controllers[T] = _LazyController<T>(creator: controllerCreator);
}
+ /// Whether a controller of type [T] has been registered for the active mode.
+ bool isRegistered<T>() {
+ final controllers = offlineDataController.showingOfflineData.value
+ ? offlineControllers
+ : this.controllers;
+ return controllers.containsKey(T);
+ }
+
/// Returns the active screen controller of type [T].
///
/// When DevTools is showing offline data, the offline screen controller will
diff --git a/packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart b/packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart
index 10a12fb..2689cd4 100644
--- a/packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart
+++ b/packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart
@@ -3,9 +3,7 @@
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
import 'dart:async';
-import 'dart:collection';
-import 'package:collection/collection.dart' show IterableExtension;
import 'package:devtools_app_shared/service.dart';
import 'package:devtools_app_shared/utils.dart';
import 'package:flutter/foundation.dart';
@@ -17,26 +15,29 @@
import '../../screens/performance/performance_screen.dart';
import '../../service/service_extensions.dart' as extensions;
import '../../service/vm_service_wrapper.dart';
-import '../diagnostics/diagnostics_node.dart';
import '../globals.dart';
import '../primitives/listenable.dart';
-import '../primitives/query_parameters.dart';
+/// Manages error badge counts for DevTools screen tabs.
+///
+/// This is a generic counter that tracks unread error counts per screen.
+/// Screen-specific error tracking logic (e.g., detailed error objects for the
+/// Inspector screen) should live in the respective screen controllers.
class ErrorBadgeManager extends DisposableController
with AutoDisposeControllerMixin {
- // TODO(https://github.com/flutter/devtools/issues/9105): Separate out
- // Inspector-specific logic from this file.
final _activeErrorCounts = <String, ValueNotifier<int>>{
InspectorScreen.id: ValueNotifier<int>(0),
PerformanceScreen.id: ValueNotifier<int>(0),
NetworkScreen.id: ValueNotifier<int>(0),
};
- final _activeErrors =
- <String, ValueNotifier<LinkedHashMap<String, DevToolsError>>>{
- InspectorScreen.id: ValueNotifier<LinkedHashMap<String, DevToolsError>>(
- LinkedHashMap<String, DevToolsError>(),
- ),
- };
+
+ /// Screen ids whose unread badge count is owned by the screen controller.
+ ///
+ /// For these screens, [clearErrorCount] is a no-op so navigating to the tab
+ /// (see scaffold) does not drop the unread badge. Controllers should update
+ /// the count via [incrementBadgeCount], [decrementBadgeCount], and
+ /// [resetErrorCount] instead.
+ final _managedErrorCounts = <String>{};
void vmServiceOpened(VmServiceWrapper service) {
// Ensure structured errors are enabled.
@@ -63,100 +64,29 @@
void _handleExtensionEvent(Event e) {
if (e.extensionKind == FlutterEvent.error) {
incrementBadgeCount(LoggingScreen.id);
-
- final inspectableError = _extractInspectableError(e);
- if (inspectableError != null) {
- appendError(InspectorScreen.id, inspectableError);
- }
}
}
- InspectableWidgetError? _extractInspectableError(Event error) {
- // TODO(https://github.com/flutter/devtools/issues/9105): Switch to using
- // the inspectorService from the serviceManager once Jacob's change to add
- // it lands.
- final node = RemoteDiagnosticsNode(
- error.extensionData!.data,
- null,
- false,
- null,
- );
-
- final errorSummaryNode = node.inlineProperties.firstWhereOrNull(
- (p) => p.type == 'ErrorSummary',
- );
- final errorMessage = errorSummaryNode?.description;
- if (errorMessage == null) {
- return null;
- }
-
- final devToolsUrlNode = node.inlineProperties.firstWhereOrNull(
- (p) =>
- p.type == 'DevToolsDeepLinkProperty' &&
- p.getStringMember('value') != null,
- );
- if (devToolsUrlNode == null) {
- return null;
- }
-
- final queryParams = DevToolsQueryParams.fromUrl(
- devToolsUrlNode.getStringMember('value')!,
- );
- final inspectorRef = queryParams.inspectorRef ?? '';
-
- return InspectableWidgetError(errorMessage, inspectorRef);
- }
-
void _handleStdErr(Event _) {
incrementBadgeCount(LoggingScreen.id);
}
+ /// Marks [screenId] as managing its own unread badge count.
+ ///
+ /// After this is called, [clearErrorCount] will not reset the badge for
+ /// [screenId] (preserving unread state across tab switches).
+ void manageErrorCount(String screenId) {
+ _managedErrorCounts.add(screenId);
+ }
+
void incrementBadgeCount(String screenId) {
- if (_activeErrors.containsKey(screenId)) {
- return;
- }
-
final notifier = _errorCountNotifier(screenId);
if (notifier == null) return;
- final currentCount = notifier.value;
- notifier.value = currentCount + 1;
- }
-
- void appendError(String screenId, DevToolsError error) {
- final errors = _activeErrors[screenId];
- if (errors == null) return;
-
- final previousError = errors.value[error.id];
-
- // Build a new map with the new error. Adding to the existing map
- // won't cause the ValueNotifier to fire (and it's not permitted to call
- // notifyListeners() directly).
- final newValue = LinkedHashMap<String, DevToolsError>.of(errors.value);
- newValue[error.id] = error;
- errors.value = newValue;
-
- if (previousError == null) {
- if (!error.read) {
- _incrementUnreadCount(screenId);
- }
- return;
- }
-
- if (previousError.read && !error.read) {
- _incrementUnreadCount(screenId);
- } else if (!previousError.read && error.read) {
- _decrementUnreadCount(screenId);
- }
- }
-
- void _incrementUnreadCount(String screenId) {
- final notifier = _errorCountNotifier(screenId);
- if (notifier == null) return;
notifier.value = notifier.value + 1;
}
- void _decrementUnreadCount(String screenId) {
+ void decrementBadgeCount(String screenId) {
final notifier = _errorCountNotifier(screenId);
if (notifier == null) return;
if (notifier.value == 0) return;
@@ -167,55 +97,23 @@
return _errorCountNotifier(screenId) ?? const FixedValueListenable<int>(0);
}
- ValueListenable<LinkedHashMap<String, DevToolsError>> erroredItemsForPage(
- String screenId,
- ) {
- return _activeErrors[screenId] ??
- FixedValueListenable<LinkedHashMap<String, DevToolsError>>(
- LinkedHashMap<String, DevToolsError>(),
- );
- }
-
ValueNotifier<int>? _errorCountNotifier(String screenId) {
return _activeErrorCounts[screenId];
}
+ /// Clears the badge count for [screenId], unless it is [manageErrorCount]d.
void clearErrorCount(String screenId) {
- if (_activeErrors.containsKey(screenId)) {
- return;
- }
+ if (_managedErrorCounts.contains(screenId)) return;
_activeErrorCounts[screenId]?.value = 0;
}
- void clearErrors(String screenId) {
- if (!_activeErrors.containsKey(screenId)) {
- clearErrorCount(screenId);
- return;
- }
-
- _activeErrors[screenId]?.value = LinkedHashMap<String, DevToolsError>();
+ /// Unconditionally resets the badge count for [screenId].
+ ///
+ /// Use from screen controllers that call [manageErrorCount] when they need
+ /// to clear their own unread state (e.g. on hot restart).
+ void resetErrorCount(String screenId) {
_activeErrorCounts[screenId]?.value = 0;
}
-
- void markErrorAsRead(String screenId, DevToolsError error) {
- final errors = _activeErrors[screenId];
- if (errors == null) return;
-
- // If this error doesn't exist anymore or is already read, nothing to do.
- if (errors.value[error.id]?.read ?? true) {
- return;
- }
-
- // Otherwise, replace the map with a new one that has the error marked
- // as read.
- errors.value = LinkedHashMap<String, DevToolsError>.fromEntries(
- errors.value.entries.map((e) {
- if (e.value != error) return e;
- return MapEntry(e.key, e.value.asRead());
- }),
- );
- _decrementUnreadCount(screenId);
- }
}
class DevToolsError {
@@ -227,11 +125,3 @@
DevToolsError asRead() => DevToolsError(errorMessage, id, read: true);
}
-
-class InspectableWidgetError extends DevToolsError {
- InspectableWidgetError(super.errorMessage, super.id, {super.read});
-
- @override
- InspectableWidgetError asRead() =>
- InspectableWidgetError(errorMessage, id, read: true);
-}
diff --git a/packages/devtools_app/test/screens/inspector/inspector_screen_controller_test.dart b/packages/devtools_app/test/screens/inspector/inspector_screen_controller_test.dart
new file mode 100644
index 0000000..02fb69d
--- /dev/null
+++ b/packages/devtools_app/test/screens/inspector/inspector_screen_controller_test.dart
@@ -0,0 +1,75 @@
+// Copyright 2025 The Flutter Authors
+// 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/devtools_app.dart';
+import 'package:devtools_app_shared/utils.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+ late ServiceConnectionManager serviceConnectionManager;
+ late InspectorScreenController controller;
+
+ setUp(() {
+ serviceConnectionManager = ServiceConnectionManager();
+ setGlobal(ServiceConnectionManager, serviceConnectionManager);
+ controller = InspectorScreenController();
+ // Mirror the badge-ownership registration performed in [init] without
+ // constructing the full Inspector tree controllers.
+ serviceConnection.errorBadgeManager.manageErrorCount(InspectorScreen.id);
+ });
+
+ int unreadBadgeCount() => serviceConnection.errorBadgeManager
+ .errorCountNotifier(InspectorScreen.id)
+ .value;
+
+ test('appendError stores the error and increments the badge', () {
+ controller.appendError(InspectableWidgetError('Overflow', 'ref-1'));
+
+ expect(controller.inspectorErrors.value.length, equals(1));
+ expect(controller.inspectorErrors.value['ref-1']!.errorMessage, 'Overflow');
+ expect(unreadBadgeCount(), equals(1));
+ });
+
+ test('appendError does not double-count the same error id', () {
+ controller.appendError(InspectableWidgetError('Overflow', 'ref-1'));
+ controller.appendError(InspectableWidgetError('Overflow again', 'ref-1'));
+
+ expect(controller.inspectorErrors.value.length, equals(1));
+ expect(unreadBadgeCount(), equals(1));
+ });
+
+ test('markErrorAsRead decrements the badge', () {
+ final error = InspectableWidgetError('Overflow', 'ref-1');
+ controller.appendError(error);
+ controller.markErrorAsRead(error);
+
+ expect(controller.inspectorErrors.value['ref-1']!.read, isTrue);
+ expect(unreadBadgeCount(), equals(0));
+ });
+
+ test('clearErrors resets errors and badge', () {
+ controller.appendError(InspectableWidgetError('Overflow', 'ref-1'));
+ controller.appendError(InspectableWidgetError('Null check', 'ref-2'));
+ expect(unreadBadgeCount(), equals(2));
+
+ controller.clearErrors();
+
+ expect(controller.inspectorErrors.value, isEmpty);
+ expect(unreadBadgeCount(), equals(0));
+ });
+
+ test(
+ 'scaffold-style clearErrorCount does not drop inspector unread badge',
+ () {
+ controller.appendError(InspectableWidgetError('Overflow', 'ref-1'));
+ expect(unreadBadgeCount(), equals(1));
+
+ // Simulates DevToolsScaffold clearing the badge on tab navigation.
+ serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
+
+ expect(unreadBadgeCount(), equals(1));
+ expect(controller.inspectorErrors.value.length, equals(1));
+ },
+ );
+}
diff --git a/packages/devtools_app/test/shared/managers/error_badge_manager_test.dart b/packages/devtools_app/test/shared/managers/error_badge_manager_test.dart
index e3c12b6..bbd3d15 100644
--- a/packages/devtools_app/test/shared/managers/error_badge_manager_test.dart
+++ b/packages/devtools_app/test/shared/managers/error_badge_manager_test.dart
@@ -13,7 +13,11 @@
import 'package:devtools_app/src/shared/managers/error_badge_manager.dart';
import 'package:flutter_test/flutter_test.dart';
-final screensWithCountOnly = [PerformanceScreen.id, NetworkScreen.id];
+final screensWithBadgeCounts = [
+ InspectorScreen.id,
+ PerformanceScreen.id,
+ NetworkScreen.id,
+];
final allScreenIds = [
InspectorScreen.id,
@@ -30,9 +34,6 @@
late ErrorBadgeManager errorBadgeManager;
group('ErrorBadgeManager', () {
- int getActiveErrorCount(String screenId) =>
- errorBadgeManager.erroredItemsForPage(screenId).value.entries.length;
-
setUp(() {
errorBadgeManager = ErrorBadgeManager();
});
@@ -51,7 +52,7 @@
allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
for (final id in allScreenIds) {
- if (screensWithCountOnly.contains(id)) {
+ if (screensWithBadgeCounts.contains(id)) {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(1));
} else {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
@@ -59,11 +60,36 @@
}
});
+ test('decrementBadgeCount decrements supported tabs', () {
+ // First increment
+ allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
+
+ for (final id in screensWithBadgeCounts) {
+ expect(errorBadgeManager.errorCountNotifier(id).value, equals(1));
+ }
+
+ // Then decrement
+ allScreenIds.forEach(errorBadgeManager.decrementBadgeCount);
+
+ for (final id in screensWithBadgeCounts) {
+ expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
+ }
+ });
+
+ test('decrementBadgeCount does not go below zero', () {
+ // Decrement without any prior increment
+ errorBadgeManager.decrementBadgeCount(InspectorScreen.id);
+ expect(
+ errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
+ equals(0),
+ );
+ });
+
test('clearErrorCount resets counts', () {
allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
for (final id in allScreenIds) {
- if (screensWithCountOnly.contains(id)) {
+ if (screensWithBadgeCounts.contains(id)) {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(1));
} else {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
@@ -77,43 +103,23 @@
}
});
- // TODO(https://github.com/flutter/devtools/issues/9105): This logic should
- // be moved to the inspector.
- test('appendError works for inspector screen only', () {
- for (final id in allScreenIds) {
- errorBadgeManager.appendError(id, DevToolsError('An error', id));
- }
-
- for (final id in allScreenIds) {
- if (id == InspectorScreen.id) {
- expect(getActiveErrorCount(id), equals(1));
- } else {
- expect(getActiveErrorCount(id), equals(0));
- }
- }
- });
-
- test('clearErrors resets counts and removes errors', () {
- expect(getActiveErrorCount(InspectorScreen.id), equals(0));
- expect(
- errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
- equals(0),
- );
-
- errorBadgeManager.appendError(
- InspectorScreen.id,
- DevToolsError('An error', InspectorScreen.id),
- );
-
- expect(getActiveErrorCount(InspectorScreen.id), equals(1));
+ test('clearErrorCount is a no-op for manageErrorCount screens', () {
+ errorBadgeManager.manageErrorCount(InspectorScreen.id);
+ errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
expect(
errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
equals(1),
);
- errorBadgeManager.clearErrors(InspectorScreen.id);
+ // Simulates scaffold clearing the badge on tab navigation.
+ errorBadgeManager.clearErrorCount(InspectorScreen.id);
+ expect(
+ errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
+ equals(1),
+ );
- expect(getActiveErrorCount(InspectorScreen.id), equals(0));
+ // Controllers that own unread state can still reset explicitly.
+ errorBadgeManager.resetErrorCount(InspectorScreen.id);
expect(
errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
equals(0),
diff --git a/packages/devtools_test/lib/src/mocks/fake_service_manager.dart b/packages/devtools_test/lib/src/mocks/fake_service_manager.dart
index 1b4027f..5b948b5 100644
--- a/packages/devtools_test/lib/src/mocks/fake_service_manager.dart
+++ b/packages/devtools_test/lib/src/mocks/fake_service_manager.dart
@@ -3,7 +3,6 @@
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
import 'dart:async';
-import 'dart:collection';
import 'package:devtools_app/devtools_app.dart';
import 'package:devtools_app_shared/service.dart';
@@ -40,9 +39,6 @@
);
for (final screen in ScreenMetaData.values) {
final screenId = screen.id;
- when(errorBadgeManager.erroredItemsForPage(screenId)).thenReturn(
- FixedValueListenable(LinkedHashMap<String, DevToolsError>()),
- );
when(
errorBadgeManager.errorCountNotifier(screenId),
).thenReturn(ValueNotifier<int>(0));
