Fix persistent and disappearing error badges for Inspector (#9805)
* Fix persistent and disappearing error badges for Inspector
* Add release note for error badge fix
* Increase timeout for asyncEval tests on Windows CI
* Revert "Increase timeout for asyncEval tests on Windows CI"
This reverts commit 9b461c3e96af82f2cd6aeba152d13d355dc50511.
* Update packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* update error badge unread count incrementally
* remove bad merge
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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 e1db0b5..1f1ed0c 100644
--- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
+++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
@@ -157,9 +157,8 @@
// TODO(kenz): When this method is called outside createState(), this post
// frame callback can be removed.
WidgetsBinding.instance.addPostFrameCallback((_) {
- serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
+ serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
});
- filterErrors();
}
void _handleConnectionStop() {
@@ -381,8 +380,6 @@
return;
}
- filterErrors();
-
return _waitForPendingUpdateDone();
}
@@ -404,13 +401,6 @@
await onForceRefresh();
}
- void filterErrors() {
- serviceConnection.errorBadgeManager.filterErrors(
- InspectorScreen.id,
- (id) => hasDiagnosticsValue(InspectorInstanceRef(id)),
- );
- }
-
void setActivate(bool enabled) {
if (!enabled) {
onIsolateStopped();
@@ -977,17 +967,12 @@
_selectedErrorIndex.value = errorIndex;
if (errorIndex != null) {
- // Mark the error as "seen" as this will render slightly differently
- // so the user can track which errored nodes they've viewed.
+ // Marking an error as read will automatically update the badge count to
+ // reflect the remaining unread errors.
serviceConnection.errorBadgeManager.markErrorAsRead(
InspectorScreen.id,
errors[inspectorRef!]!,
);
- // Also clear the error badge since new errors may have arrived while
- // the inspector was visible (normally they're cleared when visiting
- // the screen) and visiting an errored node seems an appropriate
- // acknowledgement of the errors.
- serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
}
}
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 16950eb..10a12fb 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
@@ -66,7 +66,6 @@
final inspectableError = _extractInspectableError(e);
if (inspectableError != null) {
- incrementBadgeCount(InspectorScreen.id);
appendError(InspectorScreen.id, inspectableError);
}
}
@@ -113,6 +112,10 @@
}
void incrementBadgeCount(String screenId) {
+ if (_activeErrors.containsKey(screenId)) {
+ return;
+ }
+
final notifier = _errorCountNotifier(screenId);
if (notifier == null) return;
@@ -124,12 +127,40 @@
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) {
+ final notifier = _errorCountNotifier(screenId);
+ if (notifier == null) return;
+ if (notifier.value == 0) return;
+ notifier.value = notifier.value - 1;
}
ValueListenable<int> errorCountNotifier(String screenId) {
@@ -150,25 +181,20 @@
}
void clearErrorCount(String screenId) {
+ if (_activeErrors.containsKey(screenId)) {
+ return;
+ }
_activeErrorCounts[screenId]?.value = 0;
}
void clearErrors(String screenId) {
- clearErrorCount(screenId);
- _activeErrors[screenId]?.value = LinkedHashMap<String, DevToolsError>();
- }
-
- void filterErrors(String screenId, bool Function(String id) isValid) {
- final errors = _activeErrors[screenId];
- if (errors == null) return;
-
- final oldCount = errors.value.length;
- final newValue = Map.fromEntries(
- errors.value.entries.where((e) => isValid(e.key)),
- );
- if (newValue.length != oldCount) {
- errors.value = newValue as LinkedHashMap<String, DevToolsError>;
+ if (!_activeErrors.containsKey(screenId)) {
+ clearErrorCount(screenId);
+ return;
}
+
+ _activeErrors[screenId]?.value = LinkedHashMap<String, DevToolsError>();
+ _activeErrorCounts[screenId]?.value = 0;
}
void markErrorAsRead(String screenId, DevToolsError error) {
@@ -188,6 +214,7 @@
return MapEntry(e.key, e.value.asRead());
}),
);
+ _decrementUnreadCount(screenId);
}
}
diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
index 982483f..7d6a008 100644
--- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
+++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
@@ -21,7 +21,7 @@
## Inspector updates
-TODO: Remove this section if there are not any updates.
+- Fixed an issue where the Inspector error badge count would improperly increase or disappear during navigation. [#9524](https://github.com/flutter/devtools/issues/9524)
## Performance updates
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 e5f17d1..06388d2 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,11 +13,7 @@
import 'package:devtools_app/src/shared/managers/error_badge_manager.dart';
import 'package:flutter_test/flutter_test.dart';
-final supportedScreenIds = [
- InspectorScreen.id,
- PerformanceScreen.id,
- NetworkScreen.id,
-];
+final screensWithCountOnly = [PerformanceScreen.id, NetworkScreen.id];
final allScreenIds = [
InspectorScreen.id,
@@ -55,7 +51,7 @@
allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
for (final id in allScreenIds) {
- if (supportedScreenIds.contains(id)) {
+ if (screensWithCountOnly.contains(id)) {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(1));
} else {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
@@ -67,7 +63,7 @@
allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
for (final id in allScreenIds) {
- if (supportedScreenIds.contains(id)) {
+ if (screensWithCountOnly.contains(id)) {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(1));
} else {
expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
@@ -108,7 +104,6 @@
InspectorScreen.id,
DevToolsError('An error', InspectorScreen.id),
);
- errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
expect(getActiveErrorCount(InspectorScreen.id), equals(1));
expect(
