Stop logging http to timeline by default and warn users about memory implications

packages/devtools_app/lib/src/framework/framework_core.dart

@@ -11,6 +11,7 @@ import '../screens/debugger/breakpoint_manager.dart';
 import '../service/service.dart';
 import '../service/service_manager.dart';
 import '../service/vm_service_wrapper.dart';
+import '../shared/banner_messages.dart';
 import '../shared/console/eval/eval_service.dart';
 import '../shared/framework_controller.dart';
 import '../shared/globals.dart';
@@ -36,6 +37,7 @@ class FrameworkCore {
     setGlobal(OfflineModeController, OfflineModeController());
     setGlobal(ScriptManager, ScriptManager());
     setGlobal(NotificationService, NotificationService());
+    setGlobal(BannerMessagesController, BannerMessagesController());
     setGlobal(BreakpointManager, BreakpointManager());
     setGlobal(EvalService, EvalService());
   }

packages/devtools_app/lib/src/framework/scaffold.dart

@@ -303,82 +303,79 @@ class DevToolsScaffoldState extends State<DevToolsScaffold>
     );
     final theme = Theme.of(context);
 
-    return Provider<BannerMessagesController>(
-      create: (_) => BannerMessagesController(),
-      child: Provider<ImportController>.value(
-        value: _importController,
-        builder: (context, _) {
-          final showConsole = serviceManager.connectedAppInitialized &&
-              !offlineController.offlineMode.value &&
-              _currentScreen.showConsole(widget.embed);
-
-          return DragAndDrop(
-            handleDrop: _importController.importData,
-            child: Title(
-              title: scaffoldTitle,
-              // Color is a required parameter but the color only appears to
-              // matter on Android and we do not care about Android.
-              // Using theme.primaryColor matches the default behavior of the
-              // title used by [WidgetsApp].
-              color: theme.primaryColor.withAlpha(255),
-              child: KeyboardShortcuts(
-                keyboardShortcuts: _currentScreen.buildKeyboardShortcuts(
-                  context,
-                ),
-                child: Scaffold(
-                  appBar: widget.embed
-                      ? null
-                      : PreferredSize(
-                          preferredSize: Size.fromHeight(defaultToolbarHeight),
-                          // Place the AppBar inside of a Hero widget to keep it the same across
-                          // route transitions.
-                          child: Hero(
-                            tag: _appBarTag,
-                            child: DevToolsAppBar(
-                              tabController: _tabController,
-                              title: scaffoldTitle,
-                              screens: widget.screens,
-                              actions: widget.actions,
-                            ),
+    return Provider<ImportController>.value(
+      value: _importController,
+      builder: (context, _) {
+        final showConsole = serviceManager.connectedAppInitialized &&
+            !offlineController.offlineMode.value &&
+            _currentScreen.showConsole(widget.embed);
+
+        return DragAndDrop(
+          handleDrop: _importController.importData,
+          child: Title(
+            title: scaffoldTitle,
+            // Color is a required parameter but the color only appears to
+            // matter on Android and we do not care about Android.
+            // Using theme.primaryColor matches the default behavior of the
+            // title used by [WidgetsApp].
+            color: theme.primaryColor.withAlpha(255),
+            child: KeyboardShortcuts(
+              keyboardShortcuts: _currentScreen.buildKeyboardShortcuts(
+                context,
+              ),
+              child: Scaffold(
+                appBar: widget.embed
+                    ? null
+                    : PreferredSize(
+                        preferredSize: Size.fromHeight(defaultToolbarHeight),
+                        // Place the AppBar inside of a Hero widget to keep it the same across
+                        // route transitions.
+                        child: Hero(
+                          tag: _appBarTag,
+                          child: DevToolsAppBar(
+                            tabController: _tabController,
+                            title: scaffoldTitle,
+                            screens: widget.screens,
+                            actions: widget.actions,
                           ),
                         ),
-                  body: OutlineDecoration.onlyTop(
-                    child: Padding(
-                      padding: widget.appPadding,
-                      child: showConsole
-                          ? Split(
-                              axis: Axis.vertical,
-                              splitters: [
-                                ConsolePaneHeader(
-                                  backgroundColor: theme.colorScheme.surface,
+                      ),
+                body: OutlineDecoration.onlyTop(
+                  child: Padding(
+                    padding: widget.appPadding,
+                    child: showConsole
+                        ? Split(
+                            axis: Axis.vertical,
+                            splitters: [
+                              ConsolePaneHeader(
+                                backgroundColor: theme.colorScheme.surface,
+                              ),
+                            ],
+                            initialFractions: const [0.8, 0.2],
+                            children: [
+                              Padding(
+                                padding: const EdgeInsets.only(
+                                  bottom: intermediateSpacing,
                                 ),
-                              ],
-                              initialFractions: const [0.8, 0.2],
-                              children: [
-                                Padding(
-                                  padding: const EdgeInsets.only(
-                                    bottom: intermediateSpacing,
-                                  ),
-                                  child: content,
-                                ),
-                                RoundedOutlinedBorder.onlyBottom(
-                                  child: const ConsolePane(),
-                                ),
-                              ],
-                            )
-                          : content,
-                    ),
-                  ),
-                  bottomNavigationBar: StatusLine(
-                    currentScreen: _currentScreen,
-                    isEmbedded: widget.embed,
+                                child: content,
+                              ),
+                              RoundedOutlinedBorder.onlyBottom(
+                                child: const ConsolePane(),
+                              ),
+                            ],
+                          )
+                        : content,
                   ),
                 ),
+                bottomNavigationBar: StatusLine(
+                  currentScreen: _currentScreen,
+                  isEmbedded: widget.embed,
+                ),
               ),
             ),
-          );
-        },
-      ),
+          ),
+        );
+      },
     );
   }
 }

packages/devtools_app/lib/src/screens/memory/framework/connected/connected_screen_body.dart

@@ -5,6 +5,7 @@
 import 'package:flutter/material.dart';
 
 import '../../../../shared/banner_messages.dart';
+import '../../../../shared/http/http_service.dart' as http_service;
 import '../../../../shared/primitives/auto_dispose.dart';
 import '../../../../shared/primitives/simple_items.dart';
 import '../../../../shared/theme.dart';
@@ -46,8 +47,14 @@ class _ConnectedMemoryBodyState extends State<ConnectedMemoryBody>
   void didChangeDependencies() {
     super.didChangeDependencies();
     maybePushDebugModeMemoryMessage(context, ScreenMetaData.memory.id);
+    maybePushHttpLoggingMessage(context, ScreenMetaData.memory.id);
+
     if (!initController()) return;
 
+    addAutoDisposeListener(http_service.httpLoggingState, () {
+      maybePushHttpLoggingMessage(context, ScreenMetaData.memory.id);
+    });
+
     final vmChartController = VMChartController(controller);
     _chartController = MemoryChartPaneController(
       event: EventChartController(controller),

packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_chart.dart

@@ -6,7 +6,6 @@ import 'dart:async';
 import 'dart:math' as math;
 
 import 'package:flutter/material.dart';
-import 'package:provider/provider.dart';
 
 import '../../../../framework/scaffold.dart';
 import '../../../../shared/analytics/analytics.dart' as ga;
@@ -115,7 +114,7 @@ class _FlutterFramesChartState extends State<_FlutterFramesChart> {
         Duration.zero,
         (prev, frame) => prev + frame.shaderDuration,
       );
-      Provider.of<BannerMessagesController>(context).addMessage(
+      bannerMessages.addMessage(
         ShaderJankMessage(
           offlineController.offlineMode.value
               ? SimpleScreen.id

packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart

@@ -13,7 +13,6 @@ import '../../../../shared/analytics/constants.dart' as gac;
 import '../../../../shared/analytics/metrics.dart';
 import '../../../../shared/future_work_tracker.dart';
 import '../../../../shared/globals.dart';
-import '../../../../shared/http/http_service.dart' as http_service;
 import '../../../../shared/primitives/auto_dispose.dart';
 import '../../../../shared/primitives/trace_event.dart';
 import '../../../../shared/primitives/utils.dart';
@@ -84,16 +83,6 @@ class TimelineEventsController extends PerformanceFeatureController
 
   final _workTracker = FutureWorkTracker();
 
-  // TODO(jacobr): this isn't accurate. Another page of DevTools
-  // or a different instance of DevTools could change this value. We need to
-  // sync the value with the server like we do for other vm service extensions
-  // that we track with the vm service extension manager.
-  // See https://github.com/dart-lang/sdk/issues/41823.
-  /// Whether http timeline logging is enabled.
-  ValueListenable<bool> get httpTimelineLoggingEnabled =>
-      _httpTimelineLoggingEnabled;
-  final _httpTimelineLoggingEnabled = ValueNotifier<bool>(false);
-
   Timer? _pollingTimer;
 
   int _nextPollStartMicros = 0;
@@ -135,7 +124,6 @@ class TimelineEventsController extends PerformanceFeatureController
 
   Future<void> _initForServiceConnection() async {
     await serviceManager.timelineStreamManager.setDefaultTimelineStreams();
-    await toggleHttpRequestLogging(true);
 
     autoDisposeStreamSubscription(
       serviceManager.onConnectionClosed.listen((_) {
@@ -484,11 +472,6 @@ class TimelineEventsController extends PerformanceFeatureController
     }
   }
 
-  Future<void> toggleHttpRequestLogging(bool state) async {
-    await http_service.toggleHttpRequestLogging(state);
-    _httpTimelineLoggingEnabled.value = state;
-  }
-
   Future<void> toggleUseLegacyTraceViewer(bool? value) async {
     useLegacyTraceViewer.value = value ?? false;
     await processAllTraceEvents();

packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_view.dart

@@ -11,6 +11,8 @@ import '../../../../shared/charts/flame_chart.dart';
 import '../../../../shared/common_widgets.dart';
 import '../../../../shared/dialogs.dart';
 import '../../../../shared/globals.dart';
+import '../../../../shared/http/http_service.dart' as http_service;
+import '../../../../shared/primitives/auto_dispose.dart';
 import '../../../../shared/theme.dart';
 import '../../../../shared/ui/search.dart';
 import 'legacy/legacy_events_controller.dart';
@@ -132,7 +134,7 @@ class TraceCategoriesButton extends StatelessWidget {
     unawaited(
       showDialog(
         context: context,
-        builder: (context) => TraceCategoriesDialog(controller),
+        builder: (context) => const TraceCategoriesDialog(),
       ),
     );
   }
@@ -166,10 +168,35 @@ class RefreshTimelineEventsButton extends StatelessWidget {
   }
 }
 
-class TraceCategoriesDialog extends StatelessWidget {
-  const TraceCategoriesDialog(this.timelineEventsController, {super.key});
+class TraceCategoriesDialog extends StatefulWidget {
+  const TraceCategoriesDialog({super.key});
 
-  final TimelineEventsController timelineEventsController;
+  @override
+  State<TraceCategoriesDialog> createState() => _TraceCategoriesDialogState();
+}
+
+class _TraceCategoriesDialogState extends State<TraceCategoriesDialog>
+    with AutoDisposeMixin {
+  late final ValueNotifier<bool?> _httpLogging;
+
+  @override
+  void initState() {
+    super.initState();
+    // Mirror the value of [http_service.httpLoggingState] in the [_httpLogging]
+    // notifier so that we can use [_httpLogging] for the [CheckboxSetting]
+    // widget below.
+    _httpLogging = ValueNotifier<bool>(http_service.httpLoggingEnabled);
+    addAutoDisposeListener(http_service.httpLoggingState, () {
+      _httpLogging.value = http_service.httpLoggingState.value.enabled;
+    });
+  }
+
+  @override
+  void dispose() {
+    cancelListeners();
+    _httpLogging.dispose();
+    super.dispose();
+  }
 
   @override
   Widget build(BuildContext context) {
@@ -210,10 +237,9 @@ class TraceCategoriesDialog extends StatelessWidget {
       CheckboxSetting(
         title: 'Network',
         description: 'Http traffic',
-        notifier: timelineEventsController.httpTimelineLoggingEnabled
-            as ValueNotifier<bool?>,
+        notifier: _httpLogging,
         onChanged: (value) => unawaited(
-          timelineEventsController.toggleHttpRequestLogging(value ?? false),
+          http_service.toggleHttpRequestLogging(value ?? false),
         ),
       ),
     ];

packages/devtools_app/lib/src/screens/performance/performance_screen.dart

@@ -64,7 +64,6 @@ class PerformanceScreenBodyState extends State<PerformanceScreenBody>
   void didChangeDependencies() {
     super.didChangeDependencies();
     maybePushUnsupportedFlutterVersionWarning(
-      context,
       PerformanceScreen.id,
       supportedFlutterVersion: SemanticVersion(
         major: 2,

packages/devtools_app/lib/src/screens/provider/provider_screen.dart

@@ -7,12 +7,12 @@ import 'dart:async';
 import 'package:collection/collection.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
-import 'package:provider/provider.dart' as provider show Provider;
 
 import '../../shared/analytics/analytics.dart' as ga;
 import '../../shared/banner_messages.dart';
 import '../../shared/common_widgets.dart';
 import '../../shared/dialogs.dart';
+import '../../shared/globals.dart';
 import '../../shared/primitives/simple_items.dart';
 import '../../shared/screen.dart';
 import '../../shared/split.dart';
@@ -98,7 +98,7 @@ class ProviderScreenBody extends ConsumerWidget {
         : '[No provider selected]';
 
     ref.listen<bool>(_hasErrorProvider, (_, hasError) {
-      if (hasError) showProviderErrorBanner(context);
+      if (hasError) showProviderErrorBanner();
     });
 
     return Split(
@@ -157,11 +157,8 @@ class ProviderScreenBody extends ConsumerWidget {
   }
 }
 
-void showProviderErrorBanner(BuildContext context) {
-  provider.Provider.of<BannerMessagesController>(
-    context,
-    listen: false,
-  ).addMessage(
+void showProviderErrorBanner() {
+  bannerMessages.addMessage(
     ProviderUnknownErrorBanner(screenId: ProviderScreen.id).build(),
   );
 }