Use typed integers for the Buffer MapRequestID

This will prevent mixing it up with other serial types in the future.

Bug: dawn:442
Change-Id: Ie655d57722fcd79c82acc5aac429aed2c2741c3e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28920
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/Buffer.cpp

@@ -138,7 +138,7 @@ namespace dawn_native {
     BufferBase::~BufferBase() {
         if (mState == BufferState::Mapped) {
             ASSERT(!IsError());
-            CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
+            CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
         }
     }
 
@@ -212,9 +212,9 @@ namespace dawn_native {
         }
     }
 
-    void BufferBase::CallMapCallback(uint32_t serial, WGPUBufferMapAsyncStatus status) {
+    void BufferBase::CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status) {
         ASSERT(!IsError());
-        if (mMapCallback != nullptr && serial == mMapSerial) {
+        if (mMapCallback != nullptr && mapID == mLastMapID) {
             // Tag the callback as fired before firing it, otherwise it could fire a second time if
             // for example buffer.Unmap() is called inside the application-provided callback.
             WGPUBufferMapCallback callback = mMapCallback;
@@ -249,8 +249,7 @@ namespace dawn_native {
         }
         ASSERT(!IsError());
 
-        // TODO(cwallez@chromium.org): what to do on wraparound? Could cause crashes.
-        mMapSerial++;
+        mLastMapID++;
         mMapMode = mode;
         mMapOffset = offset;
         mMapSize = size;
@@ -259,12 +258,12 @@ namespace dawn_native {
         mState = BufferState::Mapped;
 
         if (GetDevice()->ConsumedError(MapAsyncImpl(mode, offset, size))) {
-            CallMapCallback(mMapSerial, WGPUBufferMapAsyncStatus_DeviceLost);
+            CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DeviceLost);
             return;
         }
 
         MapRequestTracker* tracker = GetDevice()->GetMapRequestTracker();
-        tracker->Track(this, mMapSerial);
+        tracker->Track(this, mLastMapID);
     }
 
     void* BufferBase::GetMappedRange(size_t offset, size_t size) {
@@ -350,7 +349,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, callbackStatus);
+            CallMapCallback(mLastMapID, callbackStatus);
             UnmapImpl();
 
             mMapCallback = nullptr;
@@ -514,8 +513,8 @@ namespace dawn_native {
         mState = BufferState::Destroyed;
     }
 
-    void BufferBase::OnMapCommandSerialFinished(uint32_t mapSerial) {
-        CallMapCallback(mapSerial, WGPUBufferMapAsyncStatus_Success);
+    void BufferBase::OnMapRequestCompleted(MapRequestID mapID) {
+        CallMapCallback(mapID, WGPUBufferMapAsyncStatus_Success);
     }
 
     bool BufferBase::IsDataInitialized() const {

src/dawn_native/Buffer.h

@@ -17,6 +17,7 @@
 
 #include "dawn_native/Error.h"
 #include "dawn_native/Forward.h"
+#include "dawn_native/IntegerTypes.h"
 #include "dawn_native/ObjectBase.h"
 
 #include "dawn_native/dawn_platform.h"
@@ -52,7 +53,7 @@ namespace dawn_native {
         wgpu::BufferUsage GetUsage() const;
 
         MaybeError MapAtCreation();
-        void OnMapCommandSerialFinished(uint32_t mapSerial);
+        void OnMapRequestCompleted(MapRequestID mapID);
 
         MaybeError ValidateCanUseOnQueueNow() const;
 
@@ -91,7 +92,7 @@ namespace dawn_native {
         virtual bool IsCPUWritableAtCreation() const = 0;
         MaybeError CopyFromStagingBuffer();
         void* GetMappedRangeInternal(bool writable, size_t offset, size_t size);
-        void CallMapCallback(uint32_t serial, WGPUBufferMapAsyncStatus status);
+        void CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status);
 
         MaybeError ValidateMap(wgpu::BufferUsage requiredUsage,
                                WGPUBufferMapAsyncStatus* status) const;
@@ -113,7 +114,7 @@ namespace dawn_native {
 
         WGPUBufferMapCallback mMapCallback = nullptr;
         void* mMapUserdata = 0;
-        uint32_t mMapSerial = 0;
+        MapRequestID mLastMapID = MapRequestID(0);
         wgpu::MapMode mMapMode = wgpu::MapMode::None;
         size_t mMapOffset = 0;
         size_t mMapSize = 0;

src/dawn_native/IntegerTypes.h

@@ -43,6 +43,17 @@ namespace dawn_native {
     constexpr VertexAttributeLocation kMaxVertexAttributesTyped =
         VertexAttributeLocation(kMaxVertexAttributes);
 
+    // Serials are 64bit integers that are incremented by one each time to produce unique values.
+    // Some serials (like queue serials) are compared numerically to know which one is before
+    // another, while some serials are only checked for equality. We call serials only checked
+    // for equality IDs.
+
+    // Buffer mapping requests are stored outside of the buffer while they are being processed and
+    // cannot be invalidated. Instead they are associated with an ID, and when a map request is
+    // finished, the mapping callback is fired only if its ID matches the ID if the last request
+    // that was sent.
+    using MapRequestID = TypedInteger<struct MapRequestIDT, uint64_t>;
+
 }  // namespace dawn_native
 
 #endif  // DAWNNATIVE_INTEGERTYPES_H_

src/dawn_native/MapRequestTracker.cpp

@@ -13,12 +13,11 @@
 // limitations under the License.
 
 #include "dawn_native/MapRequestTracker.h"
+
 #include "dawn_native/Buffer.h"
 #include "dawn_native/Device.h"
 
 namespace dawn_native {
-    struct Request;
-    class DeviceBase;
 
     MapRequestTracker::MapRequestTracker(DeviceBase* device) : mDevice(device) {
     }
@@ -27,10 +26,10 @@ namespace dawn_native {
         ASSERT(mInflightRequests.Empty());
     }
 
-    void MapRequestTracker::Track(BufferBase* buffer, uint32_t mapSerial) {
+    void MapRequestTracker::Track(BufferBase* buffer, MapRequestID mapID) {
         Request request;
         request.buffer = buffer;
-        request.mapSerial = mapSerial;
+        request.id = mapID;
 
         mInflightRequests.Enqueue(std::move(request), mDevice->GetPendingCommandSerial());
         mDevice->AddFutureCallbackSerial(mDevice->GetPendingCommandSerial());
@@ -38,7 +37,7 @@ namespace dawn_native {
 
     void MapRequestTracker::Tick(Serial finishedSerial) {
         for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) {
-            request.buffer->OnMapCommandSerialFinished(request.mapSerial);
+            request.buffer->OnMapRequestCompleted(request.id);
         }
         mInflightRequests.ClearUpTo(finishedSerial);
     }

src/dawn_native/MapRequestTracker.h

@@ -16,7 +16,9 @@
 #define DAWNNATIVE_MAPREQUESTTRACKER_H_
 
 #include "common/SerialQueue.h"
-#include "dawn_native/Device.h"
+#include "dawn_native/Buffer.h"
+#include "dawn_native/Forward.h"
+#include "dawn_native/IntegerTypes.h"
 
 namespace dawn_native {
 
@@ -25,7 +27,7 @@ namespace dawn_native {
         MapRequestTracker(DeviceBase* device);
         ~MapRequestTracker();
 
-        void Track(BufferBase* buffer, uint32_t mapSerial);
+        void Track(BufferBase* buffer, MapRequestID mapID);
         void Tick(Serial finishedSerial);
 
       private:
@@ -33,7 +35,7 @@ namespace dawn_native {
 
         struct Request {
             Ref<BufferBase> buffer;
-            uint32_t mapSerial;
+            MapRequestID id;
         };
         SerialQueue<Serial, Request> mInflightRequests;
     };