Reland "Do not involve external_view_embedder in submit frame process if threads are not merged. #22275"

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 + ".",
@@ -108,14 +112,85 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
   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>(
+      /*surface=*/nullptr, /*supports_readback=*/true,
+      /*submit_callback=*/[](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)));
+              BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
+                         /*device_pixel_ratio=*/2.0,
+                         /*raster_thread_merger=*/
+                         fml::RefPtr<fml::RasterThreadMerger>(nullptr)))
+      .Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
+  EXPECT_CALL(
+      external_view_embedder,
+      EndFrame(/*should_resubmit_frame=*/false,
+               /*raster_thread_merger=*/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);
+    auto 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>(
+      /*surface=*/nullptr, /*supports_readback=*/true,
+      /*submit_callback=*/[](const SurfaceFrame&, SkCanvas*) { return true; });
+  EXPECT_CALL(*surface, AcquireFrame(SkISize()))
+      .WillOnce(Return(ByMove(std::move(surface_frame))));
+
   EXPECT_CALL(external_view_embedder,
-              EndFrame(false, fml::RefPtr<fml::RasterThreadMerger>(nullptr)));
+              BeginFrame(/*frame_size=*/SkISize(), /*context=*/nullptr,
+                         /*device_pixel_ratio=*/2.0,
+                         /*raster_thread_merger=*/_))
+      .Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(0);
+  EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
+                                               /*raster_thread_merger=*/_))
+      .Times(1);
+
   rasterizer->Setup(std::move(surface));
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
@@ -124,12 +199,66 @@ TEST(RasterizerTest, drawWithExternalViewEmbedder) {
                                                   /*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;
-    };
+    auto no_discard = [](LayerTree&) { return false; };
     rasterizer->Draw(pipeline, no_discard);
     latch.Signal();
   });
   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>(
+      /*surface=*/nullptr, /*supports_readback=*/true,
+      /*submit_callback=*/[](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(/*frame_size=*/SkISize(), /*context=*/nullptr,
+                         /*device_pixel_ratio=*/2.0,
+                         /*raster_thread_merger=*/_))
+      .Times(1);
+  EXPECT_CALL(external_view_embedder, SubmitFrame).Times(1);
+  EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
+                                               /*raster_thread_merger=*/_))
+      .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);
+  auto no_discard = [](LayerTree&) { return false; };
+  rasterizer->Draw(pipeline, no_discard);
+}
 }  // namespace flutter

shell/common/shell_unittests.cc

@@ -1051,20 +1051,25 @@ TEST_F(ShellTest,
   auto settings = CreateSettingsForFixture();
   fml::AutoResetWaitableEvent end_frame_latch;
   std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
-
+  fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
   auto end_frame_callback =
       [&](bool should_resubmit_frame,
           fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
-        external_view_embedder->UpdatePostPrerollResult(
-            PostPrerollResult::kSuccess);
+        if (!raster_thread_merger_ref) {
+          raster_thread_merger_ref = raster_thread_merger;
+        }
+        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,13 +1079,18 @@ TEST_F(ShellTest,
   ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
 
   PumpOneFrame(shell.get());
-  // `EndFrame` changed the post preroll result to `kSuccess`.
+  // `EndFrame` changed the post preroll result to `kSuccess` and merged the
+  // threads. During the frame, the threads are not merged, So no
+  // `external_view_embedder->GetSubmittedFrameCount()` is called.
   end_frame_latch.Wait();
-  ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
+  ASSERT_TRUE(raster_thread_merger_ref->IsMerged());
+  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));
 }
 
@@ -2020,14 +2030,29 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) {
   SkISize expected_size = SkISize::Make(400, 200);
 
   fml::AutoResetWaitableEvent end_frame_latch;
+  std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder;
+  fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_ref;
+  auto end_frame_callback =
+      [&](bool should_merge_thread,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        if (!raster_thread_merger_ref) {
+          raster_thread_merger_ref = 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);
@@ -2048,23 +2073,29 @@ 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()));
 
   end_frame_latch.Wait();
 
   ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());
 
+  // Threads will be merged at the end of this frame.
   PumpOneFrame(shell.get(), static_cast<double>(expected_size.width()),
                static_cast<double>(expected_size.height()));
 
   end_frame_latch.Wait();
+  // Even the threads are merged at the end of the frame,
+  // during the frame, the threads are not merged,
+  // So no `external_view_embedder->GetSubmittedFrameCount()` is called.
+  ASSERT_TRUE(raster_thread_merger_ref->IsMerged());
+  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,