Make SwapChainStateVk::createSwapChainCi returns a value object

... which stores all the data on the stack, so that in a crash dump,
VkSwapchainCreateInfoKHR will be guaranteed to be captured.

Bug: b/222118078
Test: run the emulator
Change-Id: Ib63f1abda5c57894bc3edc238f0d3f870b1eb47b

5 files changed, 129 insertions, 56 deletions

diff --git a/stream-servers/DisplayVk.cpp b/stream-servers/DisplayVk.cpp
index cc9a8522..fb6cea17 100644
--- a/stream-servers/DisplayVk.cpp
+++ b/stream-servers/DisplayVk.cpp
@@ -155,15 +155,20 @@ void DisplayVk::bindToSurface(VkSurfaceKHR surface, uint32_t width, uint32_t hei
     auto swapChainCi = SwapChainStateVk::createSwapChainCi(
         m_vk, surface, m_vkPhysicalDevice, width, height,
         {m_swapChainQueueFamilyIndex, m_compositorQueueFamilyIndex});
+    if (!swapChainCi) {
+        GFXSTREAM_ABORT(FatalError(ABORT_REASON_OTHER))
+            << "Failed to create VkSwapchainCreateInfoKHR.";
+    }
     VkFormatProperties formatProps;
-    m_vk.vkGetPhysicalDeviceFormatProperties(m_vkPhysicalDevice, swapChainCi->imageFormat,
-                                             &formatProps);
+    m_vk.vkGetPhysicalDeviceFormatProperties(m_vkPhysicalDevice,
+                                             swapChainCi->mCreateInfo.imageFormat, &formatProps);
     if (!(formatProps.optimalTilingFeatures & VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT)) {
         GFXSTREAM_ABORT(FatalError(ABORT_REASON_OTHER))
             << "DisplayVk: The image format chosen for present VkImage can't be used as the color "
                "attachment, and therefore can't be used as the render target of CompositorVk.";
     }
-    m_swapChainStateVk = std::make_unique<SwapChainStateVk>(m_vk, m_vkDevice, *swapChainCi);
+    m_swapChainStateVk =
+        std::make_unique<SwapChainStateVk>(m_vk, m_vkDevice, swapChainCi->mCreateInfo);
     m_compositorVk = CompositorVk::create(
         m_vk, m_vkDevice, m_vkPhysicalDevice, m_compositorVkQueue, m_compositorVkQueueLock,
         k_compositorVkRenderTargetFormat, VK_IMAGE_LAYOUT_UNDEFINED,
diff --git a/stream-servers/SwapChainStateVk.cpp b/stream-servers/SwapChainStateVk.cpp
index a0364b9e..2f5e7890 100644
--- a/stream-servers/SwapChainStateVk.cpp
+++ b/stream-servers/SwapChainStateVk.cpp
@@ -3,16 +3,75 @@
 #include <cinttypes>
 #include <unordered_set>
 
+#include "host-common/GfxstreamFatalError.h"
 #include "host-common/logging.h"
 #include "vulkan/vk_enum_string_helper.h"
 #include "vulkan/vk_util.h"
 
+using emugl::ABORT_REASON_OTHER;
+using emugl::FatalError;
+
 #define SWAPCHAINSTATE_VK_ERROR(fmt, ...)                                                     \
     do {                                                                                      \
         fprintf(stderr, "%s(%s:%d): " fmt "\n", __func__, __FILE__, __LINE__, ##__VA_ARGS__); \
         fflush(stderr);                                                                       \
     } while (0)
 
+namespace {
+
+void swap(SwapchainCreateInfoWrapper& a, SwapchainCreateInfoWrapper& b) {
+    std::swap(a.mQueueFamilyIndices, b.mQueueFamilyIndices);
+    std::swap(a.mCreateInfo, b.mCreateInfo);
+    // The C++ spec guarantees that after std::swap is called, all iterators and references of the
+    // container remain valid, and the past-the-end iterator is invalidated. Therefore, no need to
+    // reset the VkSwapchainCreateInfoKHR::pQueueFamilyIndices.
+}
+
+}  // namespace
+
+SwapchainCreateInfoWrapper::SwapchainCreateInfoWrapper(const VkSwapchainCreateInfoKHR& createInfo)
+    : mCreateInfo(createInfo) {
+    if (createInfo.pNext) {
+        GFXSTREAM_ABORT(FatalError(ABORT_REASON_OTHER))
+            << "VkSwapchainCreateInfoKHR with pNext in the chain is not supported.";
+    }
+
+    if (createInfo.pQueueFamilyIndices && (createInfo.queueFamilyIndexCount > 0)) {
+        setQueueFamilyIndices(std::vector<uint32_t>(
+            createInfo.pQueueFamilyIndices,
+            createInfo.pQueueFamilyIndices + createInfo.queueFamilyIndexCount));
+    } else {
+        setQueueFamilyIndices({});
+    }
+}
+
+SwapchainCreateInfoWrapper::SwapchainCreateInfoWrapper(const SwapchainCreateInfoWrapper& other)
+    : mCreateInfo(other.mCreateInfo) {
+    if (other.mCreateInfo.pNext) {
+        GFXSTREAM_ABORT(FatalError(ABORT_REASON_OTHER))
+            << "VkSwapchainCreateInfoKHR with pNext in the chain is not supported.";
+    }
+    setQueueFamilyIndices(other.mQueueFamilyIndices);
+}
+
+SwapchainCreateInfoWrapper& SwapchainCreateInfoWrapper::operator=(
+    const SwapchainCreateInfoWrapper& other) {
+    SwapchainCreateInfoWrapper tmp(other);
+    swap(*this, tmp);
+    return *this;
+}
+
+void SwapchainCreateInfoWrapper::setQueueFamilyIndices(
+    const std::vector<uint32_t>& queueFamilyIndices) {
+    mQueueFamilyIndices = queueFamilyIndices;
+    mCreateInfo.queueFamilyIndexCount = static_cast<uint32_t>(mQueueFamilyIndices.size());
+    if (mQueueFamilyIndices.empty()) {
+        mCreateInfo.pQueueFamilyIndices = nullptr;
+    } else {
+        mCreateInfo.pQueueFamilyIndices = queueFamilyIndices.data();
+    }
+}
+
 SwapChainStateVk::SwapChainStateVk(const goldfish_vk::VulkanDispatch &vk, VkDevice vkDevice,
                                    const VkSwapchainCreateInfoKHR &swapChainCi)
     : m_vk(vk),
@@ -85,9 +144,9 @@ bool SwapChainStateVk::validateQueueFamilyProperties(const goldfish_vk::VulkanDi
     return presentSupport;
 }
 
-SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainCi(
-    const goldfish_vk::VulkanDispatch &vk, VkSurfaceKHR surface, VkPhysicalDevice physicalDevice,
-    uint32_t width, uint32_t height, const std::unordered_set<uint32_t> &queueFamilyIndices) {
+std::optional<SwapchainCreateInfoWrapper> SwapChainStateVk::createSwapChainCi(
+    const goldfish_vk::VulkanDispatch& vk, VkSurfaceKHR surface, VkPhysicalDevice physicalDevice,
+    uint32_t width, uint32_t height, const std::unordered_set<uint32_t>& queueFamilyIndices) {
     uint32_t formatCount = 0;
     VK_CHECK(
         vk.vkGetPhysicalDeviceSurfaceFormatsKHR(physicalDevice, surface, &formatCount, nullptr));
@@ -124,7 +183,7 @@ SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainC
                                 ") with color space(%#" PRIx64 ") not supported.",
                                 static_cast<uint64_t>(k_vkFormat),
                                 static_cast<uint64_t>(k_vkColorSpace));
-        return nullptr;
+        return std::nullopt;
     }
 
     uint32_t presentModeCount = 0;
@@ -137,7 +196,7 @@ SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainC
     VkPresentModeKHR presentMode = VK_PRESENT_MODE_FIFO_KHR;
     if (!presentModes.count(VK_PRESENT_MODE_FIFO_KHR)) {
         SWAPCHAINSTATE_VK_ERROR("Fail to create swapchain: FIFO present mode not supported.");
-        return nullptr;
+        return std::nullopt;
     }
     VkFormatProperties formatProperties = {};
     vk.vkGetPhysicalDeviceFormatProperties(physicalDevice, k_vkFormat, &formatProperties);
@@ -151,7 +210,7 @@ SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainC
             "The format %s with the optimal tiling doesn't support VK_FORMAT_FEATURE_BLIT_DST_BIT. "
             "The supported features are %s.",
             string_VkFormat(k_vkFormat), string_VkFormatFeatureFlags(formatFeatures).c_str());
-        return nullptr;
+        return std::nullopt;
     }
     VkSurfaceCapabilitiesKHR surfaceCaps;
     VK_CHECK(vk.vkGetPhysicalDeviceSurfaceCapabilitiesKHR(physicalDevice, surface, &surfaceCaps));
@@ -160,7 +219,7 @@ SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainC
             "The supported usage flags of the presentable images is %s, and don't contain "
             "VK_IMAGE_USAGE_TRANSFER_DST_BIT.",
             string_VkImageUsageFlags(surfaceCaps.supportedUsageFlags).c_str());
-        return nullptr;
+        return std::nullopt;
     }
     std::optional<VkExtent2D> maybeExtent = std::nullopt;
     if (surfaceCaps.currentExtent.width != UINT32_MAX && surfaceCaps.currentExtent.width == width &&
@@ -176,52 +235,43 @@ SwapChainStateVk::VkSwapchainCreateInfoKHRPtr SwapChainStateVk::createSwapChainC
         SWAPCHAINSTATE_VK_ERROR("Fail to create swapchain: extent(%" PRIu64 "x%" PRIu64
                                 ") not supported.",
                                 static_cast<uint64_t>(width), static_cast<uint64_t>(height));
-        return nullptr;
+        return std::nullopt;
     }
     auto extent = maybeExtent.value();
     uint32_t imageCount = surfaceCaps.minImageCount + 1;
     if (surfaceCaps.maxImageCount != 0 && surfaceCaps.maxImageCount < imageCount) {
         imageCount = surfaceCaps.maxImageCount;
     }
-    VkSwapchainCreateInfoKHRPtr swapChainCi(
-        new VkSwapchainCreateInfoKHR{
-            .sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR,
-            .pNext = nullptr,
-            .flags = VkSwapchainCreateFlagsKHR{0},
-            .surface = surface,
-            .minImageCount = imageCount,
-            .imageFormat = iSurfaceFormat->format,
-            .imageColorSpace = iSurfaceFormat->colorSpace,
-            .imageExtent = extent,
-            .imageArrayLayers = 1,
-            .imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT,
-            .imageSharingMode = VkSharingMode{},
-            .queueFamilyIndexCount = 0,
-            .pQueueFamilyIndices = nullptr,
-            .preTransform = surfaceCaps.currentTransform,
-            .compositeAlpha = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR,
-            .presentMode = presentMode,
-            .clipped = VK_TRUE,
-            .oldSwapchain = VK_NULL_HANDLE},
-        [](VkSwapchainCreateInfoKHR *p) {
-            if (p->pQueueFamilyIndices != nullptr) {
-                delete[] p->pQueueFamilyIndices;
-            }
-        });
+    SwapchainCreateInfoWrapper swapChainCi(VkSwapchainCreateInfoKHR{
+        .sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR,
+        .pNext = nullptr,
+        .flags = VkSwapchainCreateFlagsKHR{0},
+        .surface = surface,
+        .minImageCount = imageCount,
+        .imageFormat = iSurfaceFormat->format,
+        .imageColorSpace = iSurfaceFormat->colorSpace,
+        .imageExtent = extent,
+        .imageArrayLayers = 1,
+        .imageUsage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT,
+        .imageSharingMode = VkSharingMode{},
+        .queueFamilyIndexCount = 0,
+        .pQueueFamilyIndices = nullptr,
+        .preTransform = surfaceCaps.currentTransform,
+        .compositeAlpha = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR,
+        .presentMode = presentMode,
+        .clipped = VK_TRUE,
+        .oldSwapchain = VK_NULL_HANDLE});
     if (queueFamilyIndices.empty()) {
         SWAPCHAINSTATE_VK_ERROR("Fail to create swapchain: no Vulkan queue family specified.");
-        return nullptr;
+        return std::nullopt;
     }
     if (queueFamilyIndices.size() == 1) {
-        swapChainCi->imageSharingMode = VK_SHARING_MODE_EXCLUSIVE;
-        swapChainCi->queueFamilyIndexCount = 0;
-        swapChainCi->pQueueFamilyIndices = nullptr;
+        swapChainCi.mCreateInfo.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE;
+        swapChainCi.setQueueFamilyIndices({});
     } else {
-        swapChainCi->imageSharingMode = VK_SHARING_MODE_CONCURRENT;
-        swapChainCi->queueFamilyIndexCount = static_cast<uint32_t>(queueFamilyIndices.size());
-        uint32_t *pQueueFamilyIndices = new uint32_t[queueFamilyIndices.size()];
-        std::copy(queueFamilyIndices.begin(), queueFamilyIndices.end(), pQueueFamilyIndices);
-        swapChainCi->pQueueFamilyIndices = pQueueFamilyIndices;
+        swapChainCi.mCreateInfo.imageSharingMode = VK_SHARING_MODE_CONCURRENT;
+        swapChainCi.setQueueFamilyIndices(
+            std::vector<uint32_t>(queueFamilyIndices.begin(), queueFamilyIndices.end()));
     }
     return swapChainCi;
 }
diff --git a/stream-servers/SwapChainStateVk.h b/stream-servers/SwapChainStateVk.h
index ee50c07c..71066d75 100644
--- a/stream-servers/SwapChainStateVk.h
+++ b/stream-servers/SwapChainStateVk.h
@@ -4,22 +4,41 @@
 #include <functional>
 #include <memory>
 #include <optional>
+#include <type_traits>
 #include <unordered_set>
 #include <vector>
 
 #include "vulkan/cereal/common/goldfish_vk_dispatch.h"
 
+struct SwapchainCreateInfoWrapper {
+    VkSwapchainCreateInfoKHR mCreateInfo;
+    std::vector<uint32_t> mQueueFamilyIndices;
+
+    SwapchainCreateInfoWrapper(const SwapchainCreateInfoWrapper&);
+    SwapchainCreateInfoWrapper(SwapchainCreateInfoWrapper&&) = delete;
+    SwapchainCreateInfoWrapper& operator=(const SwapchainCreateInfoWrapper&);
+    SwapchainCreateInfoWrapper& operator=(SwapchainCreateInfoWrapper&&) = delete;
+
+    SwapchainCreateInfoWrapper(const VkSwapchainCreateInfoKHR&);
+
+    void setQueueFamilyIndices(const std::vector<uint32_t>& queueFamilyIndices);
+};
+
+// Assert SwapchainCreateInfoWrapper is a copy only class.
+static_assert(std::is_copy_assignable_v<SwapchainCreateInfoWrapper> &&
+              std::is_copy_constructible_v<SwapchainCreateInfoWrapper> &&
+              !std::is_move_constructible_v<SwapchainCreateInfoWrapper> &&
+              !std::is_move_assignable_v<SwapchainCreateInfoWrapper>);
+
 class SwapChainStateVk {
    public:
     static std::vector<const char *> getRequiredInstanceExtensions();
     static std::vector<const char *> getRequiredDeviceExtensions();
     static bool validateQueueFamilyProperties(const goldfish_vk::VulkanDispatch &, VkPhysicalDevice,
                                               VkSurfaceKHR, uint32_t queueFamilyIndex);
-    using VkSwapchainCreateInfoKHRPtr =
-        std::unique_ptr<VkSwapchainCreateInfoKHR, std::function<void(VkSwapchainCreateInfoKHR *)>>;
-    static VkSwapchainCreateInfoKHRPtr createSwapChainCi(
-        const goldfish_vk::VulkanDispatch &, VkSurfaceKHR, VkPhysicalDevice, uint32_t width,
-        uint32_t height, const std::unordered_set<uint32_t> &queueFamilyIndices);
+    static std::optional<SwapchainCreateInfoWrapper> createSwapChainCi(
+        const goldfish_vk::VulkanDispatch&, VkSurfaceKHR, VkPhysicalDevice, uint32_t width,
+        uint32_t height, const std::unordered_set<uint32_t>& queueFamilyIndices);
 
     explicit SwapChainStateVk(const goldfish_vk::VulkanDispatch &, VkDevice,
                               const VkSwapchainCreateInfoKHR &);
diff --git a/stream-servers/tests/DisplayVk_unittest.cpp b/stream-servers/tests/DisplayVk_unittest.cpp
index 984cff3b..7c36164a 100644
--- a/stream-servers/tests/DisplayVk_unittest.cpp
+++ b/stream-servers/tests/DisplayVk_unittest.cpp
@@ -129,7 +129,7 @@ class DisplayVkTest : public ::testing::Test {
                     SwapChainStateVk::validateQueueFamilyProperties(*k_vk, device, m_vkSurface,
                                                                     queueFamilyIndex) &&
                     SwapChainStateVk::createSwapChainCi(*k_vk, m_vkSurface, device, k_width,
-                                                        k_height, {queueFamilyIndex}) != nullptr) {
+                                                        k_height, {queueFamilyIndex})) {
                     maybeSwapChainQueueFamilyIndex = queueFamilyIndex;
                 }
                 if (!maybeCompositorQueueFamilyIndex.has_value() &&
diff --git a/stream-servers/tests/SwapChainStateVk_unittest.cpp b/stream-servers/tests/SwapChainStateVk_unittest.cpp
index f3ed3e6e..819dd22d 100644
--- a/stream-servers/tests/SwapChainStateVk_unittest.cpp
+++ b/stream-servers/tests/SwapChainStateVk_unittest.cpp
@@ -99,9 +99,8 @@ class SwapChainStateVkTest : public ::testing::Test {
                         *k_vk, device, m_vkSurface, queueFamilyIndex)) {
                     continue;
                 }
-                if (SwapChainStateVk::createSwapChainCi(
-                        *k_vk, m_vkSurface, device, k_width, k_height,
-                        {queueFamilyIndex}) == nullptr) {
+                if (!SwapChainStateVk::createSwapChainCi(*k_vk, m_vkSurface, device, k_width,
+                                                         k_height, {queueFamilyIndex})) {
                     continue;
                 }
                 break;
@@ -149,6 +148,6 @@ TEST_F(SwapChainStateVkTest, init) {
     auto swapChainCi = SwapChainStateVk::createSwapChainCi(
         *k_vk, m_vkSurface, m_vkPhysicalDevice, k_width, k_height,
         {m_swapChainQueueFamilyIndex});
-    ASSERT_NE(swapChainCi, nullptr);
-    SwapChainStateVk swapChainState(*k_vk, m_vkDevice, *swapChainCi);
+    ASSERT_NE(swapChainCi, std::nullopt);
+    SwapChainStateVk swapChainState(*k_vk, m_vkDevice, swapChainCi->mCreateInfo);
 }
\ No newline at end of file