From 1ee02856d47de7d4a3ef6a492357b6a3a2f72f4c Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 17 Aug 2020 12:46:18 -0700 Subject: [PATCH 1/4] Revert "Revert "Create PlatformView instance right after method channel call from Dart (#20500)" (#20564)" This reverts commit 7b9ac278c97fcf58ae2c890e3610f20f11f91916. --- .../platform/PlatformViewsController.java | 86 ++++++++----------- .../platform/PlatformViewsControllerTest.java | 79 +++++++++++++---- 2 files changed, 100 insertions(+), 65 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..3c09e57a38d97 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; @@ -107,18 +106,45 @@ 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 (!validateDirection(request.direction)) { + throw new IllegalStateException( + "Trying to create a view with unknown direction value: " + + request.direction + + "(view id: " + + request.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); + } + + Object createParams = null; + if (request.params != null) { + createParams = factory.getCreateArgsCodec().decodeMessage(request.params); + } + + 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."); + } + if (view.getParent() != null) { + throw new IllegalStateException( + "The Android view returned from PlatformView#getView() was already added to a parent view."); + } + platformViews.put(request.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); @@ -378,7 +404,6 @@ public PlatformViewsController() { currentFrameUsedOverlayLayerIds = new HashSet<>(); currentFrameUsedPlatformViewIds = new HashSet<>(); - platformViewRequests = new SparseArray<>(); platformViews = new SparseArray<>(); mutatorViews = new SparseArray<>(); @@ -651,50 +676,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); 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..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; @@ -28,7 +29,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 +245,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 +284,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 +324,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 +519,7 @@ public FlutterImageView createImageView() { @Implements(FlutterJNI.class) public static class ShadowFlutterJNI { + private static SparseArray replies = new SparseArray<>(); public ShadowFlutterJNI() {} @@ -527,7 +566,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 SparseArray getResponses() { + return replies; + } } @Implements(SurfaceView.class) From ed148a882d30d73a9a7ce1c6135f6137bbc43c98 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 17 Aug 2020 12:31:59 -0700 Subject: [PATCH 2/4] Fix crash --- .../platform/PlatformViewsController.java | 76 +++++++++++------- .../platform/PlatformViewsControllerTest.java | 78 ++++++++++++++++++- 2 files changed, 123 insertions(+), 31 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java index 3c09e57a38d97..6819a380958bc 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -79,8 +79,20 @@ 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; + // The view returned by `PlatformView#getView()`. + // + // This only applies to hybrid composition. private final SparseArray platformViews; - private final SparseArray mutatorViews; + + // The platform view parent that is appended to `FlutterView`. + // If an entry in `platformViews` doesn't have an entry in this array, the platform view isn't + // in the view hierarchy. + // + // This view provides a wrapper that applies scene builder operations to the platform view. + // For example, a transform matrix, or setting opacity on the platform view layer. + // + // This is only applies to hybrid composition. + private final SparseArray platformViewParent; // Map of unique IDs to views that render overlay layers. private final SparseArray overlayLayerViews; @@ -146,12 +158,17 @@ public void createAndroidViewForPlatformView( public void disposeAndroidViewForPlatformView(int viewId) { // Hybrid view. final View platformView = platformViews.get(viewId); + final FlutterMutatorView parentView = platformViewParent.get(viewId); if (platformView != null) { - final FlutterMutatorView mutatorView = mutatorViews.get(viewId); - mutatorView.removeView(platformView); - ((FlutterView) flutterView).removeView(mutatorView); + if (parentView != null) { + parentView.removeView(platformView); + } platformViews.remove(viewId); - mutatorViews.remove(viewId); + } + + if (parentView != null) { + ((FlutterView) flutterView).removeView(parentView); + platformViewParent.remove(viewId); } } @@ -405,7 +422,7 @@ public PlatformViewsController() { currentFrameUsedPlatformViewIds = new HashSet<>(); platformViews = new SparseArray<>(); - mutatorViews = new SparseArray<>(); + platformViewParent = new SparseArray<>(); motionEventTracker = MotionEventTracker.getInstance(); } @@ -681,15 +698,15 @@ void initializePlatformViewIfNeeded(int viewId) { throw new IllegalStateException( "Platform view hasn't been initialized from the platform view channel."); } - if (mutatorViews.get(viewId) != null) { + if (platformViewParent.get(viewId) != null) { return; } - final FlutterMutatorView mutatorView = + final FlutterMutatorView parentView = new FlutterMutatorView( context, context.getResources().getDisplayMetrics().density, androidTouchProcessor); - mutatorViews.put(viewId, mutatorView); - mutatorView.addView(view); - ((FlutterView) flutterView).addView(mutatorView); + platformViewParent.put(viewId, parentView); + parentView.addView(view); + ((FlutterView) flutterView).addView(parentView); } public void attachToFlutterRenderer(FlutterRenderer flutterRenderer) { @@ -708,13 +725,14 @@ public void onDisplayPlatformView( initializeRootImageViewIfNeeded(); initializePlatformViewIfNeeded(viewId); - FlutterMutatorView mutatorView = mutatorViews.get(viewId); - mutatorView.readyToDisplay(mutatorsStack, x, y, width, height); - mutatorView.setVisibility(View.VISIBLE); - mutatorView.bringToFront(); + final FlutterMutatorView parentView = platformViewParent.get(viewId); + parentView.readyToDisplay(mutatorsStack, x, y, width, height); + parentView.setVisibility(View.VISIBLE); + parentView.bringToFront(); - FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams(viewWidth, viewHeight); - View platformView = platformViews.get(viewId); + final FrameLayout.LayoutParams layoutParams = + new FrameLayout.LayoutParams(viewWidth, viewHeight); + final View platformView = platformViews.get(viewId); platformView.setLayoutParams(layoutParams); platformView.bringToFront(); currentFrameUsedPlatformViewIds.add(viewId); @@ -723,7 +741,7 @@ public void onDisplayPlatformView( public void onDisplayOverlaySurface(int id, int x, int y, int width, int height) { initializeRootImageViewIfNeeded(); - FlutterImageView overlayView = overlayLayerViews.get(id); + final FlutterImageView overlayView = overlayLayerViews.get(id); if (overlayView.getParent() == null) { ((FlutterView) flutterView).addView(overlayView); } @@ -766,19 +784,19 @@ public void onEndFrame() { // If one of the surfaces doesn't have an image, the frame may be incomplete and must be // dropped. // For example, a toolbar widget painted by Flutter may not be rendered. - boolean isFrameRenderedUsingImageReaders = + final boolean isFrameRenderedUsingImageReaders = flutterViewConvertedToImageView && view.acquireLatestImageViewFrame(); finishFrame(isFrameRenderedUsingImageReaders); } private void finishFrame(boolean isFrameRenderedUsingImageReaders) { for (int i = 0; i < overlayLayerViews.size(); i++) { - int overlayId = overlayLayerViews.keyAt(i); - FlutterImageView overlayView = overlayLayerViews.valueAt(i); + final int overlayId = overlayLayerViews.keyAt(i); + final FlutterImageView overlayView = overlayLayerViews.valueAt(i); if (currentFrameUsedOverlayLayerIds.contains(overlayId)) { ((FlutterView) flutterView).attachOverlaySurfaceToRender(overlayView); - boolean didAcquireOverlaySurfaceImage = overlayView.acquireLatestImage(); + final boolean didAcquireOverlaySurfaceImage = overlayView.acquireLatestImage(); isFrameRenderedUsingImageReaders &= didAcquireOverlaySurfaceImage; } else { // If the background surface isn't rendered by the image view, then the @@ -792,22 +810,20 @@ private void finishFrame(boolean isFrameRenderedUsingImageReaders) { } } - for (int i = 0; i < platformViews.size(); i++) { - int viewId = platformViews.keyAt(i); - View platformView = platformViews.get(viewId); - 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); // Show platform views only if the surfaces have images available in this frame, // and if the platform view is rendered in this frame. + // The platform view is appended to a mutator view. // // Otherwise, hide the platform view, but don't remove it from the view hierarchy yet as // they are removed when the framework diposes the platform view widget. if (isFrameRenderedUsingImageReaders && currentFrameUsedPlatformViewIds.contains(viewId)) { - platformView.setVisibility(View.VISIBLE); - mutatorView.setVisibility(View.VISIBLE); + parentView.setVisibility(View.VISIBLE); } else { - platformView.setVisibility(View.GONE); - mutatorView.setVisibility(View.GONE); + parentView.setVisibility(View.GONE); } } } 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 3724666b50e48..fec7591db794e 100644 --- a/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java +++ b/shell/platform/android/test/io/flutter/plugin/platform/PlatformViewsControllerTest.java @@ -445,6 +445,80 @@ public void onEndFrame__destroysOverlaySurfaceAfterFrameOnFlutterSurfaceView() { verify(overlayImageView, times(1)).detachFromRenderer(); } + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void onEndFrame__removesPlatformView() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + final View androidView = mock(View.class); + when(platformView.getView()).thenReturn(androidView); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(false); + attach(jni, platformViewsController); + + jni.onFirstFrame(); + + // Simulate create call from the framework. + createPlatformView(jni, platformViewsController, platformViewId, "testType"); + + // Simulate first frame from the framework. + jni.onFirstFrame(); + platformViewsController.onBeginFrame(); + + platformViewsController.onEndFrame(); + verify(androidView, never()).setVisibility(View.GONE); + + final ViewParent parentView = mock(ViewParent.class); + when(androidView.getParent()).thenReturn(parentView); + } + + @Test + @Config(shadows = {ShadowFlutterSurfaceView.class, ShadowFlutterJNI.class}) + public void onEndFrame__removesPlatformViewParent() { + final PlatformViewsController platformViewsController = new PlatformViewsController(); + + final int platformViewId = 0; + assertNull(platformViewsController.getPlatformViewById(platformViewId)); + + final PlatformViewFactory viewFactory = mock(PlatformViewFactory.class); + final PlatformView platformView = mock(PlatformView.class); + final View androidView = mock(View.class); + when(platformView.getView()).thenReturn(androidView); + when(viewFactory.create(any(), eq(platformViewId), any())).thenReturn(platformView); + + platformViewsController.getRegistry().registerViewFactory("testType", viewFactory); + + final FlutterJNI jni = new FlutterJNI(); + jni.attachToNative(false); + + final FlutterView flutterView = attach(jni, platformViewsController); + + jni.onFirstFrame(); + + // Simulate create call from the framework. + createPlatformView(jni, platformViewsController, platformViewId, "testType"); + platformViewsController.initializePlatformViewIfNeeded(platformViewId); + assertEquals(flutterView.getChildCount(), 2); + + // Simulate first frame from the framework. + jni.onFirstFrame(); + platformViewsController.onBeginFrame(); + platformViewsController.onEndFrame(); + + // Simulate dispose call from the framework. + disposePlatformView(jni, platformViewsController, platformViewId); + assertEquals(flutterView.getChildCount(), 1); + } + private static byte[] encodeMethodCall(MethodCall call) { final ByteBuffer buffer = StandardMethodCodec.INSTANCE.encodeMethodCall(call); buffer.rewind(); @@ -484,7 +558,8 @@ private static void disposePlatformView( "flutter/platform_views", encodeMethodCall(platformDisposeMethodCall), /*replyId=*/ 0); } - private static void attach(FlutterJNI jni, PlatformViewsController platformViewsController) { + private static FlutterView attach( + FlutterJNI jni, PlatformViewsController platformViewsController) { final DartExecutor executor = new DartExecutor(jni, mock(AssetManager.class)); executor.onAttachedToJNI(); @@ -515,6 +590,7 @@ public FlutterImageView createImageView() { view.attachToFlutterEngine(engine); platformViewsController.attachToView(view); + return view; } @Implements(FlutterJNI.class) From dddc42a106ef3c67d740e38d5961a3d68970059f Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 17 Aug 2020 14:13:14 -0700 Subject: [PATCH 3/4] Retry LUCI job From 42329320830aea866e89deaf527244495d9924e0 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 17 Aug 2020 15:05:39 -0700 Subject: [PATCH 4/4] Nit --- .../io/flutter/plugin/platform/PlatformViewsController.java | 4 ++-- 1 file changed, 2 insertions(+), 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 6819a380958bc..2293c4971e943 100644 --- a/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +++ b/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java @@ -79,12 +79,12 @@ 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; - // The view returned by `PlatformView#getView()`. + // The views returned by `PlatformView#getView()`. // // This only applies to hybrid composition. private final SparseArray platformViews; - // The platform view parent that is appended to `FlutterView`. + // The platform view parents that are appended to `FlutterView`. // If an entry in `platformViews` doesn't have an entry in this array, the platform view isn't // in the view hierarchy. //