Buffer.MapAsync pending map error non validation error consumed

Bug: dawn:1613
Change-Id: Ib1e5014d36c2c35d7ed9b946de4d21e046cf1282
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/114280
Commit-Queue: Takahiro <hogehoge@gachapin.jp>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index f90ebec..1f8f5d9 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -340,6 +340,15 @@
                              size_t size,
                              WGPUBufferMapCallback callback,
                              void* userdata) {
+    // Check for an existing pending map first because it just
+    // rejects the callback and doesn't produce a validation error.
+    if (mState == BufferState::PendingMap) {
+        if (callback) {
+            callback(WGPUBufferMapAsyncStatus_Error, userdata);
+        }
+        return;
+    }
+
     // Handle the defaulting of size required by WebGPU, even if in webgpu_cpp.h it is not
     // possible to default the function argument (because there is the callback later in the
     // argument list)
@@ -481,7 +490,7 @@
         case BufferState::MappedAtCreation:
             return DAWN_VALIDATION_ERROR("%s is already mapped.", this);
         case BufferState::PendingMap:
-            return DAWN_VALIDATION_ERROR("%s is pending map.", this);
+            UNREACHABLE();
         case BufferState::Destroyed:
             return DAWN_VALIDATION_ERROR("%s is destroyed.", this);
         case BufferState::Unmapped:
diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp
index db9f84c..03ba28a 100644
--- a/src/dawn/tests/end2end/BufferTests.cpp
+++ b/src/dawn/tests/end2end/BufferTests.cpp
@@ -421,14 +421,17 @@
         },
         &done1);
 
-    // Call MapAsync another time, it is an error because the buffer is already being mapped so
-    // mMapOffset is not updated.
-    ASSERT_DEVICE_ERROR(buffer.MapAsync(
+    // Call MapAsync another time, the callback will be rejected with error status
+    // (but doesn't produce a validation error) and mMapOffset is not updated
+    // because the buffer is already being mapped and it doesn't allow multiple
+    // MapAsync requests.
+    buffer.MapAsync(
         wgpu::MapMode::Read, 0, 4,
         [](WGPUBufferMapAsyncStatus status, void* userdata) {
+            ASSERT_EQ(WGPUBufferMapAsyncStatus_Error, status);
             *static_cast<bool*>(userdata) = true;
         },
-        &done2));
+        &done2);
 
     while (!done1 || !done2) {
         WaitABit();
diff --git a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
index 1dac496..3e2ddf4 100644
--- a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
@@ -313,15 +313,17 @@
     }
 }
 
-// Test map async with a buffer that's pending map
+// Test MapAsync() immediately causes a pending map error
 TEST_F(BufferValidationTest, MapAsync_PendingMap) {
     // Read + overlapping range
     {
         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, nullptr);
-        AssertMapAsyncError(buffer, wgpu::MapMode::Read, 0, 4);
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, nullptr))
+        buffer.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+            .Times(1);
+        buffer.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
             .Times(1);
         WaitForAllOperations(device);
     }
@@ -330,9 +332,11 @@
     {
         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, nullptr);
-        AssertMapAsyncError(buffer, wgpu::MapMode::Read, 8, 8);
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, nullptr))
+        buffer.MapAsync(wgpu::MapMode::Read, 0, 8, ToMockBufferMapAsyncCallback, this);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+            .Times(1);
+        buffer.MapAsync(wgpu::MapMode::Read, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
             .Times(1);
         WaitForAllOperations(device);
     }
@@ -341,9 +345,11 @@
     {
         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, nullptr);
-        AssertMapAsyncError(buffer, wgpu::MapMode::Write, 0, 4);
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, nullptr))
+        buffer.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+            .Times(1);
+        buffer.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
             .Times(1);
         WaitForAllOperations(device);
     }
@@ -352,9 +358,11 @@
     {
         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, nullptr);
-        AssertMapAsyncError(buffer, wgpu::MapMode::Write, 8, 8);
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, nullptr))
+        buffer.MapAsync(wgpu::MapMode::Write, 0, 8, ToMockBufferMapAsyncCallback, this);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1))
+            .Times(1);
+        buffer.MapAsync(wgpu::MapMode::Write, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
+        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this))
             .Times(1);
         WaitForAllOperations(device);
     }
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index c3c8906..804c40e 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -456,6 +456,15 @@
 ################################################################################
 
 ################################################################################
+# Buffer.MapAsync pending map error tests need updates in CTS
+################################################################################
+crbug.com/dawn/1613 webgpu:api,validation,buffer,mapping:getMappedRange,state,mappingPending:* [ Failure ]
+crbug.com/dawn/1613 webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:* [ Failure ]
+crbug.com/dawn/1613 worker_webgpu:api,validation,buffer,mapping:getMappedRange,state,mappingPending:* [ Failure ]
+crbug.com/dawn/1613 worker_webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:* [ Failure ]
+################################################################################
+
+################################################################################
 # untriaged failures
 # KEEP
 ################################################################################