Added test to assert the vulkan embedder threadsafe vkqueue usage

shell/platform/embedder/BUILD.gn

@@ -361,6 +361,10 @@ if (enable_unittests) {
     if (test_enable_metal) {
       sources += [ "tests/embedder_metal_unittests.mm" ]
     }
+
+    if (test_enable_vulkan) {
+      sources += [ "tests/embedder_vk_unittests.cc" ]
+    }
   }
 
   executable("embedder_a11y_unittests") {

shell/platform/embedder/embedder.h

@@ -757,6 +757,11 @@ typedef struct {
   /// The queue family index of the VkQueue supplied in the next field.
   uint32_t queue_family_index;
   /// VkQueue handle.
+  /// The queue should not be used without protection from a mutex to make sure
+  /// it is not used simultaneously with other threads. That mutex should match
+  /// the one injected via the |get_instance_proc_address_callback|.
+  /// There is a proposal to remove the need for the mutex at
+  /// https://github.com/flutter/flutter/issues/134573.
   FlutterVulkanQueueHandle queue;
   /// The number of instance extensions available for enumerating in the next
   /// field.
@@ -780,6 +785,12 @@ typedef struct {
   /// For example: VK_KHR_GET_MEMORY_REQUIREMENTS_2_EXTENSION_NAME
   const char** enabled_device_extensions;
   /// The callback invoked when resolving Vulkan function pointers.
+  /// At a bare minimum this should be used to swap out any calls that operate
+  /// on vkQueue's for threadsafe variants that obtain locks for their duration.
+  /// The functions to swap out are "vkQueueSubmit" and "vkQueueWaitIdle".  An
+  /// example of how to do that can be found in the test
+  /// "EmbedderTest.CanSwapOutVulkanCalls" unit-test in
+  /// //shell/platform/embedder/tests/embedder_vk_unittests.cc.
   FlutterVulkanInstanceProcAddressCallback get_instance_proc_address_callback;
   /// The callback invoked when the engine requests a VkImage from the embedder
   /// for rendering the next frame.

shell/platform/embedder/tests/embedder_config_builder.cc

@@ -205,10 +205,18 @@ void EmbedderConfigBuilder::SetMetalRendererConfig(SkISize surface_size) {
 #endif
 }
 
-void EmbedderConfigBuilder::SetVulkanRendererConfig(SkISize surface_size) {
+void EmbedderConfigBuilder::SetVulkanRendererConfig(
+    SkISize surface_size,
+    std::optional<FlutterVulkanInstanceProcAddressCallback>
+        instance_proc_address_callback) {
 #ifdef SHELL_ENABLE_VULKAN
   renderer_config_.type = FlutterRendererType::kVulkan;
-  renderer_config_.vulkan = vulkan_renderer_config_;
+  FlutterVulkanRendererConfig vulkan_renderer_config = vulkan_renderer_config_;
+  if (instance_proc_address_callback.has_value()) {
+    vulkan_renderer_config.get_instance_proc_address_callback =
+        instance_proc_address_callback.value();
+  }
+  renderer_config_.vulkan = vulkan_renderer_config;
   context_.SetupSurface(surface_size);
 #endif
 }
@@ -519,13 +527,7 @@ void EmbedderConfigBuilder::InitializeVulkanRendererConfig() {
       static_cast<EmbedderTestContextVulkan&>(context_)
           .vulkan_context_->device_->GetQueueHandle();
   vulkan_renderer_config_.get_instance_proc_address_callback =
-      [](void* context, FlutterVulkanInstanceHandle instance,
-         const char* name) -> void* {
-    auto proc_addr = reinterpret_cast<EmbedderTestContextVulkan*>(context)
-                         ->vulkan_context_->vk_->GetInstanceProcAddr(
-                             reinterpret_cast<VkInstance>(instance), name);
-    return reinterpret_cast<void*>(proc_addr);
-  };
+      EmbedderTestContextVulkan::InstanceProcAddr;
   vulkan_renderer_config_.get_next_image_callback =
       [](void* context,
          const FlutterFrameInfo* frame_info) -> FlutterVulkanImage {

shell/platform/embedder/tests/embedder_config_builder.h

@@ -53,7 +53,10 @@ class EmbedderConfigBuilder {
 
   void SetMetalRendererConfig(SkISize surface_size);
 
-  void SetVulkanRendererConfig(SkISize surface_size);
+  void SetVulkanRendererConfig(
+      SkISize surface_size,
+      std::optional<FlutterVulkanInstanceProcAddressCallback>
+          instance_proc_address_callback = {});
 
   // Used to explicitly set an `open_gl.fbo_callback`. Using this method will
   // cause your test to fail since the ctor for this class sets

shell/platform/embedder/tests/embedder_test_context_vulkan.cc

@@ -58,5 +58,15 @@ void EmbedderTestContextVulkan::SetupCompositor() {
       surface_size_, vulkan_context_->GetGrDirectContext());
 }
 
+void* EmbedderTestContextVulkan::InstanceProcAddr(
+    void* user_data,
+    FlutterVulkanInstanceHandle instance,
+    const char* name) {
+  auto proc_addr = reinterpret_cast<EmbedderTestContextVulkan*>(user_data)
+                       ->vulkan_context_->vk_->GetInstanceProcAddr(
+                           reinterpret_cast<VkInstance>(instance), name);
+  return reinterpret_cast<void*>(proc_addr);
+}
+
 }  // namespace testing
 }  // namespace flutter

shell/platform/embedder/tests/embedder_test_context_vulkan.h

@@ -33,6 +33,10 @@ class EmbedderTestContextVulkan : public EmbedderTestContext {
 
   bool PresentImage(VkImage image);
 
+  static void* InstanceProcAddr(void* user_data,
+                                FlutterVulkanInstanceHandle instance,
+                                const char* name);
+
  private:
   std::unique_ptr<TestVulkanSurface> surface_;
 

shell/platform/embedder/tests/embedder_vk_unittests.cc

@@ -0,0 +1,128 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// 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 <cstring>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "embedder.h"
+#include "embedder_engine.h"
+#include "flutter/fml/synchronization/count_down_latch.h"
+#include "flutter/shell/platform/embedder/tests/embedder_config_builder.h"
+#include "flutter/shell/platform/embedder/tests/embedder_test.h"
+#include "flutter/shell/platform/embedder/tests/embedder_test_context_vulkan.h"
+#include "flutter/shell/platform/embedder/tests/embedder_unittests_util.h"
+#include "flutter/testing/testing.h"
+
+// CREATE_NATIVE_ENTRY is leaky by design
+// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
+
+namespace flutter {
+namespace testing {
+
+using EmbedderTest = testing::EmbedderTest;
+
+////////////////////////////////////////////////////////////////////////////////
+// Notice: Other Vulkan unit tests exist in embedder_gl_unittests.cc.
+//         See https://github.com/flutter/flutter/issues/134322
+////////////////////////////////////////////////////////////////////////////////
+
+namespace {
+
+struct VulkanProcInfo {
+  decltype(vkGetInstanceProcAddr)* get_instance_proc_addr = nullptr;
+  decltype(vkGetDeviceProcAddr)* get_device_proc_addr = nullptr;
+  decltype(vkQueueSubmit)* queue_submit_proc_addr = nullptr;
+  bool did_call_queue_submit = false;
+};
+
+static_assert(std::is_trivially_destructible_v<VulkanProcInfo>);
+
+VulkanProcInfo g_vulkan_proc_info;
+
+VkResult QueueSubmit(VkQueue queue,
+                     uint32_t submitCount,
+                     const VkSubmitInfo* pSubmits,
+                     VkFence fence) {
+  FML_DCHECK(g_vulkan_proc_info.queue_submit_proc_addr != nullptr);
+  g_vulkan_proc_info.did_call_queue_submit = true;
+  return g_vulkan_proc_info.queue_submit_proc_addr(queue, submitCount, pSubmits,
+                                                   fence);
+}
+
+template <size_t N>
+int StrcmpFixed(const char* str1, const char (&str2)[N]) {
+  return strncmp(str1, str2, N - 1);
+}
+
+PFN_vkVoidFunction GetDeviceProcAddr(VkDevice device, const char* pName) {
+  FML_DCHECK(g_vulkan_proc_info.get_device_proc_addr != nullptr);
+  if (StrcmpFixed(pName, "vkQueueSubmit") == 0) {
+    g_vulkan_proc_info.queue_submit_proc_addr =
+        reinterpret_cast<decltype(vkQueueSubmit)*>(
+            g_vulkan_proc_info.get_device_proc_addr(device, pName));
+    return reinterpret_cast<PFN_vkVoidFunction>(QueueSubmit);
+  }
+  return g_vulkan_proc_info.get_device_proc_addr(device, pName);
+}
+
+PFN_vkVoidFunction GetInstanceProcAddr(VkInstance instance, const char* pName) {
+  FML_DCHECK(g_vulkan_proc_info.get_instance_proc_addr != nullptr);
+  if (StrcmpFixed(pName, "vkGetDeviceProcAddr") == 0) {
+    g_vulkan_proc_info.get_device_proc_addr =
+        reinterpret_cast<decltype(vkGetDeviceProcAddr)*>(
+            g_vulkan_proc_info.get_instance_proc_addr(instance, pName));
+    return reinterpret_cast<PFN_vkVoidFunction>(GetDeviceProcAddr);
+  }
+  return g_vulkan_proc_info.get_instance_proc_addr(instance, pName);
+}
+
+template <typename T, typename U>
+struct CheckSameSignature : std::false_type {};
+
+template <typename Ret, typename... Args>
+struct CheckSameSignature<Ret(Args...), Ret(Args...)> : std::true_type {};
+
+static_assert(CheckSameSignature<decltype(GetInstanceProcAddr),
+                                 decltype(vkGetInstanceProcAddr)>::value);
+static_assert(CheckSameSignature<decltype(GetDeviceProcAddr),
+                                 decltype(vkGetDeviceProcAddr)>::value);
+static_assert(
+    CheckSameSignature<decltype(QueueSubmit), decltype(vkQueueSubmit)>::value);
+}  // namespace
+
+TEST_F(EmbedderTest, CanSwapOutVulkanCalls) {
+  auto& context = GetEmbedderContext(EmbedderTestContextType::kVulkanContext);
+  fml::AutoResetWaitableEvent latch;
+  context.AddIsolateCreateCallback([&latch]() { latch.Signal(); });
+  EmbedderConfigBuilder builder(context);
+  builder.SetVulkanRendererConfig(
+      SkISize::Make(1024, 1024),
+      [](void* user_data, FlutterVulkanInstanceHandle instance,
+         const char* name) -> void* {
+        if (StrcmpFixed(name, "vkGetInstanceProcAddr") == 0) {
+          g_vulkan_proc_info.get_instance_proc_addr =
+              reinterpret_cast<decltype(vkGetInstanceProcAddr)*>(
+                  EmbedderTestContextVulkan::InstanceProcAddr(user_data,
+                                                              instance, name));
+          return reinterpret_cast<void*>(GetInstanceProcAddr);
+        }
+        return EmbedderTestContextVulkan::InstanceProcAddr(user_data, instance,
+                                                           name);
+      });
+  auto engine = builder.LaunchEngine();
+  ASSERT_TRUE(engine.is_valid());
+  // Wait for the root isolate to launch.
+  latch.Wait();
+  engine.reset();
+  EXPECT_TRUE(g_vulkan_proc_info.did_call_queue_submit);
+}
+
+}  // namespace testing
+}  // namespace flutter
+
+// NOLINTEND(clang-analyzer-core.StackAddressEscape)