Performance screen cleanups (#4751)
diff --git a/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart b/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart index e998cb4..e2f7cb7 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart
@@ -498,7 +498,7 @@ ), ); } - unawaited(framesController.toggleSelectedFrame(frame)); + framesController.handleSelectedFrame(frame); } } @@ -516,7 +516,7 @@ final bool hasShaderJank; - static const double _moreInfoLinkWidth = 85.0; + static const double _moreInfoLinkWidth = 100.0; static const _textMeasurementBuffer = 4.0;
diff --git a/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart b/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart index b613b04..a7e4936 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart
@@ -140,15 +140,6 @@ return _unassignedFlutterFrames.containsKey(frameNumber); } - Future<void> toggleSelectedFrame(FlutterFrame frame) async { - handleSelectedFrame(frame); - // We do not need to block the UI on the TimelineEvents feature loading the - // selected frame. - unawaited( - performanceController.timelineEventsController.handleSelectedFrame(frame), - ); - } - void _addPendingFlutterFrames() { _pendingFlutterFrames.forEach(_maybeBadgeTabForJankyFrame); data!.frames.addAll(_pendingFlutterFrames); @@ -210,6 +201,12 @@ _data.selectedFrame = frame; _selectedFrameNotifier.value = frame; + + // We do not need to block the UI on the TimelineEvents feature loading the + // selected frame. + unawaited( + performanceController.timelineEventsController.handleSelectedFrame(frame), + ); } @override
diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_event_processor.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_event_processor.dart index 7afb8e6..9ebac6c 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_event_processor.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_event_processor.dart
@@ -42,7 +42,7 @@ /// Notifies with the current progress value of processing Timeline data. /// /// This value should sit between 0.0 and 1.0. - ValueListenable get progressNotifier => _progressNotifier; + ValueListenable<double> get progressNotifier => _progressNotifier; final _progressNotifier = ValueNotifier<double>(0.0); int _traceEventsProcessed = 0;
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 0cd511c..f08dd8d 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
@@ -37,11 +37,25 @@ /// Debugging flag to load sample trace events from [simple_trace_example.dart]. bool debugSimpleTrace = false; +enum EventsControllerStatus { + empty, + processing, + ready, +} + class TimelineEventsController extends PerformanceFeatureController with AutoDisposeControllerMixin { TimelineEventsController(super.performanceController) { legacyController = LegacyTimelineEventsController(performanceController); perfettoController = PerfettoController(performanceController); + addAutoDisposeListener(_workTracker.active, () { + final active = _workTracker.active.value; + if (active) { + _status.value = EventsControllerStatus.processing; + } else { + _status.value = EventsControllerStatus.ready; + } + }); } /// Controller that contains business logic for the legacy trace viewer. @@ -75,7 +89,10 @@ FeatureFlags.embeddedPerfetto && !useLegacyTraceViewer.value; /// Whether the recorded timeline data is currently being processed. - ValueListenable<bool> get processing => _workTracker.active; + ValueListenable<EventsControllerStatus> get status => _status; + final _status = + ValueNotifier<EventsControllerStatus>(EventsControllerStatus.empty); + final _workTracker = FutureWorkTracker(); // TODO(jacobr): this isn't accurate. Another page of DevTools @@ -251,10 +268,7 @@ ); } - legacyController.processor.primeThreadIds( - uiThreadId: uiThreadId, - rasterThreadId: rasterThreadId, - ); + _primeThreadIds(uiThreadId: uiThreadId, rasterThreadId: rasterThreadId); } } @@ -443,9 +457,10 @@ data!.traceEvents.add(trace); } - void _primeThreadIds(List<TraceEventWrapper> traceEvents) { - final uiThreadId = _threadIdForEvents({uiEventName}, traceEvents); - final rasterThreadId = _threadIdForEvents({rasterEventName}, traceEvents); + void _primeThreadIds({ + required int? uiThreadId, + required int? rasterThreadId, + }) { legacyController.processor.primeThreadIds( uiThreadId: uiThreadId, rasterThreadId: rasterThreadId, @@ -479,10 +494,17 @@ allTraceEvents ..clear() ..addAll(traceEvents); - _primeThreadIds(traceEvents); + + final uiThreadId = _threadIdForEvents({uiEventName}, traceEvents); + final rasterThreadId = _threadIdForEvents({rasterEventName}, traceEvents); + _primeThreadIds(uiThreadId: uiThreadId, rasterThreadId: rasterThreadId); await processAllTraceEvents(); await legacyController.setOfflineData(offlineData); + + if (offlineData.selectedFrame != null && _perfettoMode) { + // TODO(kenz): scroll the perfetto viewer to the selected time range. + } } @override @@ -493,6 +515,7 @@ threadNamesById.clear(); _workTracker.clear(); legacyController.clearData(); + _status.value = EventsControllerStatus.empty; if (FeatureFlags.embeddedPerfetto) { await perfettoController.clear(); }
diff --git a/packages/devtools_app/lib/src/screens/performance/performance_model.dart b/packages/devtools_app/lib/src/screens/performance/performance_model.dart index 1e9b548..f007687 100644 --- a/packages/devtools_app/lib/src/screens/performance/performance_model.dart +++ b/packages/devtools_app/lib/src/screens/performance/performance_model.dart
@@ -5,6 +5,7 @@ import 'dart:collection'; import 'dart:math' as math; +import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import '../../charts/flame_chart.dart'; @@ -333,6 +334,8 @@ final frames = framesJson .map((Map<String, dynamic> f) => FlutterFrame.parse(f)) .toList(); + final selectedFrame = + frames.firstWhereOrNull((frame) => frame.id == selectedFrameId); final Map<String, dynamic> selectedEventJson = json[PerformanceData.selectedEventKey] ?? {}; @@ -354,6 +357,7 @@ return OfflinePerformanceData._( traceEvents: traceEvents, + selectedFrame: selectedFrame, selectedFrameId: selectedFrameId, frames: frames, selectedEvent: selectedEvent,
diff --git a/packages/devtools_app/lib/src/screens/performance/performance_screen.dart b/packages/devtools_app/lib/src/screens/performance/performance_screen.dart index 94d959c..139a463 100644 --- a/packages/devtools_app/lib/src/screens/performance/performance_screen.dart +++ b/packages/devtools_app/lib/src/screens/performance/performance_screen.dart
@@ -26,7 +26,6 @@ import 'panes/controls/layer_debugging_options.dart'; import 'panes/controls/performance_settings.dart'; import 'panes/flutter_frames/flutter_frames_chart.dart'; -import 'panes/flutter_frames/flutter_frames_controller.dart'; import 'panes/timeline_events/timeline_events_controller.dart'; import 'performance_controller.dart'; import 'performance_model.dart'; @@ -66,14 +65,6 @@ AutoDisposeMixin, OfflineScreenMixin<PerformanceScreenBody, OfflinePerformanceData>, ProvidedControllerMixin<PerformanceController, PerformanceScreenBody> { - bool processing = false; - - double processingProgress = 0.0; - - late TimelineEventsController _timelineEventsController; - - late FlutterFramesController _flutterFramesController; - @override void initState() { super.initState(); @@ -101,28 +92,9 @@ if (!initController()) return; - _timelineEventsController = controller.timelineEventsController; - _flutterFramesController = controller.flutterFramesController; - cancelListeners(); - processing = controller.timelineEventsController.processing.value; - addAutoDisposeListener(controller.timelineEventsController.processing, () { - setState(() { - processing = controller.timelineEventsController.processing.value; - }); - }); - - final legacyProcessor = - _timelineEventsController.legacyController.processor; - processingProgress = legacyProcessor.progressNotifier.value; - addAutoDisposeListener(legacyProcessor.progressNotifier, () { - setState(() { - processingProgress = legacyProcessor.progressNotifier.value; - }); - }); - - addAutoDisposeListener(_flutterFramesController.selectedFrame); + addAutoDisposeListener(controller.flutterFramesController.selectedFrame); // Load offline timeline data if available. if (shouldLoadOfflineData()) { @@ -149,10 +121,6 @@ controller.offlinePerformanceData != null && controller.offlinePerformanceData!.frames.isNotEmpty; - final tabbedPerformanceView = TabbedPerformanceView( - processing: processing, - processingProgress: processingProgress, - ); final performanceScreen = Column( children: [ if (!offlineController.offlineMode.value) _buildPerformanceControls(), @@ -160,8 +128,8 @@ if (isOfflineFlutterApp || (!offlineController.offlineMode.value && serviceManager.connectedApp!.isFlutterAppNow!)) - FlutterFramesChart(_flutterFramesController), - Expanded(child: tabbedPerformanceView), + FlutterFramesChart(controller.flutterFramesController), + const Expanded(child: TabbedPerformanceView()), ], ); @@ -185,10 +153,15 @@ return Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ - _PrimaryControls( - controller: controller, - processing: processing, - onClear: () => setState(() {}), + ValueListenableBuilder<EventsControllerStatus>( + valueListenable: controller.timelineEventsController.status, + builder: (context, status, _) { + return _PrimaryControls( + controller: controller, + processing: status == EventsControllerStatus.processing, + onClear: () => setState(() {}), + ); + }, ), const SizedBox(width: defaultSpacing), SecondaryPerformanceControls(controller: controller),
diff --git a/packages/devtools_app/lib/src/screens/performance/tabbed_performance_view.dart b/packages/devtools_app/lib/src/screens/performance/tabbed_performance_view.dart index 22f1f02..cc36913 100644 --- a/packages/devtools_app/lib/src/screens/performance/tabbed_performance_view.dart +++ b/packages/devtools_app/lib/src/screens/performance/tabbed_performance_view.dart
@@ -31,14 +31,7 @@ final timelineSearchFieldKey = GlobalKey(debugLabel: 'TimelineSearchFieldKey'); class TabbedPerformanceView extends StatefulWidget { - const TabbedPerformanceView({ - required this.processing, - required this.processingProgress, - }); - - final bool processing; - - final double processingProgress; + const TabbedPerformanceView(); @override _TabbedPerformanceViewState createState() => _TabbedPerformanceViewState(); @@ -90,7 +83,6 @@ showRebuildStats = showRebuildStats && offlineData.rebuildCountModel.isNotEmpty; } - final tabRecords = <_PerformanceTabRecord>[ if (showFrameAnalysis) _frameAnalysisRecord(), if (showRasterStats) _rasterStatsRecord(), @@ -219,9 +211,6 @@ } _PerformanceTabRecord _timelineEventsRecord() { - final data = controller.data; - final hasData = data != null && !data.isEmpty; - final searchFieldEnabled = hasData && !widget.processing; return _PerformanceTabRecord( tab: _buildTab( tabName: 'Timeline Events', @@ -232,7 +221,14 @@ mainAxisAlignment: MainAxisAlignment.end, children: [ if (useLegacy || !FeatureFlags.embeddedPerfetto) ...[ - _buildSearchField(searchFieldEnabled), + ValueListenableBuilder<EventsControllerStatus>( + valueListenable: _timelineEventsController.status, + builder: (context, status, _) { + final searchFieldEnabled = + status == EventsControllerStatus.ready; + return _buildSearchField(searchFieldEnabled); + }, + ), const FlameChartHelpButton( gaScreen: PerformanceScreen.id, gaSelection: analytics_constants.timelineFlameChartHelp, @@ -252,10 +248,18 @@ builder: (context, useLegacy, _) { return (useLegacy || !FeatureFlags.embeddedPerfetto) ? KeepAliveWrapper( - child: TimelineEventsView( - controller: _timelineEventsController, - processing: widget.processing, - processingProgress: widget.processingProgress, + child: DualValueListenableBuilder<EventsControllerStatus, + double>( + firstListenable: _timelineEventsController.status, + secondListenable: _timelineEventsController + .legacyController.processor.progressNotifier, + builder: (context, status, processingProgress, _) { + return TimelineEventsView( + controller: _timelineEventsController, + processing: status == EventsControllerStatus.processing, + processingProgress: processingProgress, + ); + }, ), ) : KeepAliveWrapper(
diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 514c52d..23778fe 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml
@@ -18,6 +18,8 @@ # Pin ansicolor to version before pre-NNBD version 1.1.0, should be ^1.0.5 # See https://github.com/flutter/devtools/issues/2530 ansicolor: ^2.0.0 + # TODO: https://github.com/flutter/devtools/issues/4728 - remove constraint when archive is fixed + archive: <3.3.3 async: ^2.0.0 codicon: ^1.0.0 collection: ^1.15.0
diff --git a/packages/devtools_app/test/performance/flutter_frames/flutter_frames_controller_test.dart b/packages/devtools_app/test/performance/flutter_frames/flutter_frames_controller_test.dart index 0fa0614..54868ac 100644 --- a/packages/devtools_app/test/performance/flutter_frames/flutter_frames_controller_test.dart +++ b/packages/devtools_app/test/performance/flutter_frames/flutter_frames_controller_test.dart
@@ -84,7 +84,7 @@ // Select a frame. expect(framesController.selectedFrame.value, isNull); - await framesController.toggleSelectedFrame(frame0); + framesController.handleSelectedFrame(frame0); expect( framesController.selectedFrame.value, equals(frame0), @@ -94,14 +94,14 @@ expect(timelineControllerHandlerCalled, isTrue); // Unselect this frame. - await framesController.toggleSelectedFrame(frame0); + framesController.handleSelectedFrame(frame0); expect( framesController.selectedFrame.value, isNull, ); // Select a different frame. - await framesController.toggleSelectedFrame(frame1); + framesController.handleSelectedFrame(frame1); expect( framesController.selectedFrame.value, equals(frame1),
diff --git a/packages/devtools_app/test/performance/tabbed_performance_view_test.dart b/packages/devtools_app/test/performance/tabbed_performance_view_test.dart index 8404f93..f650a79 100644 --- a/packages/devtools_app/test/performance/tabbed_performance_view_test.dart +++ b/packages/devtools_app/test/performance/tabbed_performance_view_test.dart
@@ -60,6 +60,11 @@ when(mockTimelineEventsController.data).thenReturn(controller.data); when(mockTimelineEventsController.useLegacyTraceViewer) .thenReturn(ValueNotifier<bool>(true)); + when(mockTimelineEventsController.status).thenReturn( + const FixedValueListenable<EventsControllerStatus>( + EventsControllerStatus.ready, + ), + ); when(mockTimelineEventsController.legacyController) .thenReturn(LegacyTimelineEventsController(controller)); when(controller.timelineEventsController) @@ -90,10 +95,7 @@ await tester.pumpWidget( wrapWithControllers( - const TabbedPerformanceView( - processing: false, - processingProgress: 0.0, - ), + const TabbedPerformanceView(), performance: controller, ), );
diff --git a/packages/devtools_app/test/performance/timeline_events/timeline_flame_chart_test.dart b/packages/devtools_app/test/performance/timeline_events/timeline_flame_chart_test.dart index 147948a..8c59a63 100644 --- a/packages/devtools_app/test/performance/timeline_events/timeline_flame_chart_test.dart +++ b/packages/devtools_app/test/performance/timeline_events/timeline_flame_chart_test.dart
@@ -164,8 +164,8 @@ .addTimelineEvent(goldenRasterTimelineEvent); final data = controller.data!; expect(data.frames.length, equals(1)); - await controller.flutterFramesController - .toggleSelectedFrame(data.frames.first); + controller.flutterFramesController + .handleSelectedFrame(data.frames.first); await tester.pumpAndSettle(); }); expect(find.byType(TimelineFlameChart), findsOneWidget);