Return an error surface if surface creation fails

Fixes an ASSERT checking the created surface is non-null.

Fixed: chromium:1330113
Change-Id: Iebbcd6e69042abea5b424953d78e294a92ce5c82
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/92140
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

include/dawn/native/DawnNative.h

@@ -253,6 +253,8 @@ struct DAWN_NATIVE_EXPORT ExternalImageExportInfo {
     ExternalImageType mType;
 };
 
+DAWN_NATIVE_EXPORT bool CheckIsErrorForTesting(void* objectHandle);
+
 DAWN_NATIVE_EXPORT const char* GetObjectLabelForTesting(void* objectHandle);
 
 DAWN_NATIVE_EXPORT uint64_t GetAllocatedSizeForTesting(WGPUBuffer buffer);

src/dawn/native/DawnNative.cpp

@@ -300,6 +300,10 @@ ExternalImageType ExternalImageExportInfo::GetType() const {
     return mType;
 }
 
+bool CheckIsErrorForTesting(void* objectHandle) {
+    return reinterpret_cast<ErrorMonad*>(objectHandle)->IsError();
+}
+
 const char* GetObjectLabelForTesting(void* objectHandle) {
     ApiObjectBase* object = reinterpret_cast<ApiObjectBase*>(objectHandle);
     return object->GetLabel().c_str();

src/dawn/native/Instance.cpp

@@ -466,7 +466,7 @@ const XlibXcbFunctions* InstanceBase::GetOrCreateXlibXcbFunctions() {
 
 Surface* InstanceBase::APICreateSurface(const SurfaceDescriptor* descriptor) {
     if (ConsumedError(ValidateSurfaceDescriptor(this, descriptor))) {
-        return nullptr;
+        return Surface::MakeError(this);
     }
 
     return new Surface(this, descriptor);

src/dawn/native/ObjectBase.cpp

@@ -22,18 +22,21 @@ namespace dawn::native {
 static constexpr uint64_t kErrorPayload = 0;
 static constexpr uint64_t kNotErrorPayload = 1;
 
-ObjectBase::ObjectBase(DeviceBase* device) : RefCounted(kNotErrorPayload), mDevice(device) {}
+ErrorMonad::ErrorMonad() : RefCounted(kNotErrorPayload) {}
+ErrorMonad::ErrorMonad(ErrorTag) : RefCounted(kErrorPayload) {}
 
-ObjectBase::ObjectBase(DeviceBase* device, ErrorTag) : RefCounted(kErrorPayload), mDevice(device) {}
+bool ErrorMonad::IsError() const {
+    return GetRefCountPayload() == kErrorPayload;
+}
+
+ObjectBase::ObjectBase(DeviceBase* device) : ErrorMonad(), mDevice(device) {}
+
+ObjectBase::ObjectBase(DeviceBase* device, ErrorTag) : ErrorMonad(kError), mDevice(device) {}
 
 DeviceBase* ObjectBase::GetDevice() const {
     return mDevice.Get();
 }
 
-bool ObjectBase::IsError() const {
-    return GetRefCountPayload() == kErrorPayload;
-}
-
 ApiObjectBase::ApiObjectBase(DeviceBase* device, const char* label) : ObjectBase(device) {
     if (label) {
         mLabel = label;

src/dawn/native/ObjectBase.h

@@ -25,16 +25,23 @@ namespace dawn::native {
 
 class DeviceBase;
 
-class ObjectBase : public RefCounted {
+class ErrorMonad : public RefCounted {
   public:
     struct ErrorTag {};
     static constexpr ErrorTag kError = {};
 
+    ErrorMonad();
+    explicit ErrorMonad(ErrorTag tag);
+
+    bool IsError() const;
+};
+
+class ObjectBase : public ErrorMonad {
+  public:
     explicit ObjectBase(DeviceBase* device);
     ObjectBase(DeviceBase* device, ErrorTag tag);
 
     DeviceBase* GetDevice() const;
-    bool IsError() const;
 
   private:
     // Ref to owning device.

src/dawn/native/Surface.cpp

@@ -150,8 +150,15 @@ MaybeError ValidateSurfaceDescriptor(const InstanceBase* instance,
     return DAWN_FORMAT_VALIDATION_ERROR("Unsupported sType (%s)", descriptor->nextInChain->sType);
 }
 
+// static
+Surface* Surface::MakeError(InstanceBase* instance) {
+    return new Surface(instance, ErrorMonad::kError);
+}
+
+Surface::Surface(InstanceBase* instance, ErrorTag tag) : ErrorMonad(tag), mInstance(instance) {}
+
 Surface::Surface(InstanceBase* instance, const SurfaceDescriptor* descriptor)
-    : mInstance(instance) {
+    : ErrorMonad(), mInstance(instance) {
     ASSERT(descriptor->nextInChain != nullptr);
     const SurfaceDescriptorFromAndroidNativeWindow* androidDesc = nullptr;
     const SurfaceDescriptorFromMetalLayer* metalDesc = nullptr;
@@ -202,41 +209,49 @@ Surface::~Surface() {
 }
 
 NewSwapChainBase* Surface::GetAttachedSwapChain() {
+    ASSERT(!IsError());
     return mSwapChain.Get();
 }
 
 void Surface::SetAttachedSwapChain(NewSwapChainBase* swapChain) {
+    ASSERT(!IsError());
     mSwapChain = swapChain;
 }
 
-InstanceBase* Surface::GetInstance() {
+InstanceBase* Surface::GetInstance() const {
     return mInstance.Get();
 }
 
 Surface::Type Surface::GetType() const {
+    ASSERT(!IsError());
     return mType;
 }
 
 void* Surface::GetAndroidNativeWindow() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::AndroidWindow);
     return mAndroidNativeWindow;
 }
 
 void* Surface::GetMetalLayer() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::MetalLayer);
     return mMetalLayer;
 }
 
 void* Surface::GetHInstance() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::WindowsHWND);
     return mHInstance;
 }
 void* Surface::GetHWND() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::WindowsHWND);
     return mHWND;
 }
 
 IUnknown* Surface::GetCoreWindow() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::WindowsCoreWindow);
 #if defined(DAWN_PLATFORM_WINDOWS)
     return mCoreWindow.Get();
@@ -246,6 +261,7 @@ IUnknown* Surface::GetCoreWindow() const {
 }
 
 IUnknown* Surface::GetSwapChainPanel() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::WindowsSwapChainPanel);
 #if defined(DAWN_PLATFORM_WINDOWS)
     return mSwapChainPanel.Get();
@@ -255,10 +271,12 @@ IUnknown* Surface::GetSwapChainPanel() const {
 }
 
 void* Surface::GetXDisplay() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::XlibWindow);
     return mXDisplay;
 }
 uint32_t Surface::GetXWindow() const {
+    ASSERT(!IsError());
     ASSERT(mType == Type::XlibWindow);
     return mXWindow;
 }

src/dawn/native/Surface.h

@@ -15,9 +15,9 @@
 #ifndef SRC_DAWN_NATIVE_SURFACE_H_
 #define SRC_DAWN_NATIVE_SURFACE_H_
 
-#include "dawn/common/RefCounted.h"
 #include "dawn/native/Error.h"
 #include "dawn/native/Forward.h"
+#include "dawn/native/ObjectBase.h"
 
 #include "dawn/native/dawn_platform.h"
 
@@ -42,8 +42,10 @@ MaybeError ValidateSurfaceDescriptor(const InstanceBase* instance,
 // ObjectiveC).
 // The surface is also used to store the current swapchain so that we can detach it when it is
 // replaced.
-class Surface final : public RefCounted {
+class Surface final : public ErrorMonad {
   public:
+    static Surface* MakeError(InstanceBase* instance);
+
     Surface(InstanceBase* instance, const SurfaceDescriptor* descriptor);
 
     void SetAttachedSwapChain(NewSwapChainBase* swapChain);
@@ -59,7 +61,7 @@ class Surface final : public RefCounted {
         XlibWindow,
     };
     Type GetType() const;
-    InstanceBase* GetInstance();
+    InstanceBase* GetInstance() const;
 
     // Valid to call if the type is MetalLayer
     void* GetMetalLayer() const;
@@ -82,6 +84,7 @@ class Surface final : public RefCounted {
     uint32_t GetXWindow() const;
 
   private:
+    Surface(InstanceBase* instance, ErrorMonad::ErrorTag tag);
     ~Surface() override;
 
     Ref<InstanceBase> mInstance;

src/dawn/native/SwapChain.cpp

@@ -66,6 +66,7 @@ MaybeError ValidateSwapChainDescriptor(const DeviceBase* device,
     } else {
         DAWN_INVALID_IF(surface == nullptr,
                         "At least one of surface or implementation must be set");
+        DAWN_INVALID_IF(surface->IsError(), "[Surface] is invalid.");
 
         DAWN_TRY(ValidatePresentMode(descriptor->presentMode));
 

src/dawn/tests/end2end/SwapChainValidationTests.cpp

@@ -109,6 +109,15 @@ TEST_P(SwapChainValidationTests, CreationSuccess) {
     swapchain.Present();
 }
 
+// Test that creating a swapchain with an invalid surface is an error.
+TEST_P(SwapChainValidationTests, InvalidSurface) {
+    wgpu::SurfaceDescriptor surface_desc = {};
+    wgpu::Surface surface = GetInstance().CreateSurface(&surface_desc);
+
+    ASSERT_DEVICE_ERROR_MSG(device.CreateSwapChain(surface, &goodDescriptor),
+                            testing::HasSubstr("[Surface] is invalid"));
+}
+
 // Checks that the creation size must be a valid 2D texture size.
 TEST_P(SwapChainValidationTests, InvalidCreationSize) {
     wgpu::Limits supportedLimits = GetSupportedLimits().limits;

src/dawn/tests/end2end/WindowSurfaceTests.cpp

@@ -63,7 +63,8 @@ class WindowSurfaceInstanceTests : public testing::Test {
     }
 
     void AssertSurfaceCreation(const wgpu::SurfaceDescriptor* descriptor, bool succeeds) {
-        ASSERT_EQ(mInstance.CreateSurface(descriptor).Get() != nullptr, succeeds);
+        wgpu::Surface surface = mInstance.CreateSurface(descriptor);
+        ASSERT_EQ(dawn::native::CheckIsErrorForTesting(surface.Get()), !succeeds);
     }
 
     GLFWwindow* CreateWindow() {