Fix memory leaks in PopupMenu (#147174)

This commit is contained in:
Valentin Vignal 2024-04-23 11:40:33 +08:00 committed by GitHub
parent 89a4ffaad5
commit 4938403a7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 77 additions and 27 deletions

View File

@ -572,7 +572,7 @@ class _CheckedPopupMenuItemState<T> extends PopupMenuItemState<T, CheckedPopupMe
} }
} }
class _PopupMenu<T> extends StatelessWidget { class _PopupMenu<T> extends StatefulWidget {
const _PopupMenu({ const _PopupMenu({
super.key, super.key,
required this.itemKeys, required this.itemKeys,
@ -588,23 +588,69 @@ class _PopupMenu<T> extends StatelessWidget {
final BoxConstraints? constraints; final BoxConstraints? constraints;
final Clip clipBehavior; final Clip clipBehavior;
@override
State<_PopupMenu<T>> createState() => _PopupMenuState<T>();
}
class _PopupMenuState<T> extends State<_PopupMenu<T>> {
List<CurvedAnimation> _opacities = const <CurvedAnimation>[];
@override
void initState() {
super.initState();
_setOpacities();
}
@override
void didUpdateWidget(covariant _PopupMenu<T> oldWidget) {
super.didUpdateWidget(oldWidget);
if (
oldWidget.route.items.length != widget.route.items.length ||
oldWidget.route.animation != widget.route.animation
) {
_setOpacities();
}
}
void _setOpacities() {
for (final CurvedAnimation opacity in _opacities) {
opacity.dispose();
}
final List<CurvedAnimation> newOpacities = <CurvedAnimation>[];
final double unit = 1.0 / (widget.route.items.length + 1.5); // 1.0 for the width and 0.5 for the last item's fade.
for (int i = 0; i < widget.route.items.length; i += 1) {
final double start = (i + 1) * unit;
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0);
final CurvedAnimation opacity = CurvedAnimation(
parent: widget.route.animation!,
curve: Interval(start, end),
);
newOpacities.add(opacity);
}
_opacities = newOpacities;
}
@override
void dispose() {
for (final CurvedAnimation opacity in _opacities) {
opacity.dispose();
}
super.dispose();
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
final double unit = 1.0 / (route.items.length + 1.5); // 1.0 for the width and 0.5 for the last item's fade. final double unit = 1.0 / (widget.route.items.length + 1.5); // 1.0 for the width and 0.5 for the last item's fade.
final List<Widget> children = <Widget>[]; final List<Widget> children = <Widget>[];
final ThemeData theme = Theme.of(context); final ThemeData theme = Theme.of(context);
final PopupMenuThemeData popupMenuTheme = PopupMenuTheme.of(context); final PopupMenuThemeData popupMenuTheme = PopupMenuTheme.of(context);
final PopupMenuThemeData defaults = theme.useMaterial3 ? _PopupMenuDefaultsM3(context) : _PopupMenuDefaultsM2(context); final PopupMenuThemeData defaults = theme.useMaterial3 ? _PopupMenuDefaultsM3(context) : _PopupMenuDefaultsM2(context);
for (int i = 0; i < route.items.length; i += 1) { for (int i = 0; i < widget.route.items.length; i += 1) {
final double start = (i + 1) * unit; final CurvedAnimation opacity = _opacities[i];
final double end = clampDouble(start + 1.5 * unit, 0.0, 1.0); Widget item = widget.route.items[i];
final CurvedAnimation opacity = CurvedAnimation( if (widget.route.initialValue != null && widget.route.items[i].represents(widget.route.initialValue)) {
parent: route.animation!,
curve: Interval(start, end),
);
Widget item = route.items[i];
if (route.initialValue != null && route.items[i].represents(route.initialValue)) {
item = ColoredBox( item = ColoredBox(
color: Theme.of(context).highlightColor, color: Theme.of(context).highlightColor,
child: item, child: item,
@ -613,10 +659,10 @@ class _PopupMenu<T> extends StatelessWidget {
children.add( children.add(
_MenuItem( _MenuItem(
onLayout: (Size size) { onLayout: (Size size) {
route.itemSizes[i] = size; widget.route.itemSizes[i] = size;
}, },
child: FadeTransition( child: FadeTransition(
key: itemKeys[i], key: widget.itemKeys[i],
opacity: opacity, opacity: opacity,
child: item, child: item,
), ),
@ -626,10 +672,10 @@ class _PopupMenu<T> extends StatelessWidget {
final CurveTween opacity = CurveTween(curve: const Interval(0.0, 1.0 / 3.0)); final CurveTween opacity = CurveTween(curve: const Interval(0.0, 1.0 / 3.0));
final CurveTween width = CurveTween(curve: Interval(0.0, unit)); final CurveTween width = CurveTween(curve: Interval(0.0, unit));
final CurveTween height = CurveTween(curve: Interval(0.0, unit * route.items.length)); final CurveTween height = CurveTween(curve: Interval(0.0, unit * widget.route.items.length));
final Widget child = ConstrainedBox( final Widget child = ConstrainedBox(
constraints: constraints ?? const BoxConstraints( constraints: widget.constraints ?? const BoxConstraints(
minWidth: _kMenuMinWidth, minWidth: _kMenuMinWidth,
maxWidth: _kMenuMaxWidth, maxWidth: _kMenuMaxWidth,
), ),
@ -639,7 +685,7 @@ class _PopupMenu<T> extends StatelessWidget {
scopesRoute: true, scopesRoute: true,
namesRoute: true, namesRoute: true,
explicitChildNodes: true, explicitChildNodes: true,
label: semanticLabel, label: widget.semanticLabel,
child: SingleChildScrollView( child: SingleChildScrollView(
padding: const EdgeInsets.symmetric( padding: const EdgeInsets.symmetric(
vertical: _kMenuVerticalPadding, vertical: _kMenuVerticalPadding,
@ -651,22 +697,22 @@ class _PopupMenu<T> extends StatelessWidget {
); );
return AnimatedBuilder( return AnimatedBuilder(
animation: route.animation!, animation: widget.route.animation!,
builder: (BuildContext context, Widget? child) { builder: (BuildContext context, Widget? child) {
return FadeTransition( return FadeTransition(
opacity: opacity.animate(route.animation!), opacity: opacity.animate(widget.route.animation!),
child: Material( child: Material(
shape: route.shape ?? popupMenuTheme.shape ?? defaults.shape, shape: widget.route.shape ?? popupMenuTheme.shape ?? defaults.shape,
color: route.color ?? popupMenuTheme.color ?? defaults.color, color: widget.route.color ?? popupMenuTheme.color ?? defaults.color,
clipBehavior: clipBehavior, clipBehavior: widget.clipBehavior,
type: MaterialType.card, type: MaterialType.card,
elevation: route.elevation ?? popupMenuTheme.elevation ?? defaults.elevation!, elevation: widget.route.elevation ?? popupMenuTheme.elevation ?? defaults.elevation!,
shadowColor: route.shadowColor ?? popupMenuTheme.shadowColor ?? defaults.shadowColor, shadowColor: widget.route.shadowColor ?? popupMenuTheme.shadowColor ?? defaults.shadowColor,
surfaceTintColor: route.surfaceTintColor ?? popupMenuTheme.surfaceTintColor ?? defaults.surfaceTintColor, surfaceTintColor: widget.route.surfaceTintColor ?? popupMenuTheme.surfaceTintColor ?? defaults.surfaceTintColor,
child: Align( child: Align(
alignment: AlignmentDirectional.topEnd, alignment: AlignmentDirectional.topEnd,
widthFactor: width.evaluate(route.animation!), widthFactor: width.evaluate(widget.route.animation!),
heightFactor: height.evaluate(route.animation!), heightFactor: height.evaluate(widget.route.animation!),
child: child, child: child,
), ),
), ),

View File

@ -4,6 +4,7 @@
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
void main() { void main() {
testWidgets('Theme.wrap()', (WidgetTester tester) async { testWidgets('Theme.wrap()', (WidgetTester tester) async {
@ -145,7 +146,10 @@ void main() {
await tester.pumpAndSettle(); // menu route animation await tester.pumpAndSettle(); // menu route animation
}); });
testWidgets('Material3 - PopupMenuTheme.wrap()', (WidgetTester tester) async { testWidgets('Material3 - PopupMenuTheme.wrap()',
// TODO(polina-c): remove when fixed https://github.com/flutter/flutter/issues/145600 [leak-tracking-opt-in]
experimentalLeakTesting: LeakTesting.settings.withTracked(classes: <String>['CurvedAnimation']),
(WidgetTester tester) async {
const TextStyle textStyle = TextStyle(fontSize: 24.0, color: Color(0xFF0000FF)); const TextStyle textStyle = TextStyle(fontSize: 24.0, color: Color(0xFF0000FF));
Widget buildFrame() { Widget buildFrame() {