Unverified Commit 14e191fa authored by chunhtai's avatar chunhtai Committed by GitHub

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
parent 898767f9
......@@ -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();
}
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);
}
_scheduledSelectableUpdateCallbackId = null;
_updateSelectables();
});
}
}
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();
}
......
......@@ -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();
......
......@@ -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();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment