Unverified Commit cc9ffd3f authored by chunhtai's avatar chunhtai Committed by GitHub

SelectionContainer's listeners can remove itself during listener call… (#124624)

When swapping out delegate of  selectioncontainer, if the newly passed in delegate doesn't have any selectable content(which is usually the case), the selectioncontainerstate will notify all of the listeners. One of the listener would be SelectionRegistrant._updateSelectionRegistrarSubscription, and since it doesn't have content, it would remove itself from the listener which causes concurrent modification
parent 15cb1f84
......@@ -18,7 +18,7 @@ void main() {
expect(BrowserContextMenu.enabled, !kIsWeb);
// Allow the selection overlay geometry to be created.
await tester.pump();
await tester.pumpAndSettle();
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
......
......@@ -1477,7 +1477,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
Selectable? _endHandleLayerOwner;
bool _isHandlingSelectionEvent = false;
bool _scheduledSelectableUpdate = false;
int? _scheduledSelectableUpdateCallbackId;
bool _selectionInProgress = false;
Set<Selectable> _additions = <Selectable>{};
......@@ -1505,16 +1505,13 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
}
void _scheduleSelectableUpdate() {
if (!_scheduledSelectableUpdate) {
_scheduledSelectableUpdate = true;
SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) {
if (!_scheduledSelectableUpdate) {
return;
}
_scheduledSelectableUpdate = false;
_updateSelectables();
});
}
_scheduledSelectableUpdateCallbackId ??= SchedulerBinding.instance.scheduleFrameCallback((Duration timeStamp) {
if (_scheduledSelectableUpdateCallbackId == null) {
return;
}
_scheduledSelectableUpdateCallbackId = null;
_updateSelectables();
});
}
void _updateSelectables() {
......@@ -2045,7 +2042,9 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
selectable.removeListener(_handleSelectableGeometryChange);
}
selectables = const <Selectable>[];
_scheduledSelectableUpdate = false;
if (_scheduledSelectableUpdateCallbackId != null) {
SchedulerBinding.instance.cancelFrameCallbackWithId(_scheduledSelectableUpdateCallbackId!);
}
super.dispose();
}
......
......@@ -133,7 +133,8 @@ class _SelectionContainerState extends State<SelectionContainer> with Selectable
_listeners.forEach(widget.delegate!.addListener);
}
if (oldWidget.delegate?.value != widget.delegate?.value) {
for (final VoidCallback listener in _listeners) {
// Avoid concurrent modification.
for (final VoidCallback listener in _listeners.toList(growable: false)) {
listener();
}
}
......
......@@ -78,6 +78,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
......@@ -111,6 +112,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
expect(find.byKey(key), findsNothing);
......@@ -182,6 +184,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph2 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Good, and you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph2, 7)); // at the 'a'
addTearDown(gesture.removePointer);
......
......@@ -93,6 +93,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
await _expectColors(
tester,
find.byType(Align),
......
......@@ -47,6 +47,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse);
......@@ -85,6 +86,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse);
......@@ -118,6 +120,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse);
......@@ -171,6 +174,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse);
......@@ -210,6 +214,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
controller.jumpTo(4000);
await tester.pumpAndSettle();
......@@ -258,6 +263,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Item 0'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 2), kind: ui.PointerDeviceKind.mouse);
......@@ -305,6 +311,7 @@ void main() {
),
),
));
await tester.pumpAndSettle();
controller.jumpTo(2080);
await tester.pumpAndSettle();
......
......@@ -48,6 +48,7 @@ void main() {
),
)
);
await tester.pumpAndSettle();
final RenderSelectionSpy renderSelectionSpy = tester.renderObject<RenderSelectionSpy>(find.byKey(spy));
final TestGesture gesture = await tester.startGesture(const Offset(200.0, 200.0), kind: PointerDeviceKind.mouse);
......@@ -121,6 +122,8 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byKey(spy)));
addTearDown(gesture.removePointer);
await tester.pump(const Duration(milliseconds: 500));
......@@ -241,6 +244,7 @@ void main() {
),
)
);
await tester.pumpAndSettle();
final RenderSelectionSpy renderSelectionSpy = tester.renderObject<RenderSelectionSpy>(find.byKey(spy));
final TestGesture gesture = await tester.startGesture(const Offset(200.0, 200.0), kind: PointerDeviceKind.mouse);
......@@ -260,6 +264,7 @@ void main() {
),
)
);
await tester.pumpAndSettle();
final RenderSelectionSpy renderSelectionSpy = tester.renderObject<RenderSelectionSpy>(find.byKey(spy));
renderSelectionSpy.events.clear();
......@@ -284,6 +289,7 @@ void main() {
),
)
);
await tester.pumpAndSettle();
final RenderSelectionSpy renderSelectionSpy = tester.renderObject<RenderSelectionSpy>(find.byKey(spy));
renderSelectionSpy.events.clear();
......@@ -315,6 +321,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderSelectionSpy renderSelectionSpy = tester.renderObject<RenderSelectionSpy>(find.byKey(spy));
renderSelectionSpy.events.clear();
......@@ -348,6 +355,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph, 6)); // at the 'r'
......@@ -708,6 +716,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
focusNode.requestFocus();
// keyboard select all.
......@@ -897,6 +906,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 6)); // at the 'r'
addTearDown(gesture.removePointer);
......@@ -928,6 +938,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph2 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Good, and you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph2, 7)); // at the 'a'
......@@ -964,6 +975,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph2 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Good, and you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph2, 7)); // at the 'a'
addTearDown(gesture.removePointer);
......@@ -998,6 +1010,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph3 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Fine, thank you.'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph3, 7)); // at the 'h'
addTearDown(gesture.removePointer);
......@@ -1039,6 +1052,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph3 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Fine, thank you.'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph3, 7)); // at the 'h'
addTearDown(gesture.removePointer);
......@@ -1075,6 +1089,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph3 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Fine, thank you.'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph3, 7)); // at the 'h'
addTearDown(gesture.removePointer);
......@@ -1111,6 +1126,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph3 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Fine, thank you.'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph3, 7)); // at the 'h'
addTearDown(gesture.removePointer);
......@@ -1146,6 +1162,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph3 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('Fine, thank you.'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph3, 7)); // at the 'h'
addTearDown(gesture.removePointer);
......@@ -1621,6 +1638,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(
find.descendant(
......@@ -1676,6 +1694,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you?'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph1, 6)); // at the 'r'
......@@ -1723,6 +1742,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you?'), matching: find.byType(RichText)));
await tester.longPressAt(textOffsetToPosition(paragraph1, 6)); // at the 'r'
......@@ -1776,6 +1796,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final RenderParagraph paragraph1 = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you?'), matching: find.byType(RichText)));
await tester.longPressAt(textOffsetToPosition(paragraph1, 6)); // at the 'r'
......@@ -1830,6 +1851,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
......@@ -1913,6 +1935,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
final SelectableRegionState state =
tester.state<SelectableRegionState>(find.byType(SelectableRegion));
......
......@@ -10,9 +10,12 @@ void main() {
Future<void> pumpContainer(WidgetTester tester, Widget child) async {
await tester.pumpWidget(
DefaultSelectionStyle(
selectionColor: Colors.red,
child: child,
Directionality(
textDirection: TextDirection.ltr,
child: DefaultSelectionStyle(
selectionColor: Colors.red,
child: child,
),
),
);
}
......@@ -34,6 +37,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(registrar.selectables.length, 1);
expect(delegate.selectables.length, 3);
});
......@@ -61,6 +65,53 @@ void main() {
expect(delegate.selectables.length, 0);
});
testWidgets('Swapping out container delegate does not crash', (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.pumpAndSettle();
expect(registrar.selectables.length, 1);
expect(delegate.value.hasContent, isTrue);
final TestContainerDelegate newDelegate = TestContainerDelegate();
await pumpContainer(
tester,
SelectionContainer(
registrar: registrar,
delegate: delegate,
child: Builder(
builder: (BuildContext context) {
return SelectionContainer(
registrar: SelectionContainer.maybeOf(context),
delegate: newDelegate,
child: const Text('dummy'),
);
},
)
),
);
await tester.pumpAndSettle();
expect(registrar.selectables.length, 1);
expect(delegate.value.hasContent, isTrue);
expect(tester.takeException(), isNull);
});
testWidgets('selection container registers itself if there is a selectable child', (WidgetTester tester) async {
final TestSelectionRegistrar registrar = TestSelectionRegistrar();
final TestContainerDelegate delegate = TestContainerDelegate();
......@@ -87,6 +138,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(registrar.selectables.length, 1);
await pumpContainer(
......@@ -98,6 +150,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(registrar.selectables.length, 0);
});
......@@ -119,6 +172,7 @@ void main() {
),
),
);
await tester.pumpAndSettle();
expect(registrar.selectables.length, 1);
});
}
......
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