From 01968e67acfc2b1468315f1b9dfdba5e9c8c2e9e Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Tue, 8 Oct 2019 17:39:05 -0700 Subject: [PATCH 1/2] Fire PlatformViewController FlutterView callbacks Fixes a bug where `PlatformViewController` was not being notified of `FlutterView` attachment changes. Unfortunately the test here is just an interaction test. I refactored some of the underlying code so that FlutterEngine could be initialized without through missing native library errors, but even with that variance there isn't any obvious way to test the real behavior here. I'd need to significantly refactor FlutterEngine and PlatformViewsController to write a more effective test here. --- shell/platform/android/BUILD.gn | 1 + .../embedding/android/FlutterView.java | 4 ++ .../embedding/engine/FlutterEngine.java | 11 +++- .../flutter/embedding/engine/FlutterJNI.java | 2 +- .../engine/renderer/FlutterRenderer.java | 2 +- .../android/io/flutter/view/FlutterView.java | 3 +- .../test/io/flutter/FlutterTestSuite.java | 2 + .../embedding/android/FlutterViewTest.java | 58 +++++++++++++++++++ 8 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index c2f3d1f41e32f..bfd6f47d5071d 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -417,6 +417,7 @@ action("robolectric_tests") { "test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java", "test/io/flutter/embedding/android/FlutterActivityTest.java", "test/io/flutter/embedding/android/FlutterFragmentTest.java", + "test/io/flutter/embedding/android/FlutterViewTest.java", "test/io/flutter/embedding/engine/FlutterEngineCacheTest.java", "test/io/flutter/embedding/engine/FlutterJNITest.java", "test/io/flutter/embedding/engine/RenderingComponentTest.java", diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index c6936c97f5aa3..2d10a20dba103 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -633,6 +633,8 @@ public void attachToFlutterEngine( sendLocalesToFlutter(getResources().getConfiguration()); sendViewportMetricsToFlutter(); + flutterEngine.getPlatformViewsController().attachToView(this); + // Notify engine attachment listeners of the attachment. for (FlutterEngineAttachmentListener listener : flutterEngineAttachmentListeners) { listener.onFlutterEngineAttachedToFlutterView(flutterEngine); @@ -668,6 +670,8 @@ public void detachFromFlutterEngine() { listener.onFlutterEngineDetachedFromFlutterView(); } + flutterEngine.getPlatformViewsController().detachFromView(); + // Disconnect the FlutterEngine's PlatformViewsController from the AccessibilityBridge. flutterEngine.getPlatformViewsController().detachAccessibiltyBridge(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java index 42b0129d7298f..5a4ec289ccf2f 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterEngine.java @@ -143,14 +143,19 @@ public void onPreEngineRestart() { * and {@link FlutterLoader#ensureInitializationComplete(Context, String[])}. */ public FlutterEngine(@NonNull Context context) { - this(context, FlutterLoader.getInstance()); + this(context, FlutterLoader.getInstance(), new FlutterJNI()); } - /* package */ FlutterEngine(@NonNull Context context, @NonNull FlutterLoader flutterLoader) { + /** + * Constructs a new {@code FlutterEngine}. See {@link #FlutterEngine(Context)}. + * + * {@code flutterJNI} should be a new instance that has never been attached to an engine before. + */ + public FlutterEngine(@NonNull Context context, @NonNull FlutterLoader flutterLoader, @NonNull FlutterJNI flutterJNI) { + this.flutterJNI = flutterJNI; flutterLoader.startInitialization(context); flutterLoader.ensureInitializationComplete(context, null); - this.flutterJNI = new FlutterJNI(); flutterJNI.addEngineLifecycleListener(engineLifecycleListener); attachToJni(); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 3ab1745d44c9b..2bd663f03106a 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -121,7 +121,7 @@ public static native void nativeInit( // TODO(mattcarroll): add javadocs @UiThread - public static native boolean nativeGetIsSoftwareRenderingEnabled(); + public native boolean nativeGetIsSoftwareRenderingEnabled(); @Nullable // TODO(mattcarroll): add javadocs diff --git a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 306535a547a26..a6675964e6bd1 100644 --- a/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -281,7 +281,7 @@ private void unregisterTexture(long textureId) { // TODO(mattcarroll): describe the native behavior that this invokes public boolean isSoftwareRenderingEnabled() { - return FlutterJNI.nativeGetIsSoftwareRenderingEnabled(); + return flutterJNI.nativeGetIsSoftwareRenderingEnabled(); } // TODO(mattcarroll): describe the native behavior that this invokes diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index d1037170e1911..1546be7daec69 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -38,7 +38,6 @@ import android.view.inputmethod.InputMethodManager; import java.nio.ByteBuffer; -import java.nio.ByteOrder; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -162,7 +161,7 @@ public FlutterView(Context context, AttributeSet attrs, FlutterNativeView native dartExecutor = mNativeView.getDartExecutor(); flutterRenderer = new FlutterRenderer(mNativeView.getFlutterJNI()); - mIsSoftwareRenderingEnabled = FlutterJNI.nativeGetIsSoftwareRenderingEnabled(); + mIsSoftwareRenderingEnabled = mNativeView.getFlutterJNI().nativeGetIsSoftwareRenderingEnabled(); mMetrics = new ViewportMetrics(); mMetrics.devicePixelRatio = context.getResources().getDisplayMetrics().density; setFocusable(true); diff --git a/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/shell/platform/android/test/io/flutter/FlutterTestSuite.java index 59980291db489..b400fb5625e1c 100644 --- a/shell/platform/android/test/io/flutter/FlutterTestSuite.java +++ b/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -10,6 +10,7 @@ import io.flutter.embedding.android.FlutterActivityTest; import io.flutter.embedding.android.FlutterFragmentTest; +import io.flutter.embedding.android.FlutterViewTest; import io.flutter.embedding.engine.FlutterEngineCacheTest; import io.flutter.embedding.engine.FlutterJNITest; import io.flutter.embedding.engine.RenderingComponentTest; @@ -28,6 +29,7 @@ FlutterFragmentTest.class, FlutterJNITest.class, FlutterRendererTest.class, + FlutterViewTest.class, PlatformChannelTest.class, PreconditionsTest.class, RenderingComponentTest.class, diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java new file mode 100644 index 0000000000000..afa4c428168db --- /dev/null +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java @@ -0,0 +1,58 @@ +package io.flutter.embedding.android; + +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.loader.FlutterLoader; +import io.flutter.plugin.platform.PlatformViewsController; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +import io.flutter.embedding.engine.FlutterEngine; + +@Config(manifest = Config.NONE) +@RunWith(RobolectricTestRunner.class) +public class FlutterViewTest { + @Mock FlutterJNI mockFlutterJni; + @Mock FlutterLoader mockFlutterLoader; + @Spy PlatformViewsController platformViewsController; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + when(mockFlutterJni.isAttached()).thenReturn(true); + } + + @Test + public void attachToFlutterEngine_alertsPlatformViews() { + FlutterView flutterView = new FlutterView(RuntimeEnvironment.application); + FlutterEngine flutterEngine = spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); + when(flutterEngine.getPlatformViewsController()).thenReturn(platformViewsController); + + flutterView.attachToFlutterEngine(flutterEngine); + + verify(platformViewsController, times(1)).attachToView(flutterView); + } + + @Test + public void detachFromFlutterEngine_alertsPlatformViews() { + FlutterView flutterView = new FlutterView(RuntimeEnvironment.application); + FlutterEngine flutterEngine = spy(new FlutterEngine(RuntimeEnvironment.application, mockFlutterLoader, mockFlutterJni)); + when(flutterEngine.getPlatformViewsController()).thenReturn(platformViewsController); + + flutterView.attachToFlutterEngine(flutterEngine); + flutterView.detachFromFlutterEngine(); + + verify(platformViewsController, times(1)).detachFromView(); + } +} \ No newline at end of file From 5ad6337c29c46187bc90f03123210f4b5e28d4a0 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Wed, 9 Oct 2019 14:16:26 -0700 Subject: [PATCH 2/2] Review feedback --- .../test/io/flutter/embedding/android/FlutterViewTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java index afa4c428168db..fe87fc8954d45 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterViewTest.java @@ -55,4 +55,4 @@ public void detachFromFlutterEngine_alertsPlatformViews() { verify(platformViewsController, times(1)).detachFromView(); } -} \ No newline at end of file +}