From 96736f6be5143a74e97a55b66237c7a1d38d22f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rulong=20Chen=EF=BC=88=E9=99=88=E6=B1=9D=E9=BE=99=EF=BC=89?= <26625149+0xZOne@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:38:35 +0800 Subject: [PATCH 1/2] [Android] Fix the issue of blank or frozen pages in shared engine scenarios (#50947) Consider this scenario: In an add-to-app context, where multiple Flutter activities share the same engine, a situation occurs. When navigating from FlutterActivity1 to FlutterActivity2, the Flutter view associated with FlutterActivity1 is detached from the engine. Then, the Flutter view of FlutterActivity2 is attached. Upon navigating back to FlutterActivity1, its Flutter view is re-attached to the shared engine. The expected behavior is: When a Flutter view detaches from the shared engine, the associated surface should be released. When the Flutter view re-attaches, a new surface should be created. After #47358, no new surface is created when the Flutter view is attached again. This results in the Flutter view having no underlying surface, which causes the page to appear blank or freeze without responding. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../embedding/android/FlutterSurfaceView.java | 9 ++- .../embedding/android/FlutterTextureView.java | 9 ++- .../android/FlutterSurfaceViewTest.java | 72 +++++++++++++++++++ .../android/FlutterTextureViewTest.java | 53 +++++++++++++- 4 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java b/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java index 9fdaa4c08bb1f..c5c6493494043 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java @@ -12,6 +12,7 @@ import android.view.SurfaceView; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import io.flutter.Log; import io.flutter.embedding.engine.renderer.FlutterRenderer; import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener; @@ -168,6 +169,10 @@ public FlutterRenderer getAttachedRenderer() { return flutterRenderer; } + @VisibleForTesting + /* package */ boolean isSurfaceAvailableForRendering() { + return isSurfaceAvailableForRendering; + } /** * Invoked by the owner of this {@code FlutterSurfaceView} when it wants to begin rendering a * Flutter UI to this {@code FlutterSurfaceView}. @@ -215,8 +220,6 @@ public void detachFromRenderer() { disconnectSurfaceFromRenderer(); } - pause(); - // Make the SurfaceView invisible to avoid showing a black rectangle. setAlpha(0.0f); flutterRenderer.removeIsDisplayingFlutterUiListener(flutterUiDisplayListener); @@ -248,7 +251,7 @@ public void resume() { // If we're already attached to an Android window then we're now attached to both a renderer // and the Android window. We can begin rendering now. - if (isSurfaceAvailableForRendering) { + if (isSurfaceAvailableForRendering()) { Log.v( TAG, "Surface is available for rendering. Connecting FlutterRenderer to Android surface."); diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java b/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java index 11237a1a70348..704509aae1e87 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java @@ -124,6 +124,11 @@ public FlutterRenderer getAttachedRenderer() { return flutterRenderer; } + @VisibleForTesting + /* package */ boolean isSurfaceAvailableForRendering() { + return isSurfaceAvailableForRendering; + } + /** * Invoked by the owner of this {@code FlutterTextureView} when it wants to begin rendering a * Flutter UI to this {@code FlutterTextureView}. @@ -170,8 +175,6 @@ public void detachFromRenderer() { disconnectSurfaceFromRenderer(); } - pause(); - flutterRenderer = null; } else { Log.w(TAG, "detachFromRenderer() invoked when no FlutterRenderer was attached."); @@ -198,7 +201,7 @@ public void resume() { // If we're already attached to an Android window then we're now attached to both a renderer // and the Android window. We can begin rendering now. - if (isSurfaceAvailableForRendering) { + if (isSurfaceAvailableForRendering()) { Log.v( TAG, "Surface is available for rendering. Connecting FlutterRenderer to Android surface."); diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java new file mode 100644 index 0000000000000..33312dc7bf0c2 --- /dev/null +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java @@ -0,0 +1,72 @@ +package io.flutter.embedding.android; + +import static io.flutter.Build.API_LEVELS; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +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 android.annotation.TargetApi; +import android.view.Surface; +import android.view.SurfaceHolder; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.renderer.FlutterRenderer; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.Config; + +@Config(manifest = Config.NONE) +@RunWith(AndroidJUnit4.class) +@TargetApi(API_LEVELS.API_30) +public class FlutterSurfaceViewTest { + @Test + public void itShouldCreateANewSurfaceWhenReattachedAfterDetachingFromRenderer() { + // Consider this scenario: In an add-to-app context, where multiple Flutter activities share the + // same engine, a situation occurs. When navigating from FlutterActivity1 to FlutterActivity2, + // the Flutter view associated with FlutterActivity1 is detached from the engine. Then, the + // Flutter view of FlutterActivity2 is attached. Upon navigating back to FlutterActivity1, its + // Flutter view is re-attached to the shared engine. + // + // The expected behavior is: When a Flutter view detaches from the shared engine, the associated + // surface should be released. When the Flutter view re-attaches, a new surface should be + // created. + + // Setup the test. + final FlutterSurfaceView surfaceView = + spy(new FlutterSurfaceView(ApplicationProvider.getApplicationContext())); + + FlutterJNI fakeFlutterJNI = mock(FlutterJNI.class); + FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); + + SurfaceHolder fakeSurfaceHolder = mock(SurfaceHolder.class); + Surface fakeSurface = mock(Surface.class); + when(surfaceView.getHolder()).thenReturn(fakeSurfaceHolder); + when(fakeSurfaceHolder.getSurface()).thenReturn(fakeSurface); + when(surfaceView.isSurfaceAvailableForRendering()).thenReturn(true); + when(surfaceView.getWindowToken()).thenReturn(mock(android.os.IBinder.class)); + + // Execute the behavior under test. + surfaceView.attachToRenderer(flutterRenderer); + + // Verify the behavior under test. + verify(fakeFlutterJNI, times(1)).onSurfaceCreated(any(Surface.class)); + + // Execute the behavior under test. + surfaceView.detachFromRenderer(); + + // Verify the behavior under test. + verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed(); + + // Execute the behavior under test. + surfaceView.attachToRenderer(flutterRenderer); + + // Verify the behavior under test. + verify(fakeFlutterJNI, never()).onSurfaceWindowChanged(any(Surface.class)); + verify(fakeFlutterJNI, times(2)).onSurfaceCreated(any(Surface.class)); + } +} diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java index f6eccaa31b6f7..7711b1568c553 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java @@ -1,7 +1,13 @@ package io.flutter.embedding.android; +import static io.flutter.Build.API_LEVELS; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +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 android.annotation.TargetApi; import android.graphics.SurfaceTexture; @@ -9,13 +15,15 @@ import android.view.TextureView; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.renderer.FlutterRenderer; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.annotation.Config; @Config(manifest = Config.NONE) @RunWith(AndroidJUnit4.class) -@TargetApi(30) +@TargetApi(API_LEVELS.API_30) public class FlutterTextureViewTest { @Test public void surfaceTextureListenerReleasesRenderer() { @@ -30,4 +38,47 @@ public void surfaceTextureListenerReleasesRenderer() { verify(mockRenderSurface).release(); } + + @Test + public void itShouldCreateANewSurfaceWhenReattachedAfterDetachingFromRenderer() { + // Consider this scenario: In an add-to-app context, where multiple Flutter activities share the + // same engine, a situation occurs. When navigating from FlutterActivity1 to FlutterActivity2, + // the Flutter view associated with FlutterActivity1 is detached from the engine. Then, the + // Flutter view of FlutterActivity2 is attached. Upon navigating back to FlutterActivity1, its + // Flutter view is re-attached to the shared engine. + // + // The expected behavior is: When a Flutter view detaches from the shared engine, the associated + // surface should be released. When the Flutter view re-attaches, a new surface should be + // created. + + // Setup the test. + final FlutterTextureView textureView = + spy(new FlutterTextureView(ApplicationProvider.getApplicationContext())); + + FlutterJNI fakeFlutterJNI = mock(FlutterJNI.class); + FlutterRenderer flutterRenderer = new FlutterRenderer(fakeFlutterJNI); + + when(textureView.isSurfaceAvailableForRendering()).thenReturn(true); + when(textureView.getSurfaceTexture()).thenReturn(mock(SurfaceTexture.class)); + when(textureView.getWindowToken()).thenReturn(mock(android.os.IBinder.class)); + + // Execute the behavior under test. + textureView.attachToRenderer(flutterRenderer); + + // Verify the behavior under test. + verify(fakeFlutterJNI, times(1)).onSurfaceCreated(any(Surface.class)); + + // Execute the behavior under test. + textureView.detachFromRenderer(); + + // Verify the behavior under test. + verify(fakeFlutterJNI, times(1)).onSurfaceDestroyed(); + + // Execute the behavior under test. + textureView.attachToRenderer(flutterRenderer); + + // Verify the behavior under test. + verify(fakeFlutterJNI, never()).onSurfaceWindowChanged(any(Surface.class)); + verify(fakeFlutterJNI, times(2)).onSurfaceCreated(any(Surface.class)); + } } From 3097bd3e57d4bfb56ffd3b42bdd1e9fa5ca2373d Mon Sep 17 00:00:00 2001 From: mdnht Date: Wed, 1 May 2024 15:31:34 +0900 Subject: [PATCH 2/2] remove import io.flutter.Build.API_LEVELS and specify TargetAPI directly. --- .../io/flutter/embedding/android/FlutterSurfaceViewTest.java | 3 +-- .../io/flutter/embedding/android/FlutterTextureViewTest.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java index 33312dc7bf0c2..8c2815b649149 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterSurfaceViewTest.java @@ -1,6 +1,5 @@ package io.flutter.embedding.android; -import static io.flutter.Build.API_LEVELS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -22,7 +21,7 @@ @Config(manifest = Config.NONE) @RunWith(AndroidJUnit4.class) -@TargetApi(API_LEVELS.API_30) +@TargetApi(30) public class FlutterSurfaceViewTest { @Test public void itShouldCreateANewSurfaceWhenReattachedAfterDetachingFromRenderer() { diff --git a/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java b/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java index 7711b1568c553..5622bbf083339 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/FlutterTextureViewTest.java @@ -1,6 +1,5 @@ package io.flutter.embedding.android; -import static io.flutter.Build.API_LEVELS; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -23,7 +22,7 @@ @Config(manifest = Config.NONE) @RunWith(AndroidJUnit4.class) -@TargetApi(API_LEVELS.API_30) +@TargetApi(30) public class FlutterTextureViewTest { @Test public void surfaceTextureListenerReleasesRenderer() {