Migrate get & set preference APIs to use the standard server connecti…

…on. (flutter#8284)
Sarsilmaz02 · Sep 6, 2024 · f4cb8be · f4cb8be
1 parent 3127636
commit f4cb8be
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 52 deletions.
diff --git a/...pp/lib/src/shared/config_specific/framework_initialize/_framework_initialize_desktop.dart b/...pp/lib/src/shared/config_specific/framework_initialize/_framework_initialize_desktop.dart
@@ -29,7 +29,7 @@ class FlutterDesktopStorage implements Storage {
   }
 
   @override
-  Future setValue(String key, String value) async {
+  Future<void> setValue(String key, String value) async {
     _values[key] = value;
 
     const encoder = JsonEncoder.withIndent('  ');

diff --git a/...ls_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart b/...ls_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart
@@ -11,6 +11,7 @@ import 'package:web/web.dart' hide Storage;
 import '../../../service/service_manager.dart';
 import '../../globals.dart';
 import '../../primitives/storage.dart';
+import '../../server/server.dart' as server;
 import '../../server/server_api_client.dart';
 
 /// Return the url the application is launched from.
@@ -24,12 +25,14 @@ Future<String> initializePlatform() async {
         }.toJS,
       );
 
+  // TODO(kenz): this server connection initialized listeners that are never
+  // disposed, so this is likely leaking resources.
   // Here, we try and initialize the connection between the DevTools web app and
   // its local server. DevTools can be launched without the server however, so
   // establishing this connection is a best-effort.
   final connection = await DevToolsServerConnection.connect();
   if (connection != null) {
-    setGlobal(Storage, ServerConnectionStorage(connection));
+    setGlobal(Storage, ServerConnectionStorage());
   } else {
     setGlobal(Storage, BrowserStorage());
   }
@@ -89,18 +92,15 @@ void _sendKeyPressToParent(KeyboardEvent event) {
 }
 
 class ServerConnectionStorage implements Storage {
-  ServerConnectionStorage(this.connection);
-
-  final DevToolsServerConnection connection;
-
   @override
-  Future<String?> getValue(String key) {
-    return connection.getPreferenceValue(key);
+  Future<String?> getValue(String key) async {
+    final value = await server.getPreferenceValue(key);
+    return value == null ? null : '$value';
   }
 
   @override
   Future<void> setValue(String key, String value) async {
-    await connection.setPreferenceValue(key, value);
+    await server.setPreferenceValue(key, value);
   }
 }
 

diff --git a/packages/devtools_app/lib/src/shared/preferences/preferences.dart b/packages/devtools_app/lib/src/shared/preferences/preferences.dart
@@ -27,6 +27,23 @@ part '_performance_preferences.dart';
 
 const _thirdPartyPathSegment = 'third_party';
 
+/// DevTools preferences for UI-related settings.
+enum _UiPreferences {
+  darkMode,
+  vmDeveloperMode;
+
+  String get storageKey => '$storagePrefix.$name';
+
+  static const storagePrefix = 'ui';
+}
+
+/// DevTools preferences for general settings.
+///
+/// These values are not stored in the DevTools storage file with a prefix.
+enum _GeneralPreferences {
+  verboseLogging,
+}
+
 /// A controller for global application preferences.
 class PreferencesController extends DisposableController
     with AutoDisposeControllerMixin {
@@ -44,7 +61,6 @@ class PreferencesController extends DisposableController
 
   final verboseLoggingEnabled =
       ValueNotifier<bool>(Logger.root.level == verboseLoggingLevel);
-  static const _verboseLoggingStorageId = 'verboseLogging';
 
   // TODO(https://github.com/flutter/devtools/issues/7860): Clean-up after
   // Inspector V2 has been released.
@@ -65,44 +81,57 @@ class PreferencesController extends DisposableController
 
   Future<void> init() async {
     // Get the current values and listen for and write back changes.
-    final darkModeValue = await storage.getValue('ui.darkMode');
+    await _initDarkMode();
+    await _initVmDeveloperMode();
+    await _initVerboseLogging();
+
+    await inspector.init();
+    await memory.init();
+    await logging.init();
+    await performance.init();
+    await devToolsExtensions.init();
+
+    setGlobal(PreferencesController, this);
+  }
+
+  Future<void> _initDarkMode() async {
+    final darkModeValue =
+        await storage.getValue(_UiPreferences.darkMode.storageKey);
     final useDarkMode = (darkModeValue == null && useDarkThemeAsDefault) ||
         darkModeValue == 'true';
     ga.impression(gac.devToolsMain, gac.startingTheme(darkMode: useDarkMode));
     toggleDarkModeTheme(useDarkMode);
     addAutoDisposeListener(darkModeEnabled, () {
-      storage.setValue('ui.darkMode', '${darkModeEnabled.value}');
+      storage.setValue(
+        _UiPreferences.darkMode.storageKey,
+        '${darkModeEnabled.value}',
+      );
     });
+  }
 
+  Future<void> _initVmDeveloperMode() async {
     final vmDeveloperModeValue = await boolValueFromStorage(
-      'ui.vmDeveloperMode',
+      _UiPreferences.vmDeveloperMode.storageKey,
       defaultsTo: false,
     );
     toggleVmDeveloperMode(vmDeveloperModeValue);
     addAutoDisposeListener(vmDeveloperModeEnabled, () {
-      storage.setValue('ui.vmDeveloperMode', '${vmDeveloperModeEnabled.value}');
+      storage.setValue(
+        _UiPreferences.vmDeveloperMode.storageKey,
+        '${vmDeveloperModeEnabled.value}',
+      );
     });
-
-    await _initVerboseLogging();
-
-    await inspector.init();
-    await memory.init();
-    await logging.init();
-    await performance.init();
-    await devToolsExtensions.init();
-
-    setGlobal(PreferencesController, this);
   }
 
   Future<void> _initVerboseLogging() async {
     final verboseLoggingEnabledValue = await boolValueFromStorage(
-      _verboseLoggingStorageId,
+      _GeneralPreferences.verboseLogging.name,
       defaultsTo: false,
     );
     toggleVerboseLogging(verboseLoggingEnabledValue);
     addAutoDisposeListener(verboseLoggingEnabled, () {
       storage.setValue(
-        'verboseLogging',
+        _GeneralPreferences.verboseLogging.name,
         verboseLoggingEnabled.value.toString(),
       );
     });
@@ -118,14 +147,14 @@ class PreferencesController extends DisposableController
     super.dispose();
   }
 
-  /// Change the value for the dark mode setting.
+  /// Change the value of the dark mode setting.
   void toggleDarkModeTheme(bool? useDarkMode) {
     if (useDarkMode != null) {
       darkModeEnabled.value = useDarkMode;
     }
   }
 
-  /// Change the value for the VM developer mode setting.
+  /// Change the value of the VM developer mode setting.
   void toggleVmDeveloperMode(bool? enableVmDeveloperMode) {
     if (enableVmDeveloperMode != null) {
       vmDeveloperModeEnabled.value = enableVmDeveloperMode;

diff --git a/packages/devtools_app/lib/src/shared/server/_preferences_api.dart b/packages/devtools_app/lib/src/shared/server/_preferences_api.dart
@@ -0,0 +1,45 @@
+// 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.
+
+part of 'server.dart';
+
+/// Requests the DevTools preference for the [key].
+///
+/// This value is stored in the file '~/.flutter-devtools/.devtools'.
+Future<Object?> getPreferenceValue(String key) async {
+  if (!isDevToolsServerAvailable) return null;
+
+  final uri = Uri(
+    path: PreferencesApi.getPreferenceValue,
+    queryParameters: {
+      PreferencesApi.preferenceKeyProperty: key,
+    },
+  );
+  final resp = await request(uri.toString());
+  if (resp?.statusOk ?? false) {
+    return jsonDecode(resp!.body);
+  } else {
+    logWarning(resp, PreferencesApi.getPreferenceValue);
+    return null;
+  }
+}
+
+/// Sets the DevTools preference [value] for the [key].
+///
+/// This value is stored in the file '~/.flutter-devtools/.devtools'.
+Future<void> setPreferenceValue(String key, Object value) async {
+  if (!isDevToolsServerAvailable) return;
+
+  final uri = Uri(
+    path: PreferencesApi.setPreferenceValue,
+    queryParameters: {
+      PreferencesApi.preferenceKeyProperty: key,
+      apiParameterValueKey: value,
+    },
+  );
+  final resp = await request(uri.toString());
+  if (resp == null || !resp.statusOk) {
+    logWarning(resp, PreferencesApi.setPreferenceValue);
+  }
+}
diff --git a/packages/devtools_app/lib/src/shared/server/server.dart b/packages/devtools_app/lib/src/shared/server/server.dart
@@ -19,6 +19,7 @@ part '_analytics_api.dart';
 part '_app_size_api.dart';
 part '_deep_links_api.dart';
 part '_extensions_api.dart';
+part '_preferences_api.dart';
 part '_release_notes_api.dart';
 part '_survey_api.dart';
 part '_dtd_api.dart';

diff --git a/packages/devtools_app/lib/src/shared/server/server_api_client.dart b/packages/devtools_app/lib/src/shared/server/server_api_client.dart
@@ -204,23 +204,6 @@ class DevToolsServerConnection {
     unawaited(_callMethod('disconnected'));
   }
 
-  /// Retrieves a preference value from the DevTools configuration file at
-  /// ~/.flutter-devtools/.devtools.
-  Future<String?> getPreferenceValue(String key) {
-    return _callMethod('getPreferenceValue', {
-      'key': key,
-    });
-  }
-
-  /// Sets a preference value in the DevTools configuration file at
-  /// ~/.flutter-devtools/.devtools.
-  Future setPreferenceValue(String key, String value) async {
-    await _callMethod('setPreferenceValue', {
-      'key': key,
-      'value': value,
-    });
-  }
-
   /// Allows the server to ping the client to see that it is definitely still
   /// active and doesn't just appear to be connected because of SSE timeouts.
   void ping() {

diff --git a/packages/devtools_app/test/test_infra/flutter_test_storage.dart b/packages/devtools_app/test/test_infra/flutter_test_storage.dart
@@ -16,7 +16,7 @@ class FlutterTestStorage implements Storage {
   }
 
   @override
-  Future setValue(String key, String value) async {
+  Future<void> setValue(String key, String value) async {
     values[key] = value;
   }
 }
diff --git a/packages/devtools_shared/CHANGELOG.md b/packages/devtools_shared/CHANGELOG.md
@@ -14,6 +14,7 @@
 * Deprecate `apiGetSurveyShownCount` in favor of `SurveyApi.getSurveyShownCount`.
 * Deprecate `apiIncrementSurveyShownCount` in favor of `SurveyApi.incrementSurveyShownCount`.
 * Support Chrome's new headless mode in the integration test runner.
+* Add `PreferencesApi` to get and set preference values.
 
 # 10.0.2
 * Update dependency `web_socket_channel: '>=2.4.0 <4.0.0'`.

diff --git a/packages/devtools_shared/lib/src/devtools_api.dart b/packages/devtools_shared/lib/src/devtools_api.dart
@@ -28,6 +28,23 @@ const apiSetDevToolsEnabled = '${apiPrefix}setDevToolsEnabled';
 /// in queryParameter:
 const devToolsEnabledPropertyName = 'enabled';
 
+abstract class PreferencesApi {
+  /// Returns the preference value in the DevTools store file for the key
+  /// specified by the [preferenceKeyProperty] query parameter.
+  static const getPreferenceValue = '${apiPrefix}getPreferenceValue';
+
+  /// Sets the preference value in the DevTools store file for the key
+  /// specified by the [preferenceKeyProperty] query parameter.
+  ///
+  /// The value must be specified by the [apiParameterValueKey] query parameter.
+  static const setPreferenceValue = '${apiPrefix}setPreferenceValue';
+
+  /// The property name for the query parameter passed along with the
+  /// [getPreferenceValue] and [setPreferenceValue] requests that describes the
+  /// preference key in the DevTools store file.
+  static const preferenceKeyProperty = 'key';
+}
+
 @Deprecated(
   'Use SurveyApi.setActiveSurvey instead. '
   'This field will be removed in devtools_shared >= 11.0.0.',

diff --git a/packages/devtools_shared/lib/src/server/handlers/_preferences.dart b/packages/devtools_shared/lib/src/server/handlers/_preferences.dart
@@ -0,0 +1,50 @@
+// 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.
+
+// ignore_for_file: avoid_classes_with_only_static_members
+
+part of '../server_api.dart';
+
+abstract class _PreferencesApiHandler {
+  static shelf.Response getPreferenceValue<T>(
+    ServerApi api,
+    Map<String, String> queryParams,
+    DevToolsUsage devToolsStore,
+  ) {
+    final missingRequiredParams = ServerApi._checkRequiredParameters(
+      [PreferencesApi.preferenceKeyProperty],
+      queryParams: queryParams,
+      api: api,
+      requestName: PreferencesApi.getPreferenceValue,
+    );
+    if (missingRequiredParams != null) return missingRequiredParams;
+
+    return _StorageHandler.handleGetStorageValue<T>(
+      api,
+      devToolsStore,
+      key: queryParams[PreferencesApi.preferenceKeyProperty]!,
+    );
+  }
+
+  static shelf.Response setPreferenceValue<T>(
+    ServerApi api,
+    Map<String, String> queryParams,
+    DevToolsUsage devToolsStore,
+  ) {
+    final missingRequiredParams = ServerApi._checkRequiredParameters(
+      [PreferencesApi.preferenceKeyProperty, apiParameterValueKey],
+      queryParams: queryParams,
+      api: api,
+      requestName: PreferencesApi.setPreferenceValue,
+    );
+    if (missingRequiredParams != null) return missingRequiredParams;
+
+    return _StorageHandler.handleSetStorageValue<T>(
+      api,
+      devToolsStore,
+      key: queryParams[PreferencesApi.preferenceKeyProperty]!,
+      value: queryParams[apiParameterValueKey]! as T,
+    );
+  }
+}
diff --git a/packages/devtools_shared/lib/src/server/handlers/_storage.dart b/packages/devtools_shared/lib/src/server/handlers/_storage.dart
@@ -11,13 +11,10 @@ abstract class _StorageHandler {
     ServerApi api,
     DevToolsUsage devToolsStore, {
     required String key,
-    required T defaultValue,
+    T? defaultValue,
   }) {
-    final T value = (devToolsStore.properties[key] as T?) ?? defaultValue;
-    return ServerApi._encodeResponse(
-      value,
-      api: api,
-    );
+    final T? value = (devToolsStore.properties[key] as T?) ?? defaultValue;
+    return ServerApi._encodeResponse(value, api: api);
   }
 
   static shelf.Response handleSetStorageValue<T>(

diff --git a/packages/devtools_shared/lib/src/server/server_api.dart b/packages/devtools_shared/lib/src/server/server_api.dart
@@ -33,6 +33,7 @@ part 'handlers/_deeplink.dart';
 part 'handlers/_devtools_extensions.dart';
 part 'handlers/_dtd.dart';
 part 'handlers/_general.dart';
+part 'handlers/_preferences.dart';
 part 'handlers/_release_notes.dart';
 part 'handlers/_storage.dart';
 part 'handlers/_survey.dart';
@@ -113,6 +114,21 @@ class ServerApi {
         }
         return _encodeResponse(_devToolsStore.analyticsEnabled, api: api);
 
+      // ----- Preferences api. -----
+      case PreferencesApi.getPreferenceValue:
+        return _PreferencesApiHandler.getPreferenceValue(
+          api,
+          queryParams,
+          _devToolsStore,
+        );
+
+      case PreferencesApi.setPreferenceValue:
+        return _PreferencesApiHandler.setPreferenceValue(
+          api,
+          queryParams,
+          _devToolsStore,
+        );
+
       // ----- DevTools survey api. -----
 
       case SurveyApi.setActiveSurvey: