Ensure JNI is not called from raster thread (#21665)

Author: Emmanuel Garcia

shell/platform/android/external_view_embedder/external_view_embedder.cc

@@ -265,15 +265,17 @@ 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) {
+  if (frame_size_ != frame_size && raster_thread_merger->IsOnPlatformThread()) {
     surface_pool_->DestroyLayers(jni_facade_);
   }
-  frame_size_ = frame_size;
-  device_pixel_ratio_ = device_pixel_ratio;
+  surface_pool_->SetFrameSize(frame_size);
   // JNI method must be called on the platform thread.
   if (raster_thread_merger->IsOnPlatformThread()) {
     jni_facade_->FlutterViewBeginFrame();
   }
+
+  frame_size_ = frame_size;
+  device_pixel_ratio_ = device_pixel_ratio;
 }
 
 // |ExternalViewEmbedder|

shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

@@ -557,6 +557,91 @@ TEST(AndroidExternalViewEmbedder, DestroyOverlayLayersOnSizeChange) {
                        raster_thread_merger);
 }
 
+TEST(AndroidExternalViewEmbedder, DoesNotDestroyOverlayLayersOnSizeChange) {
+  auto jni_mock = std::make_shared<JNIMock>();
+  auto android_context =
+      std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
+
+  auto window = fml::MakeRefCounted<AndroidNativeWindow>(nullptr);
+  auto gr_context = GrDirectContext::MakeMock(nullptr);
+  auto frame_size = SkISize::Make(1000, 1000);
+  auto surface_factory =
+      [gr_context, window, frame_size](
+          std::shared_ptr<AndroidContext> android_context,
+          std::shared_ptr<PlatformViewAndroidJNI> jni_facade) {
+        auto surface_frame_1 = std::make_unique<SurfaceFrame>(
+            SkSurface::MakeNull(1000, 1000), false,
+            [](const SurfaceFrame& surface_frame, SkCanvas* canvas) {
+              return true;
+            });
+
+        auto surface_mock = std::make_unique<SurfaceMock>();
+        EXPECT_CALL(*surface_mock, AcquireFrame(frame_size))
+            .WillOnce(Return(ByMove(std::move(surface_frame_1))));
+
+        auto android_surface_mock = std::make_unique<AndroidSurfaceMock>();
+        EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true));
+
+        EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get()))
+            .WillOnce(Return(ByMove(std::move(surface_mock))));
+
+        EXPECT_CALL(*android_surface_mock, SetNativeWindow(window));
+
+        return android_surface_mock;
+      };
+
+  auto embedder = std::make_unique<AndroidExternalViewEmbedder>(
+      android_context, jni_mock, surface_factory);
+
+  // ------------------ First frame ------------------ //
+  {
+    auto raster_thread_merger = GetThreadMergerFromPlatformThread();
+    EXPECT_CALL(*jni_mock, FlutterViewBeginFrame());
+    embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger);
+
+    // Add an Android view.
+    MutatorsStack stack1;
+    // TODO(egarciad): Investigate why Flow applies the device pixel ratio to
+    // the offsetPixels, but not the sizePoints.
+    auto view_params_1 = std::make_unique<EmbeddedViewParams>(
+        SkMatrix(), SkSize::Make(200, 200), stack1);
+
+    embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1));
+
+    // This simulates Flutter UI that intersects with the Android view.
+    embedder->CompositeEmbeddedView(0)->drawRect(
+        SkRect::MakeXYWH(50, 50, 200, 200), SkPaint());
+
+    // Create a new overlay surface.
+    EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
+        .WillOnce(Return(
+            ByMove(std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
+                0, window))));
+    // The JNI call to display the Android view.
+    EXPECT_CALL(*jni_mock, FlutterViewOnDisplayPlatformView(0, 0, 0, 200, 200,
+                                                            300, 300, stack1));
+    EXPECT_CALL(*jni_mock,
+                FlutterViewDisplayOverlaySurface(0, 50, 50, 200, 200));
+
+    auto surface_frame =
+        std::make_unique<SurfaceFrame>(SkSurface::MakeNull(1000, 1000), false,
+                                       [](const SurfaceFrame& surface_frame,
+                                          SkCanvas* canvas) { return true; });
+    embedder->SubmitFrame(gr_context.get(), std::move(surface_frame));
+
+    EXPECT_CALL(*jni_mock, FlutterViewEndFrame());
+    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, FlutterViewBeginFrame()).Times(0);
+
+  embedder->BeginFrame(SkISize::Make(30, 40), nullptr, 1.0,
+                       GetThreadMergerFromRasterThread());
+}
+
 TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) {
   auto jni_mock = std::make_shared<JNIMock>();
 

shell/platform/android/external_view_embedder/surface_pool.cc

@@ -24,6 +24,11 @@ std::shared_ptr<OverlayLayer> SurfacePool::GetLayer(
     std::shared_ptr<AndroidContext> android_context,
     std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
     const AndroidSurface::Factory& surface_factory) {
+  // Destroy current layers in the pool if the frame size has changed.
+  if (requested_frame_size_ != current_frame_size_) {
+    DestroyLayers(jni_facade);
+  }
+
   intptr_t gr_context_key = reinterpret_cast<intptr_t>(gr_context);
   // Allocate a new surface if there isn't one available.
   if (available_layer_index_ >= layers_.size()) {
@@ -63,6 +68,7 @@ std::shared_ptr<OverlayLayer> SurfacePool::GetLayer(
     layer->surface = std::move(surface);
   }
   available_layer_index_++;
+  current_frame_size_ = requested_frame_size_;
   return layer;
 }
 
@@ -87,4 +93,8 @@ std::vector<std::shared_ptr<OverlayLayer>> SurfacePool::GetUnusedLayers() {
   return results;
 }
 
+void SurfacePool::SetFrameSize(SkISize frame_size) {
+  requested_frame_size_ = frame_size;
+}
+
 }  // namespace flutter

shell/platform/android/external_view_embedder/surface_pool.h

@@ -67,6 +67,11 @@ class SurfacePool {
   // Destroys all the layers in the pool.
   void DestroyLayers(std::shared_ptr<PlatformViewAndroidJNI> jni_facade);
 
+  // Sets the frame size used by the layers in the pool.
+  // If the current layers in the pool have a different frame size,
+  // then they are deallocated as soon as |GetLayer| is called.
+  void SetFrameSize(SkISize frame_size);
+
  private:
   // The index of the entry in the layers_ vector that determines the beginning
   // of the unused layers. For example, consider the following vector:
@@ -81,7 +86,15 @@ class SurfacePool {
   //  This indicates that entries starting from 1 can be reused meanwhile the
   //  entry at position 0 cannot be reused.
   size_t available_layer_index_ = 0;
+
+  // The layers in the pool.
   std::vector<std::shared_ptr<OverlayLayer>> layers_;
+
+  // The frame size of the layers in the pool.
+  SkISize current_frame_size_;
+
+  // The frame size to be used by future layers.
+  SkISize requested_frame_size_;
 };
 
 }  // namespace flutter

shell/platform/android/external_view_embedder/surface_pool_unittests.cc

@@ -202,5 +202,46 @@ TEST(SurfacePool, DestroyLayers) {
   ASSERT_TRUE(pool->GetUnusedLayers().empty());
 }
 
+TEST(SurfacePool, DestroyLayers__frameSizeChanged) {
+  auto pool = std::make_unique<SurfacePool>();
+  auto jni_mock = std::make_shared<JNIMock>();
+
+  auto gr_context = GrDirectContext::MakeMock(nullptr);
+  auto android_context =
+      std::make_shared<AndroidContext>(AndroidRenderingAPI::kSoftware);
+
+  auto window = fml::MakeRefCounted<AndroidNativeWindow>(nullptr);
+
+  auto surface_factory =
+      [gr_context, window](std::shared_ptr<AndroidContext> android_context,
+                           std::shared_ptr<PlatformViewAndroidJNI> jni_facade) {
+        auto android_surface_mock = std::make_unique<AndroidSurfaceMock>();
+        EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get()));
+        EXPECT_CALL(*android_surface_mock, SetNativeWindow(window));
+        EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true));
+        return android_surface_mock;
+      };
+  pool->SetFrameSize(SkISize::Make(10, 10));
+  EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(0);
+  EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
+      .Times(1)
+      .WillOnce(Return(
+          ByMove(std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
+              0, window))));
+
+  pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory);
+
+  pool->SetFrameSize(SkISize::Make(20, 20));
+  EXPECT_CALL(*jni_mock, FlutterViewDestroyOverlaySurfaces()).Times(1);
+  EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface())
+      .Times(1)
+      .WillOnce(Return(
+          ByMove(std::make_unique<PlatformViewAndroidJNI::OverlayMetadata>(
+              1, window))));
+  pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory);
+
+  ASSERT_TRUE(pool->GetUnusedLayers().empty());
+}
+
 }  // namespace testing
 }  // namespace flutter