Unverified Commit eae9bba7 authored by xubaolin's avatar xubaolin Committed by GitHub

fix a draggableScrollableSheet's LocalHistoryEntry leaking (#110576)

parent 9be0fa74
...@@ -2177,6 +2177,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto ...@@ -2177,6 +2177,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
final List<_StandardBottomSheet> _dismissedBottomSheets = <_StandardBottomSheet>[]; final List<_StandardBottomSheet> _dismissedBottomSheets = <_StandardBottomSheet>[];
PersistentBottomSheetController<dynamic>? _currentBottomSheet; PersistentBottomSheetController<dynamic>? _currentBottomSheet;
final GlobalKey _currentBottomSheetKey = GlobalKey(); final GlobalKey _currentBottomSheetKey = GlobalKey();
LocalHistoryEntry? _persistentSheetHistoryEntry;
void _maybeBuildPersistentBottomSheet() { void _maybeBuildPersistentBottomSheet() {
if (widget.bottomSheet != null && _currentBottomSheet == null) { if (widget.bottomSheet != null && _currentBottomSheet == null) {
...@@ -2184,22 +2185,19 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto ...@@ -2184,22 +2185,19 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
// will not be added to the Scaffold's appbar and the bottom sheet will not // will not be added to the Scaffold's appbar and the bottom sheet will not
// support drag or swipe to dismiss. // support drag or swipe to dismiss.
final AnimationController animationController = BottomSheet.createAnimationController(this)..value = 1.0; final AnimationController animationController = BottomSheet.createAnimationController(this)..value = 1.0;
LocalHistoryEntry? persistentSheetHistoryEntry;
bool persistentBottomSheetExtentChanged(DraggableScrollableNotification notification) { bool persistentBottomSheetExtentChanged(DraggableScrollableNotification notification) {
if (notification.extent > notification.initialExtent) { if (notification.extent - notification.initialExtent > precisionErrorTolerance) {
if (persistentSheetHistoryEntry == null) { if (_persistentSheetHistoryEntry == null) {
persistentSheetHistoryEntry = LocalHistoryEntry(onRemove: () { _persistentSheetHistoryEntry = LocalHistoryEntry(onRemove: () {
if (notification.extent > notification.initialExtent) {
DraggableScrollableActuator.reset(notification.context); DraggableScrollableActuator.reset(notification.context);
}
showBodyScrim(false, 0.0); showBodyScrim(false, 0.0);
_floatingActionButtonVisibilityValue = 1.0; _floatingActionButtonVisibilityValue = 1.0;
persistentSheetHistoryEntry = null; _persistentSheetHistoryEntry = null;
}); });
ModalRoute.of(context)!.addLocalHistoryEntry(persistentSheetHistoryEntry!); ModalRoute.of(context)!.addLocalHistoryEntry(_persistentSheetHistoryEntry!);
} }
} else if (persistentSheetHistoryEntry != null) { } else if (_persistentSheetHistoryEntry != null) {
ModalRoute.of(context)!.removeLocalHistoryEntry(persistentSheetHistoryEntry!); _persistentSheetHistoryEntry!.remove();
} }
return false; return false;
} }
...@@ -2298,6 +2296,15 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto ...@@ -2298,6 +2296,15 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
bool removedEntry = false; bool removedEntry = false;
bool doingDispose = false; bool doingDispose = false;
void removePersistentSheetHistoryEntryIfNeeded() {
assert(isPersistent);
if (_persistentSheetHistoryEntry != null) {
_persistentSheetHistoryEntry!.remove();
_persistentSheetHistoryEntry = null;
}
}
void removeCurrentBottomSheet() { void removeCurrentBottomSheet() {
removedEntry = true; removedEntry = true;
if (_currentBottomSheet == null) { if (_currentBottomSheet == null) {
...@@ -2307,6 +2314,10 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto ...@@ -2307,6 +2314,10 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
assert(bottomSheetKey.currentState != null); assert(bottomSheetKey.currentState != null);
_showFloatingActionButton(); _showFloatingActionButton();
if (isPersistent) {
removePersistentSheetHistoryEntryIfNeeded();
}
bottomSheetKey.currentState!.close(); bottomSheetKey.currentState!.close();
setState(() { setState(() {
_currentBottomSheet = null; _currentBottomSheet = null;
......
...@@ -22,6 +22,61 @@ void main() { ...@@ -22,6 +22,61 @@ void main() {
expect(dyDelta1, isNot(moreOrLessEquals(dyDelta2, epsilon: 0.1))); expect(dyDelta1, isNot(moreOrLessEquals(dyDelta2, epsilon: 0.1)));
} }
testWidgets('Persistent draggableScrollableSheet localHistoryEntries test', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/110123
Widget buildFrame(Widget? bottomSheet) {
return MaterialApp(
home: Scaffold(
appBar: AppBar(),
body: const Center(child: Text('body')),
bottomSheet: bottomSheet,
floatingActionButton: const FloatingActionButton(
onPressed: null,
child: Text('fab'),
),
),
);
}
final Widget draggableScrollableSheet = DraggableScrollableSheet(
expand: false,
snap: true,
initialChildSize: 0.3,
minChildSize: 0.3,
builder: (_, ScrollController controller) {
return ListView.builder(
itemExtent: 50.0,
itemCount: 50,
itemBuilder: (_, int index) => Text('Item $index'),
controller: controller,
);
},
);
await tester.pumpWidget(buildFrame(draggableScrollableSheet));
await tester.pumpAndSettle();
expect(find.byType(BackButton).hitTestable(), findsNothing);
await tester.drag(find.text('Item 2'), const Offset(0, -200.0));
await tester.pumpAndSettle();
// We've started to drag up, we should have a back button now for a11y
expect(find.byType(BackButton).hitTestable(), findsOneWidget);
await tester.fling(find.text('Item 2'), const Offset(0, 200.0), 2000.0);
await tester.pumpAndSettle();
// BackButton should be hidden
expect(find.byType(BackButton).hitTestable(), findsNothing);
// Show the back button again
await tester.drag(find.text('Item 2'), const Offset(0, -200.0));
await tester.pumpAndSettle();
expect(find.byType(BackButton).hitTestable(), findsOneWidget);
// Remove the draggableScrollableSheet should hide the back button
await tester.pumpWidget(buildFrame(null));
expect(find.byType(BackButton).hitTestable(), findsNothing);
});
// Regression test for https://github.com/flutter/flutter/issues/83668 // Regression test for https://github.com/flutter/flutter/issues/83668
testWidgets('Scaffold.bottomSheet update test', (WidgetTester tester) async { testWidgets('Scaffold.bottomSheet update test', (WidgetTester tester) async {
Widget buildFrame(Widget? bottomSheet) { Widget buildFrame(Widget? bottomSheet) {
......
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