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 crasher in DragableScrollableSheet when controller is animating and switching widgets #125721

Merged
merged 1 commit into from Apr 29, 2023

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Apr 28, 2023

We were failing to dispose of animation controllers created by animateTo when didUpdateWidget happens and we null out the _attachedController. This would cause the listener added here to fail on a null assertion:

_attachedController!.extent.updateSize(

Without the code change, the test ends up throwing exceptions related to that line. I don't have a great way to verify what this does visually though, hoping for help on that from internal customer.

b/279930163

See cl/527868355 as well.

Fixes #125709

@flutter-dashboard flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels Apr 28, 2023
@@ -1404,22 +1404,21 @@ void main() {
await tester.pumpWidget(boilerplate);
expect(controller.isAttached, true);
expect(controller.size, isNotNull);
});
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I was looking at these tests initially - no actually changes here, just fixing the whitespace)

expect(controller2.isAttached, false);


controller1.animateTo(0.5, curve: Curves.linear, duration: const Duration(milliseconds: 200));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the key difference between the test above this one - in the test above, jumpTo is called instead of animateTo. jumpTo doesn't have any ongoing state/actions between the didUpdateWidget, whereas animateTo does.

Copy link
Member

Choose a reason for hiding this comment

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

final Set<AnimationController> _animationControllers = <AnimationController>{};

I am curious why this is a collection. I think we only need one controller instead of creating a new one every time and caching it, which will cause memory overhead and leaks.

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.

This LGTM. Thank you for finding this!

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 29, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 29, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Apr 29, 2023

auto label is removed for flutter/flutter, pr: 125721, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 29, 2023
@auto-submit auto-submit bot merged commit e1e8ad8 into flutter:master Apr 29, 2023
70 checks passed
@dnfield dnfield deleted the dss branch April 29, 2023 05:39
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2023
tarrinneal pushed a commit to flutter/packages that referenced this pull request May 1, 2023
…3874)

Manual roll Flutter from 66fa4c5d301c to 828a04040e11 (79 revisions)

Manual roll requested by tarrinneal@google.com

flutter/flutter@66fa4c5...828a040

2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818)
2023-05-01 34871572+gmackall@users.noreply.github.com Revert "Add
migrator to upgrade gradle version when conflict with And…
(flutter/flutter#125813)
2023-05-01 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125801)
2023-05-01 andrewrkolos@gmail.com [tools] fix `expect` calls in
`FakeCommand` (flutter/flutter#125783)
2023-05-01 engine-flutter-autoroll@skia.org Roll Packages from
7e3f5da to de6131d (41 revisions) (flutter/flutter#125811)
2023-05-01 tessertaha@gmail.com Introduce `TabBar.tabAlignment`
(flutter/flutter#125036)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805)
2023-05-01 44755140+werainkhatri@users.noreply.github.com add support to
customize Slider interacivity (flutter/flutter#121483)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787)
2023-05-01 andrewrkolos@gmail.com [tools] Apply Android Studio version
detection logic to explicitly configured installation directory
(`flutter config --android-studio-dir`) (flutter/flutter#125596)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735)
2023-04-29 dnfield@google.com Fix crasher in DragableScrollableSheet
when controller is animating and switching widgets
(flutter/flutter#125721)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722)
2023-04-29 65850618+Anas35@users.noreply.github.com fix package template
create platform folders (flutter/flutter#125292)
2023-04-28 thkim1011@users.noreply.github.com Sliver Cross Axis Group
(flutter/flutter#123862)
2023-04-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from
2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718)
2023-04-28 zanderso@users.noreply.github.com Remove bringup from
new_gallery_skia_ios__transition_perf (flutter/flutter#125715)
2023-04-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from
98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714)
2023-04-28 109111084+yaakovschectman@users.noreply.github.com Opt into
CMake policy CMP0135 (flutter/flutter#125502)
2023-04-28 leroux_bruno@yahoo.fr Add a channel to query the engine
keyboard state (flutter/flutter#122885)
2023-04-28 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125698)
2023-04-28 36861262+QuncCccccc@users.noreply.github.com
`Checkbox.fillColor` should be applied to checkbox's background color
when it is unchecked. (flutter/flutter#125643)
2023-04-28 zanderso@users.noreply.github.com Add back one Skia test on
iOS (flutter/flutter#125663)
2023-04-28 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125447)
2023-04-28 97679004+phlippieb-discovery@users.noreply.github.com Nit:
grammar in documentation (flutter/flutter#125462)
...
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
…lutter#3874)

Manual roll Flutter from 66fa4c5d301c to 828a04040e11 (79 revisions)

Manual roll requested by tarrinneal@google.com

flutter/flutter@66fa4c5...828a040

2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
666bc34c61aa to 687f4c761db1 (2 revisions) (flutter/flutter#125818)
2023-05-01 34871572+gmackall@users.noreply.github.com Revert "Add
migrator to upgrade gradle version when conflict with And…
(flutter/flutter#125813)
2023-05-01 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125801)
2023-05-01 andrewrkolos@gmail.com [tools] fix `expect` calls in
`FakeCommand` (flutter/flutter#125783)
2023-05-01 engine-flutter-autoroll@skia.org Roll Packages from
7e3f5da to de6131d (41 revisions) (flutter/flutter#125811)
2023-05-01 tessertaha@gmail.com Introduce `TabBar.tabAlignment`
(flutter/flutter#125036)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
b0da68e7e024 to 666bc34c61aa (1 revision) (flutter/flutter#125805)
2023-05-01 44755140+werainkhatri@users.noreply.github.com add support to
customize Slider interacivity (flutter/flutter#121483)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
b4551c72487c to b0da68e7e024 (1 revision) (flutter/flutter#125800)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
605528f293d0 to b4551c72487c (1 revision) (flutter/flutter#125795)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
bba66b658cee to 605528f293d0 (2 revisions) (flutter/flutter#125793)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
2fa61b91d7c2 to bba66b658cee (1 revision) (flutter/flutter#125791)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
30c91b8180e7 to 2fa61b91d7c2 (1 revision) (flutter/flutter#125789)
2023-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
d76a22e67eea to 30c91b8180e7 (1 revision) (flutter/flutter#125787)
2023-05-01 andrewrkolos@gmail.com [tools] Apply Android Studio version
detection logic to explicitly configured installation directory
(`flutter config --android-studio-dir`) (flutter/flutter#125596)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
f234d5e1dd26 to d76a22e67eea (1 revision) (flutter/flutter#125776)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
c796390d14cb to f234d5e1dd26 (1 revision) (flutter/flutter#125773)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
e99f31f4437d to c796390d14cb (1 revision) (flutter/flutter#125762)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
1942b0c2cd9a to e99f31f4437d (1 revision) (flutter/flutter#125758)
2023-04-30 engine-flutter-autoroll@skia.org Roll Flutter Engine from
7806f8a4fb4c to 1942b0c2cd9a (1 revision) (flutter/flutter#125757)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
8167f909bc8d to 7806f8a4fb4c (2 revisions) (flutter/flutter#125750)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
900b8a89b73b to 8167f909bc8d (1 revision) (flutter/flutter#125748)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
c56ea398b0dc to 900b8a89b73b (1 revision) (flutter/flutter#125747)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
0834c886f06a to c56ea398b0dc (1 revision) (flutter/flutter#125746)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
68f2ed0a1db5 to 0834c886f06a (1 revision) (flutter/flutter#125736)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
0079bb4a20d0 to 68f2ed0a1db5 (1 revision) (flutter/flutter#125735)
2023-04-29 dnfield@google.com Fix crasher in DragableScrollableSheet
when controller is animating and switching widgets
(flutter/flutter#125721)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
8f04b29c1b98 to 0079bb4a20d0 (2 revisions) (flutter/flutter#125734)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
788d0ed5ed06 to 8f04b29c1b98 (1 revision) (flutter/flutter#125731)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
89a8affdced0 to 788d0ed5ed06 (1 revision) (flutter/flutter#125729)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
3835d975c8b0 to 89a8affdced0 (2 revisions) (flutter/flutter#125725)
2023-04-29 engine-flutter-autoroll@skia.org Roll Flutter Engine from
1ae848ce6b55 to 3835d975c8b0 (1 revision) (flutter/flutter#125722)
2023-04-29 65850618+Anas35@users.noreply.github.com fix package template
create platform folders (flutter/flutter#125292)
2023-04-28 thkim1011@users.noreply.github.com Sliver Cross Axis Group
(flutter/flutter#123862)
2023-04-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from
2a84ea55e4ef to 1ae848ce6b55 (1 revision) (flutter/flutter#125718)
2023-04-28 zanderso@users.noreply.github.com Remove bringup from
new_gallery_skia_ios__transition_perf (flutter/flutter#125715)
2023-04-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from
98b6fabc66bb to 2a84ea55e4ef (10 revisions) (flutter/flutter#125714)
2023-04-28 109111084+yaakovschectman@users.noreply.github.com Opt into
CMake policy CMP0135 (flutter/flutter#125502)
2023-04-28 leroux_bruno@yahoo.fr Add a channel to query the engine
keyboard state (flutter/flutter#122885)
2023-04-28 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125698)
2023-04-28 36861262+QuncCccccc@users.noreply.github.com
`Checkbox.fillColor` should be applied to checkbox's background color
when it is unchecked. (flutter/flutter#125643)
2023-04-28 zanderso@users.noreply.github.com Add back one Skia test on
iOS (flutter/flutter#125663)
2023-04-28 fluttergithubbot@gmail.com Roll pub packages
(flutter/flutter#125447)
2023-04-28 97679004+phlippieb-discovery@users.noreply.github.com Nit:
grammar in documentation (flutter/flutter#125462)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App customer: chalk (g3) f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash in DraggableScrollableController.animateTo
3 participants