commit a594f8fdb4d78c06ae1cf5fa4adbcc1210af9b4d
author Corentin Wallez <cwallez@chromium.org> Wed Feb 13 13:09:18 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed Feb 13 13:09:18 2019 +0000
tree 7349ab2904e7f5496b19012a92cc852ed9403e4b
parent f872e6924cbb7ff75738b9ad6560cdb789a91ac1

WebGPU error handling 1: Return error objects on errors.

Dawn used to return "nullptr" when an error happened while creating an
object. To match WebGPU we want to return valid pointers but to "error"
objects.

This commit implements WebGPU error handling for all "descriptorized"
objects and changes the nullptr error checks into "ValidateObject"
checks. This method is used both to check that the object isn't an
error, but also that all objects in a function call are from the same
device.

New validation is added to objects with methods (apart from Device) so
they check they aren't error objects.

A large number of ASSERTs were added to check that frontend objects
aren't accessed when they are errors, so that missing validation would
hit the asserts instead of crashing randomly.

The bind group validation tests were modified to test the behavior with
both nullptrs and error objects.

Future commits will change the CommandBufferBuilder to not be a builder
anymore but an "Encoder" instead with special-cased error handling. Then
once all objects are descriptorized, the notion of builders will be
removed from the code.

BUG=dawn:8

Change-Id: I8647712d5de3deb0e99e3bc58f34496f67710edd
Reviewed-on: https://dawn-review.googlesource.com/c/4360
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Device.cpp

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 8b33268..095fb0a 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -70,6 +70,16 @@
         mErrorUserdata = userdata;
     }
 
+    MaybeError DeviceBase::ValidateObject(const ObjectBase* object) const {
+        if (DAWN_UNLIKELY(object->GetDevice() != this)) {
+            return DAWN_VALIDATION_ERROR("Object from a different device.");
+        }
+        if (DAWN_UNLIKELY(object->IsError())) {
+            return DAWN_VALIDATION_ERROR("Object is an error.");
+        }
+        return {};
+    }
+
     AdapterBase* DeviceBase::GetAdapter() const {
         return mAdapter;
     }
@@ -108,7 +118,7 @@
         BindGroupBase* result = nullptr;
 
         if (ConsumedError(CreateBindGroupInternal(&result, descriptor))) {
-            return nullptr;
+            return BindGroupBase::MakeError(this);
         }
 
         return result;
@@ -118,7 +128,7 @@
         BindGroupLayoutBase* result = nullptr;
 
         if (ConsumedError(CreateBindGroupLayoutInternal(&result, descriptor))) {
-            return nullptr;
+            return BindGroupLayoutBase::MakeError(this);
         }
 
         return result;
@@ -127,7 +137,7 @@
         BufferBase* result = nullptr;
 
         if (ConsumedError(CreateBufferInternal(&result, descriptor))) {
-            return nullptr;
+            return BufferBase::MakeError(this);
         }
 
         return result;
@@ -140,7 +150,7 @@
         ComputePipelineBase* result = nullptr;
 
         if (ConsumedError(CreateComputePipelineInternal(&result, descriptor))) {
-            return nullptr;
+            return ComputePipelineBase::MakeError(this);
         }
 
         return result;
@@ -149,7 +159,7 @@
         FenceBase* result = nullptr;
 
         if (ConsumedError(CreateFenceInternal(&result, descriptor))) {
-            return nullptr;
+            return FenceBase::MakeError(this);
         }
 
         return result;
@@ -162,7 +172,7 @@
         PipelineLayoutBase* result = nullptr;
 
         if (ConsumedError(CreatePipelineLayoutInternal(&result, descriptor))) {
-            return nullptr;
+            return PipelineLayoutBase::MakeError(this);
         }
 
         return result;
@@ -171,6 +181,9 @@
         QueueBase* result = nullptr;
 
         if (ConsumedError(CreateQueueInternal(&result))) {
+            // If queue creation failure ever becomes possible, we should implement MakeError and
+            // friends for them.
+            UNREACHABLE();
             return nullptr;
         }
 
@@ -183,7 +196,7 @@
         SamplerBase* result = nullptr;
 
         if (ConsumedError(CreateSamplerInternal(&result, descriptor))) {
-            return nullptr;
+            return SamplerBase::MakeError(this);
         }
 
         return result;
@@ -193,7 +206,7 @@
         RenderPipelineBase* result = nullptr;
 
         if (ConsumedError(CreateRenderPipelineInternal(&result, descriptor))) {
-            return nullptr;
+            return RenderPipelineBase::MakeError(this);
         }
 
         return result;
@@ -202,7 +215,7 @@
         ShaderModuleBase* result = nullptr;
 
         if (ConsumedError(CreateShaderModuleInternal(&result, descriptor))) {
-            return nullptr;
+            return ShaderModuleBase::MakeError(this);
         }
 
         return result;
@@ -214,8 +227,9 @@
         TextureBase* result = nullptr;
 
         if (ConsumedError(CreateTextureInternal(&result, descriptor))) {
-            return nullptr;
+            return TextureBase::MakeError(this);
         }
+
         return result;
     }
     TextureViewBase* DeviceBase::CreateTextureView(TextureBase* texture,
@@ -223,8 +237,9 @@
         TextureViewBase* result = nullptr;
 
         if (ConsumedError(CreateTextureViewInternal(&result, texture, descriptor))) {
-            return nullptr;
+            return TextureViewBase::MakeError(this);
         }
+
         return result;
     }