Handle new errors to WGPUBufferMapAsyncStatus
This CL raises "mapping already pending", "offset out of range", and
"size out of range", and "validation error" error to make it easier
for developers to know why APIMapAsync fail in those cases.
Bug: chromium:1431622
Change-Id: I6f04751b2d67420a51d94aeac39ffbfd6e126fbf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/129740
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Fr <beaufort.francois@gmail.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index b2efa62..fe79de4 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -378,7 +378,7 @@
if (mState == BufferState::PendingMap) {
if (callback) {
GetDevice()->GetCallbackTaskManager()->AddCallbackTask(
- callback, WGPUBufferMapAsyncStatus_Error, userdata);
+ callback, WGPUBufferMapAsyncStatus_MappingAlreadyPending, userdata);
}
return;
}
@@ -516,7 +516,7 @@
*status = WGPUBufferMapAsyncStatus_DeviceLost;
DAWN_TRY(GetDevice()->ValidateIsAlive());
- *status = WGPUBufferMapAsyncStatus_Error;
+ *status = WGPUBufferMapAsyncStatus_ValidationError;
DAWN_TRY(GetDevice()->ValidateObject(this));
DAWN_INVALID_IF(uint64_t(offset) > mSize,
diff --git a/src/dawn/node/binding/GPUBuffer.cpp b/src/dawn/node/binding/GPUBuffer.cpp
index d92c840..49184f0 100644
--- a/src/dawn/node/binding/GPUBuffer.cpp
+++ b/src/dawn/node/binding/GPUBuffer.cpp
@@ -88,7 +88,7 @@
c->promise.Resolve();
c->state = State::Mapped;
break;
- case WGPUBufferMapAsyncStatus_Error:
+ case WGPUBufferMapAsyncStatus_ValidationError:
c->promise.Reject(Errors::OperationError(c->env));
break;
case WGPUBufferMapAsyncStatus_UnmappedBeforeCallback:
diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp
index 94f8d4c..cd0739c 100644
--- a/src/dawn/tests/end2end/BufferTests.cpp
+++ b/src/dawn/tests/end2end/BufferTests.cpp
@@ -428,7 +428,7 @@
buffer.MapAsync(
wgpu::MapMode::Read, 0, 4,
[](WGPUBufferMapAsyncStatus status, void* userdata) {
- ASSERT_EQ(WGPUBufferMapAsyncStatus_Error, status);
+ ASSERT_EQ(WGPUBufferMapAsyncStatus_MappingAlreadyPending, status);
*static_cast<bool*>(userdata) = true;
},
&done2);
@@ -883,7 +883,7 @@
buffer.MapAsync(
wgpu::MapMode::Write, 0, 4,
[](WGPUBufferMapAsyncStatus status, void* userdata) {
- ASSERT_EQ(WGPUBufferMapAsyncStatus_Error, status);
+ ASSERT_EQ(WGPUBufferMapAsyncStatus_ValidationError, status);
*static_cast<bool*>(userdata) = true;
},
&done);
@@ -1094,7 +1094,7 @@
ASSERT_DEVICE_ERROR(buffer.MapAsync(
wgpu::MapMode::Write, 0, 4,
[](WGPUBufferMapAsyncStatus status, void* userdata) {
- EXPECT_EQ(status, WGPUBufferMapAsyncStatus_Error);
+ EXPECT_EQ(status, WGPUBufferMapAsyncStatus_ValidationError);
*static_cast<bool*>(userdata) = true;
},
&done));
diff --git a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
index 0dead52..76a01d2 100644
--- a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
@@ -60,7 +60,8 @@
}
void AssertMapAsyncError(wgpu::Buffer buffer, wgpu::MapMode mode, size_t offset, size_t size) {
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _))
+ .Times(1);
ASSERT_DEVICE_ERROR(
buffer.MapAsync(mode, offset, size, ToMockBufferMapAsyncCallback, nullptr));
@@ -343,7 +344,8 @@
wgpu::Buffer buffer = CreateMapReadBuffer(4);
// The first map async call should succeed while the second one should fail
buffer.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this);
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this + 1))
.Times(1);
buffer.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
@@ -356,7 +358,8 @@
wgpu::Buffer buffer = CreateMapReadBuffer(16);
// The first map async call should succeed while the second one should fail
buffer.MapAsync(wgpu::MapMode::Read, 0, 8, ToMockBufferMapAsyncCallback, this);
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this + 1))
.Times(1);
buffer.MapAsync(wgpu::MapMode::Read, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
@@ -369,7 +372,8 @@
wgpu::Buffer buffer = CreateMapWriteBuffer(4);
// The first map async call should succeed while the second one should fail
buffer.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this);
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this + 1))
.Times(1);
buffer.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
@@ -382,7 +386,8 @@
wgpu::Buffer buffer = CreateMapWriteBuffer(16);
// The first map async call should succeed while the second one should fail
buffer.MapAsync(wgpu::MapMode::Write, 0, 8, ToMockBufferMapAsyncCallback, this);
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this + 1))
.Times(1);
buffer.MapAsync(wgpu::MapMode::Write, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
@@ -556,7 +561,8 @@
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, _))
.WillOnce(InvokeWithoutArgs([&]() {
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, _));
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_ValidationError, _));
// Should cause validation error because of already mapped buffer
ASSERT_DEVICE_ERROR(
buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, nullptr));
@@ -575,7 +581,7 @@
{
wgpu::Buffer buf = CreateMapReadBuffer(4);
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, _))
+ EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _))
.WillOnce(InvokeWithoutArgs([&]() {
// Retry with valid parameter and it should succeed
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, _));
@@ -617,7 +623,8 @@
Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
.WillOnce(InvokeWithoutArgs([&]() {
// MapAsync call on destroyed buffer should be invalid
- EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, _));
+ EXPECT_CALL(*mockBufferMapAsyncCallback,
+ Call(WGPUBufferMapAsyncStatus_ValidationError, _));
ASSERT_DEVICE_ERROR(
buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, nullptr));
}));
@@ -709,7 +716,7 @@
ASSERT_DEVICE_ERROR(buffer.MapAsync(
wgpu::MapMode::Read, 0, wgpu::kWholeMapSize,
[](WGPUBufferMapAsyncStatus status, void* userdata) {
- EXPECT_EQ(WGPUBufferMapAsyncStatus_Error, status);
+ EXPECT_EQ(WGPUBufferMapAsyncStatus_ValidationError, status);
},
nullptr));
WaitForAllOperations(device);
diff --git a/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp b/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
index e4bf75d..ea396bb 100644
--- a/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
@@ -137,12 +137,13 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -207,8 +208,9 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
// The callback should get called immediately with UnmappedBeforeCallback status,
// not server-side error, even if the request fails on the server side
@@ -257,8 +259,9 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
// The callback should be called with the server-side error and not the
// DestroyedBeforCallback..
@@ -297,12 +300,13 @@
// Map failure while the buffer is already mapped
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -415,12 +419,13 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Write, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Write, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -498,12 +503,13 @@
// Map failure while the buffer is already mapped
wgpuBufferMapAsync(buffer, WGPUMapMode_Write, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Write, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -663,12 +669,13 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Write, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Write, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -755,7 +762,8 @@
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, nullptr, this);
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, this)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this))
+ .Times(1);
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, this);
}
@@ -798,9 +806,10 @@
{
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
.WillOnce(InvokeWithoutArgs([&]() {
- api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error);
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
}));
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _))
+ .Times(1);
ASSERT_EQ(wgpuBufferGetMapState(buffer), WGPUBufferMapState_Unmapped);
wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback,
@@ -914,7 +923,7 @@
// The second or later map async calls in the map async callback
// should immediately fail because of pending map
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, this))
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_MappingAlreadyPending, this))
.Times(testData.numRequests - 1);
// The first map async call in the map async callback should fail
diff --git a/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp b/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
index 913d6a7..ca347b9 100644
--- a/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
@@ -427,13 +427,14 @@
// Mock a failed callback.
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Read, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
// The client receives an error callback.
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
@@ -684,13 +685,14 @@
// Mock an error callback.
EXPECT_CALL(api, OnBufferMapAsync(apiBuffer, WGPUMapMode_Write, 0, kBufferSize, _, _))
- .WillOnce(InvokeWithoutArgs(
- [&]() { api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_Error); }));
+ .WillOnce(InvokeWithoutArgs([&]() {
+ api.CallBufferMapAsyncCallback(apiBuffer, WGPUBufferMapAsyncStatus_ValidationError);
+ }));
FlushClient();
// The client receives an error callback.
- EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, _)).Times(1);
+ EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_ValidationError, _)).Times(1);
FlushServer();
diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp
index 32e9ef0..add9f96 100644
--- a/src/dawn/wire/client/Buffer.cpp
+++ b/src/dawn/wire/client/Buffer.cpp
@@ -186,7 +186,7 @@
ASSERT(GetRefcount() != 0);
if (mPendingMap) {
- return callback(WGPUBufferMapAsyncStatus_Error, userdata);
+ return callback(WGPUBufferMapAsyncStatus_MappingAlreadyPending, userdata);
}
Client* client = GetClient();
diff --git a/src/dawn/wire/server/ServerBuffer.cpp b/src/dawn/wire/server/ServerBuffer.cpp
index e5208fd..fe2558c 100644
--- a/src/dawn/wire/server/ServerBuffer.cpp
+++ b/src/dawn/wire/server/ServerBuffer.cpp
@@ -80,8 +80,12 @@
// WGPU_WHOLE_MAP_SIZE, which is by definition std::numeric_limits<size_t>::max(). Since
// client does the default size computation, we should always have a valid actual size here
// in server. All other invalid actual size can be caught by dawn native side validation.
- if (offset64 > std::numeric_limits<size_t>::max() || size64 >= WGPU_WHOLE_MAP_SIZE) {
- OnBufferMapAsyncCallback(userdata.get(), WGPUBufferMapAsyncStatus_Error);
+ if (offset64 > std::numeric_limits<size_t>::max()) {
+ OnBufferMapAsyncCallback(userdata.get(), WGPUBufferMapAsyncStatus_OffsetOutOfRange);
+ return true;
+ }
+ if (size64 >= WGPU_WHOLE_MAP_SIZE) {
+ OnBufferMapAsyncCallback(userdata.get(), WGPUBufferMapAsyncStatus_SizeOutOfRange);
return true;
}
