Implement ErrorScopes for synchronous errors

This patch implements Push/PopErrorScope except for asynchronous
or GPU commands. These commands, such as Queue::Submit will need
to hold onto the ErrorScope until GPU execution is complete.

Bug: dawn:153
Change-Id: I2d340b8b391d117a59497f35690993a9cd7503e6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10700
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

BUILD.gn

@@ -166,6 +166,8 @@ source_set("libdawn_native_sources") {
     "src/dawn_native/Error.h",
     "src/dawn_native/ErrorData.cpp",
     "src/dawn_native/ErrorData.h",
+    "src/dawn_native/ErrorScope.cpp",
+    "src/dawn_native/ErrorScope.h",
     "src/dawn_native/Extensions.cpp",
     "src/dawn_native/Extensions.h",
     "src/dawn_native/Fence.cpp",
@@ -767,6 +769,7 @@ test("dawn_unittests") {
     "src/tests/unittests/validation/DebugMarkerValidationTests.cpp",
     "src/tests/unittests/validation/DrawIndirectValidationTests.cpp",
     "src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp",
+    "src/tests/unittests/validation/ErrorScopeValidationTests.cpp",
     "src/tests/unittests/validation/FenceValidationTests.cpp",
     "src/tests/unittests/validation/QueueSubmitValidationTests.cpp",
     "src/tests/unittests/validation/RenderBundleValidationTests.cpp",

src/dawn_native/Device.cpp

@@ -24,6 +24,7 @@
 #include "dawn_native/ComputePipeline.h"
 #include "dawn_native/DynamicUploader.h"
 #include "dawn_native/ErrorData.h"
+#include "dawn_native/ErrorScope.h"
 #include "dawn_native/Fence.h"
 #include "dawn_native/FenceSignalTracker.h"
 #include "dawn_native/Instance.h"
@@ -35,6 +36,7 @@
 #include "dawn_native/ShaderModule.h"
 #include "dawn_native/SwapChain.h"
 #include "dawn_native/Texture.h"
+#include "dawn_native/ValidationUtils_autogen.h"
 
 #include <unordered_set>
 
@@ -61,7 +63,9 @@ namespace dawn_native {
     // DeviceBase
 
     DeviceBase::DeviceBase(AdapterBase* adapter, const DeviceDescriptor* descriptor)
-        : mAdapter(adapter) {
+        : mAdapter(adapter),
+          mRootErrorScope(AcquireRef(new ErrorScope())),
+          mCurrentErrorScope(mRootErrorScope.Get()) {
         mCaches = std::make_unique<DeviceBase::Caches>();
         mFenceSignalTracker = std::make_unique<FenceSignalTracker>(this);
         mDynamicUploader = std::make_unique<DynamicUploader>(this);
@@ -89,26 +93,33 @@ namespace dawn_native {
     }
 
     void DeviceBase::HandleError(dawn::ErrorType type, const char* message) {
-        if (mErrorCallback) {
+        mCurrentErrorScope->HandleError(type, message);
-            mErrorCallback(static_cast<DawnErrorType>(type), message, mErrorUserdata);
     }
+
+    void DeviceBase::HandleError(ErrorData* data) {
+        mCurrentErrorScope->HandleError(data);
     }
 
     void DeviceBase::SetUncapturedErrorCallback(dawn::ErrorCallback callback, void* userdata) {
-        mErrorCallback = callback;
+        mRootErrorScope->SetCallback(callback, userdata);
-        mErrorUserdata = userdata;
     }
 
     void DeviceBase::PushErrorScope(dawn::ErrorFilter filter) {
-        // TODO(crbug.com/dawn/153): Implement error scopes.
+        if (ConsumedError(ValidateErrorFilter(filter))) {
-        HandleError(dawn::ErrorType::Validation, "Error scopes not implemented");
+            return;
+        }
+        mCurrentErrorScope = AcquireRef(new ErrorScope(filter, mCurrentErrorScope.Get()));
     }
 
     bool DeviceBase::PopErrorScope(dawn::ErrorCallback callback, void* userdata) {
-        // TODO(crbug.com/dawn/153): Implement error scopes.
+        if (DAWN_UNLIKELY(mCurrentErrorScope.Get() == mRootErrorScope.Get())) {
-        HandleError(dawn::ErrorType::Validation, "Error scopes not implemented");
             return false;
         }
+        mCurrentErrorScope->SetCallback(callback, userdata);
+        mCurrentErrorScope = Ref<ErrorScope>(mCurrentErrorScope->GetParent());
+
+        return true;
+    }
 
     MaybeError DeviceBase::ValidateObject(const ObjectBase* object) const {
         if (DAWN_UNLIKELY(object->GetDevice() != this)) {
@@ -286,8 +297,7 @@ namespace dawn_native {
             return static_cast<AttachmentState*>(*iter);
         }
 
-        Ref<AttachmentState> attachmentState = new AttachmentState(this, *blueprint);
+        Ref<AttachmentState> attachmentState = AcquireRef(new AttachmentState(this, *blueprint));
-        attachmentState->Release();
         mCaches->attachmentStates.insert(attachmentState.Get());
         return attachmentState;
     }
@@ -680,12 +690,6 @@ namespace dawn_native {
 
     // Other implementation details
 
-    void DeviceBase::ConsumeError(ErrorData* error) {
-        ASSERT(error != nullptr);
-        HandleError(error->GetType(), error->GetMessage().c_str());
-        delete error;
-    }
-
     ResultOrError<DynamicUploader*> DeviceBase::GetDynamicUploader() const {
         if (mDynamicUploader->IsEmpty()) {
             DAWN_TRY(mDynamicUploader->CreateAndAppendBuffer());

src/dawn_native/Device.h

@@ -35,6 +35,7 @@ namespace dawn_native {
     class AdapterBase;
     class AttachmentState;
     class AttachmentStateBlueprint;
+    class ErrorScope;
     class FenceSignalTracker;
     class DynamicUploader;
     class StagingBufferBase;
@@ -45,10 +46,11 @@ namespace dawn_native {
         virtual ~DeviceBase();
 
         void HandleError(dawn::ErrorType type, const char* message);
+        void HandleError(ErrorData* error);
 
         bool ConsumedError(MaybeError maybeError) {
             if (DAWN_UNLIKELY(maybeError.IsError())) {
-                ConsumeError(maybeError.AcquireError());
+                HandleError(maybeError.AcquireError());
                 return true;
             }
             return false;
@@ -230,11 +232,13 @@ namespace dawn_native {
 
         void ApplyExtensions(const DeviceDescriptor* deviceDescriptor);
 
-        void ConsumeError(ErrorData* error);
         void SetDefaultToggles();
 
         AdapterBase* mAdapter = nullptr;
 
+        Ref<ErrorScope> mRootErrorScope;
+        Ref<ErrorScope> mCurrentErrorScope;
+
         // The object caches aren't exposed in the header as they would require a lot of
         // additional includes.
         struct Caches;
@@ -250,8 +254,6 @@ namespace dawn_native {
         std::unique_ptr<FenceSignalTracker> mFenceSignalTracker;
         std::vector<DeferredCreateBufferMappedAsync> mDeferredCreateBufferMappedAsyncResults;
 
-        dawn::ErrorCallback mErrorCallback = nullptr;
-        void* mErrorUserdata = 0;
         uint32_t mRefCount = 1;
 
         FormatTable mFormatTable;

src/dawn_native/ErrorScope.cpp

@@ -0,0 +1,114 @@
+// Copyright 2019 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "dawn_native/ErrorScope.h"
+
+#include "common/Assert.h"
+#include "dawn_native/ErrorData.h"
+
+namespace dawn_native {
+
+    ErrorScope::ErrorScope() = default;
+
+    ErrorScope::ErrorScope(dawn::ErrorFilter errorFilter, ErrorScope* parent)
+        : RefCounted(), mErrorFilter(errorFilter), mParent(parent) {
+        ASSERT(mParent.Get() != nullptr);
+    }
+
+    ErrorScope::~ErrorScope() {
+        if (mCallback == nullptr || IsRoot()) {
+            return;
+        }
+        mCallback(static_cast<DawnErrorType>(mErrorType), mErrorMessage.c_str(), mUserdata);
+    }
+
+    void ErrorScope::SetCallback(dawn::ErrorCallback callback, void* userdata) {
+        mCallback = callback;
+        mUserdata = userdata;
+    }
+
+    ErrorScope* ErrorScope::GetParent() {
+        return mParent.Get();
+    }
+
+    bool ErrorScope::IsRoot() const {
+        return mParent.Get() == nullptr;
+    }
+
+    void ErrorScope::HandleError(dawn::ErrorType type, const char* message) {
+        HandleErrorImpl(this, type, message);
+    }
+
+    void ErrorScope::HandleError(ErrorData* error) {
+        ASSERT(error != nullptr);
+        HandleErrorImpl(this, error->GetType(), error->GetMessage().c_str());
+    }
+
+    // static
+    void ErrorScope::HandleErrorImpl(ErrorScope* scope, dawn::ErrorType type, const char* message) {
+        ErrorScope* currentScope = scope;
+        for (; !currentScope->IsRoot(); currentScope = currentScope->GetParent()) {
+            ASSERT(currentScope != nullptr);
+
+            bool consumed = false;
+            switch (type) {
+                case dawn::ErrorType::Validation:
+                    if (currentScope->mErrorFilter != dawn::ErrorFilter::Validation) {
+                        // Error filter does not match. Move on to the next scope.
+                        continue;
+                    }
+                    consumed = true;
+                    break;
+
+                case dawn::ErrorType::OutOfMemory:
+                    if (currentScope->mErrorFilter != dawn::ErrorFilter::OutOfMemory) {
+                        // Error filter does not match. Move on to the next scope.
+                        continue;
+                    }
+                    consumed = true;
+                    break;
+
+                // Unknown and DeviceLost are fatal. All error scopes capture them.
+                // |consumed| is false because these should bubble to all scopes.
+                case dawn::ErrorType::Unknown:
+                case dawn::ErrorType::DeviceLost:
+                    consumed = false;
+                    break;
+
+                case dawn::ErrorType::NoError:
+                default:
+                    UNREACHABLE();
+                    return;
+            }
+
+            // Record the error if the scope doesn't have one yet.
+            if (currentScope->mErrorType == dawn::ErrorType::NoError) {
+                currentScope->mErrorType = type;
+                currentScope->mErrorMessage = message;
+            }
+
+            if (consumed) {
+                return;
+            }
+        }
+
+        // The root error scope captures all uncaptured errors.
+        ASSERT(currentScope->IsRoot());
+        if (currentScope->mCallback) {
+            currentScope->mCallback(static_cast<DawnErrorType>(type), message,
+                                    currentScope->mUserdata);
+        }
+    }
+
+}  // namespace dawn_native

src/dawn_native/ErrorScope.h

@@ -0,0 +1,68 @@
+// Copyright 2019 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNNATIVE_ERRORSCOPE_H_
+#define DAWNNATIVE_ERRORSCOPE_H_
+
+#include "dawn_native/dawn_platform.h"
+
+#include "dawn_native/RefCounted.h"
+
+#include <string>
+
+namespace dawn_native {
+
+    class ErrorData;
+
+    // Errors can be recorded into an ErrorScope by calling |HandleError|.
+    // Because an error scope should not resolve until contained
+    // commands are complete, calling the callback is deferred until it is destructed.
+    // In-flight commands or asynchronous events should hold a reference to the
+    // ErrorScope for their duration.
+    //
+    // Because parent ErrorScopes should not resolve before child ErrorScopes,
+    // ErrorScopes hold a reference to their parent.
+    //
+    // To simplify ErrorHandling, there is a sentinel root error scope which has
+    // no parent. All uncaptured errors are handled by the root error scope. Its
+    // callback is called immediately once it encounters an error.
+    class ErrorScope : public RefCounted {
+      public:
+        ErrorScope();  // Constructor for the root error scope.
+        ErrorScope(dawn::ErrorFilter errorFilter, ErrorScope* parent);
+        ~ErrorScope();
+
+        void SetCallback(dawn::ErrorCallback callback, void* userdata);
+        ErrorScope* GetParent();
+
+        void HandleError(dawn::ErrorType type, const char* message);
+        void HandleError(ErrorData* error);
+
+      private:
+        bool IsRoot() const;
+        static void HandleErrorImpl(ErrorScope* scope, dawn::ErrorType type, const char* message);
+
+        dawn::ErrorFilter mErrorFilter = dawn::ErrorFilter::None;
+        Ref<ErrorScope> mParent = nullptr;
+
+        dawn::ErrorCallback mCallback = nullptr;
+        void* mUserdata = nullptr;
+
+        dawn::ErrorType mErrorType = dawn::ErrorType::NoError;
+        std::string mErrorMessage = "";
+    };
+
+}  // namespace dawn_native
+
+#endif  // DAWNNATIVE_ERRORSCOPE_H_

src/dawn_native/RefCounted.h

@@ -120,6 +120,13 @@ namespace dawn_native {
         T* mPointee = nullptr;
     };
 
+    template <typename T>
+    Ref<T> AcquireRef(T* pointee) {
+        Ref<T> ref(pointee);
+        ref->Release();
+        return ref;
+    }
+
 }  // namespace dawn_native
 
 #endif  // DAWNNATIVE_REFCOUNTED_H_

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -330,12 +330,7 @@ namespace dawn_native { namespace vulkan {
         tempBufferDescriptor.usage = dawn::BufferUsage::CopySrc | dawn::BufferUsage::CopyDst;
 
         Device* device = ToBackend(GetDevice());
-        Ref<Buffer> tempBuffer = ToBackend(device->CreateBuffer(&tempBufferDescriptor));
+        Ref<Buffer> tempBuffer = AcquireRef(ToBackend(device->CreateBuffer(&tempBufferDescriptor)));
-        // After device->CreateBuffer(&tempBufferDescriptor) is called, the ref count of the buffer
-        // object is 1, and after assigning it to a Ref<Buffer>, the ref count of it will be 2. To
-        // prevent memory leak, we must reduce the ref count here to ensure the ref count of this
-        // object to be 0 after all the Ref<> objects that contain the buffer object are released.
-        tempBuffer->Release();
 
         BufferCopy tempBufferCopy;
         tempBufferCopy.buffer = tempBuffer.Get();

src/tests/unittests/validation/ErrorScopeValidationTests.cpp

@@ -0,0 +1,134 @@
+// Copyright 2019 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tests/unittests/validation/ValidationTest.h"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+
+class MockDevicePopErrorScopeCallback {
+  public:
+    MOCK_METHOD3(Call, void(DawnErrorType type, const char* message, void* userdata));
+};
+
+static std::unique_ptr<MockDevicePopErrorScopeCallback> mockDevicePopErrorScopeCallback;
+static void ToMockDevicePopErrorScopeCallback(DawnErrorType type,
+                                              const char* message,
+                                              void* userdata) {
+    mockDevicePopErrorScopeCallback->Call(type, message, userdata);
+}
+
+class ErrorScopeValidationTest : public ValidationTest {
+  private:
+    void SetUp() override {
+        ValidationTest::SetUp();
+        mockDevicePopErrorScopeCallback = std::make_unique<MockDevicePopErrorScopeCallback>();
+    }
+
+    void TearDown() override {
+        // Delete mocks so that expectations are checked
+        mockDevicePopErrorScopeCallback = nullptr;
+        ValidationTest::TearDown();
+    }
+};
+
+// Test the simple success case.
+TEST_F(ErrorScopeValidationTest, Success) {
+    device.PushErrorScope(dawn::ErrorFilter::Validation);
+
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this);
+}
+
+// Test the simple case where the error scope catches an error.
+TEST_F(ErrorScopeValidationTest, CatchesError) {
+    device.PushErrorScope(dawn::ErrorFilter::Validation);
+
+    dawn::BufferDescriptor desc = {};
+    desc.usage = static_cast<dawn::BufferUsage>(DAWN_BUFFER_USAGE_FORCE32);
+    device.CreateBuffer(&desc);
+
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this))
+        .Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this);
+}
+
+// Test that errors bubble to the parent scope if not handled by the current scope.
+TEST_F(ErrorScopeValidationTest, ErrorBubbles) {
+    device.PushErrorScope(dawn::ErrorFilter::Validation);
+    device.PushErrorScope(dawn::ErrorFilter::OutOfMemory);
+
+    dawn::BufferDescriptor desc = {};
+    desc.usage = static_cast<dawn::BufferUsage>(DAWN_BUFFER_USAGE_FORCE32);
+    device.CreateBuffer(&desc);
+
+    // OutOfMemory does not match Validation error.
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this);
+
+    // Parent validation error scope captures the error.
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this + 1))
+        .Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 1);
+}
+
+// Test that if an error scope matches an error, it does not bubble to the parent scope.
+TEST_F(ErrorScopeValidationTest, HandledErrorsStopBubbling) {
+    device.PushErrorScope(dawn::ErrorFilter::OutOfMemory);
+    device.PushErrorScope(dawn::ErrorFilter::Validation);
+
+    dawn::BufferDescriptor desc = {};
+    desc.usage = static_cast<dawn::BufferUsage>(DAWN_BUFFER_USAGE_FORCE32);
+    device.CreateBuffer(&desc);
+
+    // Inner scope catches the error.
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this))
+        .Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this);
+
+    // Parent scope does not see the error.
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this + 1))
+        .Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 1);
+}
+
+// Test that if no error scope handles an error, it goes to the device UncapturedError callback
+TEST_F(ErrorScopeValidationTest, UnhandledErrorsMatchUncapturedErrorCallback) {
+    device.PushErrorScope(dawn::ErrorFilter::OutOfMemory);
+
+    dawn::BufferDescriptor desc = {};
+    desc.usage = static_cast<dawn::BufferUsage>(DAWN_BUFFER_USAGE_FORCE32);
+    ASSERT_DEVICE_ERROR(device.CreateBuffer(&desc));
+
+    EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1);
+    device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this);
+}
+
+// Check that push/popping error scopes must be balanced.
+TEST_F(ErrorScopeValidationTest, PushPopBalanced) {
+    // No error scopes to pop.
+    { EXPECT_FALSE(device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this)); }
+
+    // Too many pops
+    {
+        device.PushErrorScope(dawn::ErrorFilter::Validation);
+
+        EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this + 1))
+            .Times(1);
+        device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 1);
+
+        EXPECT_FALSE(device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 2));
+    }
+}