From bb2fe0c35b76c244ff0a891bbd0ea7afa97f72c1 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Wed, 14 Aug 2019 17:06:11 -0700 Subject: [PATCH 1/5] let listener9s self remove --- shell/platform/android/BUILD.gn | 1 + .../flutter/embedding/engine/FlutterJNI.java | 10 +++- .../test/io/flutter/FlutterTestSuite.java | 2 + .../engine/renderer/FlutterRendererTest.java | 60 +++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index a5ad8f4650839..9162fd1e9000e 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -407,6 +407,7 @@ action("robolectric_tests") { "test/io/flutter/embedding/android/FlutterActivityTest.java", "test/io/flutter/embedding/android/FlutterFragmentTest.java", "test/io/flutter/embedding/engine/FlutterEngineCacheTest.java", + "test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java", "test/io/flutter/embedding/engine/systemchannels/TextInputChannelTest.java", "test/io/flutter/util/PreconditionsTest.java", ] diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index fbd63888c19d1..01c068fe97261 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -12,12 +12,14 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.UiThread; +import android.support.annotation.VisibleForTesting; import android.view.Surface; import android.view.SurfaceHolder; import java.nio.ByteBuffer; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; import io.flutter.Log; import io.flutter.embedding.engine.FlutterEngine.EngineLifecycleListener; @@ -275,15 +277,17 @@ public void removeOnFirstFrameRenderedListener(@NonNull OnFirstFrameRenderedList // Called by native to notify first Flutter frame rendered. @SuppressWarnings("unused") + @VisibleForTesting @UiThread - private void onFirstFrame() { + protected void onFirstFrame() { ensureRunningOnMainThread(); if (renderSurface != null) { renderSurface.onFirstFrameRendered(); } // TODO(mattcarroll): log dropped messages when in debug mode (https://github.com/flutter/flutter/issues/25391) - - for (OnFirstFrameRenderedListener listener : firstFrameListeners) { + CopyOnWriteArrayList mutableListeners = + new CopyOnWriteArrayList<>(firstFrameListeners); + for (OnFirstFrameRenderedListener listener : mutableListeners) { listener.onFirstFrameRendered(); } } diff --git a/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/shell/platform/android/test/io/flutter/FlutterTestSuite.java index 6c95754ccb402..9cd30c4af827d 100644 --- a/shell/platform/android/test/io/flutter/FlutterTestSuite.java +++ b/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -12,6 +12,7 @@ import io.flutter.embedding.android.FlutterActivityTest; import io.flutter.embedding.android.FlutterFragmentTest; import io.flutter.embedding.engine.FlutterEngineCacheTest; +import io.flutter.embedding.engine.renderer.FlutterRendererTest; import io.flutter.embedding.engine.systemchannels.TextInputChannelTest; import io.flutter.util.PreconditionsTest; @@ -23,6 +24,7 @@ FlutterFragmentTest.class, // FlutterActivityAndFragmentDelegateTest.class, TODO(mklim): Fix and re-enable this FlutterEngineCacheTest.class, + FlutterRendererTest.class, TextInputChannelTest.class }) /** Runs all of the unit tests listed in the {@code @SuiteClasses} annotation. */ diff --git a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java new file mode 100644 index 0000000000000..a4ca388cc418a --- /dev/null +++ b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -0,0 +1,60 @@ +package io.flutter.embedding.engine.renderer; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.renderer.FlutterRenderer; +import io.flutter.embedding.engine.renderer.OnFirstFrameRenderedListener; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import java.util.concurrent.atomic.AtomicInteger; + +@Config(manifest=Config.NONE) +@RunWith(RobolectricTestRunner.class) +public class FlutterRendererTest { + FlutterJNITest jniUnderTest; + FlutterRenderer rendererUnderTest; + + @Before + public void setUp() { + jniUnderTest = new FlutterJNITest(); + rendererUnderTest = new FlutterRenderer(jniUnderTest); + } + + @Test + public void itHoldsFlutterEngines() { + AtomicInteger callbackCalled = new AtomicInteger(0); + OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { + @Override + public void onFirstFrameRendered() { + callbackCalled.incrementAndGet(); + rendererUnderTest.removeOnFirstFrameRenderedListener(this); + }; + }; + + rendererUnderTest.addOnFirstFrameRenderedListener(callback); + jniUnderTest.onFirstFrame(); + + assertEquals(1, callbackCalled.get()); + + // The callback removed itself from the listener list. A second call doesn't call the callback. + jniUnderTest.onFirstFrame(); + assertEquals(1, callbackCalled.get()); + } + + private static class FlutterJNITest extends FlutterJNI { + protected void onFirstFrame() { + // Exposed here for tests. + super.onFirstFrame(); + } + } +} From 3b87bd80853a16d5fe145e496242e5d8cb928d61 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Wed, 14 Aug 2019 17:40:22 -0700 Subject: [PATCH 2/5] forgot to name it right --- .../flutter/embedding/engine/renderer/FlutterRendererTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index a4ca388cc418a..4c3b27285e683 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -31,7 +31,7 @@ public void setUp() { } @Test - public void itHoldsFlutterEngines() { + public void firstFrameListenerCanRemoveThemselves() { AtomicInteger callbackCalled = new AtomicInteger(0); OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { @Override From a9a9247e07aa700535c839f22e3c93bfa667d3b9 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Tue, 20 Aug 2019 14:52:27 -0700 Subject: [PATCH 3/5] add another test for flutter jni --- shell/platform/android/BUILD.gn | 1 + .../test/io/flutter/FlutterTestSuite.java | 2 + .../embedding/engine/FlutterJNITest.java | 51 +++++++++++++++++++ testing/run_tests.py | 2 +- 4 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 9162fd1e9000e..80d4046d3e4ec 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -407,6 +407,7 @@ action("robolectric_tests") { "test/io/flutter/embedding/android/FlutterActivityTest.java", "test/io/flutter/embedding/android/FlutterFragmentTest.java", "test/io/flutter/embedding/engine/FlutterEngineCacheTest.java", + "test/io/flutter/embedding/engine/FlutterJNITest.java", "test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java", "test/io/flutter/embedding/engine/systemchannels/TextInputChannelTest.java", "test/io/flutter/util/PreconditionsTest.java", diff --git a/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/shell/platform/android/test/io/flutter/FlutterTestSuite.java index 9cd30c4af827d..747af35ec5620 100644 --- a/shell/platform/android/test/io/flutter/FlutterTestSuite.java +++ b/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -12,6 +12,7 @@ import io.flutter.embedding.android.FlutterActivityTest; import io.flutter.embedding.android.FlutterFragmentTest; import io.flutter.embedding.engine.FlutterEngineCacheTest; +import io.flutter.embedding.engine.FlutterJNITest; import io.flutter.embedding.engine.renderer.FlutterRendererTest; import io.flutter.embedding.engine.systemchannels.TextInputChannelTest; import io.flutter.util.PreconditionsTest; @@ -24,6 +25,7 @@ FlutterFragmentTest.class, // FlutterActivityAndFragmentDelegateTest.class, TODO(mklim): Fix and re-enable this FlutterEngineCacheTest.class, + FlutterJNITest.class, FlutterRendererTest.class, TextInputChannelTest.class }) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java new file mode 100644 index 0000000000000..5a661931132de --- /dev/null +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -0,0 +1,51 @@ +package io.flutter.embedding.engine; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import io.flutter.embedding.engine.FlutterJNI; +import io.flutter.embedding.engine.renderer.FlutterRenderer; +import io.flutter.embedding.engine.renderer.OnFirstFrameRenderedListener; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import java.util.concurrent.atomic.AtomicInteger; + +@Config(manifest=Config.NONE) +@RunWith(RobolectricTestRunner.class) +public class FlutterJNITest { + FlutterJNI jniUnderTest; + + @Before + public void setUp() { + jniUnderTest = new FlutterJNI(); + } + + @Test + public void firstFrameListenerCanRemoveThemselves() { + AtomicInteger callbackCalled = new AtomicInteger(0); + OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { + @Override + public void onFirstFrameRendered() { + callbackCalled.incrementAndGet(); + jniUnderTest.removeOnFirstFrameRenderedListener(this); + }; + }; + + jniUnderTest.addOnFirstFrameRenderedListener(callback); + jniUnderTest.onFirstFrame(); + + assertEquals(1, callbackCalled.get()); + + // The callback removed itself from the listener list. A second call doesn't call the callback. + jniUnderTest.onFirstFrame(); + assertEquals(1, callbackCalled.get()); + } +} diff --git a/testing/run_tests.py b/testing/run_tests.py index 37951ed70beb3..dabeeb9a7ce21 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -205,7 +205,7 @@ def EnsureDebugUnoptSkyPackagesAreBuilt(): def EnsureJavaTestsAreBuilt(android_out_dir): ninja_command = [ - 'ninja', + 'autoninja', '-C', android_out_dir, 'flutter/shell/platform/android:robolectric_tests' From 52b30c996b3fae1be8df8346a37964e1bbec07bf Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Tue, 20 Aug 2019 15:19:40 -0700 Subject: [PATCH 4/5] rename tests --- .../io/flutter/embedding/engine/FlutterJNITest.java | 10 ++++++++-- .../embedding/engine/renderer/FlutterRendererTest.java | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java index 5a661931132de..41ff9bbef34e1 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -29,7 +29,8 @@ public void setUp() { } @Test - public void firstFrameListenerCanRemoveThemselves() { + public void itAllowsFirstFrameListenersToRemoveThemselves() { + // --- Test Setup --- AtomicInteger callbackCalled = new AtomicInteger(0); OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { @Override @@ -38,14 +39,19 @@ public void onFirstFrameRendered() { jniUnderTest.removeOnFirstFrameRenderedListener(this); }; }; - jniUnderTest.addOnFirstFrameRenderedListener(callback); + + // --- Execute Test --- jniUnderTest.onFirstFrame(); + // --- Verify Results --- assertEquals(1, callbackCalled.get()); + // --- Execute Test --- // The callback removed itself from the listener list. A second call doesn't call the callback. jniUnderTest.onFirstFrame(); + + // --- Verify Results --- assertEquals(1, callbackCalled.get()); } } diff --git a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 4c3b27285e683..f8dcabb3bed9b 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -31,7 +31,8 @@ public void setUp() { } @Test - public void firstFrameListenerCanRemoveThemselves() { + public void itAllowsFirstFrameListenersToRemoveThemselves() { + // --- Test Setup --- AtomicInteger callbackCalled = new AtomicInteger(0); OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { @Override @@ -40,14 +41,19 @@ public void onFirstFrameRendered() { rendererUnderTest.removeOnFirstFrameRenderedListener(this); }; }; - rendererUnderTest.addOnFirstFrameRenderedListener(callback); + + // --- Execute Test --- jniUnderTest.onFirstFrame(); + // --- Verify Results --- assertEquals(1, callbackCalled.get()); + // --- Execute Test --- // The callback removed itself from the listener list. A second call doesn't call the callback. jniUnderTest.onFirstFrame(); + + // --- Verify Results --- assertEquals(1, callbackCalled.get()); } From 738d555de88a52182817444960e22a58927140e1 Mon Sep 17 00:00:00 2001 From: Xiao Yu Date: Tue, 20 Aug 2019 15:20:58 -0700 Subject: [PATCH 5/5] name tweak --- .../test/io/flutter/embedding/engine/FlutterJNITest.java | 2 +- .../flutter/embedding/engine/renderer/FlutterRendererTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java index 41ff9bbef34e1..c35846ea530d9 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -29,7 +29,7 @@ public void setUp() { } @Test - public void itAllowsFirstFrameListenersToRemoveThemselves() { + public void itAllowsFirstFrameListenersToRemoveThemselvesInline() { // --- Test Setup --- AtomicInteger callbackCalled = new AtomicInteger(0); OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() { diff --git a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index f8dcabb3bed9b..dd9c31097b3f3 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -31,7 +31,7 @@ public void setUp() { } @Test - public void itAllowsFirstFrameListenersToRemoveThemselves() { + public void itAllowsFirstFrameListenersToRemoveThemselvesInline() { // --- Test Setup --- AtomicInteger callbackCalled = new AtomicInteger(0); OnFirstFrameRenderedListener callback = new OnFirstFrameRenderedListener() {