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>
This commit is contained in:
François Beaufort 2023-05-04 06:46:19 +00:00 committed by Dawn LUCI CQ
parent dac162716c
commit b22c5a465c
8 changed files with 69 additions and 47 deletions

View File

@ -378,7 +378,7 @@ void BufferBase::APIMapAsync(wgpu::MapMode mode,
if (mState == BufferState::PendingMap) {
if (callback) {
GetDevice()->GetCallbackTaskManager()->AddCallbackTask(
callback, WGPUBufferMapAsyncStatus_Error, userdata);
callback, WGPUBufferMapAsyncStatus_MappingAlreadyPending, userdata);
}
return;
}
@ -516,7 +516,7 @@ MaybeError BufferBase::ValidateMapAsync(wgpu::MapMode mode,
*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,

View File

@ -88,7 +88,7 @@ interop::Promise<void> GPUBuffer::mapAsync(Napi::Env env,
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:

View File

@ -428,7 +428,7 @@ TEST_P(BufferMappingTests, OffsetNotUpdatedOnError) {
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 @@ TEST_P(BufferMappedAtCreationTests, CreateThenMapBeforeUnmapFailure) {
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 @@ TEST_P(BufferTests, CreateBufferOOMMapAsync) {
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));

View File

@ -60,7 +60,8 @@ class BufferValidationTest : public ValidationTest {
}
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 @@ TEST_F(BufferValidationTest, MapAsync_PendingMap) {
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 @@ TEST_F(BufferValidationTest, MapAsync_PendingMap) {
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 @@ TEST_F(BufferValidationTest, MapAsync_PendingMap) {
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 @@ TEST_F(BufferValidationTest, MapAsync_PendingMap) {
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 @@ TEST_F(BufferValidationTest, DISABLED_MapAsync_MapAsyncInMapAsyncSuccessCallback
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 @@ TEST_F(BufferValidationTest, MapAsync_MapAsyncInMapAsyncRejectionCallback) {
{
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 @@ TEST_F(BufferValidationTest, MapAsync_MapAsyncInMapAsyncRejectionCallback) {
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 @@ TEST_F(BufferValidationTest, MapDestroyedBufferAfterUnmap) {
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);

View File

@ -137,12 +137,13 @@ TEST_F(WireBufferMappingReadTests, ErrorWhileMappingForRead) {
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 @@ TEST_F(WireBufferMappingReadTests, UnmapCalledTooEarlyForReadButServerSideError)
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 @@ TEST_F(WireBufferMappingReadTests, DestroyCalledTooEarlyForReadButServerSideErro
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 @@ TEST_F(WireBufferMappingReadTests, MappingForReadingErrorWhileAlreadyMappedUncha
// 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 @@ TEST_F(WireBufferMappingWriteTests, ErrorWhileMappingForWrite) {
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 @@ TEST_F(WireBufferMappingWriteTests, MappingForWritingErrorWhileAlreadyMapped) {
// 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 @@ TEST_F(WireBufferMappingTests, MappedAtCreationThenMapFailure) {
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 @@ TEST_F(WireBufferMappingTests, PendingMapImmediateError) {
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 @@ TEST_F(WireBufferMappingTests, GetMapState) {
{
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 @@ TEST_F(WireBufferMappingWriteTests, MapInsideCallbackBeforeDestruction) {
// 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

View File

@ -427,13 +427,14 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapReadError) {
// 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 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapWriteError) {
// 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();

View File

@ -186,7 +186,7 @@ void Buffer::MapAsync(WGPUMapModeFlags mode,
ASSERT(GetRefcount() != 0);
if (mPendingMap) {
return callback(WGPUBufferMapAsyncStatus_Error, userdata);
return callback(WGPUBufferMapAsyncStatus_MappingAlreadyPending, userdata);
}
Client* client = GetClient();

View File

@ -80,8 +80,12 @@ bool Server::DoBufferMapAsync(ObjectId bufferId,
// 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;
}