Reland: Teardown external view embedder prior to unmerging threads

shell/common/rasterizer.cc

@@ -81,6 +81,12 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
   }
 }
 
+void Rasterizer::TeardownExternalViewEmbedder() {
+  if (external_view_embedder_) {
+    external_view_embedder_->Teardown();
+  }
+}
+
 void Rasterizer::Teardown() {
   auto context_switch =
       surface_ ? surface_->MakeRenderContextCurrent() : nullptr;
@@ -97,10 +103,6 @@ void Rasterizer::Teardown() {
     raster_thread_merger_->UnMergeNowIfLastOne();
     raster_thread_merger_->SetMergeUnmergeCallback(nullptr);
   }
-
-  if (external_view_embedder_) {
-    external_view_embedder_->Teardown();
-  }
 }
 
 void Rasterizer::EnableThreadMergerIfNeeded() {

shell/common/rasterizer.h

@@ -140,6 +140,13 @@ class Rasterizer final : public SnapshotDelegate {
   ///
   void Teardown();
 
+  //----------------------------------------------------------------------------
+  /// @brief      Releases any resource used by the external view embedder.
+  ///             For example, overlay surfaces or Android views.
+  ///             On Android, this method post a task to the platform thread,
+  ///             and waits until it completes.
+  void TeardownExternalViewEmbedder();
+
   //----------------------------------------------------------------------------
   /// @brief      Notifies the rasterizer that there is a low memory situation
   ///             and it must purge as many unnecessary resources as possible.

shell/common/shell.cc

@@ -885,6 +885,11 @@ void Shell::OnPlatformViewDestroyed() {
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
                                     raster_task);
   latch.Wait();
+  // On Android, the external view embedder posts a task to the platform thread,
+  // and waits until it completes.
+  // As a result, the platform thread must not be blocked prior to calling
+  // this method.
+  rasterizer_->TeardownExternalViewEmbedder();
 }
 
 // |PlatformView::Delegate|

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<PlatformViewAndroidJNI> jni_facade,
-    std::shared_ptr<AndroidSurfaceFactory> surface_factory)
+    std::shared_ptr<AndroidSurfaceFactory> surface_factory,
+    TaskRunners task_runners)
     : ExternalViewEmbedder(),
       android_context_(android_context),
       jni_facade_(jni_facade),
       surface_factory_(surface_factory),
-      surface_pool_(std::make_unique<SurfacePool>()) {}
+      surface_pool_(std::make_unique<SurfacePool>()),
+      task_runners_(task_runners) {}
 
 // |ExternalViewEmbedder|
 void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView(
@@ -264,8 +268,8 @@ 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()) {
-    surface_pool_->DestroyLayers(jni_facade_);
+  if (frame_size_ != frame_size) {
+    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

shell/platform/android/external_view_embedder/external_view_embedder.h

@@ -7,6 +7,7 @@
 
 #include <unordered_map>
 
+#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<PlatformViewAndroidJNI> jni_facade,
-      std::shared_ptr<AndroidSurfaceFactory> surface_factory);
+      std::shared_ptr<AndroidSurfaceFactory> 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<SurfacePool> 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();
 

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 <memory>
 #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h"
 
@@ -87,12 +89,24 @@ fml::RefPtr<fml::RasterThreadMerger> 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<JNIMock>();
 
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      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 +131,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) {
 
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      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 +154,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvasesCompositeOrder) {
 TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) {
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, nullptr, nullptr);
+      android_context, nullptr, nullptr, GetTaskRunnersForFixture());
 
   ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(0));
   embedder->PrerollCompositeEmbeddedView(
@@ -156,7 +170,7 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) {
 TEST(AndroidExternalViewEmbedder, CancelFrame) {
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, nullptr, nullptr);
+      android_context, nullptr, nullptr, GetTaskRunnersForFixture());
 
   embedder->PrerollCompositeEmbeddedView(
       0, std::make_unique<EmbeddedViewParams>());
@@ -170,7 +184,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) {
   auto jni_mock = std::make_shared<JNIMock>();
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, jni_mock, nullptr);
+      android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
 
   fml::Thread rasterizer_thread("rasterizer");
   auto raster_thread_merger =
@@ -204,7 +218,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) {
   auto jni_mock = std::make_shared<JNIMock>();
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, jni_mock, nullptr);
+      android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
 
   fml::Thread rasterizer_thread("rasterizer");
   auto raster_thread_merger =
@@ -225,7 +239,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) {
 
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      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 +267,7 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRectChangedParams) {
 
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      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 +342,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame) {
         return android_surface_mock;
       });
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *android_context, jni_mock, surface_factory);
+      *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
 
   auto raster_thread_merger = GetThreadMergerFromPlatformThread();
 
@@ -521,7 +535,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFrameOverlayComposition) {
         return android_surface_mock;
       });
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *android_context, jni_mock, surface_factory);
+      *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
 
   auto raster_thread_merger = GetThreadMergerFromPlatformThread();
 
@@ -623,7 +637,7 @@ TEST(AndroidExternalViewEmbedder, SubmitFramePlatformViewWithoutAnyOverlay) {
         return android_surface_mock;
       });
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *android_context, jni_mock, surface_factory);
+      *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
 
   auto raster_thread_merger = GetThreadMergerFromPlatformThread();
 
@@ -662,7 +676,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) {
 
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      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 +725,7 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) {
       });
 
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *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 +812,7 @@ TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) {
       });
 
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *android_context, jni_mock, surface_factory);
+      *android_context, jni_mock, surface_factory, GetTaskRunnersForFixture());
 
   // ------------------ First frame ------------------ //
   {
@@ -843,9 +857,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");
@@ -857,15 +869,15 @@ TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) {
   auto jni_mock = std::make_shared<JNIMock>();
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, jni_mock, nullptr);
+      android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
   ASSERT_TRUE(embedder->SupportsDynamicThreadMerging());
 }
 
 TEST(AndroidExternalViewEmbedder, DisableThreadMerger) {
   auto jni_mock = std::make_shared<JNIMock>();
   auto android_context = AndroidContext(AndroidRenderingAPI::kSoftware);
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      android_context, jni_mock, nullptr);
+      android_context, jni_mock, nullptr, GetTaskRunnersForFixture());
 
   fml::Thread platform_thread("platform");
   auto raster_thread_merger = GetThreadMergerFromRasterThread(&platform_thread);
@@ -921,7 +933,7 @@ TEST(AndroidExternalViewEmbedder, Teardown) {
       });
 
   auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
-      *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);

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,7 @@ std::unique_ptr<Surface> PlatformViewAndroid::CreateRenderingSurface() {
 std::shared_ptr<ExternalViewEmbedder>
 PlatformViewAndroid::CreateExternalViewEmbedder() {
   return std::make_shared<AndroidExternalViewEmbedder>(
-      *android_context_, jni_facade_, surface_factory_);
+      *android_context_, jni_facade_, surface_factory_, task_runners_);
 }
 
 // |PlatformView|