Wire: Make validation error prior to OOM if mappedAtCreation == false

This patch updates the validations about CreateBuffer() with dawn_wire
to match the latest WebGPU SPEC.

According to the SPEC, the validations in CreateBuffer() should be
executed in the below order:
1. If mappedAtCreation == true, return nullptr and a RangeError will be
   generated in Chromium.
2. Validate BufferDescriptor and check if there is OOM at device timeline
3. Check if there is OOM at content timeline

Bug: dawn:1586
Change-Id: I97ff5f82a42208442ddf6e46e66381c3b3680450
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/109040
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>

dawn.json

@@ -994,7 +994,10 @@
             {
                 "name": "create error buffer",
                 "returns": "buffer",
-                "tags": ["dawn"]
+                "tags": ["dawn"],
+                "args": [
+                    {"name": "descriptor", "type": "buffer descriptor", "annotation": "const*"}
+                ]
             },
             {
                 "name": "create command encoder",
@@ -2567,7 +2570,8 @@
             {"value": 1003, "name": "dawn encoder internal usage descriptor", "tags": ["dawn"]},
             {"value": 1004, "name": "dawn instance descriptor", "tags": ["dawn", "native"]},
             {"value": 1005, "name": "dawn cache device descriptor", "tags": ["dawn", "native"]},
-            {"value": 1006, "name": "dawn adapter properties power preference", "tags": ["dawn", "native"]}
+            {"value": 1006, "name": "dawn adapter properties power preference", "tags": ["dawn", "native"]},
+            {"value": 1007, "name": "dawn buffer descriptor error info from wire client", "tags": ["dawn"]}
         ]
     },
     "texture": {
@@ -2979,5 +2983,14 @@
         "members": [
             {"name": "power preference", "type": "power preference", "default": "undefined"}
         ]
+    },
+    "dawn buffer descriptor error info from wire client": {
+        "category": "structure",
+        "chained": "in",
+        "chain roots": ["buffer descriptor"],
+        "tags": ["dawn"],
+        "members": [
+            {"name": "out of memory", "type": "bool", "default": "false"}
+        ]
     }
 }

src/dawn/native/Buffer.cpp

@@ -17,6 +17,7 @@
 #include <cstdio>
 #include <cstring>
 #include <limits>
+#include <string>
 #include <utility>
 
 #include "dawn/common/Alloc.h"
@@ -96,7 +97,7 @@ class ErrorBuffer final : public BufferBase {
 
 }  // anonymous namespace
 
-MaybeError ValidateBufferDescriptor(DeviceBase*, const BufferDescriptor* descriptor) {
+MaybeError ValidateBufferDescriptor(DeviceBase* device, const BufferDescriptor* descriptor) {
     DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr");
     DAWN_TRY(ValidateBufferUsage(descriptor->usage));
 
@@ -124,6 +125,14 @@ MaybeError ValidateBufferDescriptor(DeviceBase*, const BufferDescriptor* descrip
                     "Buffer is mapped at creation but its size (%u) is not a multiple of 4.",
                     descriptor->size);
 
+    // TODO(dawn:1525): Change to validation error after the deprecation period.
+    if (descriptor->size > device->GetLimits().v1.maxBufferSize) {
+        std::string warning =
+            absl::StrFormat("Buffer size (%u) exceeds the max buffer size limit (%u).",
+                            descriptor->size, device->GetLimits().v1.maxBufferSize);
+        device->EmitDeprecationWarning(warning.c_str());
+    }
+
     return {};
 }
 

src/dawn/native/Device.cpp

@@ -1193,9 +1193,27 @@ TextureBase* DeviceBase::APICreateTexture(const TextureDescriptor* descriptor) {
 
 // For Dawn Wire
 
-BufferBase* DeviceBase::APICreateErrorBuffer() {
+BufferBase* DeviceBase::APICreateErrorBuffer(const BufferDescriptor* desc) {
-    BufferDescriptor desc = {};
+    BufferDescriptor fakeDescriptor = *desc;
-    return BufferBase::MakeError(this, &desc);
+    fakeDescriptor.nextInChain = nullptr;
+
+    // The validation errors on BufferDescriptor should be prior to any OOM errors when
+    // MapppedAtCreation == false.
+    MaybeError maybeError = ValidateBufferDescriptor(this, &fakeDescriptor);
+    if (maybeError.IsError()) {
+        ConsumedError(maybeError.AcquireError(), "calling %s.CreateBuffer(%s).", this, desc);
+    } else {
+        const DawnBufferDescriptorErrorInfoFromWireClient* clientErrorInfo = nullptr;
+        FindInChain(desc->nextInChain, &clientErrorInfo);
+        if (clientErrorInfo != nullptr && clientErrorInfo->outOfMemory) {
+            ConsumedError(DAWN_OUT_OF_MEMORY_ERROR("Failed to allocate memory for buffer mapping"));
+        }
+    }
+
+    // Set the size of the error buffer to 0 as this function is called only when an OOM happens at
+    // the client side.
+    fakeDescriptor.size = 0;
+    return BufferBase::MakeError(this, &fakeDescriptor);
 }
 
 ExternalTextureBase* DeviceBase::APICreateErrorExternalTexture() {
@@ -1408,15 +1426,7 @@ ResultOrError<Ref<BindGroupLayoutBase>> DeviceBase::CreateBindGroupLayout(
 ResultOrError<Ref<BufferBase>> DeviceBase::CreateBuffer(const BufferDescriptor* descriptor) {
     DAWN_TRY(ValidateIsAlive());
     if (IsValidationEnabled()) {
-        DAWN_TRY_CONTEXT(ValidateBufferDescriptor(this, descriptor), "validating %s", descriptor);
+        DAWN_TRY(ValidateBufferDescriptor(this, descriptor));
-
-        // TODO(dawn:1525): Change to validation error after the deprecation period.
-        if (descriptor->size > mLimits.v1.maxBufferSize) {
-            std::string warning =
-                absl::StrFormat("Buffer size (%u) exceeds the max buffer size limit (%u).",
-                                descriptor->size, mLimits.v1.maxBufferSize);
-            EmitDeprecationWarning(warning.c_str());
-        }
     }
 
     Ref<BufferBase> buffer;

src/dawn/native/Device.h

@@ -272,7 +272,7 @@ class DeviceBase : public RefCountedWithExternalCount {
     InternalPipelineStore* GetInternalPipelineStore();
 
     // For Dawn Wire
-    BufferBase* APICreateErrorBuffer();
+    BufferBase* APICreateErrorBuffer(const BufferDescriptor* desc);
     ExternalTextureBase* APICreateErrorExternalTexture();
     TextureBase* APICreateErrorTexture(const TextureDescriptor* desc);
 

src/dawn/tests/end2end/BufferTests.cpp

@@ -819,6 +819,10 @@ TEST_P(BufferTests, CreateBufferOOM) {
     descriptor.size = 1ull << 50;
     // TODO(dawn:1525): remove warning expectation after the deprecation period.
     ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+
+    // Validation errors should always be prior to OOM.
+    descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::Uniform;
+    ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
 }
 
 // Test that a very large buffer mappedAtCreation fails gracefully.
@@ -844,12 +848,22 @@ TEST_P(BufferTests, BufferMappedAtCreationOOM) {
 
         // Test an enormous buffer fails
         descriptor.size = std::numeric_limits<uint64_t>::max();
-        ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+        if (UsesWire()) {
+            wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+            ASSERT_EQ(nullptr, buffer.Get());
+        } else {
+            ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+        }
 
         // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
         descriptor.size = 1ull << 50;
-        // TODO(dawn:1525): remove warning expectation after the deprecation period.
+        if (UsesWire()) {
-        ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+            wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+            ASSERT_EQ(nullptr, buffer.Get());
+        } else {
+            // TODO(dawn:1525): remove warning expectation after the deprecation period.
+            ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+        }
     }
 
     // Test mappable buffer
@@ -864,12 +878,22 @@ TEST_P(BufferTests, BufferMappedAtCreationOOM) {
 
         // Test an enormous buffer fails
         descriptor.size = std::numeric_limits<uint64_t>::max();
-        ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+        if (UsesWire()) {
+            wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+            ASSERT_EQ(nullptr, buffer.Get());
+        } else {
+            ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+        }
 
         // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
         descriptor.size = 1ull << 50;
-        // TODO(dawn:1525): remove warning expectation after the deprecation period.
+        if (UsesWire()) {
-        ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+            wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+            ASSERT_EQ(nullptr, buffer.Get());
+        } else {
+            // TODO(dawn:1525): remove warning expectation after the deprecation period.
+            ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+        }
     }
 }
 

src/dawn/tests/unittests/validation/BufferValidationTests.cpp

@@ -849,13 +849,20 @@ TEST_F(BufferValidationTest, GetMappedRange_OnErrorBuffer_OOM) {
 
     uint64_t kStupidLarge = uint64_t(1) << uint64_t(63);
 
-    wgpu::Buffer buffer;
+    if (UsesWire()) {
-    ASSERT_DEVICE_ERROR(buffer = BufferMappedAtCreation(
+        wgpu::Buffer buffer = BufferMappedAtCreation(
-                            kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
+            kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead);
+        ASSERT_EQ(nullptr, buffer.Get());
+    } else {
+        wgpu::Buffer buffer;
+        ASSERT_DEVICE_ERROR(
+            buffer = BufferMappedAtCreation(
+                kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
 
-    // GetMappedRange after mappedAtCreation OOM case returns nullptr.
+        // GetMappedRange after mappedAtCreation OOM case returns nullptr.
-    ASSERT_EQ(buffer.GetConstMappedRange(), nullptr);
+        ASSERT_EQ(buffer.GetConstMappedRange(), nullptr);
-    ASSERT_EQ(buffer.GetConstMappedRange(), buffer.GetMappedRange());
+        ASSERT_EQ(buffer.GetConstMappedRange(), buffer.GetMappedRange());
+    }
 }
 
 // Test validation of the GetMappedRange parameters

src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp

@@ -680,8 +680,7 @@ TEST_F(WireBufferMappingTests, MappedAtCreationThenMapFailure) {
     FlushClient();
 }
 
-// Check that trying to create a buffer of size MAX_SIZE_T is an error handling in the client
+// Check that trying to create a buffer of size MAX_SIZE_T won't get OOM error at the client side.
-// and never gets to the server-side.
 TEST_F(WireBufferMappingTests, MaxSizeMappableBufferOOMDirectly) {
     size_t kOOMSize = std::numeric_limits<size_t>::max();
     WGPUBuffer apiBuffer = api.GetNewBuffer();
@@ -694,8 +693,6 @@ TEST_F(WireBufferMappingTests, MaxSizeMappableBufferOOMDirectly) {
         descriptor.mappedAtCreation = true;
 
         wgpuDeviceCreateBuffer(device, &descriptor);
-        EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
-        EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
         FlushClient();
     }
 
@@ -706,8 +703,7 @@ TEST_F(WireBufferMappingTests, MaxSizeMappableBufferOOMDirectly) {
         descriptor.size = kOOMSize;
 
         wgpuDeviceCreateBuffer(device, &descriptor);
-        EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
+        EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice, _)).WillOnce(Return(apiBuffer));
-        EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
         FlushClient();
     }
 
@@ -718,8 +714,7 @@ TEST_F(WireBufferMappingTests, MaxSizeMappableBufferOOMDirectly) {
         descriptor.size = kOOMSize;
 
         wgpuDeviceCreateBuffer(device, &descriptor);
-        EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
+        EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice, _)).WillOnce(Return(apiBuffer));
-        EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
         FlushClient();
     }
 }

src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp

@@ -891,7 +891,7 @@ TEST_F(WireMemoryTransferServiceTests, MappedAtCreationWriteHandleCreationFailur
     descriptor.mappedAtCreation = true;
 
     WGPUBuffer buffer = wgpuDeviceCreateBuffer(device, &descriptor);
-    EXPECT_EQ(nullptr, wgpuBufferGetMappedRange(buffer, 0, sizeof(mBufferContent)));
+    EXPECT_EQ(nullptr, buffer);
 }
 
 // Test buffer creation with mappedAtCreation DeserializeWriteHandle failure.

src/dawn/wire/client/Buffer.cpp

@@ -24,6 +24,20 @@
 
 namespace dawn::wire::client {
 
+namespace {
+WGPUBuffer CreateErrorBufferOOMAtClient(Device* device, const WGPUBufferDescriptor* descriptor) {
+    if (descriptor->mappedAtCreation) {
+        return nullptr;
+    }
+    WGPUBufferDescriptor errorBufferDescriptor = *descriptor;
+    WGPUDawnBufferDescriptorErrorInfoFromWireClient errorInfo = {};
+    errorInfo.chain.sType = WGPUSType_DawnBufferDescriptorErrorInfoFromWireClient;
+    errorInfo.outOfMemory = true;
+    errorBufferDescriptor.nextInChain = &errorInfo.chain;
+    return device->CreateErrorBuffer(&errorBufferDescriptor);
+}
+}  // anonymous namespace
+
 // static
 WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor) {
     Client* wireClient = device->GetClient();
@@ -32,8 +46,7 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor
         (descriptor->usage & (WGPUBufferUsage_MapRead | WGPUBufferUsage_MapWrite)) != 0 ||
         descriptor->mappedAtCreation;
     if (mappable && descriptor->size >= std::numeric_limits<size_t>::max()) {
-        device->InjectError(WGPUErrorType_OutOfMemory, "Buffer is too large for map usage");
+        return CreateErrorBufferOOMAtClient(device, descriptor);
-        return device->CreateErrorBuffer();
     }
 
     std::unique_ptr<MemoryTransferService::ReadHandle> readHandle = nullptr;
@@ -55,8 +68,7 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor
             readHandle.reset(
                 wireClient->GetMemoryTransferService()->CreateReadHandle(descriptor->size));
             if (readHandle == nullptr) {
-                device->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
+                return CreateErrorBufferOOMAtClient(device, descriptor);
-                return CreateError(device, descriptor);
             }
             readHandleCreateInfoLength = readHandle->SerializeCreateSize();
             cmd.readHandleCreateInfoLength = readHandleCreateInfoLength;
@@ -67,8 +79,7 @@ WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor
             writeHandle.reset(
                 wireClient->GetMemoryTransferService()->CreateWriteHandle(descriptor->size));
             if (writeHandle == nullptr) {
-                device->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
+                return CreateErrorBufferOOMAtClient(device, descriptor);
-                return CreateError(device, descriptor);
             }
             writeHandleCreateInfoLength = writeHandle->SerializeCreateSize();
             cmd.writeHandleCreateInfoLength = writeHandleCreateInfoLength;
@@ -131,6 +142,8 @@ WGPUBuffer Buffer::CreateError(Device* device, const WGPUBufferDescriptor* descr
 
     DeviceCreateErrorBufferCmd cmd;
     cmd.self = ToAPI(device);
+    cmd.selfId = device->GetWireId();
+    cmd.descriptor = descriptor;
     cmd.result = buffer->GetWireHandle();
     client->SerializeCommand(cmd);
 

src/dawn/wire/client/Device.cpp

@@ -199,9 +199,8 @@ WGPUBuffer Device::CreateBuffer(const WGPUBufferDescriptor* descriptor) {
     return Buffer::Create(this, descriptor);
 }
 
-WGPUBuffer Device::CreateErrorBuffer() {
+WGPUBuffer Device::CreateErrorBuffer(const WGPUBufferDescriptor* descriptor) {
-    WGPUBufferDescriptor fakeDescriptor = {};
+    return Buffer::CreateError(this, descriptor);
-    return Buffer::CreateError(this, &fakeDescriptor);
 }
 
 WGPUQuerySet Device::CreateQuerySet(const WGPUQuerySetDescriptor* descriptor) {

src/dawn/wire/client/Device.h

@@ -41,7 +41,7 @@ class Device final : public ObjectBase {
     void InjectError(WGPUErrorType type, const char* message);
     bool PopErrorScope(WGPUErrorCallback callback, void* userdata);
     WGPUBuffer CreateBuffer(const WGPUBufferDescriptor* descriptor);
-    WGPUBuffer CreateErrorBuffer();
+    WGPUBuffer CreateErrorBuffer(const WGPUBufferDescriptor* descriptor);
     void CreateComputePipelineAsync(WGPUComputePipelineDescriptor const* descriptor,
                                     WGPUCreateComputePipelineAsyncCallback callback,
                                     void* userdata);

webgpu-cts/expectations.txt

@@ -205,11 +205,6 @@ crbug.com/dawn/1125 [ ubuntu ] webgpu:api,operation,rendering,depth_clip_clamp:d
 crbug.com/dawn/1357 webgpu:api,operation,shader_module,compilation_info:offset_and_length:valid=false;name="carriage-return" [ Failure ]
 crbug.com/dawn/1357 webgpu:api,operation,shader_module,compilation_info:offset_and_length:valid=false;name="unicode" [ Failure ]
 
-################################################################################
-# createBuffer_invalid_and_oom failures
-################################################################################
-crbug.com/dawn/0000 webgpu:api,validation,buffer,create:createBuffer_invalid_and_oom: [ Failure ]
-
 ################################################################################
 # atan2 shader execution failures
 # Very slow, many failing. Skip for now.