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

Reland "Dispose render objects when owning element is unmounted. (#82883)" (#83790) #83920

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Jun 3, 2021

This reverts commit 63c49c3.

No changes from last time, but internal CL will avoid breakages. I will try to do a full frob run to verify.

See cl/377329167

@dnfield dnfield requested a review from goderbauer June 3, 2021 18:05
@flutter-dashboard flutter-dashboard bot added d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. labels Jun 3, 2021
@google-cla google-cla bot added the cla: yes label Jun 3, 2021
@dnfield
Copy link
Contributor Author

dnfield commented Jun 3, 2021

Longer term, the plan is to patch up flare_flutter after 2d-inc/Flare-Flutter#307 lands so that it will work with these new method/lints, and undo some changes to the internal fork that have accumulated over time/migrate internal customers to Rive.

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

@dnfield
Copy link
Contributor Author

dnfield commented Jun 3, 2021

Kicked off a FROB run for this that should run more tests than Google testing does/did.

@dnfield
Copy link
Contributor Author

dnfield commented Jun 3, 2021

test/OCL:377331261:BASE:377331237:1622744968484:d1dceb5f is showing a large number of failures due to an unrelated change...Will try to patch the build file after lunch so we can at least test these changes.

This isn't compatible with some internal customers currently. We will patch them and add this back.
// child.debugDisposed!,
// '${child.runtimeType} (child of $runtimeType) must be disposed before calling super.dispose().',
// );
// });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@goderbauer just want to make sure this LGTY with this change - it will be uncommented once we can mitigate internal violations.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@dnfield
Copy link
Contributor Author

dnfield commented Jun 3, 2021

After cl/377398477 this passes internally.

@fluttergithubbot fluttergithubbot merged commit c1fe2bd into flutter:master Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants