Skip to content

Commit

Permalink
Fix memory leak in TabPageSelector (#147403)
Browse files Browse the repository at this point in the history
  • Loading branch information
ValentinVignal committed Apr 30, 2024
1 parent b597dd2 commit af27093
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 27 deletions.
102 changes: 76 additions & 26 deletions packages/flutter/lib/src/material/tabs.dart
Expand Up @@ -2219,7 +2219,7 @@ class TabPageSelectorIndicator extends StatelessWidget {
///
/// If a [TabController] is not provided, then there must be a
/// [DefaultTabController] ancestor.
class TabPageSelector extends StatelessWidget {
class TabPageSelector extends StatefulWidget {
/// Creates a compact widget that indicates which tab has been selected.
const TabPageSelector({
super.key,
Expand Down Expand Up @@ -2256,6 +2256,73 @@ class TabPageSelector extends StatelessWidget {
/// Defaults to [BorderStyle.solid] if value is not specified.
final BorderStyle? borderStyle;

@override
State<TabPageSelector> createState() => _TabPageSelectorState();
}

class _TabPageSelectorState extends State<TabPageSelector> {
TabController? _previousTabController;
TabController get _tabController {
final TabController? tabController = widget.controller ?? DefaultTabController.maybeOf(context);
assert(() {
if (tabController == null) {
throw FlutterError(
'No TabController for $runtimeType.\n'
'When creating a $runtimeType, you must either provide an explicit TabController '
'using the "controller" property, or you must ensure that there is a '
'DefaultTabController above the $runtimeType.\n'
'In this case, there was neither an explicit controller nor a default controller.',
);
}
return true;
}());
return tabController!;
}

CurvedAnimation? _animation;

@override
void initState() {
super.initState();
_setAnimation();
}

@override
void didUpdateWidget (TabPageSelector oldWidget) {
super.didUpdateWidget(oldWidget);
if (_previousTabController?.animation != _tabController.animation) {
_setAnimation();
}
if (_previousTabController != _tabController) {
_previousTabController = _tabController;
}
}

@override
void didChangeDependencies() {
super.didChangeDependencies();
if (_previousTabController?.animation != _tabController.animation) {
_setAnimation();
}
if (_previousTabController != _tabController) {
_previousTabController = _tabController;
}
}

void _setAnimation() {
_animation?.dispose();
_animation = CurvedAnimation(
parent: _tabController.animation!,
curve: Curves.fastOutSlowIn,
);
}

@override
void dispose() {
_animation?.dispose();
super.dispose();
}

Widget _buildTabIndicator(
int tabIndex,
TabController tabController,
Expand Down Expand Up @@ -2290,44 +2357,27 @@ class TabPageSelector extends StatelessWidget {
return TabPageSelectorIndicator(
backgroundColor: background,
borderColor: selectedColorTween.end!,
size: indicatorSize,
borderStyle: borderStyle ?? BorderStyle.solid,
size: widget.indicatorSize,
borderStyle: widget.borderStyle ?? BorderStyle.solid,
);
}

@override
Widget build(BuildContext context) {
final Color fixColor = color ?? Colors.transparent;
final Color fixSelectedColor = selectedColor ?? Theme.of(context).colorScheme.secondary;
final Color fixColor = widget.color ?? Colors.transparent;
final Color fixSelectedColor = widget.selectedColor ?? Theme.of(context).colorScheme.secondary;
final ColorTween selectedColorTween = ColorTween(begin: fixColor, end: fixSelectedColor);
final ColorTween previousColorTween = ColorTween(begin: fixSelectedColor, end: fixColor);
final TabController? tabController = controller ?? DefaultTabController.maybeOf(context);
final MaterialLocalizations localizations = MaterialLocalizations.of(context);
assert(() {
if (tabController == null) {
throw FlutterError(
'No TabController for $runtimeType.\n'
'When creating a $runtimeType, you must either provide an explicit TabController '
'using the "controller" property, or you must ensure that there is a '
'DefaultTabController above the $runtimeType.\n'
'In this case, there was neither an explicit controller nor a default controller.',
);
}
return true;
}());
final Animation<double> animation = CurvedAnimation(
parent: tabController!.animation!,
curve: Curves.fastOutSlowIn,
);
return AnimatedBuilder(
animation: animation,
animation: _animation!,
builder: (BuildContext context, Widget? child) {
return Semantics(
label: localizations.tabLabel(tabIndex: tabController.index + 1, tabCount: tabController.length),
label: localizations.tabLabel(tabIndex: _tabController.index + 1, tabCount: _tabController.length),
child: Row(
mainAxisSize: MainAxisSize.min,
children: List<Widget>.generate(tabController.length, (int tabIndex) {
return _buildTabIndicator(tabIndex, tabController, selectedColorTween, previousColorTween);
children: List<Widget>.generate(_tabController.length, (int tabIndex) {
return _buildTabIndicator(tabIndex, _tabController, selectedColorTween, previousColorTween);
}).toList(),
),
);
Expand Down
6 changes: 5 additions & 1 deletion packages/flutter/test/material/page_selector_test.dart
Expand Up @@ -4,6 +4,7 @@

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

const Color kSelectedColor = Color(0xFF00FF00);
const Color kUnselectedColor = Colors.transparent;
Expand Down Expand Up @@ -86,7 +87,10 @@ void main() {
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kUnselectedColor, kSelectedColor]);
});

testWidgets('PageSelector responds correctly to TabController.animateTo()', (WidgetTester tester) async {
testWidgets('PageSelector responds correctly to TabController.animateTo()',
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: const <String>['CurvedAnimation']),
(WidgetTester tester) async {
final TabController tabController = TabController(
vsync: const TestVSync(),
length: 3,
Expand Down

0 comments on commit af27093

Please sign in to comment.