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 9b461c3.

* 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>

Author: khanak0509

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

@@ -157,9 +157,8 @@ class InspectorController extends DisposableController
     // 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 @@ class InspectorController extends DisposableController
       return;
     }
 
-    filterErrors();
-
     return _waitForPendingUpdateDone();
   }
 
@@ -404,13 +401,6 @@ class InspectorController extends DisposableController
     await onForceRefresh();
   }
 
-  void filterErrors() {
-    serviceConnection.errorBadgeManager.filterErrors(
-      InspectorScreen.id,
-      (id) => hasDiagnosticsValue(InspectorInstanceRef(id)),
-    );
-  }
-
   void setActivate(bool enabled) {
     if (!enabled) {
       onIsolateStopped();
@@ -977,17 +967,12 @@ class InspectorController extends DisposableController
     _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);
     }
   }
 

packages/devtools_app/lib/src/shared/managers/error_badge_manager.dart

@@ -66,7 +66,6 @@ class ErrorBadgeManager extends DisposableController
 
       final inspectableError = _extractInspectableError(e);
       if (inspectableError != null) {
-        incrementBadgeCount(InspectorScreen.id);
         appendError(InspectorScreen.id, inspectableError);
       }
     }
@@ -113,6 +112,10 @@ class ErrorBadgeManager extends DisposableController
   }
 
   void incrementBadgeCount(String screenId) {
+    if (_activeErrors.containsKey(screenId)) {
+      return;
+    }
+
     final notifier = _errorCountNotifier(screenId);
     if (notifier == null) return;
 
@@ -124,12 +127,40 @@ class ErrorBadgeManager extends DisposableController
     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 @@ class ErrorBadgeManager extends DisposableController
   }
 
   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 @@ class ErrorBadgeManager extends DisposableController
         return MapEntry(e.key, e.value.asRead());
       }),
     );
+    _decrementUnreadCount(screenId);
   }
 }
 

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -21,7 +21,7 @@ To learn more about DevTools, check out the
 
 ## 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
 

packages/devtools_app/test/shared/managers/error_badge_manager_test.dart

@@ -13,11 +13,7 @@ import 'package:devtools_app/src/screens/profiler/profiler_screen.dart';
 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 @@ void main() {
       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 @@ void main() {
       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 @@ void main() {
         InspectorScreen.id,
         DevToolsError('An error', InspectorScreen.id),
       );
-      errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
 
       expect(getActiveErrorCount(InspectorScreen.id), equals(1));
       expect(