Address comments

derekxu16 · derekxu16 · commit 6c6074423862 · 2025-06-11T12:42:55.000-04:00
diff --git a/flutter-candidate.txt b/flutter-candidate.txt
@@ -1 +1 @@
-ead0a01bb9707b60097679a43a6262a8979f4a89
+ea83a6a072990acc34c482d8e90f4d2bef713344
diff --git a/packages/devtools_app/lib/src/screens/performance/panes/queued_microtasks/queued_microtasks_controller.dart b/packages/devtools_app/lib/src/screens/performance/panes/queued_microtasks/queued_microtasks_controller.dart
@@ -29,16 +29,16 @@ class QueuedMicrotasksController extends PerformanceFeatureController
     });
   }
 
+  ValueListenable<QueuedMicrotasksControllerStatus> get status => _status;
   final _status = ValueNotifier<QueuedMicrotasksControllerStatus>(
     QueuedMicrotasksControllerStatus.empty,
   );
-  ValueListenable<QueuedMicrotasksControllerStatus> get status => _status;
 
-  final _queuedMicrotasks = ValueNotifier<QueuedMicrotasks?>(null);
   ValueListenable<QueuedMicrotasks?> get queuedMicrotasks => _queuedMicrotasks;
+  final _queuedMicrotasks = ValueNotifier<QueuedMicrotasks?>(null);
 
-  final _selectedMicrotask = ValueNotifier<Microtask?>(null);
   ValueListenable<Microtask?> get selectedMicrotask => _selectedMicrotask;
+  final _selectedMicrotask = ValueNotifier<Microtask?>(null);
 
   final _refreshWorkTracker = FutureWorkTracker();
 
diff --git a/packages/devtools_app/lib/src/screens/performance/panes/queued_microtasks/queued_microtasks_view.dart b/packages/devtools_app/lib/src/screens/performance/panes/queued_microtasks/queued_microtasks_view.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
 
+import 'package:collection/collection.dart' show ListExtensions;
 import 'package:devtools_app_shared/ui.dart';
 import 'package:devtools_app_shared/utils.dart';
 import 'package:flutter/material.dart';
@@ -12,28 +13,26 @@ import '../../../../shared/analytics/constants.dart' as gac;
 import '../../../../shared/primitives/utils.dart' show SortDirection;
 import '../../../../shared/table/table.dart' show FlatTable;
 import '../../../../shared/table/table_data.dart';
-import '../../../../shared/ui/common_widgets.dart' show RefreshButton;
+import '../../../../shared/ui/common_widgets.dart'
+    show CenteredMessage, RefreshButton;
 import 'queued_microtasks_controller.dart';
 
 class RefreshQueuedMicrotasksButton extends StatelessWidget {
-  const RefreshQueuedMicrotasksButton({
-    super.key,
-    required QueuedMicrotasksController controller,
-  }) : _controller = controller;
+  const RefreshQueuedMicrotasksButton({super.key, required this.controller});
 
-  final QueuedMicrotasksController _controller;
+  final QueuedMicrotasksController controller;
 
   @override
   Widget build(BuildContext context) {
     return ValueListenableBuilder<QueuedMicrotasksControllerStatus>(
-      valueListenable: _controller.status,
+      valueListenable: controller.status,
       builder: (_, status, _) {
         return RefreshButton(
           iconOnly: true,
           outlined: false,
           onPressed: status == QueuedMicrotasksControllerStatus.refreshing
               ? null
-              : _controller.refresh,
+              : controller.refresh,
           tooltip:
               "Take a new snapshot of the selected isolate's microtask queue.",
           gaScreen: gac.performance,
@@ -44,23 +43,6 @@ class RefreshQueuedMicrotasksButton extends StatelessWidget {
   }
 }
 
-class QueuedMicrotasksTabControls extends StatelessWidget {
-  const QueuedMicrotasksTabControls({
-    super.key,
-    required QueuedMicrotasksController controller,
-  }) : _controller = controller;
-
-  final QueuedMicrotasksController _controller;
-
-  @override
-  Widget build(BuildContext context) {
-    return Row(
-      mainAxisAlignment: MainAxisAlignment.end,
-      children: [RefreshQueuedMicrotasksButton(controller: _controller)],
-    );
-  }
-}
-
 class RefreshQueuedMicrotasksInstructions extends StatelessWidget {
   const RefreshQueuedMicrotasksInstructions({super.key});
 
@@ -85,60 +67,52 @@ class RefreshQueuedMicrotasksInstructions extends StatelessWidget {
   }
 }
 
-// In the response returned by the VM Service, microtasks are sorted in
-// ascending order of when they will be dequeued, i.e. the microtask that will
-// run earliest is at index 0 of the returned list. We use those indices of the
-// returned list to sort the entries of the microtask selector, so that they
-// they also appear in ascending order of when they will be dequeued.
-typedef IndexedMicrotask = (int, Microtask);
+/// In the response returned by the VM Service, microtasks are sorted in
+/// ascending order of when they will be dequeued, i.e. the microtask that will
+/// run earliest is at index 0 of the returned list. We use those indices of the
+/// returned list to sort the entries of the microtask selector, so that they
+/// they also appear in ascending order of when they will be dequeued.
+typedef IndexedMicrotask = ({int index, Microtask microtask});
 
 class _MicrotaskIdColumn extends ColumnData<IndexedMicrotask> {
   _MicrotaskIdColumn()
     : super.wide('Microtask ID', alignment: ColumnAlignment.center);
 
   @override
-  int getValue(IndexedMicrotask indexedMicrotask) => indexedMicrotask.$1;
+  int getValue(IndexedMicrotask indexedMicrotask) => indexedMicrotask.index;
 
   @override
   String getDisplayValue(IndexedMicrotask indexedMicrotask) =>
-      indexedMicrotask.$2.id!.toString();
+      indexedMicrotask.microtask.id!.toString();
 }
 
 class QueuedMicrotaskSelector extends StatelessWidget {
   const QueuedMicrotaskSelector({
     super.key,
     required List<IndexedMicrotask> indexedMicrotasks,
-    required void Function(Microtask?) setSelectedMicrotask,
+    required void Function(Microtask?) onMicrotaskSelected,
   }) : _indexedMicrotasks = indexedMicrotasks,
-       _setSelectedMicrotask = setSelectedMicrotask;
+       _setSelectedMicrotask = onMicrotaskSelected;
 
   static final _idColumn = _MicrotaskIdColumn();
   final List<IndexedMicrotask> _indexedMicrotasks;
   final void Function(Microtask?) _setSelectedMicrotask;
 
   @override
-  Widget build(BuildContext context) => Column(
-    crossAxisAlignment: CrossAxisAlignment.start,
-    children: [
-      Expanded(
-        child: FlatTable<IndexedMicrotask>(
-          keyFactory: (IndexedMicrotask microtask) =>
-              ValueKey<int>(microtask.$1),
-          data: _indexedMicrotasks,
-          dataKey: 'queued-microtask-selector',
-          columns: [_idColumn],
-          defaultSortColumn: _idColumn,
-          defaultSortDirection: SortDirection.ascending,
-          onItemSelected: (indexedMicrotask) =>
-              _setSelectedMicrotask(indexedMicrotask?.$2),
-        ),
-      ),
-    ],
+  Widget build(BuildContext context) => FlatTable<IndexedMicrotask>(
+    keyFactory: (IndexedMicrotask microtask) => ValueKey<int>(microtask.index),
+    data: _indexedMicrotasks,
+    dataKey: 'queued-microtask-selector',
+    columns: [_idColumn],
+    defaultSortColumn: _idColumn,
+    defaultSortDirection: SortDirection.ascending,
+    onItemSelected: (indexedMicrotask) =>
+        _setSelectedMicrotask(indexedMicrotask?.microtask),
   );
 }
 
-class StackTraceView extends StatelessWidget {
-  const StackTraceView({super.key, required selectedMicrotask})
+class MicrotaskStackTraceView extends StatelessWidget {
+  const MicrotaskStackTraceView({super.key, required selectedMicrotask})
     : _selectedMicrotask = selectedMicrotask;
 
   final Microtask? _selectedMicrotask;
@@ -149,14 +123,11 @@ class StackTraceView extends StatelessWidget {
 
     return Column(
       children: [
-        SizedBox.fromSize(
-          size: Size.fromHeight(defaultHeaderHeight),
-          child: Container(
-            decoration: BoxDecoration(
-              border: Border(bottom: defaultBorderSide(theme)),
-            ),
-            padding: const EdgeInsets.only(left: defaultSpacing),
-            alignment: Alignment.centerLeft,
+        Container(
+          height: defaultHeaderHeight,
+          padding: const EdgeInsets.only(left: defaultSpacing),
+          alignment: Alignment.centerLeft,
+          child: OutlineDecoration.onlyBottom(
             child: const Row(
               children: [
                 Text('Stack trace captured when microtask was enqueued'),
@@ -207,20 +178,20 @@ class _QueuedMicrotasksTabViewState extends State<QueuedMicrotasksTabView>
         if (status == QueuedMicrotasksControllerStatus.empty) {
           return const RefreshQueuedMicrotasksInstructions();
         } else if (status == QueuedMicrotasksControllerStatus.refreshing) {
-          return Center(
-            child: Text(
-              style: Theme.of(context).regularTextStyle,
-              'Refreshing...',
-            ),
-          );
+          return const CenteredMessage(message: 'Refreshing...');
         } else {
           return ValueListenableBuilder(
             valueListenable: widget.controller.queuedMicrotasks,
             builder: (_, queuedMicrotasks, _) {
               assert(queuedMicrotasks != null);
-
-              final indexedMicrotasks = queuedMicrotasks!.microtasks!.indexed
-                  .cast<IndexedMicrotask>()
+              if (queuedMicrotasks == null) {
+                return const CenteredMessage(message: 'Unexpected null value');
+              }
+
+              final indexedMicrotasks = queuedMicrotasks.microtasks!
+                  .mapIndexed(
+                    (index, microtask) => (index: index, microtask: microtask),
+                  )
                   .toList();
               final formattedTimestamp = _dateTimeFormat.format(
                 DateTime.fromMicrosecondsSinceEpoch(
@@ -244,25 +215,28 @@ class _QueuedMicrotasksTabViewState extends State<QueuedMicrotasksTabView>
                       axis: Axis.horizontal,
                       initialFractions: const [0.15, 0.85],
                       children: [
-                        QueuedMicrotaskSelector(
-                          indexedMicrotasks: indexedMicrotasks,
-                          setSelectedMicrotask:
-                              widget.controller.setSelectedMicrotask,
+                        OutlineDecoration(
+                          child: QueuedMicrotaskSelector(
+                            indexedMicrotasks: indexedMicrotasks,
+                            onMicrotaskSelected:
+                                widget.controller.setSelectedMicrotask,
+                          ),
                         ),
                         ValueListenableBuilder(
                           valueListenable: widget.controller.selectedMicrotask,
                           builder: (_, selectedMicrotask, _) =>
-                              selectedMicrotask == null
-                              ? const Center(
-                                  child: Text(
-                                    'Select a microtask ID on the left '
-                                    'to see information about the '
-                                    'corresponding microtask.',
-                                  ),
-                                )
-                              : StackTraceView(
-                                  selectedMicrotask: selectedMicrotask,
-                                ),
+                              OutlineDecoration(
+                                child: selectedMicrotask == null
+                                    ? const CenteredMessage(
+                                        message:
+                                            'Select a microtask ID on the left '
+                                            'to see information about the '
+                                            'corresponding microtask.',
+                                      )
+                                    : MicrotaskStackTraceView(
+                                        selectedMicrotask: selectedMicrotask,
+                                      ),
+                              ),
                         ),
                       ],
                     ),
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
@@ -181,7 +181,7 @@ class _TabbedPerformanceViewState extends State<TabbedPerformanceView>
         (
           tab: _buildTab(
             tabName: 'Queued Microtasks',
-            trailing: QueuedMicrotasksTabControls(
+            trailing: RefreshQueuedMicrotasksButton(
               controller: controller.queuedMicrotasksController,
             ),
           ),
diff --git a/packages/devtools_app/lib/src/service/vm_flags.dart b/packages/devtools_app/lib/src/service/vm_flags.dart
@@ -16,7 +16,7 @@ const profiler = 'profiler';
 // Defined in SDK: https://github.com/dart-lang/sdk/blob/master/runtime/vm/profiler.cc#L36
 const profilePeriod = 'profile_period';
 
-// Defined in SDK: https://github.com/dart-lang/sdk/blob/main/runtime/vm/microtask_mirror_queues.cc#L18-L23.
+// Defined in SDK: https://github.com/dart-lang/sdk/blob/main/runtime/vm/microtask_mirror_queues.cc.
 const profileMicrotasks = 'profile_microtasks';
 
 class VmFlagManager with DisposerMixin {
diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml
@@ -22,7 +22,7 @@ dependencies:
   collection: ^1.15.0
   dap: ^1.1.0
   dart_service_protocol_shared: ^0.0.3
-  dds_service_extensions: ^2.0.0
+  dds_service_extensions: ^2.0.2
   devtools_app_shared:
   devtools_extensions:
   devtools_shared:
@@ -53,7 +53,7 @@ dependencies:
   stack_trace: ^1.12.0
   string_scanner: ^1.4.0
   unified_analytics: ^7.0.0
-  vm_service: '>=15.0.0 <16.0.0'
+  vm_service: ^15.0.2
   vm_service_protos: ^1.0.0
   vm_snapshot_analysis: ^0.7.6
   web: ^1.0.0
diff --git a/packages/devtools_app/test/screens/performance/queued_microtasks/queued_microtasks_controller_test.dart b/packages/devtools_app/test/screens/performance/queued_microtasks/queued_microtasks_controller_test.dart
@@ -28,7 +28,11 @@ void main() {
     });
 
     test('refresh', () async {
+      expect(controller.status.value, QueuedMicrotasksControllerStatus.empty);
+      expect(controller.queuedMicrotasks.value, null);
+
       await controller.refresh();
+
       expect(controller.status.value, QueuedMicrotasksControllerStatus.ready);
       expect(controller.queuedMicrotasks.value, testQueuedMicrotasks);
     });
diff --git a/packages/devtools_app/test/screens/performance/tabbed_performance_view_test.dart b/packages/devtools_app/test/screens/performance/tabbed_performance_view_test.dart
@@ -34,7 +34,7 @@ void main() {
         vmFlags: [
           (
             flagName: profileMicrotasks,
-            value: shouldEnableMicrotaskProfiling ? 'true' : 'false',
+            value: shouldEnableMicrotaskProfiling.toString(),
           ),
         ],
       ),
@@ -260,11 +260,14 @@ void main() {
           final mockQueuedMicrotasksController =
               controller.queuedMicrotasksController
                   as MockQueuedMicrotasksController;
-          when(mockQueuedMicrotasksController.status).thenReturn(
-            const FixedValueListenable<QueuedMicrotasksControllerStatus>(
-              QueuedMicrotasksControllerStatus.empty,
-            ),
-          );
+
+          final mockQueuedMicrotasksControllerStatus =
+              ValueNotifier<QueuedMicrotasksControllerStatus>(
+                QueuedMicrotasksControllerStatus.empty,
+              );
+          when(
+            mockQueuedMicrotasksController.status,
+          ).thenReturn(mockQueuedMicrotasksControllerStatus);
 
           await pumpView(tester);
 
@@ -284,25 +287,19 @@ void main() {
           // Then, we verify that details about the queued microtasks are shown
           // when [controller.queuedMicrotasksController]'s status is ready.
 
-          when(mockQueuedMicrotasksController.status).thenReturn(
-            const FixedValueListenable<QueuedMicrotasksControllerStatus>(
-              QueuedMicrotasksControllerStatus.ready,
-            ),
-          );
+          mockQueuedMicrotasksControllerStatus.value =
+              QueuedMicrotasksControllerStatus.ready;
           when(
             mockQueuedMicrotasksController.selectedMicrotask,
           ).thenReturn(FixedValueListenable(testMicrotask));
           when(
             mockQueuedMicrotasksController.queuedMicrotasks,
           ).thenReturn(FixedValueListenable(testQueuedMicrotasks));
 
-          await tester.tap(find.text('Timeline Events'));
-          await tester.pumpAndSettle();
-          await tester.tap(find.text('Queued Microtasks'));
           await tester.pumpAndSettle();
 
           expect(find.byType(QueuedMicrotaskSelector), findsOneWidget);
-          expect(find.byType(StackTraceView), findsOneWidget);
+          expect(find.byType(MicrotaskStackTraceView), findsOneWidget);
         });
       },
     );

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-ead0a01bb9707b60097679a43a6262a8979f4a89`
	`1`	`+ea83a6a072990acc34c482d8e90f4d2bef713344`
Original file line number	Diff line number	Diff line change
`@@ -181,7 +181,7 @@ class _TabbedPerformanceViewState extends State<TabbedPerformanceView>`
`181`	`181`	`(`
`182`	`182`	`tab: _buildTab(`
`183`	`183`	`tabName: 'Queued Microtasks',`
`184`		`- trailing: QueuedMicrotasksTabControls(`
	`184`	`+ trailing: RefreshQueuedMicrotasksButton(`
`185`	`185`	`controller: controller.queuedMicrotasksController,`
`186`	`186`	`),`
`187`	`187`	`),`