firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ScrollController creation dispatching for memory leaks tracking (#133759)

Browse Source
This commit is contained in:
Kostia Sokolovskyi 2023-08-31 17:52:06 +02:00 committed by GitHub
parent 7fe4571af2
commit 414bff1403
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 217 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
packages/flutter/lib/src/widgets/routes.dart
View File

@ -884,6 +884,7 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
@override
void dispose() {
focusScopeNode.dispose();
primaryScrollController.dispose();
super.dispose();
}

6
packages/flutter/lib/src/widgets/scroll_controller.dart
View File

@ -65,7 +65,11 @@ class ScrollController extends ChangeNotifier {
this.debugLabel,
this.onAttach,
this.onDetach,
}) : _initialScrollOffset = initialScrollOffset;
}) : _initialScrollOffset = initialScrollOffset {
if (kFlutterMemoryAllocationsEnabled) {
maybeDispatchObjectCreation();
}
}
/// The initial value to use for [offset].
///

73
packages/flutter/test/material/app_bar_test.dart
View File

@ -1609,40 +1609,45 @@ void main() {
// Generates a MaterialApp with a SliverAppBar in a CustomScrollView.
// The first cell of the scroll view contains a button at its top, and is
// initially scrolled so that it is beneath the SliverAppBar.
Widget buildWidget({
(ScrollController, Widget) buildWidget({
required bool forceMaterialTransparency,
required VoidCallback onPressed
}) {
const double appBarHeight = 120;
return MaterialApp(
home: Scaffold(
body: CustomScrollView(
controller: ScrollController(initialScrollOffset:appBarHeight),
slivers: <Widget>[
SliverAppBar(
collapsedHeight: appBarHeight,
expandedHeight: appBarHeight,
pinned: true,
elevation: 0,
backgroundColor: Colors.transparent,
forceMaterialTransparency: forceMaterialTransparency,
title: const Text('AppBar'),
final ScrollController controller = ScrollController(initialScrollOffset: appBarHeight);
return (
controller,
MaterialApp(
home: Scaffold(
body: CustomScrollView(
controller: controller,
slivers: <Widget>[
SliverAppBar(
collapsedHeight: appBarHeight,
expandedHeight: appBarHeight,
pinned: true,
elevation: 0,
backgroundColor: Colors.transparent,
forceMaterialTransparency: forceMaterialTransparency,
title: const Text('AppBar'),
),
SliverList(
delegate: SliverChildBuilderDelegate((BuildContext context, int index) {
return SizedBox(
height: appBarHeight,
child: index == 0
? Align(
alignment: Alignment.topCenter,
child: TextButton(onPressed: onPressed, child: const Text('press')))
: const SizedBox(),
);
},
childCount: 20,
),
),
SliverList(
delegate: SliverChildBuilderDelegate((BuildContext context, int index) {
return SizedBox(
height: appBarHeight,
child: index == 0
? Align(
alignment: Alignment.topCenter,
child: TextButton(onPressed: onPressed, child: const Text('press')))
: const SizedBox(),
);
},
childCount: 20,
),
),
]),
]),
),
),
);
}
@ -1650,7 +1655,7 @@ void main() {
testWidgetsWithLeakTracking(
'forceMaterialTransparency == true allows gestures beneath the app bar', (WidgetTester tester) async {
bool buttonWasPressed = false;
final Widget widget = buildWidget(
final (ScrollController controller, Widget widget) = buildWidget(
forceMaterialTransparency:true,
onPressed:() { buttonWasPressed = true; },
);
@ -1663,6 +1668,8 @@ void main() {
await tester.tap(buttonFinder);
await tester.pump();
expect(buttonWasPressed, isTrue);
controller.dispose();
});
testWidgetsWithLeakTracking(
@ -1672,7 +1679,7 @@ void main() {
WidgetController.hitTestWarningShouldBeFatal = false;
bool buttonWasPressed = false;
final Widget widget = buildWidget(
final (ScrollController controller, Widget widget) = buildWidget(
forceMaterialTransparency:false,
onPressed:() { buttonWasPressed = true; },
);
@ -1685,6 +1692,8 @@ void main() {
await tester.tap(buttonFinder, warnIfMissed:false);
await tester.pump();
expect(buttonWasPressed, isFalse);
controller.dispose();
});
});
@ -4295,6 +4304,8 @@ void main() {
);
expect(tester.takeException(), isNull);
controller.dispose();
});
testWidgetsWithLeakTracking('does not trigger on horizontal scroll', (WidgetTester tester) async {

48
packages/flutter/test/material/page_selector_test.dart
View File

@ -84,7 +84,14 @@ void main() {
await tester.pump();
expect(tabController.index, 2);
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kUnselectedColor, kSelectedColor]);
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
testWidgetsWithLeakTracking('PageSelector responds correctly to TabController.animateTo()', (WidgetTester tester) async {
final TabController tabController = TabController(
@ -127,7 +134,14 @@ void main() {
await tester.pumpAndSettle();
expect(tabController.index, 2);
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kUnselectedColor, kSelectedColor]);
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
testWidgetsWithLeakTracking('PageSelector responds correctly to TabBarView drags', (WidgetTester tester) async {
final TabController tabController = TabController(
@ -185,8 +199,14 @@ void main() {
await tester.fling(find.byType(TabBarView), const Offset(100.0, 0.0), 1000.0);
await tester.pumpAndSettle();
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kSelectedColor, kUnselectedColor]);
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
testWidgetsWithLeakTracking('PageSelector indicatorColors', (WidgetTester tester) async {
const Color kRed = Color(0xFFFF0000);
@ -205,7 +225,14 @@ void main() {
tabController.index = 0;
await tester.pumpAndSettle();
expect(indicatorColors(tester), const <Color>[kBlue, kRed, kRed]);
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
testWidgets('PageSelector indicatorSize', (WidgetTester tester) async {
final TabController tabController = TabController(
@ -228,7 +255,7 @@ void main() {
expect(tester.getSize(find.byType(TabPageSelector)).height, 24.0);
});
testWidgetsWithLeakTracking('PageSelector circle border', (WidgetTester tester) async {
testWidgetsWithLeakTracking('PageSelector circle border', (WidgetTester tester) async {
final TabController tabController = TabController(
vsync: const TestVSync(),
initialIndex: 1,
@ -272,5 +299,12 @@ void main() {
for (final TabPageSelectorIndicator indicator in indicators) {
expect(indicator.borderStyle, BorderStyle.solid);
}
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
}

14
packages/flutter/test/material/refresh_indicator_test.dart
View File

@ -423,6 +423,8 @@ void main() {
expect(controller.offset, greaterThan(lastScrollOffset));
expect(controller.offset, lessThan(0.0));
expect(refreshCalled, isTrue);
controller.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }));
testWidgetsWithLeakTracking('RefreshIndicator does not force child to relayout', (WidgetTester tester) async {
@ -618,6 +620,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the scroll animation
await tester.pump(const Duration(seconds: 1)); // finish the indicator settle animation
expect(tester.getCenter(find.byType(RefreshProgressIndicator)).dy, lessThan(300.0));
scrollController.dispose();
});
testWidgetsWithLeakTracking('Reverse RefreshIndicator(anywhere mode) should be shown when dragging from non-zero scroll position', (WidgetTester tester) async {
@ -654,6 +658,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the scroll animation
await tester.pump(const Duration(seconds: 1)); // finish the indicator settle animation
expect(tester.getCenter(find.byType(RefreshProgressIndicator)).dy, lessThan(300.0));
scrollController.dispose();
});
// Regression test for https://github.com/flutter/flutter/issues/71936
@ -691,6 +697,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the scroll animation
await tester.pump(const Duration(seconds: 1)); // finish the indicator settle animation
expect(find.byType(RefreshProgressIndicator), findsNothing);
scrollController.dispose();
});
testWidgetsWithLeakTracking('Top RefreshIndicator(onEdge mode) should not be shown when dragging from non-zero scroll position', (WidgetTester tester) async {
@ -725,6 +733,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the scroll animation
await tester.pump(const Duration(seconds: 1)); // finish the indicator settle animation
expect(find.byType(RefreshProgressIndicator), findsNothing);
scrollController.dispose();
});
testWidgetsWithLeakTracking('Reverse RefreshIndicator(onEdge mode) should be shown when dragging from non-zero scroll position', (WidgetTester tester) async {
@ -760,6 +770,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the scroll animation
await tester.pump(const Duration(seconds: 1)); // finish the indicator settle animation
expect(find.byType(RefreshProgressIndicator), findsNothing);
scrollController.dispose();
});
testWidgetsWithLeakTracking('ScrollController.jumpTo should not trigger the refresh indicator', (WidgetTester tester) async {
@ -792,6 +804,8 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the indicator hide animation
expect(refreshCalled, false);
scrollController.dispose();
});
testWidgetsWithLeakTracking('RefreshIndicator.adaptive', (WidgetTester tester) async {

49
packages/flutter/test/material/scrollbar_test.dart
View File

@ -203,6 +203,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
final AssertionError exception = tester.takeException() as AssertionError;
expect(exception, isAssertionError);
controller.dispose();
});
testWidgetsWithLeakTracking('On first render with thumbVisibility: true, the thumb shows', (WidgetTester tester) async {
@ -229,6 +231,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
await tester.pumpAndSettle();
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
});
testWidgetsWithLeakTracking('On first render with thumbVisibility: true, the thumb shows with PrimaryScrollController', (WidgetTester tester) async {
@ -261,6 +265,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
await tester.pumpAndSettle();
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
});
testWidgetsWithLeakTracking(
@ -314,6 +320,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
final AssertionError exception = tester.takeException() as AssertionError;
expect(exception, isAssertionError);
controller.dispose();
},
);
@ -341,6 +349,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
await tester.pumpAndSettle();
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
});
testWidgetsWithLeakTracking('On first render with thumbVisibility: true, the thumb shows with PrimaryScrollController', (WidgetTester tester) async {
@ -373,6 +383,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
await tester.pumpAndSettle();
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
});
testWidgetsWithLeakTracking('On first render with thumbVisibility: false, the thumb is hidden', (WidgetTester tester) async {
@ -399,6 +411,8 @@ void main() {
await tester.pumpWidget(viewWithScroll());
await tester.pumpAndSettle();
expect(find.byType(Scrollbar), isNot(paints..rect()));
controller.dispose();
});
testWidgetsWithLeakTracking(
@ -452,6 +466,8 @@ void main() {
await tester.pumpAndSettle();
// Scrollbar is not showing after scroll finishes
expect(find.byType(Scrollbar), isNot(paints..rect()));
controller.dispose();
},
);
@ -501,6 +517,8 @@ void main() {
await tester.pumpAndSettle();
// Scrollbar is not showing after scroll finishes
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
},
);
@ -561,6 +579,8 @@ void main() {
await tester.pumpAndSettle();
// Scrollbar thumb is showing after scroll finishes and timer ends.
expect(find.byType(Scrollbar), paints..rect());
controller.dispose();
},
);
@ -610,6 +630,8 @@ void main() {
await tester.tap(find.byType(FloatingActionButton));
await tester.pumpAndSettle();
expect(materialScrollbar, isNot(paints..rect()));
controller.dispose();
},
);
@ -673,6 +695,8 @@ void main() {
));
await tester.pumpAndSettle();
controller.dispose();
});
testWidgetsWithLeakTracking('Tapping the track area pages the Scroll View', (WidgetTester tester) async {
@ -763,6 +787,8 @@ void main() {
color: _kAndroidThumbIdleColor,
),
);
scrollController.dispose();
});
testWidgets('Scrollbar never goes away until finger lift', (WidgetTester tester) async {
@ -936,6 +962,8 @@ void main() {
color: _kAndroidThumbIdleColor,
),
);
scrollController.dispose();
});
testWidgetsWithLeakTracking('Scrollbar thumb color completes a hover animation', (WidgetTester tester) async {
@ -1331,6 +1359,8 @@ void main() {
expect(find.byType(CupertinoScrollbar), paints..rrect());
final CupertinoScrollbar scrollbar = tester.widget<CupertinoScrollbar>(find.byType(CupertinoScrollbar));
expect(scrollbar.controller, isNotNull);
controller.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }));
testWidgetsWithLeakTracking("Scrollbar doesn't show when scroll the inner scrollable widget", (WidgetTester tester) async {
@ -1463,6 +1493,8 @@ void main() {
await tester.pumpAndSettle();
// The offset should not have changed.
expect(scrollController.offset, scrollAmount);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.fuchsia }));
testWidgetsWithLeakTracking('Scrollbar dragging is disabled by default on Android', (WidgetTester tester) async {
@ -1557,6 +1589,8 @@ void main() {
// The offset should not have changed.
expect(scrollController.offset, scrollAmount * 2);
expect(tapCount, 2);
scrollController.dispose();
});
testWidgetsWithLeakTracking('Simultaneous dragging and pointer scrolling does not cause a crash', (WidgetTester tester) async {
@ -1729,6 +1763,8 @@ void main() {
color: const Color(0xffbcbcbc),
),
);
scrollController.dispose();
});
testWidgetsWithLeakTracking('Scrollbar.thumbVisibility triggers assertion when multiple ScrollPositions are attached.', (WidgetTester tester) async {
@ -1796,7 +1832,16 @@ void main() {
error.message,
contains('The provided ScrollController is currently attached to more than one ScrollPosition.'),
);
});
scrollController.dispose();
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 2,
},
));
testWidgetsWithLeakTracking('Scrollbar scrollOrientation works correctly', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
@ -1844,5 +1889,7 @@ void main() {
color: _kAndroidThumbIdleColor,
),
);
scrollController.dispose();
});
}

58
packages/flutter/test/material/scrollbar_theme_test.dart
View File

@ -113,6 +113,8 @@ void main() {
color: const Color(0x80000000),
),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.linux,
TargetPlatform.macOS,
@ -204,6 +206,8 @@ void main() {
color: const Color(0xff2196f3),
),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.linux,
TargetPlatform.macOS,
@ -253,6 +257,8 @@ void main() {
color: const Color(0xFF000000),
),
);
scrollController.dispose();
},
);
@ -301,6 +307,8 @@ void main() {
color: _kDefaultIdleThumbColor,
),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.fuchsia }));
testWidgetsWithLeakTracking('Scrollbar.interactive takes priority over ScrollbarTheme', (WidgetTester tester) async {
@ -349,6 +357,8 @@ void main() {
color: _kDefaultIdleThumbColor,
),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.fuchsia }));
testWidgetsWithLeakTracking('Scrollbar widget properties take priority over theme', (WidgetTester tester) async {
@ -442,6 +452,8 @@ void main() {
color: const Color(0x80000000),
),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.linux,
TargetPlatform.macOS,
@ -451,30 +463,34 @@ void main() {
);
testWidgetsWithLeakTracking('ThemeData colorScheme is used when no ScrollbarTheme is set', (WidgetTester tester) async {
Widget buildFrame(ThemeData appTheme) {
(ScrollController, Widget) buildFrame(ThemeData appTheme) {
final ScrollController scrollController = ScrollController();
return MaterialApp(
theme: appTheme,
home: ScrollConfiguration(
behavior: const NoScrollbarBehavior(),
child: Scrollbar(
thumbVisibility: true,
showTrackOnHover: true,
controller: scrollController,
child: SingleChildScrollView(
controller: scrollController,
child: const SizedBox(width: 4000.0, height: 4000.0),
return (
scrollController,
MaterialApp(
theme: appTheme,
home: ScrollConfiguration(
behavior: const NoScrollbarBehavior(),
child: Scrollbar(
thumbVisibility: true,
showTrackOnHover: true,
controller: scrollController,
child: SingleChildScrollView(
controller: scrollController,
child: const SizedBox(width: 4000.0, height: 4000.0),
),
),
),
),
),
);
);
}
// Scrollbar defaults for light themes:
// - coloring based on ColorScheme.onSurface
await tester.pumpWidget(buildFrame(ThemeData(
final (ScrollController controller1, Widget frame1) = buildFrame(ThemeData(
colorScheme: const ColorScheme.light(),
)));
));
await tester.pumpWidget(frame1);
await tester.pumpAndSettle();
// Idle scrollbar behavior
expect(
@ -545,9 +561,10 @@ void main() {
// Scrollbar defaults for dark themes:
// - coloring slightly different based on ColorScheme.onSurface
await tester.pumpWidget(buildFrame(ThemeData(
final (ScrollController controller2, Widget frame2) = buildFrame(ThemeData(
colorScheme: const ColorScheme.dark(),
)));
));
await tester.pumpWidget(frame2);
await tester.pumpAndSettle(); // Theme change animation
// Idle scrollbar behavior
@ -610,6 +627,9 @@ void main() {
color: const Color(0xa6ffffff),
),
);
controller1.dispose();
controller2.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.linux,
TargetPlatform.macOS,
@ -656,6 +676,8 @@ void main() {
)
..rrect(color: const Color(0xff4caf50)),
);
scrollController.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{
TargetPlatform.linux,
TargetPlatform.macOS,

9
packages/flutter/test/material/tabbed_scrollview_warp_test.dart
View File

@ -81,5 +81,12 @@ void main() {
// should not crash.
await tester.tap(find.text('Tab 2'));
await tester.pumpAndSettle();
});
},
// TODO(someone): remove after fixing
// https://github.com/flutter/flutter/issues/133755
leakTrackingTestConfig: const LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{
'PageController': 1,
},
));
}

18
packages/flutter/test/widgets/scroll_controller_test.dart
View File

@ -4,6 +4,7 @@
import 'dart:ui' as ui;
import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
@ -393,4 +394,21 @@ void main() {
// should have been true
expect(isScrolling, isTrue);
});
test('$ScrollController dispatches object creation in constructor', () {
final List<ObjectEvent> events = <ObjectEvent>[];
void listener(ObjectEvent event) {
if (event.object.runtimeType == ScrollController) {
events.add(event);
}
}
MemoryAllocations.instance.addListener(listener);
final ScrollController controller = ScrollController();
expect(events, hasLength(1));
controller.dispose();
MemoryAllocations.instance.removeListener(listener);
});
}