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 e1db0b59c2a..1f1ed0cf8a7 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 @@ 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); } } 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 16950eb15d6..b9fbe734161 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 @@ 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 (screenId == InspectorScreen.id || _activeErrors.containsKey(screenId)) { + return; + } + final notifier = _errorCountNotifier(screenId); if (notifier == null) return; @@ -130,6 +133,15 @@ class ErrorBadgeManager extends DisposableController final newValue = LinkedHashMap.of(errors.value); newValue[error.id] = error; errors.value = newValue; + _updateErrorCount(screenId); + } + + void _updateErrorCount(String screenId) { + final errors = _activeErrors[screenId]; + if (errors == null) return; + + final unreadCount = errors.value.values.where((e) => !e.read).length; + _activeErrorCounts[screenId]?.value = unreadCount; } ValueListenable errorCountNotifier(String screenId) { @@ -150,25 +162,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(); - } - - 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; + if (!_activeErrors.containsKey(screenId)) { + clearErrorCount(screenId); + return; } + + _activeErrors[screenId]?.value = LinkedHashMap(); + _activeErrorCounts[screenId]?.value = 0; } void markErrorAsRead(String screenId, DevToolsError error) { @@ -188,6 +195,7 @@ class ErrorBadgeManager extends DisposableController return MapEntry(e.key, e.value.asRead()); }), ); + _updateErrorCount(screenId); } } diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 639875d1ad6..d8ec1586336 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -21,6 +21,8 @@ TODO: Remove this section if there are not any updates. - Deleted the option to use the legacy inspector. [#9782](https://github.com/flutter/devtools/pull/9782) +- 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 e5f17d19665..06388d2fa92 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/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(