Skip to content

Commit a45587c

Browse files
ellietteelliette
authored
Inspector tree errors are removed on hot-reloads (#9106)
1 parent bb11f19 commit a45587c

9 files changed

Lines changed: 72 additions & 14 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
208208
);
209209

210210
// Clear error count when navigating to a screen.
211-
serviceConnection.errorBadgeManager.clearErrors(screen.screenId);
211+
serviceConnection.errorBadgeManager.clearErrorCount(screen.screenId);
212212

213213
// Update routing with the change.
214214
WidgetsBinding.instance.addPostFrameCallback((timeStamp) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class InspectorController extends DisposableController
138138
// TODO(kenz): When this method is called outside createState(), this post
139139
// frame callback can be removed.
140140
WidgetsBinding.instance.addPostFrameCallback((_) {
141-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
141+
serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
142142
});
143143
filterErrors();
144144
}
@@ -761,7 +761,7 @@ class InspectorController extends DisposableController
761761
// the inspector was visible (normally they're cleared when visiting
762762
// the screen) and visiting an errored node seems an appropriate
763763
// acknowledgement of the errors.
764-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
764+
serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
765765
}
766766
}
767767

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class InspectorController extends DisposableController
155155
// TODO(kenz): When this method is called outside createState(), this post
156156
// frame callback can be removed.
157157
WidgetsBinding.instance.addPostFrameCallback((_) {
158-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
158+
serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
159159
});
160160
filterErrors();
161161
}
@@ -474,6 +474,7 @@ class InspectorController extends DisposableController
474474
}
475475

476476
if (event.kind == EventKind.kIsolateReload) {
477+
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
477478
_receivedIsolateReloadEvent = true;
478479
}
479480
}
@@ -989,7 +990,7 @@ class InspectorController extends DisposableController
989990
// the inspector was visible (normally they're cleared when visiting
990991
// the screen) and visiting an errored node seems an appropriate
991992
// acknowledgement of the errors.
992-
serviceConnection.errorBadgeManager.clearErrors(InspectorScreen.id);
993+
serviceConnection.errorBadgeManager.clearErrorCount(InspectorScreen.id);
993994
}
994995
}
995996

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ class LoggingController extends DevToolsScreenController
289289

290290
void clear() {
291291
_updateData([]);
292-
serviceConnection.errorBadgeManager.clearErrors(LoggingScreen.id);
292+
serviceConnection.errorBadgeManager.clearErrorCount(LoggingScreen.id);
293293
}
294294

295295
void _handleConnectionStart(VmServiceWrapper service) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ class NetworkController extends DevToolsScreenController
436436
@override
437437
void filterData(Filter<NetworkRequest> filter) {
438438
super.filterData(filter);
439-
serviceConnection.errorBadgeManager.clearErrors(NetworkScreen.id);
439+
serviceConnection.errorBadgeManager.clearErrorCount(NetworkScreen.id);
440440
final queryFilter = filter.queryFilter;
441441
if (queryFilter.isEmpty) {
442442
_currentNetworkRequests.value.forEach(_checkForError);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ class PerformanceController extends DevToolsScreenController
255255
await serviceConnection.serviceManager.service!.clearVMTimeline();
256256
}
257257
offlinePerformanceData = null;
258-
serviceConnection.errorBadgeManager.clearErrors(PerformanceScreen.id);
258+
serviceConnection.errorBadgeManager.clearErrorCount(PerformanceScreen.id);
259259
await _applyToFeatureControllersAsync((c) => c.clearData(partial: partial));
260260
}
261261

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import '../primitives/query_parameters.dart';
2424

2525
class ErrorBadgeManager extends DisposableController
2626
with AutoDisposeControllerMixin {
27+
// TODO(https://github.com/flutter/devtools/issues/9105): Separate out
28+
// Inspector-specific logic from this file.
2729
final _activeErrorCounts = <String, ValueNotifier<int>>{
2830
InspectorScreen.id: ValueNotifier<int>(0),
2931
PerformanceScreen.id: ValueNotifier<int>(0),
@@ -71,8 +73,9 @@ class ErrorBadgeManager extends DisposableController
7173
}
7274

7375
InspectableWidgetError? _extractInspectableError(Event error) {
74-
// TODO(dantup): Switch to using the inspectorService from the serviceManager
75-
// once Jacob's change to add it lands.
76+
// TODO(https://github.com/flutter/devtools/issues/9105): Switch to using
77+
// the inspectorService from the serviceManager once Jacob's change to add
78+
// it lands.
7679
final node = RemoteDiagnosticsNode(
7780
error.extensionData!.data,
7881
null,
@@ -146,10 +149,15 @@ class ErrorBadgeManager extends DisposableController
146149
return _activeErrorCounts[screenId];
147150
}
148151

149-
void clearErrors(String screenId) {
152+
void clearErrorCount(String screenId) {
150153
_activeErrorCounts[screenId]?.value = 0;
151154
}
152155

156+
void clearErrors(String screenId) {
157+
clearErrorCount(screenId);
158+
_activeErrors[screenId]?.value = LinkedHashMap<String, DevToolsError>();
159+
}
160+
153161
void filterErrors(String screenId, bool Function(String id) isValid) {
154162
final errors = _activeErrors[screenId];
155163
if (errors == null) return;

‎packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ pausing on breakpoint on connection. -
4545

4646
## Inspector updates
4747

48-
TODO: Remove this section if there are not any general updates.
48+
* Fixed bug where errors in the inspector tree (e.g. RenderFlex overflow
49+
errors) were not removed after a hot-reload. -
50+
[#9106](https://github.com/flutter/devtools/pull/9106)
4951

5052
## Performance updates
5153

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

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ void main() {
3434
late ErrorBadgeManager errorBadgeManager;
3535

3636
group('ErrorBadgeManager', () {
37+
int getActiveErrorCount(screenId) =>
38+
errorBadgeManager.erroredItemsForPage(screenId).value.entries.length;
39+
3740
setUp(() {
3841
errorBadgeManager = ErrorBadgeManager();
3942
});
@@ -60,7 +63,7 @@ void main() {
6063
}
6164
});
6265

63-
test('clearErrors resets counts', () {
66+
test('clearErrorCount resets counts', () {
6467
allScreenIds.forEach(errorBadgeManager.incrementBadgeCount);
6568

6669
for (final id in allScreenIds) {
@@ -71,11 +74,55 @@ void main() {
7174
}
7275
}
7376

74-
allScreenIds.forEach(errorBadgeManager.clearErrors);
77+
allScreenIds.forEach(errorBadgeManager.clearErrorCount);
7578

7679
for (final id in allScreenIds) {
7780
expect(errorBadgeManager.errorCountNotifier(id).value, equals(0));
7881
}
7982
});
83+
84+
// TODO(https://github.com/flutter/devtools/issues/9105): This logic should
85+
// be moved to the inspector.
86+
test('appendError works for inspector screen only', () {
87+
for (final id in allScreenIds) {
88+
errorBadgeManager.appendError(id, DevToolsError('An error', id));
89+
}
90+
91+
for (final id in allScreenIds) {
92+
if (id == InspectorScreen.id) {
93+
expect(getActiveErrorCount(id), equals(1));
94+
} else {
95+
expect(getActiveErrorCount(id), equals(0));
96+
}
97+
}
98+
});
99+
100+
test('clearErrors resets counts and removes errors', () {
101+
expect(getActiveErrorCount(InspectorScreen.id), equals(0));
102+
expect(
103+
errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
104+
equals(0),
105+
);
106+
107+
errorBadgeManager.appendError(
108+
InspectorScreen.id,
109+
DevToolsError('An error', InspectorScreen.id),
110+
);
111+
errorBadgeManager.incrementBadgeCount(InspectorScreen.id);
112+
113+
expect(getActiveErrorCount(InspectorScreen.id), equals(1));
114+
expect(
115+
errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
116+
equals(1),
117+
);
118+
119+
errorBadgeManager.clearErrors(InspectorScreen.id);
120+
121+
expect(getActiveErrorCount(InspectorScreen.id), equals(0));
122+
expect(
123+
errorBadgeManager.errorCountNotifier(InspectorScreen.id).value,
124+
equals(0),
125+
);
126+
});
80127
});
81128
}

0 commit comments

Comments
 (0)