GPUBuffer: Rely on Dawn's validation for mapping methods

Bug: dawn:445
Change-Id: I3b435dc158bba190b5499da428a1efe7433b4831
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446229
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818876}

third_party/blink/renderer/modules/webgpu/gpu_buffer.cc

@@ -25,38 +25,11 @@ namespace blink {
 
 namespace {
 
-// crbug.com/951196
-// Currently, this value is less than the maximum ArrayBuffer length which is
-// theoretically 2^53 - 1 (Number.MAX_SAFE_INTEGER). However, creating a typed
-// array from an ArrayBuffer of size greater than TypedArray::kMaxLength crashes
-// DevTools and gives obscure errors.
-constexpr size_t kLargestMappableSize = v8::TypedArray::kMaxLength;
-
-bool ValidateRangeCreation(ExceptionState& exception_state,
-                           const char* function_name,
-                           uint64_t mapping_offset,
-                           uint64_t mapping_size,
-                           size_t max_size) {
-  if (mapping_size > uint64_t(max_size) ||
-      mapping_offset > uint64_t(max_size) - mapping_size) {
-    exception_state.ThrowRangeError(
-        WTF::String::Format("%s offset (%" PRIu64 " bytes) and size (%" PRIu64
-                            " bytes) are too large for this implementation.",
-                            function_name, mapping_offset, mapping_size));
-    return false;
-  }
-
-  // TODO(crbug.com/dawn/22): Move this validation into Dawn (in both
-  // getMappedRange and mapAsync).
-  if (mapping_offset % 8 != 0) {
-    exception_state.ThrowRangeError(WTF::String::Format(
-        "%s offset (%" PRIu64 " bytes) is not a multiple of 8.", function_name,
-        mapping_offset));
-    return false;
-  }
-
-  return true;
-}
+// A size that if used to create a dawn_wire buffer, will guarantee we'll OOM
+// immediately. It is an implementation detail of dawn_wire but that's tested
+// on CQ in Dawn.
+constexpr uint64_t kGuaranteedBufferOOMSize =
+    std::numeric_limits<size_t>::max();
 
 WGPUBufferDescriptor AsDawnType(const GPUBufferDescriptor* webgpu_desc,
                                 std::string* label) {
@@ -85,20 +58,25 @@ GPUBuffer* GPUBuffer::Create(GPUDevice* device,
 
   std::string label;
   WGPUBufferDescriptor dawn_desc = AsDawnType(webgpu_desc, &label);
+
+  // If the buffer is mappable, make sure the size stays in a size_t but still
+  // guarantees that we have an OOM.
+  bool is_mappable =
+      dawn_desc.usage & (WGPUBufferUsage_MapRead | WGPUBufferUsage_MapWrite) ||
+      dawn_desc.mappedAtCreation;
+  if (is_mappable) {
+    dawn_desc.size = std::min(dawn_desc.size, kGuaranteedBufferOOMSize);
+  }
+
   return MakeGarbageCollected<GPUBuffer>(
-      device, dawn_desc.size, dawn_desc.mappedAtCreation,
+      device, dawn_desc.size,
       device->GetProcs().deviceCreateBuffer(device->GetHandle(), &dawn_desc));
 }
 
 GPUBuffer::GPUBuffer(GPUDevice* device,
                      uint64_t size,
-                     bool mapped_at_creation,
                      WGPUBuffer buffer)
     : DawnObject<WGPUBuffer>(device, buffer), size_(size) {
-  if (mapped_at_creation) {
-    map_start_ = 0;
-    map_end_ = size;
-  }
 }
 
 GPUBuffer::~GPUBuffer() {
@@ -164,33 +142,28 @@ ScriptPromise GPUBuffer::MapAsyncImpl(ScriptState* script_state,
     size_defaulted = size_ - offset;
   }
 
+  // We need to convert from uint64_t to size_t. Either of these two variables
+  // are bigger or equal to the guaranteed OOM size then mapAsync should be an
+  // error so. That OOM size fits in a size_t so we can clamp size and offset
+  // with it.
+  size_t map_offset =
+      static_cast<size_t>(std::min(offset, kGuaranteedBufferOOMSize));
+  size_t map_size =
+      static_cast<size_t>(std::min(size_defaulted, kGuaranteedBufferOOMSize));
+
   ScriptPromiseResolver* resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
-  // Check the offset and size are within the limits of the platform.
-  // (Note this also checks for an 8-byte alignment, which is an ArrayBuffer
-  // restriction, even though an ArrayBuffer is not created here.)
-  if (!ValidateRangeCreation(exception_state, "mapAsync", offset,
-                             size_defaulted,
-                             std::numeric_limits<size_t>::max())) {
-    GetProcs().deviceInjectError(device_->GetHandle(), WGPUErrorType_Validation,
-                                 "mapAsync arguments were invalid");
-    resolver->Reject(exception_state);
-    return promise;
-  }
-  size_t map_offset = static_cast<size_t>(offset);
-  size_t map_size = static_cast<size_t>(size_defaulted);
-
   // And send the command, leaving remaining validation to Dawn.
-
   auto* callback =
       BindDawnCallback(&GPUBuffer::OnMapAsyncCallback, WrapPersistent(this),
-                       WrapPersistent(resolver), offset, size_defaulted);
+                       WrapPersistent(resolver));
 
   GetProcs().bufferMapAsync(GetHandle(), mode, map_offset, map_size,
                             callback->UnboundCallback(),
                             callback->AsUserdata());
+
   // WebGPU guarantees that promises are resolved in finite time so we
   // need to ensure commands are flushed.
   EnsureFlush();
@@ -200,27 +173,40 @@ ScriptPromise GPUBuffer::MapAsyncImpl(ScriptState* script_state,
 DOMArrayBuffer* GPUBuffer::GetMappedRangeImpl(uint64_t offset,
                                               base::Optional<uint64_t> size,
                                               ExceptionState& exception_state) {
-  // Compute the defaulted size (which is "until the end of the buffer".)
-  // (First, guard against overflows on (size_ - offset).)
-  if (offset > size_) {
+  // Compute the defaulted size which is "until the end of the buffer" or 0 if
+  // offset is past the end of the buffer.
+  uint64_t size_defaulted = 0;
+  if (size) {
+    size_defaulted = *size;
+  } else if (offset <= size_) {
+    size_defaulted = size_ - offset;
+  }
+
+  // We need to convert from uint64_t to size_t. Either of these two variables
+  // are bigger or equal to the guaranteed OOM size then getMappedRange should
+  // be an error so. That OOM size fits in a size_t so we can clamp size and
+  // offset with it.
+  size_t range_offset =
+      static_cast<size_t>(std::min(offset, kGuaranteedBufferOOMSize));
+  size_t range_size =
+      static_cast<size_t>(std::min(size_defaulted, kGuaranteedBufferOOMSize));
+
+  // The maximum size that can be mapped in JS so that we can ensure we don't
+  // create mappable buffers bigger than it.
+  // This could eventually be upgrade to the max ArrayBuffer size instead of the
+  // max TypedArray size. See crbug.com/951196
+  if (range_size > v8::TypedArray::kMaxLength) {
     exception_state.ThrowDOMException(
         DOMExceptionCode::kOperationError,
-        WTF::String::Format("getMappedRange offset (%" PRIu64
-                            " bytes) is larger than the buffer (%" PRIu64
-                            " bytes).",
-                            offset, size_));
-    return nullptr;
+        "getMappedRange failed, size is too large for the implementation");
   }
-  uint64_t size_defaulted = size ? *size : (size_ - offset);
 
-  // Check the offset and size are within the limits of the platform and the
-  // ArrayBuffer spec+implementation.
-  if (!ValidateRangeCreation(exception_state, "getMappedRange", offset,
-                             size_defaulted, kLargestMappableSize)) {
+  if (range_size > std::numeric_limits<size_t>::max() - range_offset) {
+    exception_state.ThrowDOMException(
+        DOMExceptionCode::kOperationError,
+        "getMappedRange failed, offset + size overflows size_t");
     return nullptr;
   }
-  size_t range_offset = static_cast<size_t>(offset);
-  size_t range_size = static_cast<size_t>(size_defaulted);
   size_t range_end = range_offset + range_size;
 
   // Check if an overlapping range has already been returned.
@@ -242,16 +228,10 @@ DOMArrayBuffer* GPUBuffer::GetMappedRangeImpl(uint64_t offset,
   }
 
   // And send the command, leaving remaining validation to Dawn.
-
   const void* map_data_const = GetProcs().bufferGetConstMappedRange(
       GetHandle(), range_offset, range_size);
-  // It is safe to const_cast the |data| pointer because it is a shadow
-  // copy that Dawn wire makes and does not point to the mapped GPU
-  // data. Dawn wire's copy of the data is not used outside of tests.
-  uint8_t* map_data =
-      const_cast<uint8_t*>(static_cast<const uint8_t*>(map_data_const));
 
-  if (!map_data) {
+  if (!map_data_const) {
     // TODO: have explanatory error messages here (or just leave them to the
     // asynchronous error reporting).
     exception_state.ThrowDOMException(DOMExceptionCode::kOperationError,
@@ -259,18 +239,20 @@ DOMArrayBuffer* GPUBuffer::GetMappedRangeImpl(uint64_t offset,
     return nullptr;
   }
 
+  // It is safe to const_cast the |data| pointer because it is a shadow
+  // copy that Dawn wire makes and does not point to the mapped GPU
+  // data. Dawn wire's copy of the data is not used outside of tests.
+  uint8_t* map_data =
+      const_cast<uint8_t*>(static_cast<const uint8_t*>(map_data_const));
+
   mapped_ranges_.push_back(std::make_pair(range_offset, range_end));
   return CreateArrayBufferForMappedData(map_data, range_size);
 }
 
 void GPUBuffer::OnMapAsyncCallback(ScriptPromiseResolver* resolver,
-                                   uint64_t map_start,
-                                   uint64_t map_end,
                                    WGPUBufferMapAsyncStatus status) {
   switch (status) {
     case WGPUBufferMapAsyncStatus_Success:
-      map_start_ = map_start;
-      map_end_ = map_end;
       resolver->Resolve();
       break;
     case WGPUBufferMapAsyncStatus_Error:
@@ -303,7 +285,7 @@ void GPUBuffer::OnMapAsyncCallback(ScriptPromiseResolver* resolver,
 DOMArrayBuffer* GPUBuffer::CreateArrayBufferForMappedData(void* data,
                                                           size_t data_length) {
   DCHECK(data);
-  DCHECK_LE(data_length, kLargestMappableSize);
+  DCHECK_LE(static_cast<uint64_t>(data_length), v8::TypedArray::kMaxLength);
 
   ArrayBufferContents contents(data, data_length,
                                v8::BackingStore::EmptyDeleter);
@@ -314,8 +296,6 @@ DOMArrayBuffer* GPUBuffer::CreateArrayBufferForMappedData(void* data,
 }
 
 void GPUBuffer::ResetMappingState(ScriptState* script_state) {
-  map_start_ = 0;
-  map_end_ = 0;
   mapped_ranges_.clear();
 
   for (Member<DOMArrayBuffer>& mapped_array_buffer : mapped_array_buffers_) {

third_party/blink/renderer/modules/webgpu/gpu_buffer.h

@@ -26,7 +26,6 @@ class GPUBuffer : public DawnObject<WGPUBuffer> {
                            const GPUBufferDescriptor* webgpu_desc);
   explicit GPUBuffer(GPUDevice* device,
                      uint64_t size,
-                     bool mapped_at_creation,
                      WGPUBuffer buffer);
   ~GPUBuffer() override;
 
@@ -49,7 +48,6 @@ class GPUBuffer : public DawnObject<WGPUBuffer> {
                                  ExceptionState& exception_state);
   void unmap(ScriptState* script_state);
   void destroy(ScriptState* script_state);
-  // TODO(crbug.com/877147): implement GPUBuffer.
 
  private:
   ScriptPromise MapAsyncImpl(ScriptState* script_state,
@@ -62,8 +60,6 @@ class GPUBuffer : public DawnObject<WGPUBuffer> {
                                      ExceptionState& exception_state);
 
   void OnMapAsyncCallback(ScriptPromiseResolver* resolver,
-                          uint64_t map_start,
-                          uint64_t map_end,
                           WGPUBufferMapAsyncStatus status);
 
   DOMArrayBuffer* CreateArrayBufferForMappedData(void* data,
@@ -76,8 +72,6 @@ class GPUBuffer : public DawnObject<WGPUBuffer> {
   // mapWriteAsync.
   HeapVector<Member<DOMArrayBuffer>> mapped_array_buffers_;
 
-  uint64_t map_start_ = 0;
-  uint64_t map_end_ = 0;
   // List of ranges currently returned by getMappedRange, to avoid overlaps.
   Vector<std::pair<size_t, size_t>> mapped_ranges_;
 

third_party/blink/web_tests/WebGPUExpectations

@@ -52,6 +52,9 @@ crbug.com/1083478 [ Win ] wpt_internal/webgpu/webgpu/web-platform/reftests/canva
 crbug.com/dawn/542 wpt_internal/webgpu/cts.html?q=webgpu:api,validation,setViewport:* [ Failure ]
 crbug.com/dawn/542 wpt_internal/webgpu/cts.html?q=webgpu:api,validation,setScissorRect:* [ Failure ]
 
+# This test should be updated to expect an OperationError instead of a RangeError
+crbug.com/dawn/445 wpt_internal/webgpu/cts.html?q=webgpu:api,operation,buffers,map_oom:mappedAtCreation:* [ Failure ]
+
 #
 # Platform-independent failures
 #