Use MapAsync callback for server->client map callbacks.

This changes the format of the server->client callback for async mapping
to match MapAsync. Previously there were two callbacks, one for
MapReadAsync and one for MapWriteAsync.

Bug: dawn:445

Change-Id: I3330c07ac8bb6d1fa9019563e9c946875e852639
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24821
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>

dawn_wire.json

@@ -70,17 +70,12 @@
         ]
     },
     "return commands": {
-        "buffer map read async callback": [
+        "buffer map async callback": [
             { "name": "buffer", "type": "ObjectHandle", "handle_type": "buffer" },
             { "name": "request serial", "type": "uint32_t" },
             { "name": "status", "type": "uint32_t" },
-            { "name": "initial data info length", "type": "uint64_t" },
+            { "name": "read initial data info length", "type": "uint64_t" },
-            { "name": "initial data info", "type": "uint8_t", "annotation": "const*", "length": "initial data info length", "skip_serialize": true }
+            { "name": "read initial data info", "type": "uint8_t", "annotation": "const*", "length": "read initial data info length", "skip_serialize": true }
-        ],
-        "buffer map write async callback": [
-            { "name": "buffer", "type": "ObjectHandle", "handle_type": "buffer" },
-            { "name": "request serial", "type": "uint32_t" },
-            { "name": "status", "type": "uint32_t" }
         ],
         "device uncaptured error callback": [
             { "name": "type", "type": "error type"},

src/dawn_wire/client/Buffer.cpp

@@ -310,10 +310,10 @@ namespace dawn_wire { namespace client {
         }
     }
 
-    bool Buffer::OnMapReadAsyncCallback(uint32_t requestSerial,
+    bool Buffer::OnMapAsyncCallback(uint32_t requestSerial,
                                     uint32_t status,
-                                        uint64_t initialDataInfoLength,
+                                    uint64_t readInitialDataInfoLength,
-                                        const uint8_t* initialDataInfo) {
+                                    const uint8_t* readInitialDataInfo) {
         // The requests can have been deleted via an Unmap so this isn't an error.
         auto requestIt = mRequests.find(requestSerial);
         if (requestIt == mRequests.end()) {
@@ -325,115 +325,75 @@ namespace dawn_wire { namespace client {
         // second time. If, for example, buffer.Unmap() is called inside the callback.
         mRequests.erase(requestIt);
 
+        auto FailRequest = [&request]() -> bool {
+            if (request.readCallback != nullptr) {
+                request.readCallback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0,
+                                     request.userdata);
+            }
+            if (request.writeCallback != nullptr) {
+                request.writeCallback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0,
+                                      request.userdata);
+            }
+            return false;
+        };
+
+        bool isRead = request.readHandle != nullptr;
+        bool isWrite = request.writeHandle != nullptr;
+        ASSERT(isRead != isWrite);
+
         size_t mappedDataLength = 0;
         const void* mappedData = nullptr;
-
-        auto GetMappedData = [&]() -> bool {
-            // It is an error for the server to call the read callback when we asked for a map write
-            if (request.writeHandle) {
-                return false;
-            }
-
         if (status == WGPUBufferMapAsyncStatus_Success) {
             if (mReadHandle || mWriteHandle) {
                 // Buffer is already mapped.
-                    return false;
+                return FailRequest();
             }
-                if (initialDataInfoLength > std::numeric_limits<size_t>::max()) {
+
+            if (isRead) {
+                if (readInitialDataInfoLength > std::numeric_limits<size_t>::max()) {
                     // This is the size of data deserialized from the command stream, which must be
                     // CPU-addressable.
-                    return false;
+                    return FailRequest();
                 }
-                ASSERT(request.readHandle != nullptr);
 
                 // The server serializes metadata to initialize the contents of the ReadHandle.
                 // Deserialize the message and return a pointer and size of the mapped data for
                 // reading.
                 if (!request.readHandle->DeserializeInitialData(
-                        initialDataInfo, static_cast<size_t>(initialDataInfoLength), &mappedData,
+                        readInitialDataInfo, static_cast<size_t>(readInitialDataInfoLength),
-                        &mappedDataLength)) {
+                        &mappedData, &mappedDataLength)) {
                     // Deserialization shouldn't fail. This is a fatal error.
-                    return false;
+                    return FailRequest();
                 }
                 ASSERT(mappedData != nullptr);
 
-                // The MapRead request was successful. The buffer now owns the ReadHandle until
-                // Unmap().
-                mReadHandle = std::move(request.readHandle);
-            }
-
-            return true;
-        };
-
-        if (!GetMappedData()) {
-            // Dawn promises that all callbacks are called in finite time. Even if a fatal error
-            // occurs, the callback is called.
-            request.readCallback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, request.userdata);
-            return false;
             } else {
-            mMappedData = const_cast<void*>(mappedData);
-            request.readCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mMappedData,
-                                 static_cast<uint64_t>(mappedDataLength), request.userdata);
-            return true;
-        }
-    }
-
-    bool Buffer::OnMapWriteAsyncCallback(uint32_t requestSerial, uint32_t status) {
-        // The requests can have been deleted via an Unmap so this isn't an error.
-        auto requestIt = mRequests.find(requestSerial);
-        if (requestIt == mRequests.end()) {
-            return true;
-        }
-
-        auto request = std::move(requestIt->second);
-        // Delete the request before calling the callback otherwise the callback could be fired a
-        // second time. If, for example, buffer.Unmap() is called inside the callback.
-        mRequests.erase(requestIt);
-
-        size_t mappedDataLength = 0;
-        void* mappedData = nullptr;
-
-        auto GetMappedData = [&]() -> bool {
-            // It is an error for the server to call the write callback when we asked for a map read
-            if (request.readHandle) {
-                return false;
-            }
-
-            if (status == WGPUBufferMapAsyncStatus_Success) {
-                if (mReadHandle || mWriteHandle) {
-                    // Buffer is already mapped.
-                    return false;
-                }
-                ASSERT(request.writeHandle != nullptr);
-
                 // Open the WriteHandle. This returns a pointer and size of mapped memory.
                 // On failure, |mappedData| may be null.
                 std::tie(mappedData, mappedDataLength) = request.writeHandle->Open();
 
                 if (mappedData == nullptr) {
-                    return false;
+                    return FailRequest();
+                }
             }
 
-                // The MapWrite request was successful. The buffer now owns the WriteHandle until
+            // The MapAsync request was successful. The buffer now owns the Read/Write handles
-                // Unmap().
+            // until Unmap().
+            mReadHandle = std::move(request.readHandle);
             mWriteHandle = std::move(request.writeHandle);
         }
 
-            return true;
+        mMappedData = const_cast<void*>(mappedData);
-        };
 
-        if (!GetMappedData()) {
+        if (isRead) {
-            // Dawn promises that all callbacks are called in finite time. Even if a fatal error
+            request.readCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mMappedData,
-            // occurs, the callback is called.
+                                 static_cast<uint64_t>(mappedDataLength), request.userdata);
-            request.writeCallback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0,
+        } else {
-                                  request.userdata);
+            request.writeCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mMappedData,
-            return false;
-        } else {
-            mMappedData = mappedData;
-            request.writeCallback(static_cast<WGPUBufferMapAsyncStatus>(status), mappedData,
                                   static_cast<uint64_t>(mappedDataLength), request.userdata);
-            return true;
         }
+
+        return true;
     }
 
     void* Buffer::GetMappedRange() {
@@ -483,6 +443,7 @@ namespace dawn_wire { namespace client {
         } else if (mReadHandle) {
             mReadHandle = nullptr;
         }
+
         mMappedData = nullptr;
         mMapOffset = 0;
         ClearMapRequests(WGPUBufferMapAsyncStatus_Unknown);

src/dawn_wire/client/Buffer.h

@@ -38,11 +38,10 @@ namespace dawn_wire { namespace client {
 
         void MapReadAsync(WGPUBufferMapReadCallback callback, void* userdata);
         void MapWriteAsync(WGPUBufferMapWriteCallback callback, void* userdata);
-        bool OnMapReadAsyncCallback(uint32_t requestSerial,
+        bool OnMapAsyncCallback(uint32_t requestSerial,
                                 uint32_t status,
-                                    uint64_t initialDataInfoLength,
+                                uint64_t readInitialDataInfoLength,
-                                    const uint8_t* initialDataInfo);
+                                const uint8_t* readInitialDataInfo);
-        bool OnMapWriteAsyncCallback(uint32_t requestSerial, uint32_t status);
         void MapAsync(WGPUMapModeFlags mode,
                       size_t offset,
                       size_t size,

src/dawn_wire/client/ClientDoers.cpp

@@ -46,29 +46,18 @@ namespace dawn_wire { namespace client {
         return mDevice->OnPopErrorScopeCallback(requestSerial, errorType, message);
     }
 
-    bool Client::DoBufferMapReadAsyncCallback(Buffer* buffer,
+    bool Client::DoBufferMapAsyncCallback(Buffer* buffer,
                                           uint32_t requestSerial,
                                           uint32_t status,
-                                              uint64_t initialDataInfoLength,
+                                          uint64_t readInitialDataInfoLength,
-                                              const uint8_t* initialDataInfo) {
+                                          const uint8_t* readInitialDataInfo) {
         // The buffer might have been deleted or recreated so this isn't an error.
         if (buffer == nullptr) {
             return true;
         }
 
-        return buffer->OnMapReadAsyncCallback(requestSerial, status, initialDataInfoLength,
+        return buffer->OnMapAsyncCallback(requestSerial, status, readInitialDataInfoLength,
-                                              initialDataInfo);
+                                          readInitialDataInfo);
-    }
-
-    bool Client::DoBufferMapWriteAsyncCallback(Buffer* buffer,
-                                               uint32_t requestSerial,
-                                               uint32_t status) {
-        // The buffer might have been deleted or recreated so this isn't an error.
-        if (buffer == nullptr) {
-            return true;
-        }
-
-        return buffer->OnMapWriteAsyncCallback(requestSerial, status);
     }
 
     bool Client::DoFenceUpdateCompletedValue(Fence* fence, uint64_t value) {

src/dawn_wire/server/ServerBuffer.cpp

@@ -235,15 +235,34 @@ namespace dawn_wire { namespace server {
             return;
         }
 
-        if (data->mode & WGPUMapMode_Write) {
+        bool isRead = data->mode & WGPUMapMode_Read;
-            ReturnBufferMapWriteAsyncCallbackCmd cmd;
+        bool isSuccess = status == WGPUBufferMapAsyncStatus_Success;
+
+        ReturnBufferMapAsyncCallbackCmd cmd;
         cmd.buffer = data->buffer;
         cmd.requestSerial = data->requestSerial;
         cmd.status = status;
+        cmd.readInitialDataInfoLength = 0;
+        cmd.readInitialDataInfo = nullptr;
 
-            SerializeCommand(cmd);
+        const void* readData = nullptr;
+        if (isSuccess && isRead) {
+            // Get the serialization size of the message to initialize ReadHandle data.
+            readData = mProcs.bufferGetConstMappedRange(data->bufferObj);
+            cmd.readInitialDataInfoLength =
+                data->readHandle->SerializeInitialDataSize(readData, data->size);
+        }
 
-            if (status == WGPUBufferMapAsyncStatus_Success) {
+        char* readHandleSpace = SerializeCommand(cmd, cmd.readInitialDataInfoLength);
+
+        if (isSuccess) {
+            if (isRead) {
+                // Serialize the initialization message into the space after the command.
+                data->readHandle->SerializeInitialData(readData, data->size, readHandleSpace);
+                // The in-flight map request returned successfully.
+                // Move the ReadHandle so it is owned by the buffer.
+                bufferData->readHandle = std::move(data->readHandle);
+            } else {
                 // The in-flight map request returned successfully.
                 // Move the WriteHandle so it is owned by the buffer.
                 bufferData->writeHandle = std::move(data->writeHandle);
@@ -252,38 +271,6 @@ namespace dawn_wire { namespace server {
                 bufferData->writeHandle->SetTarget(mProcs.bufferGetMappedRange(data->bufferObj),
                                                    data->size);
             }
-        } else {
-            ASSERT(data->mode & WGPUMapMode_Read);
-
-            size_t initialDataInfoLength = 0;
-            size_t mappedDataLength = 0;
-            const void* mappedData = nullptr;
-            if (status == WGPUBufferMapAsyncStatus_Success) {
-                // Get the serialization size of the message to initialize ReadHandle data.
-                mappedDataLength = data->size;
-                mappedData = mProcs.bufferGetConstMappedRange(data->bufferObj);
-                initialDataInfoLength =
-                    data->readHandle->SerializeInitialDataSize(mappedData, mappedDataLength);
-            }
-
-            ReturnBufferMapReadAsyncCallbackCmd cmd;
-            cmd.buffer = data->buffer;
-            cmd.requestSerial = data->requestSerial;
-            cmd.status = status;
-            cmd.initialDataInfoLength = initialDataInfoLength;
-            cmd.initialDataInfo = nullptr;
-
-            char* readHandleSpace = SerializeCommand(cmd, initialDataInfoLength);
-
-            if (status == WGPUBufferMapAsyncStatus_Success) {
-                // Serialize the initialization message into the space after the command.
-                data->readHandle->SerializeInitialData(mappedData, mappedDataLength,
-                                                       readHandleSpace);
-
-                // The in-flight map request returned successfully.
-                // Move the ReadHandle so it is owned by the buffer.
-                bufferData->readHandle = std::move(data->readHandle);
-            }
         }
     }