Reland "remove surface dependance on external view embedder (#22468)"

ci/licenses_golden/licenses_flutter

@@ -667,7 +667,6 @@ FILE: ../../../flutter/shell/common/vsync_waiter_fallback.cc
 FILE: ../../../flutter/shell/common/vsync_waiter_fallback.h
 FILE: ../../../flutter/shell/common/vsync_waiters_test.cc
 FILE: ../../../flutter/shell/common/vsync_waiters_test.h
-FILE: ../../../flutter/shell/gpu/gpu_surface_delegate.h
 FILE: ../../../flutter/shell/gpu/gpu_surface_gl.cc
 FILE: ../../../flutter/shell/gpu/gpu_surface_gl.h
 FILE: ../../../flutter/shell/gpu/gpu_surface_gl_delegate.cc

flow/surface.cc

@@ -10,10 +10,6 @@ Surface::Surface() = default;
 
 Surface::~Surface() = default;
 
-flutter::ExternalViewEmbedder* Surface::GetExternalViewEmbedder() {
-  return nullptr;
-}
-
 std::unique_ptr<GLContextResult> Surface::MakeRenderContextCurrent() {
   return std::make_unique<GLContextDefaultResult>(true);
 }

flow/surface.h

@@ -29,8 +29,6 @@ class Surface {
 
   virtual GrDirectContext* GetContext() = 0;
 
-  virtual flutter::ExternalViewEmbedder* GetExternalViewEmbedder();
-
   virtual std::unique_ptr<GLContextResult> MakeRenderContextCurrent();
 
   virtual bool ClearRenderContext();

shell/common/platform_view.h

@@ -563,6 +563,8 @@ class PlatformView {
   ComputePlatformResolvedLocales(
       const std::vector<std::string>& supported_locale_data);
 
+  virtual std::shared_ptr<ExternalViewEmbedder> CreateExternalViewEmbedder();
+
  protected:
   PlatformView::Delegate& delegate_;
   const TaskRunners task_runners_;
@@ -575,8 +577,6 @@ class PlatformView {
   // GPU task runner.
   virtual std::unique_ptr<Surface> CreateRenderingSurface();
 
-  virtual std::shared_ptr<ExternalViewEmbedder> CreateExternalViewEmbedder();
-
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(PlatformView);
 };

shell/common/rasterizer.cc

@@ -76,8 +76,8 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
                              user_override_resource_cache_bytes_);
   }
   compositor_context_->OnGrContextCreated();
-  if (surface_->GetExternalViewEmbedder() &&
-      surface_->GetExternalViewEmbedder()->SupportsDynamicThreadMerging() &&
+  if (external_view_embedder_ &&
+      external_view_embedder_->SupportsDynamicThreadMerging() &&
       !raster_thread_merger_) {
     const auto platform_id =
         delegate_.GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId();
@@ -191,9 +191,9 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
   }
 
   // EndFrame should perform cleanups for the external_view_embedder.
-  if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) {
-    surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame,
-                                                  raster_thread_merger_);
+  if (surface_ && external_view_embedder_) {
+    external_view_embedder_->EndFrame(should_resubmit_frame,
+                                      raster_thread_merger_);
   }
 
   // Consume as many pipeline items as possible. But yield the event loop
@@ -423,14 +423,12 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
   // for instrumentation.
   compositor_context_->ui_time().SetLapTime(layer_tree.build_time());
 
-  auto* external_view_embedder = surface_->GetExternalViewEmbedder();
-
   SkCanvas* embedder_root_canvas = nullptr;
-  if (external_view_embedder != nullptr) {
-    external_view_embedder->BeginFrame(
+  if (external_view_embedder_) {
+    external_view_embedder_->BeginFrame(
         layer_tree.frame_size(), surface_->GetContext(),
         layer_tree.device_pixel_ratio(), raster_thread_merger_);
-    embedder_root_canvas = external_view_embedder->GetRootCanvas();
+    embedder_root_canvas = external_view_embedder_->GetRootCanvas();
   }
 
   // On Android, the external view embedder deletes surfaces in `BeginFrame`.
@@ -453,13 +451,13 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
       embedder_root_canvas ? embedder_root_canvas : frame->SkiaCanvas();
 
   auto compositor_frame = compositor_context_->AcquireFrame(
-      surface_->GetContext(),       // skia GrContext
-      root_surface_canvas,          // root surface canvas
-      external_view_embedder,       // external view embedder
-      root_surface_transformation,  // root surface transformation
-      true,                         // instrumentation enabled
-      frame->supports_readback(),   // surface supports pixel reads
-      raster_thread_merger_         // thread merger
+      surface_->GetContext(),         // skia GrContext
+      root_surface_canvas,            // root surface canvas
+      external_view_embedder_.get(),  // external view embedder
+      root_surface_transformation,    // root surface transformation
+      true,                           // instrumentation enabled
+      frame->supports_readback(),     // surface supports pixel reads
+      raster_thread_merger_           // thread merger
   );
 
   if (compositor_frame) {
@@ -468,11 +466,11 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
         raster_status == RasterStatus::kSkipAndRetry) {
       return raster_status;
     }
-    if (external_view_embedder != nullptr &&
+    if (external_view_embedder_ &&
         (!raster_thread_merger_ || raster_thread_merger_->IsMerged())) {
       FML_DCHECK(!frame->IsSubmitted());
-      external_view_embedder->SubmitFrame(surface_->GetContext(),
-                                          std::move(frame));
+      external_view_embedder_->SubmitFrame(surface_->GetContext(),
+                                           std::move(frame));
     } else {
       frame->Submit();
     }
@@ -653,6 +651,11 @@ void Rasterizer::SetNextFrameCallback(const fml::closure& callback) {
   next_frame_callback_ = callback;
 }
 
+void Rasterizer::SetExternalViewEmbedder(
+    const std::shared_ptr<ExternalViewEmbedder>& view_embedder) {
+  external_view_embedder_ = view_embedder;
+}
+
 void Rasterizer::FireNextFrameCallbackIfPresent() {
   if (!next_frame_callback_) {
     return;

shell/common/rasterizer.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <optional>
 
+#include "flow/embedded_views.h"
 #include "flutter/common/settings.h"
 #include "flutter/common/task_runners.h"
 #include "flutter/flow/compositor_context.h"
@@ -349,6 +350,16 @@ class Rasterizer final : public SnapshotDelegate {
   ///
   void SetNextFrameCallback(const fml::closure& callback);
 
+  //----------------------------------------------------------------------------
+  /// @brief Set the External View Embedder. This is done on shell
+  ///        initialization. This is non-null on platforms that support
+  ///        embedding externally composited views.
+  ///
+  /// @param[in] view_embedder The external view embedder object.
+  ///
+  void SetExternalViewEmbedder(
+      const std::shared_ptr<ExternalViewEmbedder>& view_embedder);
+
   //----------------------------------------------------------------------------
   /// @brief      Returns a pointer to the compositor context used by this
   ///             rasterizer. This pointer will never be `nullptr`.
@@ -437,6 +448,7 @@ class Rasterizer final : public SnapshotDelegate {
   std::optional<size_t> max_cache_bytes_;
   fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
   fml::TaskRunnerAffineWeakPtrFactory<Rasterizer> weak_factory_;
+  std::shared_ptr<ExternalViewEmbedder> external_view_embedder_;
 
   // |SnapshotDelegate|
   sk_sp<SkImage> MakeRasterSnapshot(sk_sp<SkPicture> picture,

shell/common/rasterizer_unittests.cc

@@ -113,25 +113,25 @@ TEST(RasterizerTest,
   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));
+  std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
+      std::make_shared<MockExternalViewEmbedder>();
+  rasterizer->SetExternalViewEmbedder(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,
+  EXPECT_CALL(*external_view_embedder,
               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, SubmitFrame).Times(1);
   EXPECT_CALL(
-      external_view_embedder,
+      *external_view_embedder,
       EndFrame(/*should_resubmit_frame=*/false,
                /*raster_thread_merger=*/fml::RefPtr<fml::RasterThreadMerger>(
                    nullptr)))
@@ -170,25 +170,25 @@ TEST(
   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)
+  std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
+      std::make_shared<MockExternalViewEmbedder>();
+  rasterizer->SetExternalViewEmbedder(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,
+  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(0);
-  EXPECT_CALL(external_view_embedder, EndFrame(/*should_resubmit_frame=*/false,
-                                               /*raster_thread_merger=*/_))
+  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));
@@ -229,26 +229,26 @@ TEST(
   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));
+  std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
+      std::make_shared<MockExternalViewEmbedder>();
+  rasterizer->SetExternalViewEmbedder(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)
+  EXPECT_CALL(*external_view_embedder, SupportsDynamicThreadMerging)
       .WillRepeatedly(Return(true));
 
-  EXPECT_CALL(external_view_embedder,
+  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=*/_))
+  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));
@@ -261,4 +261,40 @@ TEST(
   auto no_discard = [](LayerTree&) { return false; };
   rasterizer->Draw(pipeline, no_discard);
 }
+
+TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) {
+  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));
+  auto rasterizer = std::make_unique<Rasterizer>(delegate);
+
+  std::shared_ptr<MockExternalViewEmbedder> external_view_embedder =
+      std::make_shared<MockExternalViewEmbedder>();
+  rasterizer->SetExternalViewEmbedder(external_view_embedder);
+
+  EXPECT_CALL(
+      *external_view_embedder,
+      EndFrame(/*should_resubmit_frame=*/false,
+               /*raster_thread_merger=*/fml::RefPtr<fml::RasterThreadMerger>(
+                   nullptr)))
+      .Times(0);
+
+  fml::AutoResetWaitableEvent latch;
+  thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
+    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto no_discard = [](LayerTree&) { return false; };
+    rasterizer->Draw(pipeline, no_discard);
+    latch.Signal();
+  });
+  latch.Wait();
+}
 }  // namespace flutter

shell/common/shell.cc

@@ -547,6 +547,10 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,
   rasterizer_ = std::move(rasterizer);
   io_manager_ = std::move(io_manager);
 
+  // Set the external view embedder for the rasterizer.
+  auto view_embedder = platform_view_->CreateExternalViewEmbedder();
+  rasterizer_->SetExternalViewEmbedder(view_embedder);
+
   // The weak ptr must be generated in the platform thread which owns the unique
   // ptr.
   weak_engine_ = engine_->GetWeakPtr();

shell/common/shell_test_platform_view_gl.cc

@@ -79,10 +79,5 @@ ShellTestPlatformViewGL::GetGLProcResolver() const {
   };
 }
 
-// |GPUSurfaceGLDelegate|
-ExternalViewEmbedder* ShellTestPlatformViewGL::GetExternalViewEmbedder() {
-  return shell_test_external_view_embedder_.get();
-}
-
 }  // namespace testing
 }  // namespace flutter

shell/common/shell_test_platform_view_gl.h

@@ -64,9 +64,6 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView,
   // |GPUSurfaceGLDelegate|
   GLProcResolver GetGLProcResolver() const override;
 
-  // |GPUSurfaceGLDelegate|
-  ExternalViewEmbedder* GetExternalViewEmbedder() override;
-
   FML_DISALLOW_COPY_AND_ASSIGN(ShellTestPlatformViewGL);
 };
 

shell/common/shell_test_platform_view_vulkan.cc

@@ -198,10 +198,5 @@ SkMatrix ShellTestPlatformViewVulkan::OffScreenSurface::GetRootTransformation()
   return matrix;
 }
 
-flutter::ExternalViewEmbedder*
-ShellTestPlatformViewVulkan::OffScreenSurface::GetExternalViewEmbedder() {
-  return shell_test_external_view_embedder_.get();
-}
-
 }  // namespace testing
 }  // namespace flutter

shell/common/shell_test_platform_view_vulkan.h

@@ -47,8 +47,6 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView {
     // |Surface|
     GrDirectContext* GetContext() override;
 
-    flutter::ExternalViewEmbedder* GetExternalViewEmbedder() override;
-
    private:
     bool valid_;
     fml::RefPtr<vulkan::VulkanProcTable> vk_;

shell/gpu/BUILD.gn

@@ -16,7 +16,6 @@ gpu_common_deps = [
 
 source_set("gpu_surface_software") {
   sources = [
-    "gpu_surface_delegate.h",
     "gpu_surface_software.cc",
     "gpu_surface_software.h",
     "gpu_surface_software_delegate.cc",
@@ -28,7 +27,6 @@ source_set("gpu_surface_software") {
 
 source_set("gpu_surface_gl") {
   sources = [
-    "gpu_surface_delegate.h",
     "gpu_surface_gl.cc",
     "gpu_surface_gl.h",
     "gpu_surface_gl_delegate.cc",
@@ -40,7 +38,6 @@ source_set("gpu_surface_gl") {
 
 source_set("gpu_surface_vulkan") {
   sources = [
-    "gpu_surface_delegate.h",
     "gpu_surface_vulkan.cc",
     "gpu_surface_vulkan.h",
     "gpu_surface_vulkan_delegate.cc",
@@ -52,7 +49,6 @@ source_set("gpu_surface_vulkan") {
 
 source_set("gpu_surface_metal") {
   sources = [
-    "gpu_surface_delegate.h",
     "gpu_surface_metal.h",
     "gpu_surface_metal.mm",
   ]