Inspector tree errors are removed on hot-reloads (#9106)

Author: elliette

packages/devtools_app/lib/src/framework/scaffold/scaffold.dart

@@ -208,7 +208,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
         );
 
         // Clear error count when navigating to a screen.
-        serviceConnection.errorBadgeManager.clearErrors(screen.screenId);
+        serviceConnection.errorBadgeManager.clearErrorCount(screen.screenId);
 
         // Update routing with the change.
         WidgetsBinding.instance.addPostFrameCallback((timeStamp) {

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

@@ -138,7 +138,7 @@ 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.clearErrors(InspectorScreen.id);
+      serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
     });
     filterErrors();
   }
@@ -761,7 +761,7 @@ class InspectorController extends DisposableController
       // 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.clearErrors(InspectorScreen.id);
+      serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
     }
   }
 

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

@@ -155,7 +155,7 @@ 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.clearErrors(InspectorScreen.id);
+      serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
     });
     filterErrors();
   }
@@ -474,6 +474,7 @@ class InspectorController extends DisposableController
     }
 
     if (event.kind == EventKind.kIsolateReload) {
+      serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
       _receivedIsolateReloadEvent = true;
     }
   }
@@ -989,7 +990,7 @@ class InspectorController extends DisposableController
       // 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.clearErrors(InspectorScreen.id);
+      serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
     }
   }
 

packages/devtools_app/lib/src/screens/logging/logging_controller.dart

@@ -289,7 +289,7 @@ class LoggingController extends DevToolsScreenController
 
   void clear() {
     _updateData([]);
-    serviceConnection.errorBadgeManager.clearErrors(LoggingScreen.id);
+    serviceConnection.errorBadgeManager.clearErrorCount(LoggingScreen.id);
   }
 
   void _handleConnectionStart(VmServiceWrapper service) {

packages/devtools_app/lib/src/screens/network/network_controller.dart

@@ -436,7 +436,7 @@ class NetworkController extends DevToolsScreenController
   @override
   void filterData(Filter<NetworkRequest> filter) {
     super.filterData(filter);
-    serviceConnection.errorBadgeManager.clearErrors(NetworkScreen.id);
+    serviceConnection.errorBadgeManager.clearErrorCount(NetworkScreen.id);
     final queryFilter = filter.queryFilter;
     if (queryFilter.isEmpty) {
       _currentNetworkRequests.value.forEach(_checkForError);

packages/devtools_app/lib/src/screens/performance/performance_controller.dart

@@ -255,7 +255,7 @@ class PerformanceController extends DevToolsScreenController
       await serviceConnection.serviceManager.service!.clearVMTimeline();
     }
     offlinePerformanceData = null;
-    serviceConnection.errorBadgeManager.clearErrors(PerformanceScreen.id);
+    serviceConnection.errorBadgeManager.clearErrorCount(PerformanceScreen.id);
     await _applyToFeatureControllersAsync((c) => c.clearData(partial: partial));
   }
 

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

@@ -24,6 +24,8 @@ import '../primitives/query_parameters.dart';
 
 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),
@@ -71,8 +73,9 @@ class ErrorBadgeManager extends DisposableController
   }
 
   InspectableWidgetError? _extractInspectableError(Event error) {
-    // TODO(dantup): Switch to using the inspectorService from the serviceManager
-    //  once Jacob's change to add it lands.
+    // 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,
@@ -146,10 +149,15 @@ class ErrorBadgeManager extends DisposableController
     return _activeErrorCounts[screenId];
   }
 
-  void clearErrors(String screenId) {
+  void clearErrorCount(String screenId) {
     _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;

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -45,7 +45,9 @@ pausing on breakpoint on connection. -
 
 ## Inspector updates
 
-TODO: Remove this section if there are not any general updates.
+* Fixed bug where errors in the inspector tree (e.g. RenderFlex overflow
+errors) were not removed after a hot-reload. - 
+[#9106](https://github.com/flutter/devtools/pull/9106)
 
 ## Performance updates
 

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

@@ -34,6 +34,9 @@ void main() {
   late ErrorBadgeManager errorBadgeManager;
 
   group('ErrorBadgeManager', () {
+    int getActiveErrorCount(screenId) =>
+        errorBadgeManager.erroredItemsForPage(screenId).value.entries.length;
+
     setUp(() {
       errorBadgeManager = ErrorBadgeManager();
     });
@@ -60,7 +63,7 @@ void main() {
       }
     });
 
-    test('clearErrors resets counts', () {
+    test('clearErrorCount resets counts', () {
       allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
 
       for (final id in allScreenIds) {
@@ -71,11 +74,55 @@ void main() {
         }
       }
 
-      allScreenIds.forEach(errorBadgeManager.clearErrors);
+      allScreenIds.forEach(errorBadgeManager.clearErrorCount);
 
       for (final id in allScreenIds) {
         expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
       }
     });
+
+    // 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),
+      );
+      errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
+
+      expect(getActiveErrorCount(InspectorScreen.id), equals(1));
+      expect(
+        errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
+        equals(1),
+      );
+
+      errorBadgeManager.clearErrors(InspectorScreen.id);
+
+      expect(getActiveErrorCount(InspectorScreen.id), equals(0));
+      expect(
+        errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
+        equals(0),
+      );
+    });
   });
 }