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

Reland: Create PlatformView instance right after method channel call from Dart #20568

Merged
merged 4 commits into from
Aug 17, 2020

Conversation

blasten
Copy link

@blasten blasten commented Aug 17, 2020

Relands #20500

1f15b7e fixes the issue.

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

lgtm, just a few notes. Thanks for the added test.

View mutatorView = mutatorViews.get(viewId);
for (int i = 0; i < platformViewParent.size(); i++) {
final int viewId = platformViewParent.keyAt(i);
final View parentView = platformViewParent.get(viewId);
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible parentView will be null?

Copy link
Author

Choose a reason for hiding this comment

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

It's not possible. This is iterating over the existing entries, and the code never puts null in the dictionary.

@blasten blasten added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 17, 2020
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM % nits

@@ -79,9 +79,20 @@
// it is associated with(e.g if a platform view creates other views in the same virtual display.
private final HashMap<Context, View> contextToPlatformView;

private final SparseArray<PlatformViewsChannel.PlatformViewCreationRequest> platformViewRequests;
// The view returned by `PlatformView#getView()`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nits: views

Copy link
Author

Choose a reason for hiding this comment

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

Done

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux Web Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 17, 2020
@blasten blasten merged commit 6156798 into flutter:master Aug 17, 2020
@blasten blasten deleted the reland_pv_order branch August 17, 2020 23:29
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants