Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak in CupertinoActionSheet #134885

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

ksokolovskyi
Copy link
Contributor

This PR fixes memory leaks in the CupertinoActionSheet widget, which did not dispose ScrollControllers created internally.

Description

Tests

  • Updates cupertino/action_sheet_test.dart to use testWidgetsWithLeakTracking.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Sep 16, 2023
@ksokolovskyi
Copy link
Contributor Author

cc @polina-c

@polina-c
Copy link
Contributor

polina-c commented Sep 16, 2023

I think we duplicated each other: #13479
And the changes are almost identical.

I will merge yours, because in this case I do not need approver from Flutter team.

@ksokolovskyi
Copy link
Contributor Author

I think we duplicated each other: #13479 And the changes are almost identical.

I will merge yours, because in this case I do not need approver from Flutter team.

Thanks a lot for the review, and sorry for the duplication of your fix. Next time, I will be more attentive.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 17, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 17, 2023
flutter/flutter@e5e36ad...1b18b13

2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from e07bb975ee85 to 57a4ff3c7ff8 (1 revision) (flutter/flutter#134895)
2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 444689fd4060 to e07bb975ee85 (1 revision) (flutter/flutter#134889)
2023-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8c2203bb6c3b to 444689fd4060 (1 revision) (flutter/flutter#134886)
2023-09-16 sokolovskyi.konstantin@gmail.com Fix memory leak in CupertinoActionSheet (flutter/flutter#134885)
2023-09-16 polinach@google.com Add disposal mechanism for created Layers to TestRecordingPaintingContext. (flutter/flutter#134768)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 490925676b91 to 8c2203bb6c3b (6 revisions) (flutter/flutter#134882)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from cdcbdcc7ccba to 490925676b91 (5 revisions) (flutter/flutter#134879)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 30b7e9ded7a0 to cdcbdcc7ccba (5 revisions) (flutter/flutter#134877)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC camillesimon@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
…#4940)

flutter/flutter@e5e36ad...1b18b13

2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from e07bb975ee85 to 57a4ff3c7ff8 (1 revision) (flutter/flutter#134895)
2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 444689fd4060 to e07bb975ee85 (1 revision) (flutter/flutter#134889)
2023-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8c2203bb6c3b to 444689fd4060 (1 revision) (flutter/flutter#134886)
2023-09-16 sokolovskyi.konstantin@gmail.com Fix memory leak in CupertinoActionSheet (flutter/flutter#134885)
2023-09-16 polinach@google.com Add disposal mechanism for created Layers to TestRecordingPaintingContext. (flutter/flutter#134768)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 490925676b91 to 8c2203bb6c3b (6 revisions) (flutter/flutter#134882)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from cdcbdcc7ccba to 490925676b91 (5 revisions) (flutter/flutter#134879)
2023-09-16 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 30b7e9ded7a0 to cdcbdcc7ccba (5 revisions) (flutter/flutter#134877)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC camillesimon@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScrollController not disposed in tests
2 participants