From 14e191fa75e0eff535d057a525cf68fe5ea75c4a Mon Sep 17 00:00:00 2001
From: chunhtai <47866232+chunhtai@users.noreply.github.com>
Date: Wed, 19 Apr 2023 12:48:12 -0700
Subject: [PATCH] Revert selectable update back to be a postframecallback or
 microtask (#125140)

The regression was caused by the previous pr https://github.com/flutter/flutter/pull/124624 changes postframecallback to scheduleframecallback. The reason is that if a new postframecallback was scheduled when running a postframecallback. The newly added postframecallback will be execute on the next frame. However, adding postframecallback will not schedule a new frame. So if there isn't other widget that schedule a new frame, the newly added postframecallback will never gets run.

After changing to scheduleframecallback, it causes an issue that transient callback may be called when rendering tree contains dirty layout information that are waiting to be rebuilt.

Therefore, I use microtask to get around of the postframecallback issue instead of scheduleframecallback.

fixes https://github.com/flutter/flutter/issues/125065
---
 .../lib/src/widgets/selectable_region.dart    | 36 +++++++++++++------
 .../test/widgets/selectable_region_test.dart  | 14 ++++----
 .../widgets/selection_container_test.dart     | 27 ++++++++++++++
 3 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/packages/flutter/lib/src/widgets/selectable_region.dart b/packages/flutter/lib/src/widgets/selectable_region.dart
index ab368357f2..6e6c639892 100644
--- a/packages/flutter/lib/src/widgets/selectable_region.dart
+++ b/packages/flutter/lib/src/widgets/selectable_region.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.
 
+import 'dart:async';
 import 'dart:math';
 
 import 'package:flutter/foundation.dart';
@@ -1477,7 +1478,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
   Selectable? _endHandleLayerOwner;
 
   bool _isHandlingSelectionEvent = false;
-  int? _scheduledSelectableUpdateCallbackId;
+  bool _scheduledSelectableUpdate = false;
   bool _selectionInProgress = false;
   Set<Selectable> _additions = <Selectable>{};
 
@@ -1493,6 +1494,10 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
   @override
   void remove(Selectable selectable) {
     if (_additions.remove(selectable)) {
+      // The same selectable was added in the same frame and is not yet
+      // incorporated into the selectables.
+      //
+      // Removing such selectable doesn't require selection geometry update.
       return;
     }
     _removeSelectable(selectable);
@@ -1505,13 +1510,26 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
   }
 
   void _scheduleSelectableUpdate() {
-    _scheduledSelectableUpdateCallbackId ??= SchedulerBinding.instance.scheduleFrameCallback((Duration timeStamp) {
-      if (_scheduledSelectableUpdateCallbackId == null) {
-        return;
+    if (!_scheduledSelectableUpdate) {
+      _scheduledSelectableUpdate = true;
+      void runScheduledTask([Duration? duration]) {
+        if (!_scheduledSelectableUpdate) {
+          return;
+        }
+        _scheduledSelectableUpdate = false;
+        _updateSelectables();
       }
-      _scheduledSelectableUpdateCallbackId = null;
-      _updateSelectables();
-    });
+
+      if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.postFrameCallbacks) {
+        // A new task can be scheduled as a result of running the scheduled task
+        // from another MultiSelectableSelectionContainerDelegate. This can
+        // happen if nesting two SelectionContainers. The selectable can be
+        // safely updated in the same frame in this case.
+        scheduleMicrotask(runScheduledTask);
+      } else {
+        SchedulerBinding.instance.addPostFrameCallback(runScheduledTask);
+      }
+    }
   }
 
   void _updateSelectables() {
@@ -2042,9 +2060,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
       selectable.removeListener(_handleSelectableGeometryChange);
     }
     selectables = const <Selectable>[];
-    if (_scheduledSelectableUpdateCallbackId != null) {
-      SchedulerBinding.instance.cancelFrameCallbackWithId(_scheduledSelectableUpdateCallbackId!);
-    }
+    _scheduledSelectableUpdate = false;
     super.dispose();
   }
 
diff --git a/packages/flutter/test/widgets/selectable_region_test.dart b/packages/flutter/test/widgets/selectable_region_test.dart
index 7dabebc405..362740e79f 100644
--- a/packages/flutter/test/widgets/selectable_region_test.dart
+++ b/packages/flutter/test/widgets/selectable_region_test.dart
@@ -40,13 +40,13 @@ void main() {
     testWidgets('mouse selection sends correct events', (WidgetTester tester) async {
       final UniqueKey spy = UniqueKey();
       await tester.pumpWidget(
-          MaterialApp(
-            home: SelectableRegion(
-              focusNode: FocusNode(),
-              selectionControls: materialTextSelectionControls,
-              child: SelectionSpy(key: spy),
-            ),
-          )
+        MaterialApp(
+          home: SelectableRegion(
+            focusNode: FocusNode(),
+            selectionControls: materialTextSelectionControls,
+            child: SelectionSpy(key: spy),
+          ),
+        ),
       );
       await tester.pumpAndSettle();
 
diff --git a/packages/flutter/test/widgets/selection_container_test.dart b/packages/flutter/test/widgets/selection_container_test.dart
index 385b0d1fa2..3c7648c920 100644
--- a/packages/flutter/test/widgets/selection_container_test.dart
+++ b/packages/flutter/test/widgets/selection_container_test.dart
@@ -112,6 +112,33 @@ void main() {
     expect(tester.takeException(), isNull);
   });
 
+  testWidgets('Can update within one frame', (WidgetTester tester) async {
+    final TestSelectionRegistrar registrar = TestSelectionRegistrar();
+    final TestContainerDelegate delegate = TestContainerDelegate();
+    final TestContainerDelegate childDelegate = TestContainerDelegate();
+    await pumpContainer(
+      tester,
+      SelectionContainer(
+          registrar: registrar,
+          delegate: delegate,
+          child: Builder(
+            builder: (BuildContext context) {
+              return SelectionContainer(
+                registrar: SelectionContainer.maybeOf(context),
+                delegate: childDelegate,
+                child: const Text('dummy'),
+              );
+            },
+          )
+      ),
+    );
+    await tester.pump();
+    // Should finish update after flushing the micro tasks.
+    await tester.idle();
+    expect(registrar.selectables.length, 1);
+    expect(delegate.value.hasContent, isTrue);
+  });
+
   testWidgets('selection container registers itself if there is a selectable child', (WidgetTester tester) async {
     final TestSelectionRegistrar registrar = TestSelectionRegistrar();
     final TestContainerDelegate delegate = TestContainerDelegate();