EmbedderTest: Extract backend-specific user_data (flutter/engine#56642)

cbracken · web-flow · commit 67608588967b · 2024-11-18T18:38:27.000Z
Replace the top-level public EmbedderBackingStoreProducer::UserData struct with backend-specific UserData classes, and make them internal to those backends. UserData internals shouldn't be visible to/manipulated by unit tests and compositors, but instead constrained to the backing store producer itself. Issue: flutter#158998 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer.h
@@ -21,29 +21,6 @@ namespace flutter::testing {
 
 class EmbedderTestBackingStoreProducer {
  public:
-  struct UserData {
-    UserData() : surface(nullptr), image(nullptr){};
-
-    explicit UserData(sk_sp<SkSurface> surface)
-        : surface(std::move(surface)), image(nullptr){};
-
-    UserData(sk_sp<SkSurface> surface, FlutterVulkanImage* vk_image)
-        : surface(std::move(surface)), image(vk_image){};
-
-    sk_sp<SkSurface> surface;
-    FlutterVulkanImage* image;
-#ifdef SHELL_ENABLE_GL
-    UserData(sk_sp<SkSurface> surface,
-             FlutterVulkanImage* vk_image,
-             std::unique_ptr<TestGLOnscreenOnlySurface> gl_surface)
-        : surface(std::move(surface)),
-          image(vk_image),
-          gl_surface(std::move(gl_surface)){};
-
-    std::unique_ptr<TestGLOnscreenOnlySurface> gl_surface;
-#endif
-  };
-
   enum class RenderTargetType {
     kSoftwareBuffer,
     kSoftwareBuffer2,
@@ -61,6 +38,12 @@ class EmbedderTestBackingStoreProducer {
   virtual bool Create(const FlutterBackingStoreConfig* config,
                       FlutterBackingStore* backing_store_out) = 0;
 
+  virtual sk_sp<SkSurface> GetSurface(
+      const FlutterBackingStore* backing_store) const = 0;
+
+  virtual sk_sp<SkImage> MakeImageSnapshot(
+      const FlutterBackingStore* backing_store) const = 0;
+
  protected:
   sk_sp<GrDirectContext> context_;
   RenderTargetType type_;
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_gl.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_gl.cc
@@ -14,6 +14,13 @@
 
 namespace flutter::testing {
 
+namespace {
+struct UserData {
+  sk_sp<SkSurface> surface;
+  std::unique_ptr<TestGLOnscreenOnlySurface> gl_surface;
+};
+}  // namespace
+
 EmbedderTestBackingStoreProducerGL::EmbedderTestBackingStoreProducerGL(
     sk_sp<GrDirectContext> context,
     RenderTargetType type,
@@ -39,6 +46,30 @@ bool EmbedderTestBackingStoreProducerGL::Create(
   };
 }
 
+sk_sp<SkSurface> EmbedderTestBackingStoreProducerGL::GetSurface(
+    const FlutterBackingStore* backing_store) const {
+  UserData* user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  return user_data->surface;
+}
+
+sk_sp<SkImage> EmbedderTestBackingStoreProducerGL::MakeImageSnapshot(
+    const FlutterBackingStore* backing_store) const {
+  UserData* user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  if (user_data->gl_surface != nullptr) {
+    // This backing store is an OpenGL Surface.
+    // We need to make it current so we can snapshot it.
+    user_data->gl_surface->MakeCurrent();
+
+    // GetRasterSurfaceSnapshot() does two
+    // gl_surface->makeImageSnapshot()'s. Doing a single
+    // ->makeImageSnapshot() will not work.
+    return user_data->gl_surface->GetRasterSurfaceSnapshot();
+  }
+
+  // Otherwise, it's a GL Texture or FrameBuffer.
+  return user_data->surface->makeImageSnapshot();
+}
+
 bool EmbedderTestBackingStoreProducerGL::CreateFramebuffer(
     const FlutterBackingStoreConfig* config,
     FlutterBackingStore* backing_store_out) {
@@ -75,8 +106,7 @@ bool EmbedderTestBackingStoreProducerGL::CreateFramebuffer(
     return false;
   }
 
-  auto user_data = new UserData(surface);
-
+  auto user_data = new UserData{.surface = surface};
   backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
   backing_store_out->user_data = user_data;
   backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;
@@ -124,8 +154,7 @@ bool EmbedderTestBackingStoreProducerGL::CreateTexture(
     return false;
   }
 
-  auto user_data = new UserData(surface);
-
+  auto user_data = new UserData{.surface = surface};
   backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
   backing_store_out->user_data = user_data;
   backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeTexture;
@@ -165,8 +194,10 @@ bool EmbedderTestBackingStoreProducerGL::CreateSurface(
 
   auto sk_surface = surface->GetOnscreenSurface();
 
-  auto user_data = new UserData(sk_surface, nullptr, std::move(surface));
-
+  auto user_data = new UserData{
+      .surface = sk_surface,
+      .gl_surface = std::move(surface),
+  };
   backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
   backing_store_out->user_data = user_data;
   backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeSurface;
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_gl.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_gl.h
@@ -23,8 +23,14 @@ class EmbedderTestBackingStoreProducerGL
 
   virtual ~EmbedderTestBackingStoreProducerGL();
 
-  virtual bool Create(const FlutterBackingStoreConfig* config,
-                      FlutterBackingStore* backing_store_out);
+  bool Create(const FlutterBackingStoreConfig* config,
+              FlutterBackingStore* backing_store_out) override;
+
+  sk_sp<SkSurface> GetSurface(
+      const FlutterBackingStore* backing_store) const override;
+
+  sk_sp<SkImage> MakeImageSnapshot(
+      const FlutterBackingStore* backing_store) const override;
 
  private:
   bool CreateFramebuffer(const FlutterBackingStoreConfig* config,
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_metal.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_metal.h
@@ -18,8 +18,14 @@ class EmbedderTestBackingStoreProducerMetal
 
   virtual ~EmbedderTestBackingStoreProducerMetal();
 
-  virtual bool Create(const FlutterBackingStoreConfig* config,
-                      FlutterBackingStore* backing_store_out);
+  bool Create(const FlutterBackingStoreConfig* config,
+              FlutterBackingStore* backing_store_out) override;
+
+  sk_sp<SkSurface> GetSurface(
+      const FlutterBackingStore* backing_store) const override;
+
+  sk_sp<SkImage> MakeImageSnapshot(
+      const FlutterBackingStore* backing_store) const override;
 
  private:
   std::unique_ptr<TestMetalContext> test_metal_context_;
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_metal.mm b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_metal.mm
@@ -4,6 +4,8 @@
 
 #include "flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_metal.h"
 
+#include <exception>
+
 #include "flutter/fml/logging.h"
 #include "third_party/skia/include/core/SkColorSpace.h"
 #include "third_party/skia/include/gpu/ganesh/GrBackendSurface.h"
@@ -56,4 +58,16 @@
   return true;
 }
 
+sk_sp<SkSurface> EmbedderTestBackingStoreProducerMetal::GetSurface(
+    const FlutterBackingStore* backing_store) const {
+  FML_LOG(FATAL) << "Unimplemented.";
+  std::terminate();
+}
+
+sk_sp<SkImage> EmbedderTestBackingStoreProducerMetal::MakeImageSnapshot(
+    const FlutterBackingStore* backing_store) const {
+  FML_LOG(FATAL) << "Unimplemented.";
+  std::terminate();
+}
+
 }  // namespace flutter::testing
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_software.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_software.cc
@@ -12,6 +12,12 @@
 
 namespace flutter::testing {
 
+namespace {
+struct UserData {
+  sk_sp<SkSurface> surface;
+};
+}  // namespace
+
 EmbedderTestBackingStoreProducerSoftware::
     EmbedderTestBackingStoreProducerSoftware(
         sk_sp<GrDirectContext> context,
@@ -46,6 +52,18 @@ bool EmbedderTestBackingStoreProducerSoftware::Create(
   }
 }
 
+sk_sp<SkSurface> EmbedderTestBackingStoreProducerSoftware::GetSurface(
+    const FlutterBackingStore* backing_store) const {
+  UserData* user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  return user_data->surface;
+}
+
+sk_sp<SkImage> EmbedderTestBackingStoreProducerSoftware::MakeImageSnapshot(
+    const FlutterBackingStore* backing_store) const {
+  auto user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  return user_data->surface->makeImageSnapshot();
+}
+
 bool EmbedderTestBackingStoreProducerSoftware::CreateSoftware(
     const FlutterBackingStoreConfig* config,
     FlutterBackingStore* backing_store_out) {
@@ -64,8 +82,7 @@ bool EmbedderTestBackingStoreProducerSoftware::CreateSoftware(
     return false;
   }
 
-  auto user_data = new UserData(surface);
-
+  auto user_data = new UserData{.surface = surface};
   backing_store_out->type = kFlutterBackingStoreTypeSoftware;
   backing_store_out->user_data = user_data;
   backing_store_out->software.allocation = pixmap.addr();
@@ -101,8 +118,7 @@ bool EmbedderTestBackingStoreProducerSoftware::CreateSoftware2(
     return false;
   }
 
-  auto user_data = new UserData(surface);
-
+  auto user_data = new UserData{.surface = surface};
   backing_store_out->type = kFlutterBackingStoreTypeSoftware2;
   backing_store_out->user_data = user_data;
   backing_store_out->software2.struct_size =
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_software.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_software.h
@@ -20,8 +20,14 @@ class EmbedderTestBackingStoreProducerSoftware
 
   virtual ~EmbedderTestBackingStoreProducerSoftware();
 
-  virtual bool Create(const FlutterBackingStoreConfig* config,
-                      FlutterBackingStore* backing_store_out);
+  bool Create(const FlutterBackingStoreConfig* config,
+              FlutterBackingStore* backing_store_out) override;
+
+  sk_sp<SkSurface> GetSurface(
+      const FlutterBackingStore* backing_store) const override;
+
+  sk_sp<SkImage> MakeImageSnapshot(
+      const FlutterBackingStore* backing_store) const override;
 
  private:
   bool CreateSoftware(const FlutterBackingStoreConfig* config,
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_vulkan.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_vulkan.cc
@@ -13,6 +13,13 @@
 
 namespace flutter::testing {
 
+namespace {
+struct UserData {
+  sk_sp<SkSurface> surface;
+  FlutterVulkanImage* image;
+};
+}  // namespace
+
 EmbedderTestBackingStoreProducerVulkan::EmbedderTestBackingStoreProducerVulkan(
     sk_sp<GrDirectContext> context,
     RenderTargetType type)
@@ -83,7 +90,7 @@ bool EmbedderTestBackingStoreProducerVulkan::Create(
 
   // Collect all allocated resources in the destruction_callback.
   {
-    auto user_data = new UserData(surface, image);
+    auto user_data = new UserData{.surface = surface, .image = image};
     backing_store_out->user_data = user_data;
     backing_store_out->vulkan.user_data = user_data;
     backing_store_out->vulkan.destruction_callback = [](void* user_data) {
@@ -96,4 +103,16 @@ bool EmbedderTestBackingStoreProducerVulkan::Create(
   return true;
 }
 
+sk_sp<SkSurface> EmbedderTestBackingStoreProducerVulkan::GetSurface(
+    const FlutterBackingStore* backing_store) const {
+  UserData* user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  return user_data->surface;
+}
+
+sk_sp<SkImage> EmbedderTestBackingStoreProducerVulkan::MakeImageSnapshot(
+    const FlutterBackingStore* backing_store) const {
+  UserData* user_data = reinterpret_cast<UserData*>(backing_store->user_data);
+  return user_data->surface->makeImageSnapshot();
+}
+
 }  // namespace flutter::testing
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_vulkan.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_backingstore_producer_vulkan.h
@@ -19,8 +19,14 @@ class EmbedderTestBackingStoreProducerVulkan
 
   virtual ~EmbedderTestBackingStoreProducerVulkan();
 
-  virtual bool Create(const FlutterBackingStoreConfig* config,
-                      FlutterBackingStore* backing_store_out);
+  bool Create(const FlutterBackingStoreConfig* config,
+              FlutterBackingStore* backing_store_out) override;
+
+  sk_sp<SkSurface> GetSurface(
+      const FlutterBackingStore* backing_store) const override;
+
+  sk_sp<SkImage> MakeImageSnapshot(
+      const FlutterBackingStore* backing_store) const override;
 
  private:
   fml::RefPtr<TestVulkanContext> test_vulkan_context_;
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor.cc
@@ -51,6 +51,11 @@ bool EmbedderTestCompositor::CollectBackingStore(
   return true;
 }
 
+sk_sp<SkSurface> EmbedderTestCompositor::GetSurface(
+    const FlutterBackingStore* backing_store) const {
+  return backingstore_producer_->GetSurface(backing_store);
+}
+
 sk_sp<SkImage> EmbedderTestCompositor::GetLastComposition() {
   return last_composition_;
 }
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor.h b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor.h
@@ -45,6 +45,8 @@ class EmbedderTestCompositor {
   void SetPlatformViewRendererCallback(
       const PlatformViewRendererCallback& callback);
 
+  sk_sp<SkSurface> GetSurface(const FlutterBackingStore* backing_store) const;
+
   //----------------------------------------------------------------------------
   /// @brief      Allows tests to install a callback to notify them when the
   ///             entire render tree has been finalized so they can run their
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_gl.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_gl.cc
@@ -89,24 +89,8 @@ bool EmbedderTestCompositorGL::UpdateOffscrenComposition(
 
     switch (layer->type) {
       case kFlutterLayerContentTypeBackingStore: {
-        auto gl_user_data =
-            reinterpret_cast<EmbedderTestBackingStoreProducer::UserData*>(
-                layer->backing_store->user_data);
-
-        if (gl_user_data->gl_surface != nullptr) {
-          // This backing store is a OpenGL Surface.
-          // We need to make it current so we can snapshot it.
-
-          gl_user_data->gl_surface->MakeCurrent();
-
-          // GetRasterSurfaceSnapshot() does two
-          // gl_surface->makeImageSnapshot()'s. Doing a single
-          // ->makeImageSnapshot() will not work.
-          layer_image = gl_user_data->gl_surface->GetRasterSurfaceSnapshot();
-        } else {
-          layer_image = gl_user_data->surface->makeImageSnapshot();
-        }
-
+        layer_image =
+            backingstore_producer_->MakeImageSnapshot(layer->backing_store);
         // We don't clear the current surface here because we need the
         // EGL context to be current for surface->makeImageSnapshot() below.
         break;
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_software.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_software.cc
@@ -68,13 +68,11 @@ bool EmbedderTestCompositorSoftware::UpdateOffscrenComposition(
     SkIPoint canvas_offset = SkIPoint::Make(0, 0);
 
     switch (layer->type) {
-      case kFlutterLayerContentTypeBackingStore:
+      case kFlutterLayerContentTypeBackingStore: {
         layer_image =
-            reinterpret_cast<EmbedderTestBackingStoreProducer::UserData*>(
-                layer->backing_store->user_data)
-                ->surface->makeImageSnapshot();
-
+            backingstore_producer_->MakeImageSnapshot(layer->backing_store);
         break;
+      }
       case kFlutterLayerContentTypePlatformView:
         layer_image = platform_view_renderer_callback_
                           ? platform_view_renderer_callback_(*layer, nullptr)
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_test_compositor_vulkan.cc
@@ -82,9 +82,7 @@ bool EmbedderTestCompositorVulkan::UpdateOffscrenComposition(
     switch (layer->type) {
       case kFlutterLayerContentTypeBackingStore:
         layer_image =
-            reinterpret_cast<EmbedderTestBackingStoreProducer::UserData*>(
-                layer->backing_store->user_data)
-                ->surface->makeImageSnapshot();
+            backingstore_producer_->MakeImageSnapshot(layer->backing_store);
         break;
       case kFlutterLayerContentTypePlatformView:
         layer_image =
diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,11 @@ bool EmbedderTestCompositor::CollectBackingStore(`
`51`	`51`	`return true;`
`52`	`52`	`}`
`53`	`53`
	`54`	`+sk_sp<SkSurface> EmbedderTestCompositor::GetSurface(`
	`55`	`+ const FlutterBackingStore* backing_store) const {`
	`56`	`+ return backingstore_producer_->GetSurface(backing_store);`
	`57`	`+}`
	`58`	`+`
`54`	`59`	`sk_sp<SkImage> EmbedderTestCompositor::GetLastComposition() {`
`55`	`60`	`return last_composition_;`
`56`	`61`	`}`