Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) by gawi151 · Pull Request #148298 · flutter/flutter

gawi151 · 2024-05-14T07:57:09Z

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 #138977

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gawi151 · 2024-05-14T07:58:07Z

Hi, @Piinks Would you be so kind to review the fix?

Piinks

LGTM with some nits, thank you for sending a PR!

Piinks · 2024-05-21T21:46:50Z

packages/flutter/test/widgets/two_dimensional_utils.dart

Super clever!

packages/flutter/test/widgets/two_dimensional_viewport_test.dart

…pt alive

Signed-off-by: Lukasz Gawron <[email protected]>

gawi151 · 2024-05-22T05:05:22Z

@Piinks Thx for fixing wording for me ;)

gawi151 · 2024-05-23T11:34:59Z

@Piinks, could you please assist with adding the auto-submit label to this PR? We would greatly appreciate your help in getting it merged promptly. It's crucial for us to have this included in the next bugfix release, as we are currently unable to upgrade beyond Flutter 3.13. Thank you for your support!

Piinks · 2024-05-23T19:55:31Z

This PR still needs one more review before it can be merged, I am working on getting that now.

justinmc

LGTM 👍

justinmc · 2024-05-23T20:05:49Z

packages/flutter/test/widgets/two_dimensional_viewport_test.dart

+      await tester.pump();
+      expect(find.byKey(checkBoxKey), findsNothing);
+
+      // Bring back into view


Nit: Period here at the end.

…ved (when no longer should be kept alive) (flutter/flutter#148298)

flutter/flutter@8dd0831...cb26a01 2024-05-25 [email protected] Roll Flutter Engine from e0207131ea13 to 1d154ff93a87 (1 revision) (flutter/flutter#149070) 2024-05-25 [email protected] Roll Flutter Engine from f693fdd840e8 to e0207131ea13 (7 revisions) (flutter/flutter#149068) 2024-05-25 [email protected] Roll Flutter Engine from b5e98f9b8d0f to f693fdd840e8 (4 revisions) (flutter/flutter#149057) 2024-05-24 [email protected] fix popup menu offset when using `useRootNavigator` in `PopupMenu` (flutter/flutter#144670) 2024-05-24 [email protected] [GAR][a11y] Update assessment app to unblock tester from testing dark mode (flutter/flutter#149055) 2024-05-24 [email protected] [a11y] Slider should respect bold text setting (flutter/flutter#149053) 2024-05-24 [email protected] Add tests for editable_text.on_changed.0.dart API example. (flutter/flutter#148874) 2024-05-24 [email protected] Roll Flutter Engine from 979214444aca to b5e98f9b8d0f (1 revision) (flutter/flutter#149051) 2024-05-24 [email protected] Add test for text_editing_controller.0.dart API example. (flutter/flutter#148872) 2024-05-24 [email protected] Roll Flutter Engine from 4ab442475223 to 979214444aca (3 revisions) (flutter/flutter#149045) 2024-05-24 [email protected] Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) (flutter/flutter#148298) 2024-05-24 [email protected] Roll Packages from 1008d9e to 7b423f5 (2 revisions) (flutter/flutter#149043) 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 [email protected],[email protected],[email protected] 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…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

… (when no longer should be kept alive) (#149639) Cherry pick request of #148298 to stable. Fixes problems in 2D viewport when using keep alive widgets (affects users using Material Ink components).

…ved (when no longer should be kept alive) (flutter/flutter#148298)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels May 14, 2024

gawi151 changed the title ~~Fix two dimensional keep alive child not always removed~~ Fix two dimensional's viewport keep alive child not always removed May 14, 2024

gawi151 changed the title ~~Fix two dimensional's viewport keep alive child not always removed~~ Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) May 14, 2024

goderbauer requested a review from Piinks May 14, 2024 22:09

Piinks approved these changes May 21, 2024

View reviewed changes

gawi151 and others added 7 commits May 21, 2024 16:53

fixes child not removed from keep alive bucket when it's no longer ke…

8fc0aca

…pt alive

fix comments

f26468d

Signed-off-by: Lukasz Gawron <[email protected]>

rename state

de07d95

Signed-off-by: Lukasz Gawron <[email protected]>

fix analysis

db4c062

Signed-off-by: Lukasz Gawron <[email protected]>

fix typo

eb4bbb0

Signed-off-by: Lukasz Gawron <[email protected]>

Update packages/flutter/test/widgets/two_dimensional_viewport_test.dart

ff4a262

Update packages/flutter/test/widgets/two_dimensional_viewport_test.dart

807923d

Piinks force-pushed the fix/#138977-two-dimensional-keep-alive-child-not-always-removed branch from c997e8a to 807923d Compare May 21, 2024 21:53

justinmc approved these changes May 23, 2024

View reviewed changes

bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2024

auto-submit bot merged commit a53b78d into flutter:master May 24, 2024

gawi151 deleted the fix/#138977-two-dimensional-keep-alive-child-not-always-removed branch May 25, 2024 14:14

engine-flutter-autoroll mentioned this pull request May 25, 2024

Roll Flutter from 8dd08314d2e4 to cb26a01a40e9 (12 revisions) flutter/packages#6794

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 25, 2024

a53b78d Fix TwoDimensionalViewport's keep alive child not always remo…

dd96609

…ved (when no longer should be kept alive) (flutter/flutter#148298)

Piinks mentioned this pull request May 31, 2024

Flutter Framework Team: Notable Commits #121415

Closed

gawi151 mentioned this pull request Jun 3, 2024

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

Closed

gawi151 mentioned this pull request Jun 4, 2024

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

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024

a53b78d Fix TwoDimensionalViewport's keep alive child not always remo…

e8e5466

…ved (when no longer should be kept alive) (flutter/flutter#148298)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive)#148298

Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive)#148298
auto-submit[bot] merged 7 commits intoflutter:masterfrom
gawi151:fix/#138977-two-dimensional-keep-alive-child-not-always-removed

gawi151 commented May 14, 2024

Uh oh!

gawi151 commented May 14, 2024 •

edited

Loading

Uh oh!

Piinks left a comment

Uh oh!

Piinks May 21, 2024

Uh oh!

gawi151 May 22, 2024

Uh oh!

Uh oh!

Uh oh!

gawi151 commented May 22, 2024

Uh oh!

gawi151 commented May 23, 2024

Uh oh!

Piinks commented May 23, 2024

Uh oh!

justinmc left a comment

Uh oh!

justinmc May 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

gawi151 commented May 14, 2024

Pre-launch Checklist

Uh oh!

gawi151 commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks May 21, 2024

Choose a reason for hiding this comment

Uh oh!

gawi151 May 22, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gawi151 commented May 22, 2024

Uh oh!

gawi151 commented May 23, 2024

Uh oh!

Piinks commented May 23, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc May 23, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gawi151 commented May 14, 2024 •

edited

Loading