Skip to content

Commit

Permalink
Fix IconButton leaks its internal MaterialStatesController (flutter#1…
Browse files Browse the repository at this point in the history
…30720)

## Description

This PR adds a call to dispose the internal `MaterialStatesController` instantiated by `_SelectableIconButtonState`.

I found this memory leak while working on M2/M3 test update for `about_test.dart`. This memory leak only happens when using M3 because `IconButton` relies on `_SelectableIconButton` only when useMaterial3 is true:

https://github.com/flutter/flutter/blob/3a1190a5a85c3e6a0cf3a9c30f34548fdd48ac1e/packages/flutter/lib/src/material/icon_button.dart#L671-L721

## Related Issue

Fixes flutter#130708

## Tests

Adds 1 test.
  • Loading branch information
bleroux authored Jul 20, 2023
1 parent ec50831 commit 0aba94f
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 11 deletions.
6 changes: 6 additions & 0 deletions packages/flutter/lib/src/material/icon_button.dart
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,12 @@ class _SelectableIconButtonState extends State<_SelectableIconButton> {
),
);
}

@override
void dispose() {
statesController.dispose();
super.dispose();
}
}

class _IconButtonM3 extends ButtonStyleButton {
Expand Down
42 changes: 31 additions & 11 deletions packages/flutter/test/material/icon_button_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';

import '../foundation/leak_tracking.dart';
import '../rendering/mock_canvas.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';
Expand Down Expand Up @@ -2729,18 +2730,18 @@ void main() {
final ColorScheme lightScheme = const ColorScheme.light().copyWith(onSurfaceVariant: const Color(0xffe91e60));
// Brightness.dark
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(colorScheme: lightScheme, useMaterial3: true,),
home: Scaffold(
body: IconTheme.merge(
data: const IconThemeData(size: 26),
child: IconButton(
icon: const Icon(Icons.account_box),
onPressed: () {},
),
),
)
MaterialApp(
theme: ThemeData(colorScheme: lightScheme, useMaterial3: true,),
home: Scaffold(
body: IconTheme.merge(
data: const IconThemeData(size: 26),
child: IconButton(
icon: const Icon(Icons.account_box),
onPressed: () {},
),
),
)
)
);

Color? iconColor0() => _iconStyle(tester, Icons.account_box)?.color;
Expand Down Expand Up @@ -2770,6 +2771,25 @@ void main() {

expect(tester.takeException(), isNull);
});

testWidgetsWithLeakTracking('Material3 - IconButton memory leak', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/130708.
Widget buildWidget(bool showIconButton) {
return showIconButton
? MaterialApp(
theme: ThemeData(useMaterial3: true),
home: IconButton(
onPressed: () { },
icon: const Icon(Icons.search),
),
)
: const SizedBox();
}
await tester.pumpWidget(buildWidget(true));
await tester.pumpWidget(buildWidget(false));

// No exception is thrown.
});
});
}

Expand Down

0 comments on commit 0aba94f

Please sign in to comment.