Guard against usage after async callbacks in RenderAndroidView, unregister listener by dnfield · Pull Request #108496 · flutter/flutter · GitHub
Skip to content

Guard against usage after async callbacks in RenderAndroidView, unregister listener - #108496

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
dnfield:pview_size
Jul 28, 2022
Merged

auto-submit[bot] merged 3 commits into
flutter:masterfrom
dnfield:pview_size

Conversation

@dnfield

@dnfield dnfield commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

xref b/186978412

When this code was written, render object disposal wasn't a thing. When I implemented that, I didn't realize there were async methods being used in this render object.

The test in this patch is basically a clone of an existing test, but it disposes the RO before completing some of the async operations. Without the changes in the implementation the test fails.

@flutter-dashboard flutter-dashboard Bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 27, 2022
@dnfield
dnfield requested a review from bparrishMines July 27, 2022 23:27

@stuartmorgan-g stuartmorgan-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like a mock in another test needs updating, but otherwise LGTM. Thanks for investigating this!

We should definitely cherry-pick this to beta (and maybe even stable?).

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 28, 2022

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@auto-submit

auto-submit Bot commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 28, 2022
@dnfield

dnfield commented Jul 28, 2022

Copy link
Copy Markdown
Contributor Author

Is the flutter_plugins failure a known issue? Looks like mockito deprecated something that plugins is using?

@goderbauer

goderbauer commented Jul 28, 2022 •

Copy link
Copy Markdown
Member

Is the flutter_plugins failure a known issue? Looks like mockito deprecated something that plugins is using?

/cc @Piinks Is this the same issue that turned the tree red?

@Piinks

Piinks commented Jul 28, 2022

Copy link
Copy Markdown
Contributor

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 28, 2022
@auto-submit
auto-submit Bot merged commit 0cc7db5 into flutter:master Jul 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jul 29, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 29, 2022
godofredoc added a commit that referenced this pull request Aug 2, 2022
…108831)

* Guard against usage after async callbacks in RenderAndroidView, unregister listener (#108496)

* Roll flutter/plugins (#108556)

Co-authored-by: Dan Field <dnfield@google.com>
Co-authored-by: stuartmorgan <stuartmorgan@google.com>
@KourosBey KourosBey mentioned this pull request Aug 10, 2022
8 tasks
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
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 framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants