Skip to content

Fix stale "no timeline event analysis data" message in Frame Analysis tab #10050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ishaquehassan wants to merge 2 commits into
base: master
Choose a base branch
from
+61 −2
Open

Fix stale "no timeline event analysis data" message in Frame Analysis tab #10050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
811a8cc
Fix stale "no timeline event analysis data" message in Frame Analysis…
ishaquehassan Oct 21, 2026
7f8931b
Use a named method to notify selected frame listeners
ishaquehassan Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion ...tools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class FlutterFramesController extends PerformanceFeatureController {

/// The currently selected timeline frame.
ValueListenable<FlutterFrame?> get selectedFrame => _selectedFrameNotifier;
final _selectedFrameNotifier = ValueNotifier<FlutterFrame?>(null);
final _selectedFrameNotifier = _SelectedFrameNotifier(null);

/// The flutter frames in the chart.
ValueListenable<List<FlutterFrame>> get flutterFrames => _flutterFrames;
Expand Down Expand Up @@ -123,6 +123,12 @@ class FlutterFramesController extends PerformanceFeatureController {
_unassignedFlutterFrames.remove(frameNumber);
_updateFirstWellFormedFrameMicros(frame);
}
if (_selectedFrameNotifier.value == frame) {
// [frame] was selected before its timeline events were available. Notify
// listeners again so that views built from [selectedFrame], like the
// Frame Analysis tab, rebuild with the data that is now available.
_selectedFrameNotifier.notifyFrameDataChanged();
}
}

bool hasUnassignedFlutterFrame(int frameNumber) {
Expand Down Expand Up @@ -223,3 +229,16 @@ class FlutterFramesController extends PerformanceFeatureController {
super.dispose();
}
}

/// Notifier for the selected [FlutterFrame].
///
/// Timeline events are assigned to a [FlutterFrame] in place, which can happen
/// after the frame has been selected. [ValueNotifier] only notifies when
/// [value] changes, and [notifyListeners] is protected, so this subclass
/// exposes [notifyFrameDataChanged] for that case.
class _SelectedFrameNotifier extends ValueNotifier<FlutterFrame?> {

@srawlins srawlins Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this class is introduced. It only contains a controller which passes the value to super, and overrides notifyListeners to call super. On the surface, it looks like a complete no-op from just using ValueNotifier<FlutterFrame?>.

@ishaquehassan ishaquehassan Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah fair, by itself it does look like a no-op. notifyListeners is @protected and @visibleForTesting on ChangeNotifier, so calling it on a plain ValueNotifier from the controller trips invalid_use_of_protected_member and analyze fails. the override was only there to re-expose it (same as ListValueNotifier), but agree that was too hidden, so i swapped it for a named notifyFrameDataChanged() and put the reason in the doc comment 👍

_SelectedFrameNotifier(super.value);

/// Notifies listeners that the selected frame's data changed in place.
void notifyFrameDataChanged() => notifyListeners();
}
Comment on lines +239 to +244

@gemini-code-assist gemini-code-assist Bot Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[MUST-FIX] Calling notifyListeners() on a disposed ValueNotifier will throw an assertion error or exception. Since assignEventToFrame can be called asynchronously (e.g., when a pending timeline poll completes after the controller has been disposed), we should guard notifyListeners() to ensure it is not called after dispose().

Suggested change
class _SelectedFrameNotifier extends ValueNotifier<FlutterFrame?> {
_SelectedFrameNotifier(super.value);
@override
// This override is needed to change visibility of the method.
void notifyListeners() {
super.notifyListeners();
}
}
class _SelectedFrameNotifier extends ValueNotifier<FlutterFrame?> {
_SelectedFrameNotifier(super.value);
bool _disposed = false;
@override
void dispose() {
_disposed = true;
super.dispose();
}
@override
// This override is needed to change visibility of the method.
void notifyListeners() {
if (!_disposed) {
super.notifyListeners();
}
}
}

5 changes: 4 additions & 1 deletion packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ TODO: Remove this section if there are not any updates.

## Performance updates

TODO: Remove this section if there are not any updates.
* Fixed the Frame Analysis tab showing "No timeline event analysis data
available for this frame" when a frame is selected before its timeline events
have been fetched from the VM.
[#10050](https://github.com/flutter/devtools/pull/10050)

## CPU profiler updates

Expand Down
37 changes: 37 additions & 0 deletions .../devtools_app/test/screens/performance/flutter_frames/flutter_frames_controller_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,43 @@ void main() {
expect(framesController.selectedFrame.value, equals(FlutterFrame4.frame));
});

test('notifies listeners when events are assigned to selected frame', () {
when(
// ignore: discarded_futures, by design
mockTimelineEventsController.handleSelectedFrame(any),
).thenAnswer((_) => Future.value());

final frame = FlutterFrame.fromJson({
'number': 3,
'startTime': 40000,
'elapsed': 20000,
'build': 16000,
'raster': 16000,
'vsyncOverhead': 1000,
});
framesController.addFrame(frame);
framesController.handleSelectedFrame(frame);
expect(frame.frameAnalysis, isNull);

var notifyCount = 0;
framesController.selectedFrame.addListener(() => notifyCount++);

// Assigning an event to the selected frame should notify listeners, even
// though the selected frame instance has not changed.
framesController.assignEventToFrame(frame.id, FlutterFrame2.uiEvent);
expect(notifyCount, equals(1));
expect(framesController.selectedFrame.value, equals(frame));
expect(frame.frameAnalysis, isNotNull);

// Assigning an event to a frame that is not selected should not notify.
framesController.addFrame(testFrame2);
framesController.assignEventToFrame(
testFrame2.id,
FlutterFrame2.rasterEvent,
);
expect(notifyCount, equals(1));
});

test('can setOfflineData', () async {
// Ensure we are starting in an empty state.
expect(framesController.flutterFrames.value, isEmpty);
Expand Down
Loading