Fix IconButton leaks its internal MaterialStatesController

[bleroux]

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:

flutter/packages/flutter/lib/src/material/icon_button.dart

Lines 671 to 721 in 3a1190a

	if (theme.useMaterial3) {
	final Size? minSize = constraints == null
	? null
	: Size(constraints!.minWidth, constraints!.minHeight);
	final Size? maxSize = constraints == null
	? null
	: Size(constraints!.maxWidth, constraints!.maxHeight);
	ButtonStyle adjustedStyle = styleFrom(
	visualDensity: visualDensity,
	foregroundColor: color,
	disabledForegroundColor: disabledColor,
	focusColor: focusColor,
	hoverColor: hoverColor,
	highlightColor: highlightColor,
	padding: padding,
	minimumSize: minSize,
	maximumSize: maxSize,
	iconSize: iconSize,
	alignment: alignment,
	enabledMouseCursor: mouseCursor,
	disabledMouseCursor: mouseCursor,
	enableFeedback: enableFeedback,
	);
	if (style != null) {
	adjustedStyle = style!.merge(adjustedStyle);
	}
	Widget effectiveIcon = icon;
	if ((isSelected ?? false) && selectedIcon != null) {
	effectiveIcon = selectedIcon!;
	}
	Widget iconButton = effectiveIcon;
	if (tooltip != null) {
	iconButton = Tooltip(
	message: tooltip,
	child: effectiveIcon,
	);
	}
	return _SelectableIconButton(
	style: adjustedStyle,
	onPressed: onPressed,
	autofocus: autofocus,
	focusNode: focusNode,
	isSelected: isSelected,
	variant: _variant,
	child: iconButton,
	);
	}

Related Issue

Fixes #130708

Tests

Adds 1 test.

[QuncCccccc]

Thanks for add this fix! Just left one comment for the unit test:)

[QuncCccccc]

Should we move this test to packages/flutter/test/material/about_test.dart? Seems the LicensePage() is only tested in that file.

[bleroux]

Usually we keep the test as close as possible to the fix. I agree that relying on LicensePage is not great but I was not able to figure out another way to reproduce the memory leak.
It's also true that this very same test will be part of the PR I'm working on for about_test.dart M3 migration.

I'm not used to work on PR related to leaks, maybe someone can give us guidance?

[QuncCccccc]

I think to reproduce the issue, in icon_button_test.dart, we can try to pump an IconButton for the first time and then pump other widgets to see if there's any memory leak. Also checked other examples, looks like we also need to call TestWidgetsWithLeakingTracking instead of TestWidgets.

Try this. The memory leak can be reproduced when we don't have the fix.

testWidgetsWithLeakTracking('Material3 - IconButton memory leak', (WidgetTester tester) async {
  bool isSelected = false;
  Widget buildWidget(bool showIconButton) {
    return showIconButton
      ? MaterialApp(
        theme: ThemeData(useMaterial3: true),
        home: IconButton(
          onPressed: () { },
          isSelected: isSelected,
          icon: const Icon(Icons.search),
        ))
      : const SizedBox();
  }
  await tester.pumpWidget(buildWidget(true));
  await tester.pumpWidget(buildWidget(false));
});

[bleroux]

Revisiting this, I was able to write a test which does not depend on LicensePage.

[QuncCccccc]

Oh nice! I added a comment yesterday but didn't actually press the "submit review" button😂Sorry about that! But the response is similar to what you added.

[bleroux]

Great! I really thought I tried something similar. This is way cleaner that my previous attempt, thanks!

[QuncCccccc]

Seems we still didn't use IconButton here. I think the example I provided above might be enough to show there's no leaking.

[QuncCccccc]

LGTM:) Just left one nit below.

[bleroux]

Oops I misclicked.