From df36d878ce3f9c0a7a8e185d9b3b60b501e8cd1b Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 14 Jan 2022 19:48:45 -0800 Subject: [PATCH 01/10] Teardown external view embedder prior to unmerging threads --- shell/common/rasterizer.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 2fec73ab23793..71a922d05b456 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -91,16 +91,16 @@ void Rasterizer::Teardown() { surface_.reset(); last_layer_tree_.reset(); + if (external_view_embedder_) { + external_view_embedder_->Teardown(); + } + if (raster_thread_merger_.get() != nullptr && raster_thread_merger_.get()->IsMerged()) { FML_DCHECK(raster_thread_merger_->IsEnabled()); raster_thread_merger_->UnMergeNowIfLastOne(); raster_thread_merger_->SetMergeUnmergeCallback(nullptr); } - - if (external_view_embedder_) { - external_view_embedder_->Teardown(); - } } void Rasterizer::EnableThreadMergerIfNeeded() { From 087febfdb501a4da96f034a6ece1aeb1e6f9d7cc Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 14 Jan 2022 20:26:05 -0800 Subject: [PATCH 02/10] Test --- flow/embedded_views.cc | 3 ++- flow/embedded_views.h | 3 ++- shell/common/rasterizer.cc | 4 ++-- .../external_view_embedder.cc | 7 +++++-- .../external_view_embedder.h | 3 ++- .../external_view_embedder_unittests.cc | 20 +++++++++++++++++-- 6 files changed, 31 insertions(+), 9 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index c85ad66f8dc75..d76ad083ab6ec 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -64,6 +64,7 @@ bool ExternalViewEmbedder::SupportsDynamicThreadMerging() { return false; } -void ExternalViewEmbedder::Teardown() {} +void ExternalViewEmbedder::Teardown( + fml::RefPtr raster_thread_merger) {} } // namespace flutter diff --git a/flow/embedded_views.h b/flow/embedded_views.h index f62e4e86fe443..07d1028428c1c 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -333,7 +333,8 @@ class ExternalViewEmbedder { // Called when the rasterizer is being torn down. // This method provides a way to release resources associated with the current // embedder. - virtual void Teardown(); + virtual void Teardown( + fml::RefPtr raster_thread_merger); FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 71a922d05b456..c4a060772bfea 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -91,8 +91,8 @@ void Rasterizer::Teardown() { surface_.reset(); last_layer_tree_.reset(); - if (external_view_embedder_) { - external_view_embedder_->Teardown(); + if (external_view_embedder_ && raster_thread_merger_.get() != nullptr) { + external_view_embedder_->Teardown(raster_thread_merger_); } if (raster_thread_merger_.get() != nullptr && diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 0c326eb950b03..08fb7287f496d 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -299,8 +299,11 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { } // |ExternalViewEmbedder| -void AndroidExternalViewEmbedder::Teardown() { - surface_pool_->DestroyLayers(jni_facade_); +void AndroidExternalViewEmbedder::Teardown( + fml::RefPtr raster_thread_merger) { + if (raster_thread_merger->IsOnPlatformThread()) { + surface_pool_->DestroyLayers(jni_facade_); + } } } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index 278d7693e7661..e98bcd5049a33 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -73,7 +73,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { bool SupportsDynamicThreadMerging() override; - void Teardown() override; + void Teardown( + fml::RefPtr raster_thread_merger) override; // Gets the rect based on the device pixel ratio of a platform view displayed // on the screen. diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index c5a3c19701a3f..ca9518efe3238 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -953,9 +953,25 @@ TEST(AndroidExternalViewEmbedder, Teardown) { embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); + // Teardown while on platform thread. + ASSERT_TRUE(raster_thread_merger->IsOnPlatformThread()); EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); - // Teardown. - embedder->Teardown(); + embedder->Teardown(raster_thread_merger); + + // Teardown while on raster thread. + { + fml::Thread raster_thread("raster"); + auto raster_thread_merger = fml::MakeRefCounted( + raster_thread.GetTaskRunner() + ->GetTaskQueueId(), // Make the platform thread be the raster + // thread. + raster_thread.GetTaskRunner()->GetTaskQueueId()); + + ASSERT_FALSE(raster_thread_merger->IsOnPlatformThread()); + + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); + embedder->Teardown(raster_thread_merger); + } } } // namespace testing From 15314aef1bb5e5af8ef23d272b71c94fd922e632 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 18 Jan 2022 13:34:50 -0800 Subject: [PATCH 03/10] Allow DestroyLayers to be called from any thread --- flow/embedded_views.cc | 3 +- flow/embedded_views.h | 3 +- shell/common/rasterizer.cc | 8 ++-- .../external_view_embedder.cc | 9 ++-- .../external_view_embedder.h | 3 +- .../external_view_embedder_unittests.cc | 20 +-------- .../flutter/embedding/engine/FlutterJNI.java | 41 ++++++++++++++++--- .../embedding/engine/FlutterJNITest.java | 37 +++++++++++++++++ 8 files changed, 84 insertions(+), 40 deletions(-) diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index d76ad083ab6ec..c85ad66f8dc75 100644 --- a/flow/embedded_views.cc +++ b/flow/embedded_views.cc @@ -64,7 +64,6 @@ bool ExternalViewEmbedder::SupportsDynamicThreadMerging() { return false; } -void ExternalViewEmbedder::Teardown( - fml::RefPtr raster_thread_merger) {} +void ExternalViewEmbedder::Teardown() {} } // namespace flutter diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 07d1028428c1c..f62e4e86fe443 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -333,8 +333,7 @@ class ExternalViewEmbedder { // Called when the rasterizer is being torn down. // This method provides a way to release resources associated with the current // embedder. - virtual void Teardown( - fml::RefPtr raster_thread_merger); + virtual void Teardown(); FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index c4a060772bfea..2fec73ab23793 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -91,16 +91,16 @@ void Rasterizer::Teardown() { surface_.reset(); last_layer_tree_.reset(); - if (external_view_embedder_ && raster_thread_merger_.get() != nullptr) { - external_view_embedder_->Teardown(raster_thread_merger_); - } - if (raster_thread_merger_.get() != nullptr && raster_thread_merger_.get()->IsMerged()) { FML_DCHECK(raster_thread_merger_->IsEnabled()); raster_thread_merger_->UnMergeNowIfLastOne(); raster_thread_merger_->SetMergeUnmergeCallback(nullptr); } + + if (external_view_embedder_) { + external_view_embedder_->Teardown(); + } } void Rasterizer::EnableThreadMergerIfNeeded() { diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 08fb7287f496d..159f2ff128e06 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -264,7 +264,7 @@ void AndroidExternalViewEmbedder::BeginFrame( // The surface size changed. Therefore, destroy existing surfaces as // the existing surfaces in the pool can't be recycled. - if (frame_size_ != frame_size && raster_thread_merger->IsOnPlatformThread()) { + if (frame_size_ != frame_size) { surface_pool_->DestroyLayers(jni_facade_); } surface_pool_->SetFrameSize(frame_size); @@ -299,11 +299,8 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { } // |ExternalViewEmbedder| -void AndroidExternalViewEmbedder::Teardown( - fml::RefPtr raster_thread_merger) { - if (raster_thread_merger->IsOnPlatformThread()) { - surface_pool_->DestroyLayers(jni_facade_); - } +void AndroidExternalViewEmbedder::Teardown() { + surface_pool_->DestroyLayers(jni_facade_); } } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index e98bcd5049a33..278d7693e7661 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -73,8 +73,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { bool SupportsDynamicThreadMerging() override; - void Teardown( - fml::RefPtr raster_thread_merger) override; + void Teardown() override; // Gets the rect based on the device pixel ratio of a platform view displayed // on the screen. diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index ca9518efe3238..c5a3c19701a3f 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -953,25 +953,9 @@ TEST(AndroidExternalViewEmbedder, Teardown) { embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); - // Teardown while on platform thread. - ASSERT_TRUE(raster_thread_merger->IsOnPlatformThread()); EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()); - embedder->Teardown(raster_thread_merger); - - // Teardown while on raster thread. - { - fml::Thread raster_thread("raster"); - auto raster_thread_merger = fml::MakeRefCounted( - raster_thread.GetTaskRunner() - ->GetTaskQueueId(), // Make the platform thread be the raster - // thread. - raster_thread.GetTaskRunner()->GetTaskQueueId()); - - ASSERT_FALSE(raster_thread_merger->IsOnPlatformThread()); - - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); - embedder->Teardown(raster_thread_merger); - } + // Teardown. + embedder->Teardown(); } } // namespace testing diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 128eb5c6a8e8d..e3a4de6c6f16e 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -11,6 +11,7 @@ import android.graphics.ImageDecoder; import android.graphics.SurfaceTexture; import android.os.Build; +import android.os.Handler; import android.os.Looper; import android.util.Size; import android.view.Surface; @@ -1171,12 +1172,25 @@ public FlutterOverlaySurface createOverlaySurface() { @SuppressWarnings("unused") @UiThread public void destroyOverlaySurfaces() { - ensureRunningOnMainThread(); - if (platformViewsController == null) { - throw new RuntimeException( - "platformViewsController must be set before attempting to destroy an overlay surface"); - } - platformViewsController.destroyOverlaySurfaces(); + // Normally, this method is called on the Android main thread. + // + // However, it may be called from a different thread after the thread merger lease expired + // and the rasterizer is being torn down. + // + // The platform view embedder may keep these surfaces even after the lease have expired in + // case they are used in a future frame that contains platform views. + // + // This is particularly an issue when a "cached" engine is used. + // + // See Rasterizer::Teardown in C++, and FlutterEngineCache in Java. + runOnMainThread( + () -> { + if (platformViewsController == null) { + throw new RuntimeException( + "platformViewsController must be set before attempting to destroy an overlay surface"); + } + platformViewsController.destroyOverlaySurfaces(); + }); } // ----- End Engine Lifecycle Support ---- @@ -1406,6 +1420,21 @@ private void ensureRunningOnMainThread() { } } + /** + * Causes the runnable r to be run on the Android main thread. If the current thread is different, + * then it adds the runnable to the message queue of the main thread. + * + * @param r The runnable that will be executed. + */ + private void runOnMainThread(Runnable r) { + if (Looper.myLooper() == mainLooper) { + r.run(); + return; + } + final Handler handler = new Handler(mainLooper); + handler.post(r); + } + /** * Delegate responsible for creating and updating Android-side caches of Flutter's semantics tree * and custom accessibility actions. 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 5f057c36609a0..7e9455387c4ac 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -1,10 +1,13 @@ package io.flutter.embedding.engine; +import static android.os.Looper.getMainLooper; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; import android.annotation.TargetApi; import android.content.Context; @@ -19,7 +22,9 @@ import io.flutter.plugin.platform.PlatformViewsController; import java.nio.ByteBuffer; import java.util.Locale; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -245,4 +250,36 @@ public void invokePlatformMessageResponseCallback__wantsDirectBuffer() { ByteBuffer buffer = ByteBuffer.allocate(4); flutterJNI.invokePlatformMessageResponseCallback(0, buffer, buffer.position()); } + + @Test + public void destroyOverlaySurfaces__calledFromDifferentThread() throws Exception { + PlatformViewsController platformViewsController = mock(PlatformViewsController.class); + + FlutterJNI flutterJNI = new FlutterJNI(); + flutterJNI.setPlatformViewsController(platformViewsController); + + AtomicReference exception = new AtomicReference<>(); + + // --- Execute Test --- + CountDownLatch latch = new CountDownLatch(1); + Thread thread = + new Thread( + () -> { + try { + flutterJNI.destroyOverlaySurfaces(); + } catch (Exception e) { + exception.set(e); + } + latch.countDown(); + }); + + thread.start(); + latch.await(); + shadowOf(getMainLooper()).idle(); + + // --- Verify Results --- + if (exception.get() != null) { + fail("Unexpected exception: " + exception.get().toString()); + } + } } From dd7b46253534a23bbeaec671cb2047c2089d0eaf Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 18 Jan 2022 15:59:08 -0800 Subject: [PATCH 04/10] Clarify a bit more --- .../android/io/flutter/embedding/engine/FlutterJNI.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index e3a4de6c6f16e..a5d4a12f125ad 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -1180,7 +1180,9 @@ public void destroyOverlaySurfaces() { // The platform view embedder may keep these surfaces even after the lease have expired in // case they are used in a future frame that contains platform views. // - // This is particularly an issue when a "cached" engine is used. + // This is particularly an issue when a "cached" engine is used. In this case, this method + // would not be called if it requires to run on the main thread. For this reason, this + // method hops to the Android main thread if necessary. // // See Rasterizer::Teardown in C++, and FlutterEngineCache in Java. runOnMainThread( From a4e4e7823411040fd88d82e175997e77636a2947 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 18 Jan 2022 18:12:44 -0800 Subject: [PATCH 05/10] Fix test --- .../external_view_embedder_unittests.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index c5a3c19701a3f..0f7043b41993a 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -843,9 +843,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { embedder->EndFrame(/*should_resubmit_frame=*/false, raster_thread_merger); } - // Changing the frame size from the raster thread does not make JNI calls. - - EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0); + EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1); EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); fml::Thread platform_thread("platform"); From 20250bca6113eb5790bd02a5d52a28dc9feeeed2 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 19 Jan 2022 19:47:30 -0800 Subject: [PATCH 06/10] Add lock --- .../flutter/embedding/engine/FlutterJNI.java | 54 +++++++++++++----- .../embedding/engine/FlutterJNITest.java | 57 +++++++++++-------- 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index a5d4a12f125ad..6d16ad31f84b6 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -42,6 +42,7 @@ import java.util.Locale; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantReadWriteLock; /** @@ -73,7 +74,7 @@ * *
{@code
  * // Instantiate FlutterJNI and attach to the native side.
- * FlutterJNI flutterJNI = new FlutterJNI();
+ * FlutterJNI flutterJNI = new FlutterJNI(Looper.getMainLooper());
  * flutterJNI.attachToNative();
  *
  * // Use FlutterJNI as desired. flutterJNI.dispatchPointerDataPacket(...);
@@ -107,11 +108,13 @@ public class FlutterJNI {
   // platform thread and doesn't require locking.
   private ReentrantReadWriteLock shellHolderLock = new ReentrantReadWriteLock();
 
-  // Prefer using the FlutterJNI.Factory so it's easier to test.
+  /** @param looper The main looper. Typically, Looper.getMainLooper(). */
+  public FlutterJNI(@NonNull Looper looper) {
+    mainLooper = looper;
+  }
+
   public FlutterJNI() {
-    // We cache the main looper so that we can ensure calls are made on the main thread
-    // without consistently paying the synchronization cost of getMainLooper().
-    mainLooper = Looper.getMainLooper();
+    this(Looper.getMainLooper());
   }
 
   /**
@@ -1170,8 +1173,7 @@ public FlutterOverlaySurface createOverlaySurface() {
   }
 
   @SuppressWarnings("unused")
-  @UiThread
-  public void destroyOverlaySurfaces() {
+  public void destroyOverlaySurfaces() throws Exception {
     // Normally, this method is called on the Android main thread.
     //
     // However, it may be called from a different thread after the thread merger lease expired
@@ -1185,14 +1187,15 @@ public void destroyOverlaySurfaces() {
     // method hops to the Android main thread if necessary.
     //
     // See Rasterizer::Teardown in C++, and FlutterEngineCache in Java.
-    runOnMainThread(
+    runOnLooper(
         () -> {
           if (platformViewsController == null) {
             throw new RuntimeException(
                 "platformViewsController must be set before attempting to destroy an overlay surface");
           }
           platformViewsController.destroyOverlaySurfaces();
-        });
+        },
+        mainLooper);
   }
   // ----- End Engine Lifecycle Support ----
 
@@ -1423,18 +1426,39 @@ private void ensureRunningOnMainThread() {
   }
 
   /**
-   * Causes the runnable r to be run on the Android main thread. If the current thread is different,
-   * then it adds the runnable to the message queue of the main thread.
+   * Causes the runnable r to be run on the thread associated with the given looper l. Then, it
+   * waits for the runnable to run.
+   *
+   * 

If the runnable throws, then the exception is captured and rethrown in the current thread. * * @param r The runnable that will be executed. + * @param l The looper where the runnable is added. */ - private void runOnMainThread(Runnable r) { - if (Looper.myLooper() == mainLooper) { + private void runOnLooper(Runnable r, Looper l) throws Exception { + if (Looper.myLooper() == l) { r.run(); return; } - final Handler handler = new Handler(mainLooper); - handler.post(r); + final AtomicReference exception = new AtomicReference<>(); + final Handler handler = new Handler(l); + handler.post( + () -> { + try { + r.run(); + } catch (Exception e) { + exception.set(e); + } + synchronized (handler) { + handler.notify(); + } + }); + synchronized (handler) { + handler.wait(); + } + if (exception.get() != null) { + // Rethrow the exception on the current thread. + throw exception.get(); + } } /** 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 7e9455387c4ac..22f09c7ae286d 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -1,19 +1,19 @@ package io.flutter.embedding.engine; -import static android.os.Looper.getMainLooper; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.robolectric.Shadows.shadowOf; import android.annotation.TargetApi; import android.content.Context; import android.content.res.Configuration; import android.content.res.Resources; +import android.os.Handler; import android.os.LocaleList; +import android.os.Looper; +import android.os.Message; import io.flutter.embedding.engine.dart.DartExecutor; import io.flutter.embedding.engine.mutatorsstack.FlutterMutatorsStack; import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener; @@ -22,7 +22,6 @@ import io.flutter.plugin.platform.PlatformViewsController; import java.nio.ByteBuffer; import java.util.Locale; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; @@ -253,33 +252,45 @@ public void invokePlatformMessageResponseCallback__wantsDirectBuffer() { @Test public void destroyOverlaySurfaces__calledFromDifferentThread() throws Exception { + AtomicReference flutterJNI = new AtomicReference<>(); PlatformViewsController platformViewsController = mock(PlatformViewsController.class); - FlutterJNI flutterJNI = new FlutterJNI(); - flutterJNI.setPlatformViewsController(platformViewsController); - - AtomicReference exception = new AtomicReference<>(); - - // --- Execute Test --- - CountDownLatch latch = new CountDownLatch(1); - Thread thread = + Object lock = new Object(); + // This thread acts as the Android main thread. + // This is required because the test itself runs on the main thread, and without this + // workaround, + // a deadlock is produced. + Thread mainThreadImpostor = new Thread( () -> { - try { - flutterJNI.destroyOverlaySurfaces(); - } catch (Exception e) { - exception.set(e); + Looper.prepare(); + + flutterJNI.set(new FlutterJNI(Looper.myLooper())); + flutterJNI.get().setPlatformViewsController(platformViewsController); + + synchronized (lock) { + lock.notify(); } - latch.countDown(); + + new Handler(Looper.myLooper()) { + public void handleMessage(Message msg) { + msg.getCallback().run(); + } + }; + Looper.loop(); }); - thread.start(); - latch.await(); - shadowOf(getMainLooper()).idle(); + mainThreadImpostor.start(); + synchronized (lock) { + lock.wait(); + } + + // --- Execute Test --- + flutterJNI.get().destroyOverlaySurfaces(); // --- Verify Results --- - if (exception.get() != null) { - fail("Unexpected exception: " + exception.get().toString()); - } + verify(platformViewsController, times(1)).destroyOverlaySurfaces(); + + mainThreadImpostor.interrupt(); } } From 6371bb0c2d2968a039636c73822ec27cdd918f78 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 19 Jan 2022 20:02:17 -0800 Subject: [PATCH 07/10] Format --- .../test/io/flutter/embedding/engine/FlutterJNITest.java | 3 +-- 1 file changed, 1 insertion(+), 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 22f09c7ae286d..dbfd45f26e200 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -258,8 +258,7 @@ public void destroyOverlaySurfaces__calledFromDifferentThread() throws Exception Object lock = new Object(); // This thread acts as the Android main thread. // This is required because the test itself runs on the main thread, and without this - // workaround, - // a deadlock is produced. + // workaround, a deadlock is produced. Thread mainThreadImpostor = new Thread( () -> { From 9c579533e6ee2cd2a4c9721757c7babd39bb7eec Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 21 Jan 2022 16:15:09 -0800 Subject: [PATCH 08/10] Reuse C++ API --- .../external_view_embedder.cc | 23 +++++- .../external_view_embedder.h | 12 ++- .../external_view_embedder_unittests.cc | 46 +++++++---- .../flutter/embedding/engine/FlutterJNI.java | 81 +++---------------- .../platform/android/platform_view_android.cc | 10 +-- .../embedding/engine/FlutterJNITest.java | 47 ----------- tools/android_lint/project.xml | 4 +- 7 files changed, 79 insertions(+), 144 deletions(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 159f2ff128e06..84f2ab9378125 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -4,6 +4,8 @@ #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/fml/task_runner.h" #include "flutter/fml/trace_event.h" #include "flutter/shell/platform/android/surface/android_surface.h" @@ -12,12 +14,14 @@ namespace flutter { AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( const AndroidContext& android_context, std::shared_ptr jni_facade, - std::shared_ptr surface_factory) + std::shared_ptr surface_factory, + TaskRunners task_runners) : ExternalViewEmbedder(), android_context_(android_context), jni_facade_(jni_facade), surface_factory_(surface_factory), - surface_pool_(std::make_unique()) {} + surface_pool_(std::make_unique()), + task_runners_(task_runners) {} // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( @@ -265,7 +269,7 @@ void AndroidExternalViewEmbedder::BeginFrame( // The surface size changed. Therefore, destroy existing surfaces as // the existing surfaces in the pool can't be recycled. if (frame_size_ != frame_size) { - surface_pool_->DestroyLayers(jni_facade_); + DestroySurfaces(); } surface_pool_->SetFrameSize(frame_size); // JNI method must be called on the platform thread. @@ -300,7 +304,18 @@ bool AndroidExternalViewEmbedder::SupportsDynamicThreadMerging() { // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::Teardown() { - surface_pool_->DestroyLayers(jni_facade_); + DestroySurfaces(); +} + +// |ExternalViewEmbedder| +void AndroidExternalViewEmbedder::DestroySurfaces() { + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask(task_runners_.GetPlatformTaskRunner(), + [&]() { + surface_pool_->DestroyLayers(jni_facade_); + latch.Signal(); + }); + latch.Wait(); } } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index 278d7693e7661..2ec13297bb9ad 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -7,6 +7,7 @@ #include +#include "flutter/common/task_runners.h" #include "flutter/flow/embedded_views.h" #include "flutter/flow/rtree.h" #include "flutter/shell/platform/android/context/android_context.h" @@ -32,7 +33,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { AndroidExternalViewEmbedder( const AndroidContext& android_context, std::shared_ptr jni_facade, - std::shared_ptr surface_factory); + std::shared_ptr surface_factory, + TaskRunners task_runners); // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView( @@ -99,6 +101,9 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // Holds surfaces. Allows to recycle surfaces or allocate new ones. const std::unique_ptr surface_pool_; + // The task runners. + const TaskRunners task_runners_; + // The size of the root canvas. SkISize frame_size_; @@ -126,6 +131,11 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // The number of platform views in the previous frame. int64_t previous_frame_view_count_; + // Destroys the surfaces created from the surface factory. + // This method schedules a task on the platform thread, and waits for + // the task until it completes. + void DestroySurfaces(); + // Resets the state. void Reset(); diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index 0f7043b41993a..fa0e65a9f1dd7 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -87,12 +87,24 @@ fml::RefPtr GetThreadMergerFromRasterThread( rasterizer_queue_id); } +TaskRunners GetTaskRunnersForFixture() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + auto& loop = fml::MessageLoop::GetCurrent(); + return { + "test", + loop.GetTaskRunner(), // platform + loop.GetTaskRunner(), // raster + loop.GetTaskRunner(), // ui + loop.GetTaskRunner() // io + }; +} + TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { auto jni_mock = std::make_shared(); auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); @@ -117,7 +129,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); @@ -140,7 +152,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) { TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, nullptr, nullptr); + android_context, nullptr, nullptr, GetTaskRunnersForFixture()); ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(0)); embedder->PrerollCompositeEmbeddedView( @@ -156,7 +168,7 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { TEST(AndroidExternalViewEmbedder, CancelFrame) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, nullptr, nullptr); + android_context, nullptr, nullptr, GetTaskRunnersForFixture()); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -170,7 +182,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { auto jni_mock = std::make_shared(); auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = @@ -204,7 +216,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { auto jni_mock = std::make_shared(); auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = @@ -225,7 +237,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); @@ -253,7 +265,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRectChangedParams) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); @@ -328,7 +340,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) { return android_surface_mock; }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); auto raster_thread_merger = GetThreadMergerFromPlatformThread(); @@ -521,7 +533,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) { return android_surface_mock; }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); auto raster_thread_merger = GetThreadMergerFromPlatformThread(); @@ -623,7 +635,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) { return android_surface_mock; }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); auto raster_thread_merger = GetThreadMergerFromPlatformThread(); @@ -662,7 +674,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) { auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); // While on the raster thread, don't make JNI calls as these methods can only // run on the platform thread. @@ -711,7 +723,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) { }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); @@ -798,7 +810,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) { }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); // ------------------ First frame ------------------ // { @@ -855,7 +867,7 @@ TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { auto jni_mock = std::make_shared(); auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); ASSERT_TRUE(embedder->SupportsDynamicThreadMerging()); } @@ -863,7 +875,7 @@ TEST(AndroidExternalViewEmbedder, DisableThreadMerger) { auto jni_mock = std::make_shared(); auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware); auto embedder = std::make_unique( - android_context, jni_mock, nullptr); + android_context, jni_mock, nullptr, GetTaskRunnersForFixture()); fml::Thread platform_thread("platform"); auto raster_thread_merger = GetThreadMergerFromRasterThread(&platform_thread); @@ -919,7 +931,7 @@ TEST(AndroidExternalViewEmbedder, Teardown) { }); auto embedder = std::make_unique( - *android_context, jni_mock, surface_factory); + *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture()); fml::Thread rasterizer_thread("rasterizer"); auto raster_thread_merger = GetThreadMergerFromPlatformThread(&rasterizer_thread); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 6d16ad31f84b6..128eb5c6a8e8d 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -11,7 +11,6 @@ import android.graphics.ImageDecoder; import android.graphics.SurfaceTexture; import android.os.Build; -import android.os.Handler; import android.os.Looper; import android.util.Size; import android.view.Surface; @@ -42,7 +41,6 @@ import java.util.Locale; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantReadWriteLock; /** @@ -74,7 +72,7 @@ * *

{@code
  * // Instantiate FlutterJNI and attach to the native side.
- * FlutterJNI flutterJNI = new FlutterJNI(Looper.getMainLooper());
+ * FlutterJNI flutterJNI = new FlutterJNI();
  * flutterJNI.attachToNative();
  *
  * // Use FlutterJNI as desired. flutterJNI.dispatchPointerDataPacket(...);
@@ -108,13 +106,11 @@ public class FlutterJNI {
   // platform thread and doesn't require locking.
   private ReentrantReadWriteLock shellHolderLock = new ReentrantReadWriteLock();
 
-  /** @param looper The main looper. Typically, Looper.getMainLooper(). */
-  public FlutterJNI(@NonNull Looper looper) {
-    mainLooper = looper;
-  }
-
+  // Prefer using the FlutterJNI.Factory so it's easier to test.
   public FlutterJNI() {
-    this(Looper.getMainLooper());
+    // We cache the main looper so that we can ensure calls are made on the main thread
+    // without consistently paying the synchronization cost of getMainLooper().
+    mainLooper = Looper.getMainLooper();
   }
 
   /**
@@ -1173,29 +1169,14 @@ public FlutterOverlaySurface createOverlaySurface() {
   }
 
   @SuppressWarnings("unused")
-  public void destroyOverlaySurfaces() throws Exception {
-    // Normally, this method is called on the Android main thread.
-    //
-    // However, it may be called from a different thread after the thread merger lease expired
-    // and the rasterizer is being torn down.
-    //
-    // The platform view embedder may keep these surfaces even after the lease have expired in
-    // case they are used in a future frame that contains platform views.
-    //
-    // This is particularly an issue when a "cached" engine is used. In this case, this method
-    // would not be called if it requires to run on the main thread. For this reason, this
-    // method hops to the Android main thread if necessary.
-    //
-    // See Rasterizer::Teardown in C++, and FlutterEngineCache in Java.
-    runOnLooper(
-        () -> {
-          if (platformViewsController == null) {
-            throw new RuntimeException(
-                "platformViewsController must be set before attempting to destroy an overlay surface");
-          }
-          platformViewsController.destroyOverlaySurfaces();
-        },
-        mainLooper);
+  @UiThread
+  public void destroyOverlaySurfaces() {
+    ensureRunningOnMainThread();
+    if (platformViewsController == null) {
+      throw new RuntimeException(
+          "platformViewsController must be set before attempting to destroy an overlay surface");
+    }
+    platformViewsController.destroyOverlaySurfaces();
   }
   // ----- End Engine Lifecycle Support ----
 
@@ -1425,42 +1406,6 @@ private void ensureRunningOnMainThread() {
     }
   }
 
-  /**
-   * Causes the runnable r to be run on the thread associated with the given looper l. Then, it
-   * waits for the runnable to run.
-   *
-   * 

If the runnable throws, then the exception is captured and rethrown in the current thread. - * - * @param r The runnable that will be executed. - * @param l The looper where the runnable is added. - */ - private void runOnLooper(Runnable r, Looper l) throws Exception { - if (Looper.myLooper() == l) { - r.run(); - return; - } - final AtomicReference exception = new AtomicReference<>(); - final Handler handler = new Handler(l); - handler.post( - () -> { - try { - r.run(); - } catch (Exception e) { - exception.set(e); - } - synchronized (handler) { - handler.notify(); - } - }); - synchronized (handler) { - handler.wait(); - } - if (exception.get() != null) { - // Rethrow the exception on the current thread. - throw exception.get(); - } - } - /** * Delegate responsible for creating and updating Android-side caches of Flutter's semantics tree * and custom accessibility actions. diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index c3d4c696628f4..97c58de4c3298 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -14,13 +14,12 @@ #include "flutter/shell/platform/android/android_external_texture_gl.h" #include "flutter/shell/platform/android/android_surface_gl.h" #include "flutter/shell/platform/android/android_surface_software.h" -#include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" -#include "flutter/shell/platform/android/surface/android_surface.h" -#include "flutter/shell/platform/android/surface/snapshot_surface_producer.h" - #include "flutter/shell/platform/android/context/android_context.h" +#include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "flutter/shell/platform/android/platform_message_response_android.h" +#include "flutter/shell/platform/android/surface/android_surface.h" +#include "flutter/shell/platform/android/surface/snapshot_surface_producer.h" #include "flutter/shell/platform/android/vsync_waiter_android.h" namespace flutter { @@ -255,7 +254,8 @@ std::unique_ptr PlatformViewAndroid::CreateRenderingSurface() { std::shared_ptr PlatformViewAndroid::CreateExternalViewEmbedder() { return std::make_shared( - *android_context_, jni_facade_, surface_factory_); + *android_context_, jni_facade_, surface_factory_, + std::move(task_runners_)); } // |PlatformView| 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 dbfd45f26e200..5f057c36609a0 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java @@ -10,10 +10,7 @@ import android.content.Context; import android.content.res.Configuration; import android.content.res.Resources; -import android.os.Handler; import android.os.LocaleList; -import android.os.Looper; -import android.os.Message; import io.flutter.embedding.engine.dart.DartExecutor; import io.flutter.embedding.engine.mutatorsstack.FlutterMutatorsStack; import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener; @@ -23,7 +20,6 @@ import java.nio.ByteBuffer; import java.util.Locale; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -249,47 +245,4 @@ public void invokePlatformMessageResponseCallback__wantsDirectBuffer() { ByteBuffer buffer = ByteBuffer.allocate(4); flutterJNI.invokePlatformMessageResponseCallback(0, buffer, buffer.position()); } - - @Test - public void destroyOverlaySurfaces__calledFromDifferentThread() throws Exception { - AtomicReference flutterJNI = new AtomicReference<>(); - PlatformViewsController platformViewsController = mock(PlatformViewsController.class); - - Object lock = new Object(); - // This thread acts as the Android main thread. - // This is required because the test itself runs on the main thread, and without this - // workaround, a deadlock is produced. - Thread mainThreadImpostor = - new Thread( - () -> { - Looper.prepare(); - - flutterJNI.set(new FlutterJNI(Looper.myLooper())); - flutterJNI.get().setPlatformViewsController(platformViewsController); - - synchronized (lock) { - lock.notify(); - } - - new Handler(Looper.myLooper()) { - public void handleMessage(Message msg) { - msg.getCallback().run(); - } - }; - Looper.loop(); - }); - - mainThreadImpostor.start(); - synchronized (lock) { - lock.wait(); - } - - // --- Execute Test --- - flutterJNI.get().destroyOverlaySurfaces(); - - // --- Verify Results --- - verify(platformViewsController, times(1)).destroyOverlaySurfaces(); - - mainThreadImpostor.interrupt(); - } } diff --git a/tools/android_lint/project.xml b/tools/android_lint/project.xml index b37e7f0029a4c..27c279ba89da7 100644 --- a/tools/android_lint/project.xml +++ b/tools/android_lint/project.xml @@ -52,8 +52,8 @@ - + @@ -178,8 +178,8 @@ - + From d60f166998f6419f21ad1c859dd58de47e7f5219 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 21 Jan 2022 16:17:45 -0800 Subject: [PATCH 09/10] Revert change --- tools/android_lint/project.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/android_lint/project.xml b/tools/android_lint/project.xml index 27c279ba89da7..b37e7f0029a4c 100644 --- a/tools/android_lint/project.xml +++ b/tools/android_lint/project.xml @@ -52,8 +52,8 @@ - + @@ -178,8 +178,8 @@ - + From 0b22d375ae392efeede9b036e5ad6c42e0894de6 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 26 Jan 2022 11:19:18 -0800 Subject: [PATCH 10/10] Add FML_USED_ON_EMBEDDER --- .../external_view_embedder/external_view_embedder_unittests.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index fa0e65a9f1dd7..57966440d0d05 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#define FML_USED_ON_EMBEDDER + #include #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h"