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; }