Report more detailed error information for the failures of mapAsync

This patch adds two new buffer map async status "destroyed before
callback" and "unmapped before callback" to replace the status "unknown"
so that the developers can get more details when meeting such errors in
the call of buffer mapAsync.

Note that this patch still preserves "unknown" as it is still being used
in Chromium.

BUG=dawn:533
TEST=dawn_unittests

Change-Id: I12deefb49311ea6adea72c24e4e40797dd7eb4a1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28883
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>

dawn.json

@@ -240,7 +240,9 @@
             {"value": 0, "name": "success"},
             {"value": 1, "name": "error"},
             {"value": 2, "name": "unknown"},
-            {"value": 3, "name": "device lost"}
+            {"value": 3, "name": "device lost"},
+            {"value": 4, "name": "destroyed before callback"},
+            {"value": 5, "name": "unmapped before callback"}
         ]
     },
     "buffer usage": {

src/dawn_native/Buffer.cpp

@@ -138,7 +138,7 @@ namespace dawn_native {
     BufferBase::~BufferBase() {
         if (mState == BufferState::Mapped) {
             ASSERT(!IsError());
-            CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_Unknown);
+            CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
         }
     }
 
@@ -302,13 +302,13 @@ namespace dawn_native {
         ASSERT(!IsError());
 
         if (mState == BufferState::Mapped) {
-            Unmap();
+            UnmapInternal(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
         } else if (mState == BufferState::MappedAtCreation) {
             if (mStagingBuffer != nullptr) {
                 mStagingBuffer.reset();
             } else if (mSize != 0) {
                 ASSERT(IsCPUWritableAtCreation());
-                Unmap();
+                UnmapInternal(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
             }
         }
 
@@ -330,6 +330,10 @@ namespace dawn_native {
     }
 
     void BufferBase::Unmap() {
+        UnmapInternal(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback);
+    }
+
+    void BufferBase::UnmapInternal(WGPUBufferMapAsyncStatus callbackStatus) {
         if (IsError()) {
             // It is an error to call Unmap() on an ErrorBuffer, but we still need to reclaim the
             // fake mapped staging data.
@@ -346,7 +350,7 @@ namespace dawn_native {
             // completed before the Unmap.
             // Callbacks are not fired if there is no callback registered, so this is correct for
             // mappedAtCreation = true.
-            CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_Unknown);
+            CallMapCallback(mMapSerial, callbackStatus);
             UnmapImpl();
 
             mMapCallback = nullptr;

src/dawn_native/Buffer.h

@@ -79,8 +79,6 @@ namespace dawn_native {
 
         void DestroyInternal();
 
-        bool IsMapped() const;
-
         MaybeError MapAtCreationInternal();
 
       private:
@@ -104,6 +102,7 @@ namespace dawn_native {
         MaybeError ValidateUnmap() const;
         MaybeError ValidateDestroy() const;
         bool CanGetMappedRange(bool writable, size_t offset, size_t size) const;
+        void UnmapInternal(WGPUBufferMapAsyncStatus callbackStatus);
 
         uint64_t mSize = 0;
         wgpu::BufferUsage mUsage = wgpu::BufferUsage::None;

src/dawn_wire/client/Buffer.cpp

@@ -102,8 +102,8 @@ namespace dawn_wire { namespace client {
 
     Buffer::~Buffer() {
         // Callbacks need to be fired in all cases, as they can handle freeing resources
-        // so we call them with "Unknown" status.
+        // so we call them with "DestroyedBeforeCallback" status.
-        ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);
+        ClearMapRequests(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
     }
 
     void Buffer::ClearMapRequests(WGPUBufferMapAsyncStatus status) {
@@ -326,7 +326,7 @@ namespace dawn_wire { namespace client {
         mMappedData = nullptr;
         mMapOffset = 0;
         mMapSize = 0;
-        ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);
+        ClearMapRequests(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback);
 
         BufferUnmapCmd cmd;
         cmd.self = ToAPI(this);
@@ -338,7 +338,7 @@ namespace dawn_wire { namespace client {
         mWriteHandle = nullptr;
         mReadHandle = nullptr;
         mMappedData = nullptr;
-        ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);
+        ClearMapRequests(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
 
         BufferDestroyCmd cmd;
         cmd.self = ToAPI(this);

src/tests/unittests/validation/BufferValidationTests.cpp

@@ -317,7 +317,8 @@ TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResult) {
         wgpu::Buffer buf = CreateMapReadBuffer(4);
         buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, nullptr);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, _))
             .Times(1);
         buf.Unmap();
 
@@ -328,7 +329,8 @@ TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResult) {
         wgpu::Buffer buf = CreateMapWriteBuffer(4);
         buf.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, nullptr);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, _))
             .Times(1);
         buf.Unmap();
 
@@ -344,7 +346,8 @@ TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResultAndMapAgain) {
         wgpu::Buffer buf = CreateMapReadBuffer(4);
         buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this + 0);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, this + 0))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, this + 0))
             .Times(1);
         buf.Unmap();
 
@@ -357,7 +360,8 @@ TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResultAndMapAgain) {
         wgpu::Buffer buf = CreateMapWriteBuffer(4);
         buf.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this + 0);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, this + 0))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, this + 0))
             .Times(1);
         buf.Unmap();
 
@@ -374,7 +378,8 @@ TEST_F(BufferValidationTest, MapAsync_DestroyBeforeResult) {
         wgpu::Buffer buf = CreateMapReadBuffer(4);
         buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, nullptr);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
             .Times(1);
         buf.Destroy();
 
@@ -385,7 +390,8 @@ TEST_F(BufferValidationTest, MapAsync_DestroyBeforeResult) {
         wgpu::Buffer buf = CreateMapWriteBuffer(4);
         buf.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, nullptr);
 
-        EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _))
+        EXPECT_CALL(*mockBufferMapAsyncCallback,
+                    Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
             .Times(1);
         buf.Destroy();
 

src/tests/unittests/wire/WireBufferMappingTests.cpp

@@ -137,8 +137,10 @@ TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) {
     EXPECT_CALL(api, BufferGetConstMappedRange(apiBuffer, 0, kBufferSize))
         .WillOnce(Return(&bufferContent));
 
-    // Destroy before the client gets the success, so the callback is called with unknown.
+    // Destroy before the client gets the success, so the callback is called with
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    // DestroyedBeforeCallback.
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
+        .Times(1);
     wgpuBufferRelease(buffer);
     EXPECT_CALL(api, BufferRelease(apiBuffer));
 
@@ -146,8 +148,8 @@ TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) {
     FlushServer();
 }
 
-// Check the map read callback is called with UNKNOWN when the map request would have worked, but
+// Check the map read callback is called with "UnmappedBeforeCallback" when the map request would
-// Unmap was called
+// have worked, but Unmap was called
 TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForRead) {
     wgpuBufferMapAsync(buffer, WGPUMapMode_Read, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
 
@@ -161,7 +163,8 @@ TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForRead) {
     FlushClient();
 
     // Oh no! We are calling Unmap too early!
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, _))
+        .Times(1);
     wgpuBufferUnmap(buffer);
 
     // The callback shouldn't get called, even when the request succeeded on the server side
@@ -304,8 +307,8 @@ TEST_F(WireBufferMappingTests, ErrorWhileMappingForWrite) {
     EXPECT_EQ(nullptr, wgpuBufferGetMappedRange(buffer, 0, kBufferSize));
 }
 
-// Check that the map write callback is called with UNKNOWN when the buffer is destroyed before the
+// Check that the map write callback is called with "DestroyedBeforeCallback" when the buffer is
-// request is finished
+// destroyed before the request is finished
 TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) {
     wgpuBufferMapAsync(buffer, WGPUMapMode_Write, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
 
@@ -317,8 +320,10 @@ TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) {
     EXPECT_CALL(api, BufferGetMappedRange(apiBuffer, 0, kBufferSize))
         .WillOnce(Return(&bufferContent));
 
-    // Destroy before the client gets the success, so the callback is called with unknown.
+    // Destroy before the client gets the success, so the callback is called with
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    // DestroyedBeforeCallback.
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
+        .Times(1);
     wgpuBufferRelease(buffer);
     EXPECT_CALL(api, BufferRelease(apiBuffer));
 
@@ -326,8 +331,8 @@ TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) {
     FlushServer();
 }
 
-// Check the map read callback is called with UNKNOWN when the map request would have worked, but
+// Check the map read callback is called with "UnmappedBeforeCallback" when the map request would
-// Unmap was called
+// have worked, but Unmap was called
 TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForWrite) {
     wgpuBufferMapAsync(buffer, WGPUMapMode_Write, 0, kBufferSize, ToMockBufferMapCallback, nullptr);
 
@@ -341,7 +346,8 @@ TEST_F(WireBufferMappingTests, UnmapCalledTooEarlyForWrite) {
     FlushClient();
 
     // Oh no! We are calling Unmap too early!
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, _))
+        .Times(1);
     wgpuBufferUnmap(buffer);
 
     // The callback shouldn't get called, even when the request succeeded on the server side

src/tests/unittests/wire/WireMemoryTransferServiceTests.cpp

@@ -450,7 +450,8 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapReadDeserializeReadHandleFailure
 
     // The server received a fatal failure and the client callback was never returned.
     // It is called when the wire is destructed.
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
+        .Times(1);
 
     EXPECT_CALL(clientMemoryTransferService, OnReadHandleDestroy(clientHandle)).Times(1);
 }
@@ -680,7 +681,8 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapWriteDeserializeWriteHandleFailu
 
     // The server hit a fatal failure and never returned the callback. The client callback is
     // called when the wire is destructed.
-    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Unknown, _)).Times(1);
+    EXPECT_CALL(*mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback, _))
+        .Times(1);
 
     EXPECT_CALL(clientMemoryTransferService, OnWriteHandleDestroy(clientHandle)).Times(1);
 }