diff --git a/packages/devtools_app/lib/src/screens/performance/panes/controls/performance_controls.dart b/packages/devtools_app/lib/src/screens/performance/panes/controls/performance_controls.dart index d1d5ba807c9..6da95046cd0 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/controls/performance_controls.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/controls/performance_controls.dart @@ -148,7 +148,10 @@ class _SecondaryPerformanceControls extends StatelessWidget { const SizedBox(width: denseSpacing), OpenSaveButtonGroup( screenId: ScreenMetaData.performance.id, - onSave: (_) => controller.exportData(), + onSave: (_) async { + await controller.fetchFullDataBeforeExport(); + controller.exportData(); + }, ), if (isFlutterApp) ...[ const SizedBox(width: denseSpacing), diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart index 043bac51281..1dfb79897b1 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart @@ -118,6 +118,9 @@ class TimelineEventsController extends PerformanceFeatureController int _nextPollStartMicros = 0; + /// The most recent pull of the VM timeline, if any. + Future? _latestPull; + /// The number of requests per second that we allow for pulling the VM /// timeline. /// @@ -170,7 +173,26 @@ class TimelineEventsController extends PerformanceFeatureController }); } - Future _pullPerfettoVmTimeline({bool isInitialPull = false}) async { + /// Pulls the VM timeline events that have not been received yet. + /// + /// Pulls can be started by polling, [forceRefresh], and + /// [pullLatestTimelineEvents], so each pull waits for the previous one to + /// finish. Otherwise two pulls could request the same time range and add + /// the same events to the trace twice. + Future _pullPerfettoVmTimeline({bool isInitialPull = false}) { + final previousPull = _latestPull; + return _latestPull = () async { + if (previousPull != null) { + // Errors from the previous pull are handled by its own caller. + await previousPull.catchError((_) {}); + } + await _pullPerfettoVmTimelineHelper(isInitialPull: isInitialPull); + }(); + } + + Future _pullPerfettoVmTimelineHelper({ + bool isInitialPull = false, + }) async { final service = serviceConnection.serviceManager.service; if (service == null) return; final currentVmTime = await service.getVMTimelineMicros(); @@ -342,6 +364,18 @@ class TimelineEventsController extends PerformanceFeatureController await loadPerfettoTrace(); } + /// Pulls any timeline events from the VM that have not been received yet. + /// + /// Unlike [forceRefresh], this does not reload the Perfetto trace viewer. + Future pullLatestTimelineEvents() async { + try { + await _pullPerfettoVmTimeline(); + } catch (e, st) { + // Don't block the export of the data we already have. + _log.warning('Failed to pull the latest timeline events.', e, st); + } + } + void processTrackEvents() { if (!_isFlutterAppHelper()) { debugTraceCallback( diff --git a/packages/devtools_app/lib/src/screens/performance/performance_controller.dart b/packages/devtools_app/lib/src/screens/performance/performance_controller.dart index b9e158009c5..bc4d295efd0 100644 --- a/packages/devtools_app/lib/src/screens/performance/performance_controller.dart +++ b/packages/devtools_app/lib/src/screens/performance/performance_controller.dart @@ -291,6 +291,20 @@ class PerformanceController extends DevToolsScreenController ).toJson(), ); + /// Pulls the latest timeline events so that they are included in the export. + /// + /// Timeline events are only polled periodically, so the most recent events + /// would otherwise be missing from the export. + Future fetchFullDataBeforeExport() async { + final serviceManager = serviceConnection.serviceManager; + if (offlineDataController.showingOfflineData.value || + !serviceManager.connectedState.value.connected || + (serviceManager.connectedApp?.isDartWebAppNow ?? false)) { + return; + } + await timelineEventsController.pullLatestTimelineEvents(); + } + @override FutureOr releaseMemory({bool partial = false}) async { await clearData(partial: partial, clearVmTimeline: !partial); diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 338e629da83..3163d93b0e0 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -44,6 +44,10 @@ TODO: Remove this section if there are not any updates. 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) +* Fixed a bug where data exported from the Performance screen could be + missing the most recent timeline events. - + [#10055](https://github.com/flutter/devtools/pull/10055), + [#9731](https://github.com/flutter/devtools/issues/9731) ## CPU profiler updates diff --git a/packages/devtools_app/test/screens/performance/performance_controller_test.dart b/packages/devtools_app/test/screens/performance/performance_controller_test.dart index b33f7381334..d239790cf27 100644 --- a/packages/devtools_app/test/screens/performance/performance_controller_test.dart +++ b/packages/devtools_app/test/screens/performance/performance_controller_test.dart @@ -49,5 +49,17 @@ void main() { expect(controller.flutterFramesController.isActiveFeature, isTrue); expect(controller.timelineEventsController.isActiveFeature, isTrue); }); + + test('fetchFullDataBeforeExport does not pull timeline events when showing ' + 'offline data', () async { + final mockServiceManager = + mockServiceConnection.serviceManager as MockServiceManager; + clearInteractions(mockServiceManager); + + await controller.fetchFullDataBeforeExport(); + + verifyNever(mockServiceManager.service); + expect(controller.timelineEventsController.fullPerfettoTrace, isEmpty); + }); }); } diff --git a/packages/devtools_app/test/screens/performance/timeline_events/timeline_events_controller_test.dart b/packages/devtools_app/test/screens/performance/timeline_events/timeline_events_controller_test.dart index 740325d17c5..3b9ec5ac2ee 100644 --- a/packages/devtools_app/test/screens/performance/timeline_events/timeline_events_controller_test.dart +++ b/packages/devtools_app/test/screens/performance/timeline_events/timeline_events_controller_test.dart @@ -10,6 +10,7 @@ import 'package:devtools_app_shared/utils.dart'; import 'package:devtools_test/devtools_test.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; +import 'package:vm_service/vm_service.dart'; import '../../../test_infra/test_data/performance/sample_performance_data.dart'; @@ -82,6 +83,64 @@ void main() { ); }); + group('pullLatestTimelineEvents', () { + setUp(() { + setGlobal(PreferencesController, PreferencesController()); + }); + + test('adds the latest VM timeline to the trace', () async { + expect(eventsController.fullPerfettoTrace, isEmpty); + + await eventsController.pullLatestTimelineEvents(); + + expect(eventsController.fullPerfettoTrace, isNotEmpty); + }); + + test('completes without throwing when the pull fails', () async { + // This fake service throws when the VM timeline is requested because + // it was not provided any timeline data. + setGlobal(ServiceConnectionManager, FakeServiceConnectionManager()); + + await expectLater( + eventsController.pullLatestTimelineEvents(), + completes, + ); + expect(eventsController.fullPerfettoTrace, isEmpty); + }); + + test('does not request the same time range twice', () async { + final mockService = MockVmServiceWrapper(); + var currentVmTimeMicros = 0; + when(mockService.getVMTimelineMicros()).thenAnswer((_) async { + currentVmTimeMicros += 100; + return Timestamp(timestamp: currentVmTimeMicros); + }); + final requestedTimeOrigins = []; + when( + mockService.getPerfettoVMTimeline( + timeOriginMicros: anyNamed('timeOriginMicros'), + timeExtentMicros: anyNamed('timeExtentMicros'), + ), + ).thenAnswer((invocation) async { + requestedTimeOrigins.add( + invocation.namedArguments[#timeOriginMicros] as int?, + ); + return perfettoVmTimeline; + }); + final serviceConnectionWithMock = FakeServiceConnectionManager(); + serviceConnectionWithMock.serviceManager.service = mockService; + setGlobal(ServiceConnectionManager, serviceConnectionWithMock); + + // Start the second pull before the first one has finished. + await [ + eventsController.pullLatestTimelineEvents(), + eventsController.pullLatestTimelineEvents(), + ].wait; + + expect(requestedTimeOrigins, equals([0, 101])); + }); + }); + group('can clearData', () { setUp(() async { // Set some data in the controller. It does not matter if it is connected