Skip to content

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

Merged
auto-submit[bot] merged 1 commit intoflutter:flutter-3.22-candidate.0from
gawi151:cp-issue-fix-138977
Jun 5, 2024
Merged

[CP] Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive)#149639
auto-submit[bot] merged 1 commit intoflutter:flutter-3.22-candidate.0from
gawi151:cp-issue-fix-138977

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
761747b [CP] Fix TwoDimensionalViewport's keep alive child not always…
d688778 
… 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