Cleanup UX for widget build counts feature (flutter#7847)

packages/devtools_app/lib/devtools_app.dart

@@ -39,6 +39,7 @@ export 'src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart';
 export 'src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart';
 export 'src/screens/performance/panes/frame_analysis/frame_analysis_model.dart';
 export 'src/screens/performance/panes/raster_stats/raster_stats_controller.dart';
+export 'src/screens/performance/panes/rebuild_stats/rebuild_stats_controller.dart';
 export 'src/screens/performance/panes/rebuild_stats/rebuild_stats_model.dart';
 export 'src/screens/performance/panes/timeline_events/perfetto/tracing/model.dart';
 export 'src/screens/performance/panes/timeline_events/timeline_events_controller.dart';

packages/devtools_app/lib/src/screens/performance/panes/controls/more_debugging_options.dart

@@ -29,7 +29,7 @@ class MoreDebuggingOptionsButton extends StatelessWidget {
         extensions.disableClipLayers,
         extensions.disableOpacityLayers,
         extensions.disablePhysicalShapeLayers,
-        if (FeatureFlags.widgetRebuildStats) extensions.trackRebuildWidgets,
+        if (FeatureFlags.widgetRebuildStats) extensions.trackWidgetBuildCounts,
       ],
       overlayDescription: Column(
         crossAxisAlignment: CrossAxisAlignment.start,

packages/devtools_app/lib/src/screens/performance/panes/frame_analysis/frame_analysis.dart

@@ -2,32 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:devtools_app_shared/service.dart';
 import 'package:devtools_app_shared/ui.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 
 import '../../../../service/service_extension_widgets.dart';
 import '../../../../service/service_extensions.dart' as extensions;
-import '../../../../shared/common_widgets.dart';
 import '../../../../shared/feature_flags.dart';
 import '../../../../shared/globals.dart';
+import '../../../../shared/primitives/utils.dart';
 import '../controls/enhance_tracing/enhance_tracing_controller.dart';
+import '../flutter_frames/flutter_frame_model.dart';
 import '../rebuild_stats/rebuild_stats.dart';
 import '../rebuild_stats/rebuild_stats_model.dart';
-import 'frame_analysis_model.dart';
 import 'frame_hints.dart';
 import 'frame_time_visualizer.dart';
 
 class FlutterFrameAnalysisView extends StatelessWidget {
   const FlutterFrameAnalysisView({
     super.key,
-    required this.frameAnalysis,
+    required this.frame,
     required this.enhanceTracingController,
     required this.rebuildCountModel,
     required this.displayRefreshRateNotifier,
   });
 
-  final FrameAnalysis? frameAnalysis;
+  final FlutterFrame frame;
 
   final EnhanceTracingController enhanceTracingController;
 
@@ -37,16 +38,8 @@ class FlutterFrameAnalysisView extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    final frameAnalysis = this.frameAnalysis;
-    if (frameAnalysis == null) {
-      return const CenteredMessage(
-        'No analysis data available for this frame. This means that the '
-        'timeline events\nfor this frame occurred too long ago and DevTools '
-        'could not access them.\n\nTo avoid this, open the DevTools Performance '
-        'page earlier.',
-      );
-    }
-    final rebuilds = rebuildCountModel.rebuildsForFrame(frameAnalysis.frame.id);
+    final frameAnalysis = frame.frameAnalysis;
+    final rebuilds = rebuildCountModel.rebuildsForFrame(frame.id);
     final theme = Theme.of(context);
     return Padding(
       padding: const EdgeInsets.all(defaultSpacing),
@@ -61,65 +54,60 @@ class FlutterFrameAnalysisView extends StatelessWidget {
                   style: theme.regularTextStyle,
                 ),
                 TextSpan(
-                  text: '${frameAnalysis.frame.id}',
+                  text: '${frame.id}',
                   style: theme.fixedFontStyle
                       .copyWith(color: theme.colorScheme.primary),
                 ),
               ],
             ),
           ),
-          const PaddedDivider(
-            padding: EdgeInsets.only(
-              bottom: denseSpacing,
+          const PaddedDivider.noPadding(),
+          if (frameAnalysis == null) ...[
+            const Text(
+              'No timeline event analysis data available for this frame. This '
+              'means that the timeline events for this frame occurred too long '
+              'ago and DevTools could not access them. To avoid this, open the '
+              'DevTools Performance page earlier.',
             ),
-          ),
-          // TODO(jacobr): we might have so many frame hints that this content
-          // needs to scroll. Supporting that would be hard as the RebuildTable
-          // also needs to scroll and the devtools table functionality does not
-          // support the shrinkWrap property and has features that would make
-          //it difficult to handle robustly.
-          ValueListenableBuilder(
-            valueListenable: displayRefreshRateNotifier,
-            builder: (context, refreshRate, _) {
-              return FrameHints(
-                frameAnalysis: frameAnalysis,
-                enhanceTracingController: enhanceTracingController,
-                displayRefreshRate: refreshRate,
-              );
-            },
-          ),
-          const PaddedDivider(
-            padding: EdgeInsets.only(
-              top: denseSpacing,
-              bottom: denseSpacing,
-            ),
-          ),
-          FrameTimeVisualizer(frameAnalysis: frameAnalysis),
-          const PaddedDivider(
-            padding: EdgeInsets.only(
-              top: denseSpacing,
-              bottom: denseSpacing,
+          ] else ...[
+            // TODO(jacobr): we might have so many frame hints that this content
+            // needs to scroll. Supporting that would be hard as the RebuildTable
+            // also needs to scroll and the devtools table functionality does not
+            // support the shrinkWrap property and has features that would make
+            //it difficult to handle robustly.
+            ValueListenableBuilder(
+              valueListenable: displayRefreshRateNotifier,
+              builder: (context, refreshRate, _) {
+                return FrameHints(
+                  frameAnalysis: frameAnalysis,
+                  enhanceTracingController: enhanceTracingController,
+                  displayRefreshRate: refreshRate,
+                );
+              },
             ),
-          ),
 
+            const PaddedDivider.noPadding(),
+            FrameTimeVisualizer(frameAnalysis: frameAnalysis),
+          ],
           if (FeatureFlags.widgetRebuildStats) ...[
-            if (rebuilds == null || rebuilds.isEmpty)
-              ValueListenableBuilder<bool>(
+            if (rebuilds.isNullOrEmpty) ...[
+              const PaddedDivider.noPadding(),
+              ValueListenableBuilder<ServiceExtensionState>(
                 valueListenable: serviceConnection
                     .serviceManager.serviceExtensionManager
-                    .hasServiceExtension(
-                  extensions.trackRebuildWidgets.extension,
+                    .getServiceExtensionState(
+                  extensions.trackWidgetBuildCounts.extension,
                 ),
-                builder: (context, hasExtension, _) {
-                  if (hasExtension) {
+                builder: (context, extensionState, _) {
+                  if (!extensionState.enabled) {
                     return Row(
                       children: [
                         const Text(
                           'To see widget rebuilds for Flutter frames, enable',
                         ),
                         Flexible(
                           child: ServiceExtensionCheckbox(
-                            serviceExtension: extensions.trackRebuildWidgets,
+                            serviceExtension: extensions.trackWidgetBuildCounts,
                             showDescription: false,
                           ),
                         ),
@@ -129,21 +117,25 @@ class FlutterFrameAnalysisView extends StatelessWidget {
                   return const SizedBox();
                 },
               ),
+            ],
             if (rebuilds == null)
               const Text(
                 'Rebuild information not available for this frame.',
               )
             else if (rebuilds.isEmpty)
               const Text(
-                'No widget rebuilds occurred for widgets that were directly created in your project.',
+                'No widget rebuilds occurred for widgets that were directly '
+                'created in your project.',
               )
-            else
+            else ...[
+              const SizedBox(height: defaultSpacing),
               Expanded(
                 child: RebuildTable(
                   metricNames: const ['Rebuild Count'],
                   metrics: combineStats([rebuilds]),
                 ),
               ),
+            ],
           ],
         ],
       ),

packages/devtools_app/lib/src/screens/performance/panes/frame_analysis/frame_hints.dart

@@ -41,12 +41,12 @@ class FrameHints extends StatelessWidget {
       return const Text('No suggestions for this frame - no jank detected.');
     }
 
+    final theme = Theme.of(context);
     final saveLayerCount = frameAnalysis.saveLayerCount;
     final intrinsicOperationsCount = frameAnalysis.intrinsicOperationsCount;
-
     final uiHints = showUiJankHints
         ? [
-            const Text('UI Jank Detected'),
+            Text('UI Jank Detected', style: theme.errorTextStyle),
             const SizedBox(height: denseSpacing),
             EnhanceTracingHint(
               longestPhase: frameAnalysis.longestUiPhase,
@@ -60,7 +60,7 @@ class FrameHints extends StatelessWidget {
         : <Widget>[];
     final rasterHints = showRasterJankHints
         ? [
-            const Text('Raster Jank Detected'),
+            Text('Raster Jank Detected', style: theme.errorTextStyle),
             const SizedBox(height: denseSpacing),
             if (saveLayerCount > 0) CanvasSaveLayerHint(saveLayerCount),
             const SizedBox(height: denseSpacing),

packages/devtools_app/lib/src/screens/performance/panes/frame_analysis/frame_time_visualizer.dart

@@ -234,7 +234,7 @@ class _FramePhaseBlock extends StatelessWidget {
     required this.width,
   });
 
-  static const _height = 30.0;
+  static const _height = 24.0;
 
   static const minBlockWidth = defaultIconSizeBeforeScaling + densePadding * 8;
 

packages/devtools_app/lib/src/screens/performance/panes/rebuild_stats/rebuild_stats.dart

@@ -105,13 +105,15 @@ class _RebuildStatsViewState extends State<RebuildStatsView>
                   gaSelection: gac.PerformanceEvents.clearRebuildStats.name,
                   onPressed: widget.model.clearAllCounts,
                 ),
-                const SizedBox(width: denseSpacing),
-                Flexible(
-                  child: ServiceExtensionCheckbox(
-                    serviceExtension: extensions.trackRebuildWidgets,
+                Expanded(
+                  child: Padding(
+                    padding:
+                        const EdgeInsets.symmetric(horizontal: denseSpacing),
+                    child: ServiceExtensionCheckbox(
+                      serviceExtension: extensions.trackWidgetBuildCounts,
+                    ),
                   ),
                 ),
-                const Spacer(),
               ],
             ),
           ),
@@ -121,7 +123,7 @@ class _RebuildStatsViewState extends State<RebuildStatsView>
             valueListenable: serviceConnection
                 .serviceManager.serviceExtensionManager
                 .getServiceExtensionState(
-              extensions.trackRebuildWidgets.extension,
+              extensions.trackWidgetBuildCounts.extension,
             ),
             builder: (context, state, _) {
               if (metrics.isEmpty && !state.enabled) {
@@ -142,6 +144,7 @@ class _RebuildStatsViewState extends State<RebuildStatsView>
                 key: const Key('Rebuild Table'),
                 metricNames: metricNames,
                 metrics: metrics,
+                includeBorder: false,
               );
             },
           ),
@@ -156,10 +159,12 @@ class RebuildTable extends StatefulWidget {
     super.key,
     required this.metricNames,
     required this.metrics,
+    this.includeBorder = true,
   });
 
   final List<String> metricNames;
   final List<RebuildLocationStats> metrics;
+  final bool includeBorder;
 
   @override
   State<RebuildTable> createState() => _RebuildTableState();
@@ -196,32 +201,27 @@ class _RebuildTableState extends State<RebuildTable> {
 
   @override
   Widget build(BuildContext context) {
-    final borderSide = defaultBorderSide(Theme.of(context));
-    return Container(
-      decoration: BoxDecoration(
-        border: Border(right: borderSide),
-      ),
-      child: FlatTable<RebuildLocationStats>(
-        dataKey: 'RebuildMetricsTable',
-        columns: _columns,
-        data: widget.metrics,
-        keyFactory: (RebuildLocationStats location) =>
-            ValueKey<String?>('${location.location.id}'),
-        defaultSortColumn: _metricsColumns.first,
-        defaultSortDirection: sortDirection,
-        onItemSelected: (item) async {
-          final location = item?.location;
-          if (location?.fileUriString != null) {
-            await _service?.navigateToCode(
-              fileUriString: location?.fileUriString ?? '',
-              line: location?.line ?? 0,
-              column: location?.column ?? 0,
-              source: 'devtools.rebuildStats',
-            );
-          }
-        },
-      ),
+    final table = FlatTable<RebuildLocationStats>(
+      dataKey: 'RebuildMetricsTable',
+      columns: _columns,
+      data: widget.metrics,
+      keyFactory: (RebuildLocationStats location) =>
+          ValueKey<String?>('${location.location.id}'),
+      defaultSortColumn: _metricsColumns.first,
+      defaultSortDirection: sortDirection,
+      onItemSelected: (item) async {
+        final location = item?.location;
+        if (location?.fileUriString != null) {
+          await _service?.navigateToCode(
+            fileUriString: location?.fileUriString ?? '',
+            line: location?.line ?? 0,
+            column: location?.column ?? 0,
+            source: 'devtools.rebuildStats',
+          );
+        }
+      },
     );
+    return widget.includeBorder ? OutlineDecoration(child: table) : table;
   }
 }
 

packages/devtools_app/lib/src/screens/performance/panes/rebuild_stats/rebuild_stats_controller.dart

@@ -0,0 +1,32 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import '../../performance_controller.dart';
+import '../../performance_model.dart';
+import '../flutter_frames/flutter_frame_model.dart';
+
+// TODO(kenz): merge this control with the Rebuild Stats model class so that
+// this feature conforms to the patterns of other Performance page features.
+// This is just a temporary placeholder to ensure that we do not switch off of
+// the Rebuild Stats tab when frame selections occur.
+class RebuildStatsController extends PerformanceFeatureController {
+  RebuildStatsController(super.performanceController);
+
+  @override
+  FutureOr<void> clearData() {}
+
+  @override
+  void handleSelectedFrame(FlutterFrame frame) {}
+
+  @override
+  Future<void> init() async {}
+
+  @override
+  void onBecomingActive() {}
+
+  @override
+  Future<void> setOfflineData(OfflinePerformanceData offlineData) async {}
+}