From 4cdbb02b42ddabe42fc86c6389bac56689a04154 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 13 Aug 2020 14:50:47 -0700 Subject: [PATCH 1/5] Initialize PlatformView earlier --- .../platform/PlatformViewsController.java | 90 +++++++++---------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 7c064a6f90818..d6dc2cd968ef6 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -107,18 +107,51 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega @Override public void createAndroidViewForPlatformView( @NonNull PlatformViewsChannel.PlatformViewCreationRequest request) { - // API level 19 is required for android.graphics.ImageReader. + // API level 19 is required for `android.graphics.ImageReader`. ensureValidAndroidVersion(Build.VERSION_CODES.KITKAT); - platformViewRequests.put(request.viewId, request); + + if (request == null) { + throw new IllegalStateException( + "Platform view hasn't been initialized from the platform view channel."); + } + + if (!validateDirection(request.direction)) { + throw new IllegalStateException( + "Trying to create a view with unknown direction value: " + + request.direction + + "(view id: " + + viewId + + ")"); + } + + final PlatformViewFactory factory = registry.getFactory(request.viewType); + if (factory == null) { + throw new IllegalStateException( + "Trying to create a platform view of unregistered type: " + request.viewType); + } + + final Object createParams = null; + if (request.params != null) { + createParams = factory.getCreateArgsCodec().decodeMessage(request.params); + } + + final PlatformView platformView = factory.create(context, viewId, createParams); + final View view = platformView.getView(); + + if (view == null) { + throw new IllegalStateException( + "PlatformView#getView() returned null, but an Android view reference was expected."); + } + if (view.getParent() != null) { + throw new IllegalStateException( + "The Android view returned from PlatformView#getView() was already added to a parent view."); + } + platformViews.put(viewId, view); } @Override public void disposeAndroidViewForPlatformView(int viewId) { // Hybrid view. - if (platformViewRequests.get(viewId) != null) { - platformViewRequests.remove(viewId); - } - final View platformView = platformViews.get(viewId); if (platformView != null) { final FlutterMutatorView mutatorView = mutatorViews.get(viewId); @@ -651,50 +684,15 @@ private void initializeRootImageViewIfNeeded() { @VisibleForTesting void initializePlatformViewIfNeeded(int viewId) { - if (platformViews.get(viewId) != null) { - return; - } - - PlatformViewsChannel.PlatformViewCreationRequest request = platformViewRequests.get(viewId); - if (request == null) { - throw new IllegalStateException( - "Platform view hasn't been initialized from the platform view channel."); - } - - if (!validateDirection(request.direction)) { - throw new IllegalStateException( - "Trying to create a view with unknown direction value: " - + request.direction - + "(view id: " - + viewId - + ")"); - } - - PlatformViewFactory factory = registry.getFactory(request.viewType); - if (factory == null) { - throw new IllegalStateException( - "Trying to create a platform view of unregistered type: " + request.viewType); - } - - Object createParams = null; - if (request.params != null) { - createParams = factory.getCreateArgsCodec().decodeMessage(request.params); - } - - PlatformView platformView = factory.create(context, viewId, createParams); - View view = platformView.getView(); - + final View view = platformViews.get(viewId); if (view == null) { throw new IllegalStateException( - "PlatformView#getView() returned null, but an Android view reference was expected."); + "Platform view hasn't been initialized from the platform view channel."); } - if (view.getParent() != null) { - throw new IllegalStateException( - "The Android view returned from PlatformView#getView() was already added to a parent view."); + if (mutatorViews.get(viewId) != null) { + return; } - platformViews.put(viewId, view); - - FlutterMutatorView mutatorView = + final FlutterMutatorView mutatorView = new FlutterMutatorView( context, context.getResources().getDisplayMetrics().density, androidTouchProcessor); mutatorViews.put(viewId, mutatorView); From d9906e515a637da88c827b10bf84a22cfc34aaf1 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 13 Aug 2020 19:06:51 -0700 Subject: [PATCH 2/5] Tests --- .../platform/PlatformViewsController.java | 9 +-- .../platform/PlatformViewsControllerTest.java | 78 +++++++++++++++---- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index d6dc2cd968ef6..b734a9bb905eb 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -120,7 +120,7 @@ public void createAndroidViewForPlatformView( "Trying to create a view with unknown direction value: " + request.direction + "(view id: " - + viewId + + request.viewId + ")"); } @@ -130,14 +130,13 @@ public void createAndroidViewForPlatformView( "Trying to create a platform view of unregistered type: " + request.viewType); } - final Object createParams = null; + Object createParams = null; if (request.params != null) { createParams = factory.getCreateArgsCodec().decodeMessage(request.params); } - final PlatformView platformView = factory.create(context, viewId, createParams); + final PlatformView platformView = factory.create(context, request.viewId, createParams); final View view = platformView.getView(); - if (view == null) { throw new IllegalStateException( "PlatformView#getView() returned null, but an Android view reference was expected."); @@ -146,7 +145,7 @@ public void createAndroidViewForPlatformView( throw new IllegalStateException( "The Android view returned from PlatformView#getView() was already added to a parent view."); } - platformViews.put(viewId, view); + platformViews.put(request.viewId, view); } @Override diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index 659056afc5132..ba025e73dd6c6 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -28,7 +28,9 @@ import io.flutter.embedding.engine.systemchannels.MouseCursorChannel; import io.flutter.embedding.engine.systemchannels.SettingsChannel; import io.flutter.embedding.engine.systemchannels.TextInputChannel; +import io.flutter.plugin.common.FlutterException; import io.flutter.plugin.common.MethodCall; +import io.flutter.plugin.common.StandardMessageCodec; import io.flutter.plugin.common.StandardMethodCodec; import io.flutter.plugin.localization.LocalizationPlugin; import java.nio.ByteBuffer; @@ -242,7 +244,29 @@ public void getPlatformViewById__hybridComposition() { @Test @Config(shadows = {ShadowFlutterJNI.class}) - public void initializePlatformViewIfNeeded__throwsIfViewIsNull() { + public void createPlatformViewMessage__initializesAndroidView() { + PlatformViewsController platformViewsController = new PlatformViewsController(); + + int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + PlatformView platformView = mock(PlatformView.class); + when(platformView.getView()).thenReturn(mock(View.class)); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + FlutterJNI jni = new FlutterJNI(); + attach(jni, platformViewsController); + + // Simulate create call from the framework. + createPlatformView(jni, platformViewsController, platformViewId, "testType"); + verify(platformView, times(1)).getView(); + } + + @Test + @Config(shadows = {ShadowFlutterJNI.class}) + public void createPlatformViewMessage__throwsIfViewIsNull() { PlatformViewsController platformViewsController = new PlatformViewsController(); int platformViewId = 0; @@ -259,22 +283,28 @@ public void initializePlatformViewIfNeeded__throwsIfViewIsNull() { // Simulate create call from the framework. createPlatformView(jni, platformViewsController, platformViewId, "testType"); + assertEquals(ShadowFlutterJNI.getResponses().size(), 1); + final ByteBuffer responseBuffer = ShadowFlutterJNI.getResponses().get(0); + responseBuffer.rewind(); + + StandardMethodCodec methodCodec = new StandardMethodCodec(new StandardMessageCodec()); try { - platformViewsController.initializePlatformViewIfNeeded(platformViewId); - } catch (Exception exception) { - assertTrue(exception instanceof IllegalStateException); - assertEquals( - exception.getMessage(), - "PlatformView#getView() returned null, but an Android view reference was expected."); + methodCodec.decodeEnvelope(responseBuffer); + } catch (FlutterException exception) { + assertTrue( + exception + .getMessage() + .contains( + "PlatformView#getView() returned null, but an Android view reference was expected.")); return; } - assertTrue(false); + assertFalse(true); } @Test @Config(shadows = {ShadowFlutterJNI.class}) - public void initializePlatformViewIfNeeded__throwsIfViewHasParent() { + public void createPlatformViewMessage__throwsIfViewHasParent() { PlatformViewsController platformViewsController = new PlatformViewsController(); int platformViewId = 0; @@ -293,16 +323,23 @@ public void initializePlatformViewIfNeeded__throwsIfViewHasParent() { // Simulate create call from the framework. createPlatformView(jni, platformViewsController, platformViewId, "testType"); + assertEquals(ShadowFlutterJNI.getResponses().size(), 1); + + final ByteBuffer responseBuffer = ShadowFlutterJNI.getResponses().get(0); + responseBuffer.rewind(); + + StandardMethodCodec methodCodec = new StandardMethodCodec(new StandardMessageCodec()); try { - platformViewsController.initializePlatformViewIfNeeded(platformViewId); - } catch (Exception exception) { - assertTrue(exception instanceof IllegalStateException); - assertEquals( - exception.getMessage(), - "The Android view returned from PlatformView#getView() was already added to a parent view."); + methodCodec.decodeEnvelope(responseBuffer); + } catch (FlutterException exception) { + assertTrue( + exception + .getMessage() + .contains( + "The Android view returned from PlatformView#getView() was already added to a parent view.")); return; } - assertTrue(false); + assertFalse(true); } @Test @@ -481,6 +518,7 @@ public FlutterImageView createImageView() { @Implements(FlutterJNI.class) public static class ShadowFlutterJNI { + private static Map replies = new HashMap<>(); public ShadowFlutterJNI() {} @@ -527,7 +565,13 @@ public void setViewportMetrics( @Implementation public void invokePlatformMessageResponseCallback( - int responseId, ByteBuffer message, int position) {} + int responseId, ByteBuffer message, int position) { + replies.put(responseId, message); + } + + public static Map getResponses() { + return replies; + } } @Implements(SurfaceView.class) From 151840ee45ecb1b18c7d0deee28067267a1d30aa Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 13 Aug 2020 19:22:39 -0700 Subject: [PATCH 3/5] Map->SparseArray --- .../flutter/plugin/platform/PlatformViewsControllerTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java index ba025e73dd6c6..3724666b50e48 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -7,6 +7,7 @@ import android.content.Context; import android.content.res.AssetManager; +import android.util.SparseArray; import android.view.MotionEvent; import android.view.Surface; import android.view.SurfaceHolder; @@ -518,7 +519,7 @@ public FlutterImageView createImageView() { @Implements(FlutterJNI.class) public static class ShadowFlutterJNI { - private static Map replies = new HashMap<>(); + private static SparseArray replies = new SparseArray<>(); public ShadowFlutterJNI() {} @@ -569,7 +570,7 @@ public void invokePlatformMessageResponseCallback( replies.put(responseId, message); } - public static Map getResponses() { + public static SparseArray getResponses() { return replies; } } From ed76300b2b49784baa530a8300104edf39ce258c Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 13 Aug 2020 19:24:20 -0700 Subject: [PATCH 4/5] Remove dead code --- .../io/flutter/plugin/platform/PlatformViewsController.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index b734a9bb905eb..1a4e7637d0b20 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -79,7 +79,6 @@ public class PlatformViewsController implements PlatformViewsAccessibilityDelega // it is associated with(e.g if a platform view creates other views in the same virtual display. private final HashMap contextToPlatformView; - private final SparseArray platformViewRequests; private final SparseArray platformViews; private final SparseArray mutatorViews; @@ -410,7 +409,6 @@ public PlatformViewsController() { currentFrameUsedOverlayLayerIds = new HashSet<>(); currentFrameUsedPlatformViewIds = new HashSet<>(); - platformViewRequests = new SparseArray<>(); platformViews = new SparseArray<>(); mutatorViews = new SparseArray<>(); From 31fcfb6d24518eb7f53a461740e91c9cf1e6da1d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 14 Aug 2020 12:19:32 -0700 Subject: [PATCH 5/5] Remove irrelevant check --- .../io/flutter/plugin/platform/PlatformViewsController.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 1a4e7637d0b20..3c09e57a38d97 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -109,11 +109,6 @@ public void createAndroidViewForPlatformView( // API level 19 is required for `android.graphics.ImageReader`. ensureValidAndroidVersion(Build.VERSION_CODES.KITKAT); - if (request == null) { - throw new IllegalStateException( - "Platform view hasn't been initialized from the platform view channel."); - } - if (!validateDirection(request.direction)) { throw new IllegalStateException( "Trying to create a view with unknown direction value: "