dawn_wire: Forward client-generated errors to the server

This fixes a problem where client-generated errors weren't properly
captured in error scopes.

Bug: chromium:1004368
Change-Id: Ic0f6e5bc5e281c676ea7154dd217cfc8dd51be5c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11642
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

dawn.json

@@ -539,6 +539,14 @@
                     {"name": "descriptor", "type": "texture descriptor", "annotation": "const*"}
                 ]
             },
+            {
+                "name": "inject error",
+                "args": [
+                    {"name": "type", "type": "error type"},
+                    {"name": "message", "type": "char", "annotation": "const*"}
+                ],
+                "TODO": "enga@: Make this a Dawn extension"
+            },
             {
                 "name": "tick"
             },

src/dawn_native/Device.cpp

@@ -98,6 +98,13 @@ namespace dawn_native {
         mCurrentErrorScope->HandleError(type, message);
     }
 
+    void DeviceBase::InjectError(dawn::ErrorType type, const char* message) {
+        if (ConsumedError(ValidateErrorType(type))) {
+            return;
+        }
+        mCurrentErrorScope->HandleError(type, message);
+    }
+
     void DeviceBase::ConsumeError(ErrorData* error) {
         ASSERT(error != nullptr);
         HandleError(error->GetType(), error->GetMessage().c_str());

src/dawn_native/Device.h

@@ -149,6 +149,8 @@ namespace dawn_native {
         TextureViewBase* CreateTextureView(TextureBase* texture,
                                            const TextureViewDescriptor* descriptor);
 
+        void InjectError(dawn::ErrorType type, const char* message);
+
         void Tick();
 
         void SetUncapturedErrorCallback(dawn::ErrorCallback callback, void* userdata);

src/dawn_wire/client/ApiProcs.cpp

@@ -285,8 +285,9 @@ namespace dawn_wire { namespace client {
                                  void* userdata) {
         Fence* fence = reinterpret_cast<Fence*>(cFence);
         if (value > fence->signaledValue) {
-            fence->device->HandleError(DAWN_ERROR_TYPE_VALIDATION,
+            ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
-                                       "Value greater than fence signaled value");
+                                    DAWN_ERROR_TYPE_VALIDATION,
+                                    "Value greater than fence signaled value");
             callback(DAWN_FENCE_COMPLETION_STATUS_ERROR, userdata);
             return;
         }
@@ -394,14 +395,15 @@ namespace dawn_wire { namespace client {
         Fence* fence = reinterpret_cast<Fence*>(cFence);
         Queue* queue = reinterpret_cast<Queue*>(cQueue);
         if (fence->queue != queue) {
-            fence->device->HandleError(
+            ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
-                DAWN_ERROR_TYPE_VALIDATION,
+                                    DAWN_ERROR_TYPE_VALIDATION,
-                "Fence must be signaled on the queue on which it was created.");
+                                    "Fence must be signaled on the queue on which it was created.");
             return;
         }
         if (signalValue <= fence->signaledValue) {
-            fence->device->HandleError(DAWN_ERROR_TYPE_VALIDATION,
+            ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
-                                       "Fence value less than or equal to signaled value");
+                                    DAWN_ERROR_TYPE_VALIDATION,
+                                    "Fence value less than or equal to signaled value");
             return;
         }
         fence->signaledValue = signalValue;

src/tests/end2end/FenceTests.cpp

@@ -18,6 +18,8 @@
 #include <array>
 #include <cstring>
 
+using namespace testing;
+
 class MockFenceOnCompletionCallback {
   public:
     MOCK_METHOD2(Call, void(DawnFenceCompletionStatus status, void* userdata));
@@ -27,6 +29,17 @@ static std::unique_ptr<MockFenceOnCompletionCallback> mockFenceOnCompletionCallb
 static void ToMockFenceOnCompletionCallback(DawnFenceCompletionStatus status, void* userdata) {
     mockFenceOnCompletionCallback->Call(status, userdata);
 }
+
+class MockPopErrorScopeCallback {
+  public:
+    MOCK_METHOD3(Call, void(DawnErrorType type, const char* message, void* userdata));
+};
+
+static std::unique_ptr<MockPopErrorScopeCallback> mockPopErrorScopeCallback;
+static void ToMockPopErrorScopeCallback(DawnErrorType type, const char* message, void* userdata) {
+    mockPopErrorScopeCallback->Call(type, message, userdata);
+}
+
 class FenceTests : public DawnTest {
   private:
     struct CallbackInfo {
@@ -50,10 +63,12 @@ class FenceTests : public DawnTest {
     void SetUp() override {
         DawnTest::SetUp();
         mockFenceOnCompletionCallback = std::make_unique<MockFenceOnCompletionCallback>();
+        mockPopErrorScopeCallback = std::make_unique<MockPopErrorScopeCallback>();
     }
 
     void TearDown() override {
         mockFenceOnCompletionCallback = nullptr;
+        mockPopErrorScopeCallback = nullptr;
         DawnTest::TearDown();
     }
 
@@ -206,4 +221,22 @@ TEST_P(FenceTests, DISABLED_DestroyBeforeOnCompletionEnd) {
     WaitForCompletedValue(fence, 1);
 }
 
+// Regression test that validation errors that are tracked client-side are captured
+// in error scopes.
+TEST_P(FenceTests, ClientValidationErrorInErrorScope) {
+    dawn::FenceDescriptor descriptor;
+    descriptor.initialValue = 0u;
+    dawn::Fence fence = queue.CreateFence(&descriptor);
+
+    queue.Signal(fence, 4);
+
+    device.PushErrorScope(dawn::ErrorFilter::Validation);
+    queue.Signal(fence, 2);
+
+    EXPECT_CALL(*mockPopErrorScopeCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this)).Times(1);
+    device.PopErrorScope(ToMockPopErrorScopeCallback, this);
+
+    WaitForCompletedValue(fence, 4);
+}
+
 DAWN_INSTANTIATE_TEST(FenceTests, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);

src/tests/unittests/wire/WireFenceTests.cpp

@@ -19,17 +19,6 @@ using namespace dawn_wire;
 
 namespace {
 
-    // Mock classes to add expectations on the wire calling callbacks
-    class MockDeviceErrorCallback {
-      public:
-        MOCK_METHOD3(Call, void(DawnErrorType type, const char* message, void* userdata));
-    };
-
-    std::unique_ptr<StrictMock<MockDeviceErrorCallback>> mockDeviceErrorCallback;
-    void ToMockDeviceErrorCallback(DawnErrorType type, const char* message, void* userdata) {
-        mockDeviceErrorCallback->Call(type, message, userdata);
-    }
-
     class MockFenceOnCompletionCallback {
       public:
         MOCK_METHOD2(Call, void(DawnFenceCompletionStatus status, void* userdata));
@@ -51,7 +40,6 @@ class WireFenceTests : public WireTest {
     void SetUp() override {
         WireTest::SetUp();
 
-        mockDeviceErrorCallback = std::make_unique<StrictMock<MockDeviceErrorCallback>>();
         mockFenceOnCompletionCallback =
             std::make_unique<StrictMock<MockFenceOnCompletionCallback>>();
 
@@ -77,14 +65,12 @@ class WireFenceTests : public WireTest {
     void TearDown() override {
         WireTest::TearDown();
 
-        mockDeviceErrorCallback = nullptr;
         mockFenceOnCompletionCallback = nullptr;
     }
 
     void FlushServer() {
         WireTest::FlushServer();
 
-        Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback);
         Mock::VerifyAndClearExpectations(&mockFenceOnCompletionCallback);
     }
 
@@ -117,37 +103,23 @@ TEST_F(WireFenceTests, QueueSignalSuccess) {
     FlushServer();
 }
 
-// Without any flushes, it is valid to signal a value greater than the current
+// Errors should be generated when signaling a value less
-// signaled value
-TEST_F(WireFenceTests, QueueSignalSynchronousValidationSuccess) {
-    dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(_, _, _)).Times(0);
-
-    dawnQueueSignal(queue, fence, 2u);
-    dawnQueueSignal(queue, fence, 4u);
-    dawnQueueSignal(queue, fence, 5u);
-}
-
-// Without any flushes, errors should be generated when signaling a value less
 // than or equal to the current signaled value
-TEST_F(WireFenceTests, QueueSignalSynchronousValidationError) {
+TEST_F(WireFenceTests, QueueSignalValidationError) {
-    dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
-
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
     dawnQueueSignal(queue, fence, 0u);  // Error
-    EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
     dawnQueueSignal(queue, fence, 1u);  // Error
-    EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(_, _, _)).Times(0);
+    DoQueueSignal(4u);  // Success
-    dawnQueueSignal(queue, fence, 4u);  // Success
+    FlushClient();
-    EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
 
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
     dawnQueueSignal(queue, fence, 3u);  // Error
-    EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 }
 
 // Check that callbacks are immediately called if the fence is already finished
@@ -214,16 +186,16 @@ TEST_F(WireFenceTests, OnCompletionSynchronousValidationSuccess) {
         .Times(3);
 }
 
-// Without any flushes, errors should be generated when waiting on a value greater
+// Errors should be generated when waiting on a value greater
 // than the last signaled value
-TEST_F(WireFenceTests, OnCompletionSynchronousValidationError) {
+TEST_F(WireFenceTests, OnCompletionValidationError) {
-    dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, this + 1);
-
     EXPECT_CALL(*mockFenceOnCompletionCallback, Call(DAWN_FENCE_COMPLETION_STATUS_ERROR, this + 0))
         .Times(1);
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this + 1)).Times(1);
 
     dawnFenceOnCompletion(fence, 2u, ToMockFenceOnCompletionCallback, this + 0);
+
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 }
 
 // Check that the fence completed value is initialized
@@ -262,9 +234,9 @@ TEST_F(WireFenceTests, SignalWrongQueue) {
     EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
     FlushClient();
 
-    dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
     dawnQueueSignal(queue2, fence, 2u);  // error
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 }
 
 // Test that signaling a fence on a wrong queue does not update fence signaled value
@@ -274,9 +246,9 @@ TEST_F(WireFenceTests, SignalWrongQueueDoesNotUpdateValue) {
     EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
     FlushClient();
 
-    dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
-    EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
     dawnQueueSignal(queue2, fence, 2u);  // error
+    EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+    FlushClient();
 
     // Fence value should be unchanged.
     FlushClient();