Skip to content

[CP] Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) #149639

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2024
Merged

[CP] Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) #149639

merged 1 commit into from
Jun 5, 2024

Conversation

@gawi151
Copy link
Contributor

@gawi151 gawi151 commented Jun 4, 2024

Cherry pick request of #148298 to stable.

Fixes problems in 2D viewport when using keep alive widgets (affects users using Material Ink components).

@gawi151
Fix TwoDimensionalViewport's keep alive child not always removed (whe…
c025f2b 
…n no longer should be kept alive) (flutter#148298)

- Fixes a child not removed from `_keepAliveBucket` when widget is no longer kept alive offscreen. Bug was triggering assert in performLayout.
- Adds test to cover the case from bug report

Fixes flutter#138977
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Jun 4, 2024
@gawi151 gawi151 mentioned this pull request Jun 4, 2024
Closed
@Piinks
Copy link
Contributor

Piinks commented Jun 4, 2024 •
edited
Loading

@itsjustkevin don't we use a bot for CP's? Maybe this being put up manually is what has made CI unhappy?

@itsjustkevin itsjustkevin requested a review from Piinks June 4, 2024 19:24
@itsjustkevin
Copy link
Contributor

itsjustkevin commented Jun 4, 2024

@itsjustkevin don't we use a bot for CP's? Maybe this being put up manually is what has made CI unhappy?

Yes, but the bot doesn't really do anything special, just generates a normal cherry-pick. This seems like an infra flake.

@gawi151
Copy link
Contributor Author

gawi151 commented Jun 5, 2024

Seems like all checks passed. Is there anything else I can do to help here?

@itsjustkevin itsjustkevin self-requested a review June 5, 2024 06:52
@itsjustkevin itsjustkevin added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 5, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 5, 2024

auto label is removed for flutter/flutter/149639, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 5, 2024
@itsjustkevin
Copy link
Contributor

itsjustkevin commented Jun 5, 2024

This is going to need a second reviewer cc @Piinks

@itsjustkevin itsjustkevin added cp: review Cherry-picks in the review queue cp: approved Approved cherry-pick request labels Jun 5, 2024
@itsjustkevin
Copy link
Contributor

itsjustkevin commented Jun 5, 2024

@gawi151 this may miss the current release build and make it in the next, but it is still on our radar 🙂.

Piinks
Piinks approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM

@itsjustkevin itsjustkevin added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 5, 2024
@auto-submit auto-submit bot merged commit 761747b into flutter:flutter-3.22-candidate.0 Jun 5, 2024
@gawi151 gawi151 deleted the cp-issue-fix-138977 branch June 5, 2024 21:44
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2024
@engine-flutter-autoroll
761747bfc [CP] Fix TwoDimensionalViewport's keep alive child not alwa…
d688778 
…ys removed (when no longer should be kept alive) (flutter/flutter#149639)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@itsjustkevin itsjustkevin itsjustkevin approved these changes

@Piinks Piinks Piinks approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: approved Approved cherry-pick request cp: review Cherry-picks in the review queue f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gawi151 @Piinks @itsjustkevin