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

_RenderChip should not create OpacityLayer without disposing. #134708

Merged
merged 24 commits into from
Sep 22, 2023

Conversation

polina-c
Copy link
Contributor

@polina-c polina-c commented Sep 14, 2023

Fixes #134394
Prerequisite: #134768

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Sep 14, 2023
@github-actions github-actions bot added the a: tests "flutter test", flutter_test, or one of our tests label Sep 14, 2023
@polina-c polina-c marked this pull request as ready for review September 14, 2023 20:01
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@polina-c
Copy link
Contributor Author

Comment from @goderbauer : #134701 (comment)

@polina-c
Copy link
Contributor Author

polina-c commented Sep 15, 2023

All tests passed, except Google testing that failed goldens:

https://fusion2.corp.google.com/presubmit/565529126/OCL:565529126:BASE:565532441:1694740599247:ccbdcd1/targets

@polina-c polina-c marked this pull request as ready for review September 15, 2023 00:58
@polina-c polina-c marked this pull request as draft September 16, 2023 18:40
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polina-c
Copy link
Contributor Author

@goderbauer , it seems goldens are affected: cl/567461418
I am little scared to merge this. Can you check?

@goderbauer
Copy link
Member

I didn't look at all of them, but for the ones I spot-checked it looked like it was just shifting by a pixel here and there. If they are all just those tiny differences, this looks fine.

@polina-c
Copy link
Contributor Author

Yes, all of them are tiny.

@polina-c polina-c merged commit 99ac6b8 into flutter:master Sep 22, 2023
67 checks passed
@polina-c polina-c deleted the _RenderChip branch September 22, 2023 20:44
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker a: tests "flutter test", flutter_test, or one of our tests f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instances of OpacityLayer, created by _RenderChip, should be disposed.
2 participants