Fix Android platform view creation flow#109232
Fix Android platform view creation flow#109232auto-submit[bot] merged 2 commits intoflutter:masterfrom
Conversation
Fixes regressions from flutter#101399: - For Hybrid Composition, there is a common race where `create` will be called by the `PlatformViewLink` even though it has already been successfully called by the plugin itself. This causes exceptions on use of many Hybrid Composition plugins, and blocks adopting the new forced-HC mode in the google_maps_flutter plugin, for instance. - The documented behavior of `AndroidViewController`, which is that `setSize` would create it on demand, was removed. That logic had been moved to an internal call site, but there may have been public callers expecting that behavior as well so it's better to keep the logic in the code documented to have that behavior. Part of flutter#107297
flutter-dashboard
bot
added
the
framework
|
@cyanglaz You reviewed the initial PR, so if you could do the primary review that would be great since you have the most context. @bparrishMines @dnfield If either or both of you could take a look that would be great. This is fixing a regression on |
| // Depending on the implementation, the initial size can be used to size | ||
| // the platform view. | ||
| return _PlatformViewPlaceHolder(onLayout: (Size size) { | ||
| if (controller.awaitingCreation) { |
There was a problem hiding this comment.
This change (and the new awaitCreation logic) is the core of the fix. Hybrid Composition was pretty broken because plugins call create is called in the initial view build, per our docs, and if onLayout here fired before that async operation was completed (which is when _platformViewCreated will actually change, down in _onPlatformViewCreated), then it would start a second call to create.
TLHC and VD weren't affected by this because the initial create doesn't have a size, and thus in those implementations doesn't do anything.
| Size targetSize; | ||
| do { | ||
| targetSize = size; | ||
| if (_viewController.isCreated) { |
There was a problem hiding this comment.
This conditional logic moved back into setSize, so no longer needed to exist here.
There was a problem hiding this comment.
consider adding an assert, e.g. assert(_viewController.isCreated); // created in setSize
There was a problem hiding this comment.
I'm pretty sure that's not a valid assertion, for the same reason that you had added the _isDisposed check just after create in the code I'm removing: setSize is guaranteed to start creating it if necessary, but there's no guarantee that it's not then disposed later in the await.
| Size targetSize; | ||
| do { | ||
| targetSize = size; | ||
| if (_viewController.isCreated) { |
There was a problem hiding this comment.
consider adding an assert, e.g. assert(_viewController.isCreated); // created in setSize
stuartmorgan-g
added
the
autosubmit
This reverts commit a47b3cc.
|
This is reverted at #109389 due to an internal test failure |
3 participants
Fixes regressions from #101399:
createwill becalled by the
PlatformViewLinkeven though it has already beensuccessfully called by the plugin itself. This causes exceptions on
use of many Hybrid Composition plugins, and blocks adopting the new
forced-HC mode in the google_maps_flutter plugin, for instance.
AndroidViewController, which is thatsetSizewould create it on demand, was removed. That logic had beenmoved to an internal call site in the referenced PR, but there may have
been public callers expecting that behavior as well so it's better to keep
the logic in the code documented to have that behavior.
Fixes (at least part of) #107297
Pre-launch Checklist
///).