Do not involve external_view_embedder in submit frame process if threads are not merged.

shell/common/rasterizer.cc

@@ -190,8 +190,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
     consume_result = PipelineConsumeResult::MoreAvailable;
   }
 
-  // Merging the thread as we know the next `Draw` should be run on the platform
-  // thread.
+  // EndFrame should perform cleanups for the external_view_embedder.
   if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) {
     surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame,
                                                   raster_thread_merger_);
@@ -469,7 +468,8 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
         raster_status == RasterStatus::kSkipAndRetry) {
       return raster_status;
     }
-    if (external_view_embedder != nullptr) {
+    if (external_view_embedder != nullptr &&
+        (!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
       FML_DCHECK(!frame->IsSubmitted());
       external_view_embedder->SubmitFrame(surface_->GetContext(),
                                           std::move(frame));

shell/common/rasterizer_unittests.cc

@@ -2,13 +2,16 @@
 // 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 "flutter/shell/common/rasterizer.h"
 
 #include "flutter/shell/common/thread_host.h"
 #include "flutter/testing/testing.h"
 #include "gmock/gmock.h"
 
 using testing::_;
+using testing::ByMove;
 using testing::Return;
 using testing::ReturnRef;
 
@@ -92,7 +95,8 @@ TEST(RasterizerTest, drawEmptyPipeline) {
   latch.Wait();
 }
 
-TEST(RasterizerTest, drawWithExternalViewEmbedder) {
+TEST(RasterizerTest,
+     drawWithExternalViewEmbedder_ExternalViewEmbedderSubmitFrameCalled) {
   std::string test_name =
       ::testing::UnitTest::GetInstance()->current_test_info()->name();
   ThreadHost thread_host("io.flutter.test." + test_name + ".",
@@ -111,11 +115,70 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
   MockExternalViewEmbedder external_view_embedder;
   EXPECT_CALL(*surface, GetExternalViewEmbedder())
       .WillRepeatedly(Return(&external_view_embedder));
+
+  auto surface_frame = std::make_unique<SurfaceFrame>(
+      nullptr, true, [](const SurfaceFrame&, SkCanvas*) { return true; });
+  EXPECT_CALL(*surface, AcquireFrame(SkISize()))
+      .WillOnce(Return(ByMove(std::move(surface_frame))));
+
   EXPECT_CALL(external_view_embedder,
               BeginFrame(SkISize(), nullptr, 2.0,
-                         fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
+                         fml::RefPtr<fml::RasterThreadMerger>(nullptr)))
+      .Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
   EXPECT_CALL(external_view_embedder,
-              EndFrame(false, fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
+              EndFrame(false, fml::RefPtr<fml::RasterThreadMerger>(nullptr)))
+      .Times(1);
+
+  rasterizer->Setup(std::move(surface));
+  fml::AutoResetWaitableEvent latch;
+  thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
+    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
+                                                  /*device_pixel_ratio=*/2.0f);
+    bool result = pipeline->Produce().Complete(std::move(layer_tree));
+    EXPECT_TRUE(result);
+    std::function<bool(LayerTree&)> no_discard = [](LayerTree&) {
+      return false;
+    };
+    rasterizer->Draw(pipeline, no_discard);
+    latch.Signal();
+  });
+  latch.Wait();
+}
+
+TEST(
+    RasterizerTest,
+    drawWithExternalViewEmbedderAndThreadMergerNotMerged_ExternalViewEmbedderSubmitFrameNotCalled) {
+  std::string test_name =
+      ::testing::UnitTest::GetInstance()->current_test_info()->name();
+  ThreadHost thread_host("io.flutter.test." + test_name + ".",
+                         ThreadHost::Type::Platform | ThreadHost::Type::GPU |
+                             ThreadHost::Type::IO | ThreadHost::Type::UI);
+  TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(),
+                           thread_host.raster_thread->GetTaskRunner(),
+                           thread_host.ui_thread->GetTaskRunner(),
+                           thread_host.io_thread->GetTaskRunner());
+  MockDelegate delegate;
+  EXPECT_CALL(delegate, GetTaskRunners())
+      .WillRepeatedly(ReturnRef(task_runners));
+  EXPECT_CALL(delegate, OnFrameRasterized(_));
+  auto rasterizer = std::make_unique<Rasterizer>(delegate);
+  auto surface = std::make_unique<MockSurface>();
+  MockExternalViewEmbedder external_view_embedder;
+  EXPECT_CALL(*surface, GetExternalViewEmbedder())
+      .WillRepeatedly(Return(&external_view_embedder));
+  EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging)
+      .WillRepeatedly(Return(true));
+  auto surface_frame = std::make_unique<SurfaceFrame>(
+      nullptr, true, [](const SurfaceFrame&, SkCanvas*) { return true; });
+  EXPECT_CALL(*surface, AcquireFrame(SkISize()))
+      .WillOnce(Return(ByMove(std::move(surface_frame))));
+
+  EXPECT_CALL(external_view_embedder, BeginFrame).Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(0);
+  EXPECT_CALL(external_view_embedder, EndFrame).Times(1);
+
   rasterizer->Setup(std::move(surface));
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
@@ -132,4 +195,52 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
   });
   latch.Wait();
 }
+
+TEST(
+    RasterizerTest,
+    drawWithExternalViewEmbedderAndThreadsMerged_ExternalViewEmbedderSubmitFrameCalled) {
+  std::string test_name =
+      ::testing::UnitTest::GetInstance()->current_test_info()->name();
+  ThreadHost thread_host("io.flutter.test." + test_name + ".",
+                         ThreadHost::Type::Platform | ThreadHost::Type::GPU |
+                             ThreadHost::Type::IO | ThreadHost::Type::UI);
+  fml::MessageLoop::EnsureInitializedForCurrentThread();
+  TaskRunners task_runners("test",
+                           fml::MessageLoop::GetCurrent().GetTaskRunner(),
+                           fml::MessageLoop::GetCurrent().GetTaskRunner(),
+                           thread_host.ui_thread->GetTaskRunner(),
+                           thread_host.io_thread->GetTaskRunner());
+
+  MockDelegate delegate;
+  EXPECT_CALL(delegate, GetTaskRunners())
+      .WillRepeatedly(ReturnRef(task_runners));
+  EXPECT_CALL(delegate, OnFrameRasterized(_));
+
+  auto rasterizer = std::make_unique<Rasterizer>(delegate);
+  auto surface = std::make_unique<MockSurface>();
+
+  MockExternalViewEmbedder external_view_embedder;
+  EXPECT_CALL(*surface, GetExternalViewEmbedder())
+      .WillRepeatedly(Return(&external_view_embedder));
+  auto surface_frame = std::make_unique<SurfaceFrame>(
+      nullptr, true, [](const SurfaceFrame&, SkCanvas*) { return true; });
+  EXPECT_CALL(*surface, AcquireFrame(SkISize()))
+      .WillOnce(Return(ByMove(std::move(surface_frame))));
+  EXPECT_CALL(external_view_embedder, SupportsDynamicThreadMerging)
+      .WillRepeatedly(Return(true));
+
+  EXPECT_CALL(external_view_embedder, BeginFrame).Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
+  EXPECT_CALL(external_view_embedder, EndFrame).Times(1);
+
+  rasterizer->Setup(std::move(surface));
+
+  auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+  auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
+                                                /*device_pixel_ratio=*/2.0f);
+  bool result = pipeline->Produce().Complete(std::move(layer_tree));
+  EXPECT_TRUE(result);
+  std::function<bool(LayerTree&)> no_discard = [](LayerTree&) { return false; };
+  rasterizer->Draw(pipeline, no_discard);
+}
 }  // namespace flutter

shell/common/shell_unittests.cc

@@ -1054,16 +1054,18 @@ TEST_F(ShellTest,
   auto end_frame_callback =
       [&](bool should_resubmit_frame,
           fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
-        external_view_embedder->UpdatePostPrerollResult(
-            PostPrerollResult::kSuccess);
+        if (should_resubmit_frame && !raster_thread_merger->IsMerged()) {
+          raster_thread_merger->MergeWithLease(10);
+          external_view_embedder->UpdatePostPrerollResult(
+              PostPrerollResult::kSuccess);
+        }
         end_frame_latch.Signal();
       };
   external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
       end_frame_callback, PostPrerollResult::kResubmitFrame, true);
 
   auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
                            false, external_view_embedder);
-
   PlatformViewNotifyCreated(shell.get());
 
   auto configuration = RunConfiguration::InferFromSettings(settings);
@@ -1074,12 +1076,16 @@ TEST_F(ShellTest,
 
   PumpOneFrame(shell.get());
   // `EndFrame` changed the post preroll result to `kSuccess`.
+  // First frame hasn't merged the threads yet, no
+  // `external_view_embedder->GetSubmittedFrameCount()` is called.
   end_frame_latch.Wait();
-  ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
+  ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
 
+  // This is the resubmitted frame, which threads are also merged.
   end_frame_latch.Wait();
-  ASSERT_EQ(2, external_view_embedder->GetSubmittedFrameCount());
+  ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
 
+  PlatformViewNotifyDestroyed(shell.get());
   DestroyShell(std::move(shell));
 }
 
@@ -2019,14 +2025,25 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
   SkISize expected_size = SkISize::Make(400, 200);
 
   fml::AutoResetWaitableEvent end_frame_latch;
+  std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
+  auto end_frame_callback =
+      [&](bool should_merge_thread,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        if (should_merge_thread) {
+          // TODO(cyanglaz): This test used external_view_embedder so we need to
+          // merge the threads here. However, the scenario it is testing is
+          // unrelated to platform views. We should consider to update this test
+          // so it doesn't require external_view_embedder.
+          // https://github.com/flutter/flutter/issues/69895
+          raster_thread_merger->MergeWithLease(10);
+          external_view_embedder->UpdatePostPrerollResult(
+              PostPrerollResult::kSuccess);
+        }
+        end_frame_latch.Signal();
+      };
 
-  auto end_frame_callback = [&](bool, fml::RefPtr<fml::RasterThreadMerger>) {
-    end_frame_latch.Signal();
-  };
-
-  std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder =
-      std::make_shared<ShellTestExternalViewEmbedder>(
-          std::move(end_frame_callback), PostPrerollResult::kSuccess, true);
+  external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
+      std::move(end_frame_callback), PostPrerollResult::kResubmitFrame, true);
 
   std::unique_ptr<Shell> shell = CreateShell(
       settings, GetTaskRunnersForFixture(), false, external_view_embedder);
@@ -2047,8 +2064,6 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
 
   RunEngine(shell.get(), std::move(configuration));
 
-  fml::WeakPtr<RuntimeDelegate> runtime_delegate = shell->GetEngine();
-
   PumpOneFrame(shell.get(), static_cast<double>(wrong_size.width()),
                static_cast<double>(wrong_size.height()));
 
@@ -2060,10 +2075,14 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
                static_cast<double>(expected_size.height()));
 
   end_frame_latch.Wait();
+  // Threads not merged.
+  ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
 
+  end_frame_latch.Wait();
   ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
   ASSERT_EQ(expected_size, external_view_embedder->GetLastSubmittedFrameSize());
 
+  PlatformViewNotifyDestroyed(shell.get());
   DestroyShell(std::move(shell));
 }
 

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

@@ -342,6 +342,9 @@
 
 void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack,
                                                    UIView* embedded_view) {
+  if (flutter_view_ == nullptr) {
+    return;
+  }
   FML_DCHECK(CATransform3DEqualToTransform(embedded_view.layer.transform, CATransform3DIdentity));
   ResetAnchor(embedded_view.layer);
   ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview;
@@ -398,7 +401,6 @@
 
 void FlutterPlatformViewsController::CompositeWithParams(int view_id,
                                                          const EmbeddedViewParams& params) {
-  FML_DCHECK(flutter_view_);
   CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height());
   UIView* touchInterceptor = touch_interceptors_[view_id].get();
   touchInterceptor.layer.transform = CATransform3DIdentity;
@@ -419,9 +421,8 @@
 }
 
 SkCanvas* FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) {
-  FML_DCHECK(flutter_view_);
-  // TODO(amirh): assert that this is running on the platform thread once we support the iOS
-  // embedded views thread configuration.
+  // Any UIKit related code has to run on main thread.
+  FML_DCHECK([[NSThread currentThread] isMainThread]);
 
   // Do nothing if the view doesn't need to be composited.
   if (views_to_recomposite_.count(view_id) == 0) {
@@ -469,12 +470,11 @@
 bool FlutterPlatformViewsController::SubmitFrame(GrDirectContext* gr_context,
                                                  std::shared_ptr<IOSContext> ios_context,
                                                  std::unique_ptr<SurfaceFrame> frame) {
-  FML_DCHECK(flutter_view_);
-
   // Any UIKit related code has to run on main thread.
-  // When on a non-main thread, we only allow the rest of the method to run if there is no
-  // Pending UIView operations.
-  FML_DCHECK([[NSThread currentThread] isMainThread] || !HasPlatformViewThisOrNextFrame());
+  FML_DCHECK([[NSThread currentThread] isMainThread]);
+  if (flutter_view_ == nullptr) {
+    return frame->Submit();
+  }
 
   DisposeViews();
 
@@ -558,8 +558,6 @@
   BringLayersIntoView(platform_view_layers);
   // Mark all layers as available, so they can be used in the next frame.
   layer_pool_->RecycleLayers();
-  // Reset the composition order, so next frame starts empty.
-  composition_order_.clear();
 
   did_submit &= frame->Submit();
 
@@ -599,7 +597,10 @@
 
 void FlutterPlatformViewsController::EndFrame(
     bool should_resubmit_frame,
-    fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {}
+    fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+  // Reset the composition order, so next frame starts empty.
+  composition_order_.clear();
+}
 
 std::shared_ptr<FlutterPlatformViewLayer> FlutterPlatformViewsController::GetLayer(
     GrDirectContext* gr_context,

shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm

@@ -602,6 +602,57 @@ - (void)testSetFlutterViewControllerAfterCreateCanStillDispatchTouchEvents {
   flutterPlatformViewsController->Reset();
 }
 
+- (void)testFlutterPlatformViewControllerSubmitFrameWithoutFlutterViewNotCrashing {
+  flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
+  auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+  auto surface_factory = flutter::IOSSurfaceFactory::Create(flutter::IOSRenderingAPI::kSoftware);
+  auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+      /*delegate=*/mock_delegate,
+      /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+      /*ios_surface_factory=*/surface_factory,
+      /*task_runners=*/runners);
+
+  auto flutterPlatformViewsController =
+      std::make_shared<flutter::FlutterPlatformViewsController>(surface_factory);
+  surface_factory->SetPlatformViewsController(flutterPlatformViewsController);
+
+  FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
+      [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
+  flutterPlatformViewsController->RegisterViewFactory(
+      factory, @"MockFlutterPlatformView",
+      FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
+  FlutterResult result = ^(id result) {
+  };
+  flutterPlatformViewsController->OnMethodCall(
+      [FlutterMethodCall
+          methodCallWithMethodName:@"create"
+                         arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}],
+      result);
+
+  XCTAssertNotNil(gMockPlatformView);
+
+  // Create embedded view params
+  flutter::MutatorsStack stack;
+  SkMatrix finalMatrix;
+
+  auto embeddedViewParams =
+      std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
+
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams));
+  flutterPlatformViewsController->CompositeEmbeddedView(2);
+  auto mock_surface = std::make_unique<flutter::SurfaceFrame>(
+      nullptr, true,
+      [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; });
+  flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface));
+
+  flutterPlatformViewsController->Reset();
+}
+
 - (int)alphaOfPoint:(CGPoint)point onView:(UIView*)view {
   unsigned char pixel[4] = {0};