Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Sep 8, 2022

Removes the assertion about contexts in Android platform view's Virtual Display fallback.

When #33599 re-introduced Virtual Display, it added an assertion about context preservation that wasn't previously present, which changed what would previously have been a silently failure for some specific VD workarounds (which would not be necessary for all plugins) to function into an assertion failure.

In theory plugins should be updated to avoid tripping the assertion, but in practice a common pattern (using LayoutInflator) ends up violating this assertion, and there does not appear to be a straightforward fix in that case (especially if the view is crated by an SDK that the plugin wraps without controlling). Rather than leaving affected plugins more broken than they were pre-3.0, this restores the previous behavior of working, but with those specific workarounds not activated for those plugins. The assertion is replaced with a warning, so that plugin developers can be made aware of the issue, and includes suggestions for resolving it.

Fixes flutter/flutter#110146

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

public class PlatformViewsActivity extends TestActivity {
// WARNING: These strings must all be exactly the same length to avoid
// breaking the 'create' method's manual encoding in the test. See the
// TODO(stuartmorgan) about encoding alignment in platform_view.dart
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 lost at least an hour to figuring out why my new factory wasn't working at all 😐 I didn't want to yak-shave right now since this is an important fix, and one that I want to scope-limit so that we can cherry-pick it to stable, so I just left comments so that at least the problem will be obvious to the next person.

@stuartmorgan-g stuartmorgan-g changed the title Android platform view vd remove assertion Remove context assertion from Android platform views using Virtual Display Sep 8, 2022
@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 9, 2022
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #35997 at sha f2ab551

@auto-submit auto-submit bot merged commit dfe5c9e into flutter:main Sep 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2022
stuartmorgan-g added a commit to stuartmorgan-g/engine that referenced this pull request Sep 9, 2022
@stuartmorgan-g stuartmorgan-g deleted the android-platform-view-vd-remove-assertion branch September 9, 2022 16:27
itsjustkevin pushed a commit that referenced this pull request Sep 13, 2022
…rtual Display (#36046)

* Remove context assertion from Android platform views using Virtual Display (#35997)

* empty commit

Co-authored-by: Casey Hillers <[email protected]>
cfontas pushed a commit to cfontas/engine that referenced this pull request Sep 14, 2022
Oleh-Sv pushed a commit to Oleh-Sv/engine that referenced this pull request Sep 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-android will affect goldens

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android plugin crash: IllegalStateException: Unexpected platform view context

3 participants