Skip to content

Commit

Permalink
Reland fix memory leaks for tab selector (#147689)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ValentinVignal
ValentinVignal authored May 5, 2024
1 parent 145e940 commit 02d239a
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 27 deletions.
96 changes: 70 additions & 26 deletions packages/flutter/lib/src/material/tabs.dart
View file
Original file line number Diff line number Diff line change
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,67 @@ 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 didUpdateWidget (TabPageSelector oldWidget) {
super.didUpdateWidget(oldWidget);
if (_previousTabController?.animation != _tabController.animation) {
_setAnimation();
}
if (_previousTabController != _tabController) {
_previousTabController = _tabController;
}
}

@override
void didChangeDependencies() {
super.didChangeDependencies();
if (_animation == null || _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 +2351,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
90 changes: 89 additions & 1 deletion packages/flutter/test/material/page_selector_test.dart
View file
Original file line number Diff line number Diff line change
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 Expand Up @@ -277,4 +281,88 @@ void main() {
expect(indicator.borderStyle, BorderStyle.solid);
}
});

testWidgets('PageSelector responds correctly to TabController.animateTo() from the default tab controller',
// 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 {
await tester.pumpWidget(
Localizations(
locale: const Locale('en', 'US'),
delegates: const <LocalizationsDelegate<dynamic>>[
DefaultMaterialLocalizations.delegate,
DefaultWidgetsLocalizations.delegate,
],
child: Directionality(
textDirection: TextDirection.ltr,
child: Theme(
data: ThemeData(colorScheme: const ColorScheme.light().copyWith(secondary: kSelectedColor)),
child: const SizedBox.expand(
child: Center(
child: SizedBox(
width: 400.0,
height: 400.0,
child: DefaultTabController(
length: 3,
child: Column(
children: <Widget>[
TabPageSelector(
),
Flexible(
child: TabBarView(
children: <Widget>[
Center(child: Text('0')),
Center(child: Text('1')),
Center(child: Text('2')),
],
),
),
],
),
),
),
),
),
),
),
),
);

final TabController tabController = DefaultTabController.of(tester.element(find.byType(TabPageSelector)));

expect(tabController.index, 0);
expect(indicatorColors(tester), const <Color>[kSelectedColor, kUnselectedColor, kUnselectedColor]);

tabController.animateTo(1, duration: const Duration(milliseconds: 200));
await tester.pump();
// Verify that indicator 0's color is becoming increasingly transparent,
// and indicator 1's color is becoming increasingly opaque during the
// 200ms animation. Indicator 2 remains transparent throughout.
await tester.pump(const Duration(milliseconds: 10));
List<Color> colors = indicatorColors(tester);
expect(colors[0].alpha, greaterThan(colors[1].alpha));
expect(colors[2], kUnselectedColor);
await tester.pump(const Duration(milliseconds: 175));
colors = indicatorColors(tester);
expect(colors[0].alpha, lessThan(colors[1].alpha));
expect(colors[2], kUnselectedColor);
await tester.pumpAndSettle();
expect(tabController.index, 1);
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kSelectedColor, kUnselectedColor]);

tabController.animateTo(2, duration: const Duration(milliseconds: 200));
await tester.pump();
// Same animation test as above for indicators 1 and 2.
await tester.pump(const Duration(milliseconds: 10));
colors = indicatorColors(tester);
expect(colors[1].alpha, greaterThan(colors[2].alpha));
expect(colors[0], kUnselectedColor);
await tester.pump(const Duration(milliseconds: 175));
colors = indicatorColors(tester);
expect(colors[1].alpha, lessThan(colors[2].alpha));
expect(colors[0], kUnselectedColor);
await tester.pumpAndSettle();
expect(tabController.index, 2);
expect(indicatorColors(tester), const <Color>[kUnselectedColor, kUnselectedColor, kSelectedColor]);
});
}

0 comments on commit 02d239a

Please sign in to comment.