[Property Editor] Debounce calls to the Analysis Server for editableArguments (#9017)

Author: elliette

packages/devtools_app/lib/src/framework/observer/memory_observer.dart

@@ -47,7 +47,7 @@ class MemoryObserver extends DisposableController {
 
   static const _memoryPressureLimitGb = 3.0;
 
-  DebounceTimer? _timer;
+  PeriodicDebouncer? _timer;
 
   /// Tracks the most recent memory usage measurement.
   ///
@@ -58,7 +58,7 @@ class MemoryObserver extends DisposableController {
   @override
   void init() {
     super.init();
-    _timer = DebounceTimer.periodic(_pollingDuration, _pollForMemoryUsage);
+    _timer = PeriodicDebouncer.run(_pollingDuration, _pollForMemoryUsage);
   }
 
   @override

packages/devtools_app/lib/src/screens/memory/panes/chart/controller/chart_connection.dart

@@ -36,7 +36,7 @@ class ChartVmConnection extends DisposableController
 
   bool initialized = false;
 
-  DebounceTimer? _polling;
+  PeriodicDebouncer? _polling;
 
   late final bool isDeviceAndroid;
 
@@ -79,7 +79,7 @@ class ChartVmConnection extends DisposableController
       ),
     );
 
-    _polling = DebounceTimer.periodic(chartUpdateDelay, ({
+    _polling = PeriodicDebouncer.run(chartUpdateDelay, ({
       DebounceCancelledCallback? cancelledCallback,
     }) async {
       if (!_isConnected) {

packages/devtools_app/lib/src/screens/network/network_controller.dart

@@ -165,7 +165,7 @@ class NetworkController extends DevToolsScreenController
   /// This timestamp is on the monotonic clock used by the timeline.
   int lastSocketDataRefreshMicros = 0;
 
-  DebounceTimer? _pollingTimer;
+  PeriodicDebouncer? _pollingTimer;
 
   @visibleForTesting
   bool get isPolling => _pollingTimer != null;
@@ -277,7 +277,7 @@ class NetworkController extends DevToolsScreenController
 
   void _updatePollingState(bool recording) {
     if (recording) {
-      _pollingTimer ??= DebounceTimer.periodic(
+      _pollingTimer ??= PeriodicDebouncer.run(
         // TODO(kenz): look into improving performance by caching more data.
         // Polling less frequently helps performance.
         _pollingDuration,

packages/devtools_app/lib/src/shared/utils/utils.dart

@@ -185,17 +185,43 @@ extension IsKeyType on KeyEvent {
   bool get isKeyDownOrRepeat => this is KeyDownEvent || this is KeyRepeatEvent;
 }
 
+/// A helper class for [Timer] functionality, where the callbacks are debounced.
+class Debouncer extends Disposable {
+  Debouncer({required this.duration});
+
+  final Duration duration;
+  Timer? _activeTimer;
+
+  /// Invokes the [callback] once after the specified [duration] has elapsed.
+  ///
+  /// If multiple invokations are called while the timer is running, only the
+  /// last one will be called once no more invokations happen within the given
+  /// [duration].
+  void run(void Function() callback) {
+    if (disposed) return;
+    _activeTimer?.cancel();
+    _activeTimer = Timer(duration, callback);
+  }
+
+  @override
+  void dispose() {
+    _activeTimer?.cancel();
+    _activeTimer = null;
+    super.dispose();
+  }
+}
+
 typedef DebounceCancelledCallback = bool Function();
 
-/// A helper class for [Timer] functionality, where the callbacks are debounced.
-class DebounceTimer {
-  /// A periodic timer that ensures [callback] is only called at most once
-  /// per [duration].
+/// A periodic debouncer that calls the given [callback] at most once per
+/// [duration].
+class PeriodicDebouncer {
+  /// Start running the periodic debouncer.
   ///
   /// [callback] is triggered once immediately, and then every [duration] the
   /// timer checks to see if the previous [callback] call has finished running.
   /// If it has finished, then then next call to [callback] will begin.
-  DebounceTimer.periodic(
+  PeriodicDebouncer.run(
     Duration duration,
     Future<void> Function({DebounceCancelledCallback? cancelledCallback})
     callback,

packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart

@@ -9,6 +9,7 @@ import '../../../shared/analytics/analytics.dart' as ga;
 import '../../../shared/analytics/constants.dart' as gac;
 import '../../../shared/editor/api_classes.dart';
 import '../../../shared/editor/editor_client.dart';
+import '../../../shared/utils/utils.dart';
 
 typedef EditableWidgetData =
     ({List<EditableArgument> args, String? name, String? documentation});
@@ -22,7 +23,7 @@ typedef EditArgumentFunction =
 class PropertyEditorController extends DisposableController
     with AutoDisposeControllerMixin {
   PropertyEditorController(this.editorClient) {
-    _init();
+    init();
   }
 
   final EditorClient editorClient;
@@ -36,7 +37,15 @@ class PropertyEditorController extends DisposableController
       _editableWidgetData;
   final _editableWidgetData = ValueNotifier<EditableWidgetData?>(null);
 
-  void _init() {
+  late final Debouncer _editableArgsDebouncer;
+
+  static const _editableArgsDebounceDuration = Duration(milliseconds: 600);
+
+  @override
+  void init() {
+    super.init();
+    _editableArgsDebouncer = Debouncer(duration: _editableArgsDebounceDuration);
+
     autoDisposeStreamSubscription(
       editorClient.activeLocationChangedStream.listen((event) async {
         final textDocument = event.textDocument;
@@ -45,34 +54,33 @@ class PropertyEditorController extends DisposableController
         if (textDocument == null) {
           return;
         }
-        // Don't do anything if the event corresponds to the current position.
-        if (textDocument == _currentDocument &&
+        // Don't do anything if the event corresponds to the current position
+        // and document version.
+        //
+        // Note: This is only checked if the document version is not null. For
+        // IntelliJ, the document verison is always null, so identical events
+        // indicating a valid change are possible.
+        if (textDocument.version != null &&
+            textDocument == _currentDocument &&
             cursorPosition == _currentCursorPosition) {
           return;
         }
-        _currentDocument = textDocument;
-        _currentCursorPosition = cursorPosition;
-        // Get the editable arguments for the current position.
-        final result = await editorClient.getEditableArguments(
-          textDocument: textDocument,
-          position: cursorPosition,
-        );
-        final args = result?.args ?? <EditableArgument>[];
-        final name = result?.name;
-        _editableWidgetData.value = (
-          args: args,
-          name: name,
-          documentation: result?.documentation,
-        );
-        // Register impression.
-        ga.impression(
-          gaId,
-          gac.PropertyEditorSidebar.widgetPropertiesUpdate(name: name),
+        _editableArgsDebouncer.run(
+          () => _updateWithEditableArgs(
+            textDocument: textDocument,
+            cursorPosition: cursorPosition,
+          ),
         );
       }),
     );
   }
 
+  @override
+  void dispose() {
+    _editableArgsDebouncer.dispose();
+    super.dispose();
+  }
+
   Future<EditArgumentResponse?> editArgument<T>({
     required String name,
     required T value,
@@ -88,6 +96,31 @@ class PropertyEditorController extends DisposableController
     );
   }
 
+  Future<void> _updateWithEditableArgs({
+    required TextDocument textDocument,
+    required CursorPosition cursorPosition,
+  }) async {
+    _currentDocument = textDocument;
+    _currentCursorPosition = cursorPosition;
+    // Get the editable arguments for the current position.
+    final result = await editorClient.getEditableArguments(
+      textDocument: textDocument,
+      position: cursorPosition,
+    );
+    final args = result?.args ?? <EditableArgument>[];
+    final name = result?.name;
+    _editableWidgetData.value = (
+      args: args,
+      name: name,
+      documentation: result?.documentation,
+    );
+    // Register impression.
+    ga.impression(
+      gaId,
+      gac.PropertyEditorSidebar.widgetPropertiesUpdate(name: name),
+    );
+  }
+
   @visibleForTesting
   void initForTestsOnly({
     EditableArgumentsResult? editableArgsResult,

packages/devtools_app/test/shared/utils/utils_test.dart

@@ -12,11 +12,11 @@ import 'package:fake_async/fake_async.dart';
 import 'package:flutter_test/flutter_test.dart';
 
 void main() {
-  group('DebounceTimer', () {
+  group('PeriodicDebouncer', () {
     test('the callback happens immediately', () {
       fakeAsync((async) {
         int callbackCounter = 0;
-        DebounceTimer.periodic(const Duration(seconds: 1), ({
+        PeriodicDebouncer.run(const Duration(seconds: 1), ({
           DebounceCancelledCallback? cancelledCallback,
         }) async {
           callbackCounter++;
@@ -30,7 +30,7 @@ void main() {
     test('only triggers another callback after the first is done', () {
       fakeAsync((async) {
         int callbackCounter = 0;
-        DebounceTimer.periodic(const Duration(milliseconds: 500), ({
+        PeriodicDebouncer.run(const Duration(milliseconds: 500), ({
           DebounceCancelledCallback? cancelledCallback,
         }) async {
           callbackCounter++;
@@ -44,7 +44,7 @@ void main() {
     test('calls the callback at the beginning and then once per period', () {
       fakeAsync((async) {
         int callbackCounter = 0;
-        DebounceTimer.periodic(const Duration(seconds: 1), ({
+        PeriodicDebouncer.run(const Duration(seconds: 1), ({
           DebounceCancelledCallback? cancelledCallback,
         }) async {
           callbackCounter++;
@@ -60,7 +60,7 @@ void main() {
       () {
         fakeAsync((async) {
           int callbackCounter = 0;
-          final timer = DebounceTimer.periodic(const Duration(seconds: 1), ({
+          final timer = PeriodicDebouncer.run(const Duration(seconds: 1), ({
             DebounceCancelledCallback? cancelledCallback,
           }) async {
             callbackCounter++;
@@ -82,7 +82,7 @@ void main() {
       () {
         fakeAsync((async) {
           int callbackCounter = 0;
-          final timer = DebounceTimer.periodic(const Duration(seconds: 1), ({
+          final timer = PeriodicDebouncer.run(const Duration(seconds: 1), ({
             DebounceCancelledCallback? cancelledCallback,
           }) async {
             callbackCounter++;
@@ -102,7 +102,7 @@ void main() {
     test('cancels the callback when cancelled during the first callback', () {
       fakeAsync((async) {
         int callbackCounter = 0;
-        final timer = DebounceTimer.periodic(const Duration(seconds: 1), ({
+        final timer = PeriodicDebouncer.run(const Duration(seconds: 1), ({
           DebounceCancelledCallback? cancelledCallback,
         }) async {
           callbackCounter++;
@@ -123,7 +123,7 @@ void main() {
     test('cancels the callback when cancelled during the Nth callback', () {
       fakeAsync((async) {
         int callbackCounter = 0;
-        final timer = DebounceTimer.periodic(const Duration(seconds: 1), ({
+        final timer = PeriodicDebouncer.run(const Duration(seconds: 1), ({
           DebounceCancelledCallback? cancelledCallback,
         }) async {
           callbackCounter++;
@@ -142,6 +142,57 @@ void main() {
     });
   });
 
+  group('Debouncer', () {
+    late Debouncer debouncer;
+    late int callbackCount;
+
+    setUp(() {
+      callbackCount = 0;
+      debouncer = Debouncer(duration: const Duration(milliseconds: 100));
+    });
+
+    tearDown(() {
+      debouncer.dispose();
+    });
+
+    test('calls callback after duration', () async {
+      debouncer.run(() => callbackCount++);
+      expect(callbackCount, 0);
+      await Future.delayed(const Duration(milliseconds: 150));
+      expect(callbackCount, 1);
+    });
+
+    test('debounces multiple calls', () async {
+      debouncer.run(() => callbackCount++);
+      debouncer.run(() => callbackCount++);
+      debouncer.run(() => callbackCount++);
+      expect(callbackCount, 0);
+      await Future.delayed(const Duration(milliseconds: 150));
+      expect(callbackCount, 1);
+    });
+
+    test('calls callback after multiple calls with delay', () async {
+      debouncer.run(() => callbackCount++);
+      await Future.delayed(const Duration(milliseconds: 50));
+      debouncer.run(() => callbackCount++);
+      await Future.delayed(const Duration(milliseconds: 50));
+      debouncer.run(() => callbackCount++);
+      expect(callbackCount, 0);
+      await Future.delayed(const Duration(milliseconds: 150));
+      expect(callbackCount, 1);
+    });
+
+    test('dispose cancels timer', () async {
+      debouncer.run(() => callbackCount++);
+      debouncer.dispose();
+      await Future.delayed(const Duration(milliseconds: 150));
+      expect(callbackCount, 0);
+      debouncer.run(() => callbackCount++);
+      await Future.delayed(const Duration(milliseconds: 150));
+      expect(callbackCount, 0);
+    });
+  });
+
   group('InterruptableChunkWorker', () {
     late InterruptableChunkWorker worker;
     late List<int> indexes;