From 0ae00a187d6d4bd06ea622a776e26e2df9aceecc Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 28 Mar 2019 17:12:47 +0000 Subject: [PATCH] Nuke Builders Part 1: remove the testing BufferBuilder. This requires deleting wire tests for builders that were using it, and leads to small simplifications in the WireTest harness. Also allows removing the BuilderBase class from dawn_native. BUG=dawn:125 Change-Id: I3cbac609207aa652cdc9d37e0b700cce3ac6e093 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6120 Reviewed-by: Kai Ninomiya Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez --- BUILD.gn | 4 +- dawn.json | 20 -- generator/templates/mock_api.cpp | 15 -- generator/templates/mock_api.h | 9 - src/dawn_native/Buffer.h | 13 - src/dawn_native/Builder.cpp | 93 ------- src/dawn_native/Builder.h | 105 -------- src/dawn_native/Device.h | 5 - src/dawn_native/Pipeline.h | 1 - src/dawn_native/Queue.h | 1 - src/dawn_native/ShaderModule.h | 1 - src/dawn_native/SwapChain.h | 1 - src/dawn_native/Texture.h | 1 - .../unittests/wire/WireArgumentTests.cpp | 2 +- src/tests/unittests/wire/WireBasicTests.cpp | 2 +- .../unittests/wire/WireBufferMappingTests.cpp | 2 +- .../unittests/wire/WireCallbackTests.cpp | 251 ------------------ .../unittests/wire/WireErrorCallbackTests.cpp | 75 ++++++ src/tests/unittests/wire/WireFenceTests.cpp | 2 +- .../unittests/wire/WireInjectTextureTests.cpp | 2 +- .../unittests/wire/WireOptionalTests.cpp | 2 +- src/tests/unittests/wire/WireTest.cpp | 5 +- src/tests/unittests/wire/WireTest.h | 3 +- 23 files changed, 84 insertions(+), 531 deletions(-) delete mode 100644 src/dawn_native/Builder.cpp delete mode 100644 src/dawn_native/Builder.h delete mode 100644 src/tests/unittests/wire/WireCallbackTests.cpp create mode 100644 src/tests/unittests/wire/WireErrorCallbackTests.cpp diff --git a/BUILD.gn b/BUILD.gn index b69343a884..836ed1e674 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -92,8 +92,6 @@ source_set("libdawn_native_sources") { "src/dawn_native/BindGroupLayout.h", "src/dawn_native/Buffer.cpp", "src/dawn_native/Buffer.h", - "src/dawn_native/Builder.cpp", - "src/dawn_native/Builder.h", "src/dawn_native/CommandAllocator.cpp", "src/dawn_native/CommandAllocator.h", "src/dawn_native/CommandBuffer.cpp", @@ -592,7 +590,7 @@ test("dawn_unittests") { "src/tests/unittests/wire/WireArgumentTests.cpp", "src/tests/unittests/wire/WireBasicTests.cpp", "src/tests/unittests/wire/WireBufferMappingTests.cpp", - "src/tests/unittests/wire/WireCallbackTests.cpp", + "src/tests/unittests/wire/WireErrorCallbackTests.cpp", "src/tests/unittests/wire/WireFenceTests.cpp", "src/tests/unittests/wire/WireInjectTextureTests.cpp", "src/tests/unittests/wire/WireOptionalTests.cpp", diff --git a/dawn.json b/dawn.json index d0c7e2616b..dcb6392c9a 100644 --- a/dawn.json +++ b/dawn.json @@ -181,22 +181,6 @@ } ] }, - "buffer builder": { - "_comment": "This builder is kept for testing only", - "category": "object", - "methods": [ - { - "name": "get result", - "returns": "buffer" - }, - { - "name": "set size", - "args": [ - {"name": "size", "type": "uint32_t"} - ] - } - ] - }, "buffer copy view": { "category": "structure", "extensible": true, @@ -440,10 +424,6 @@ {"name": "descriptor", "type": "buffer descriptor", "annotation": "const*"} ] }, - { - "name": "create buffer builder for testing", - "returns": "buffer builder" - }, { "name": "create command encoder", "returns": "command encoder" diff --git a/generator/templates/mock_api.cpp b/generator/templates/mock_api.cpp index 5c9b87006f..7e6982e5ea 100644 --- a/generator/templates/mock_api.cpp +++ b/generator/templates/mock_api.cpp @@ -89,10 +89,6 @@ void ProcTableAsClass::CallDeviceErrorCallback(DawnDevice device, const char* me auto object = reinterpret_cast(device); object->deviceErrorCallback(message, object->userdata1); } -void ProcTableAsClass::CallBuilderErrorCallback(void* builder , DawnBuilderErrorStatus status, const char* message) { - auto object = reinterpret_cast(builder); - object->builderErrorCallback(status, message, object->userdata1, object->userdata2); -} void ProcTableAsClass::CallMapReadCallback(DawnBuffer buffer, DawnBufferMapAsyncStatus status, const void* data, uint32_t dataLength) { auto object = reinterpret_cast(buffer); object->mapReadCallback(status, data, dataLength, object->userdata1); @@ -109,17 +105,6 @@ void ProcTableAsClass::CallFenceOnCompletionCallback(DawnFence fence, object->fenceOnCompletionCallback(status, object->userdata1); } -{% for type in by_category["object"] if type.is_builder %} - void ProcTableAsClass::{{as_MethodSuffix(type.name, Name("set error callback"))}}({{as_cType(type.name)}} self, DawnBuilderErrorCallback callback, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2) { - auto object = reinterpret_cast(self); - object->builderErrorCallback = callback; - object->userdata1 = userdata1; - object->userdata2 = userdata2; - - OnBuilderSetErrorCallback(reinterpret_cast(self), callback, userdata1, userdata2); - } -{% endfor %} - {% for type in by_category["object"] %} {{as_cType(type.name)}} ProcTableAsClass::GetNew{{type.name.CamelCase()}}() { mObjects.emplace_back(new Object); diff --git a/generator/templates/mock_api.h b/generator/templates/mock_api.h index 198d5677de..e3675f6d1d 100644 --- a/generator/templates/mock_api.h +++ b/generator/templates/mock_api.h @@ -48,11 +48,6 @@ class ProcTableAsClass { {% endfor %} virtual void {{as_MethodSuffix(type.name, Name("reference"))}}({{as_cType(type.name)}} self) = 0; virtual void {{as_MethodSuffix(type.name, Name("release"))}}({{as_cType(type.name)}} self) = 0; - - // Stores callback and userdata and calls OnBuilderSetErrorCallback - {% if type.is_builder %} - void {{as_MethodSuffix(type.name, Name("set error callback"))}}({{as_cType(type.name)}} self, DawnBuilderErrorCallback callback, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2); - {% endif %} {% endfor %} // Stores callback and userdata and calls the On* methods @@ -66,7 +61,6 @@ class ProcTableAsClass { // Special cased mockable methods virtual void OnDeviceSetErrorCallback(DawnDevice device, DawnDeviceErrorCallback callback, DawnCallbackUserdata userdata) = 0; - virtual void OnBuilderSetErrorCallback(DawnBufferBuilder builder, DawnBuilderErrorCallback callback, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2) = 0; virtual void OnBufferMapReadAsyncCallback(DawnBuffer buffer, DawnBufferMapReadCallback callback, DawnCallbackUserdata userdata) = 0; virtual void OnBufferMapWriteAsyncCallback(DawnBuffer buffer, DawnBufferMapWriteCallback callback, DawnCallbackUserdata userdata) = 0; virtual void OnFenceOnCompletionCallback(DawnFence fence, @@ -76,7 +70,6 @@ class ProcTableAsClass { // Calls the stored callbacks void CallDeviceErrorCallback(DawnDevice device, const char* message); - void CallBuilderErrorCallback(void* builder , DawnBuilderErrorStatus status, const char* message); void CallMapReadCallback(DawnBuffer buffer, DawnBufferMapAsyncStatus status, const void* data, uint32_t dataLength); void CallMapWriteCallback(DawnBuffer buffer, DawnBufferMapAsyncStatus status, void* data, uint32_t dataLength); void CallFenceOnCompletionCallback(DawnFence fence, DawnFenceCompletionStatus status); @@ -84,7 +77,6 @@ class ProcTableAsClass { struct Object { ProcTableAsClass* procs = nullptr; DawnDeviceErrorCallback deviceErrorCallback = nullptr; - DawnBuilderErrorCallback builderErrorCallback = nullptr; DawnBufferMapReadCallback mapReadCallback = nullptr; DawnBufferMapWriteCallback mapWriteCallback = nullptr; DawnFenceOnCompletionCallback fenceOnCompletionCallback = nullptr; @@ -120,7 +112,6 @@ class MockProcTable : public ProcTableAsClass { {% endfor %} MOCK_METHOD3(OnDeviceSetErrorCallback, void(DawnDevice device, DawnDeviceErrorCallback callback, DawnCallbackUserdata userdata)); - MOCK_METHOD4(OnBuilderSetErrorCallback, void(DawnBufferBuilder builder, DawnBuilderErrorCallback callback, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2)); MOCK_METHOD3(OnBufferMapReadAsyncCallback, void(DawnBuffer buffer, DawnBufferMapReadCallback callback, DawnCallbackUserdata userdata)); MOCK_METHOD3(OnBufferMapWriteAsyncCallback, void(DawnBuffer buffer, DawnBufferMapWriteCallback callback, DawnCallbackUserdata userdata)); MOCK_METHOD4(OnFenceOnCompletionCallback, diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h index 9c3d40281f..7ad78a950e 100644 --- a/src/dawn_native/Buffer.h +++ b/src/dawn_native/Buffer.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_BUFFER_H_ #define DAWNNATIVE_BUFFER_H_ -#include "dawn_native/Builder.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" @@ -94,18 +93,6 @@ namespace dawn_native { BufferState mState; }; - // This builder class is kept around purely for testing but should not be used. - class BufferBuilder : public Builder { - public: - BufferBuilder(DeviceBase* device) : Builder(device) { - UNREACHABLE(); - } - - void SetSize(uint32_t) { - UNREACHABLE(); - } - }; - } // namespace dawn_native #endif // DAWNNATIVE_BUFFER_H_ diff --git a/src/dawn_native/Builder.cpp b/src/dawn_native/Builder.cpp deleted file mode 100644 index 8c77632d46..0000000000 --- a/src/dawn_native/Builder.cpp +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2017 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/Builder.h" - -#include "common/Assert.h" -#include "dawn_native/Device.h" - -namespace dawn_native { - - bool BuilderBase::CanBeUsed() const { - return !mIsConsumed && !mGotStatus; - } - - void BuilderBase::HandleError(const char* message) { - SetStatus(dawn::BuilderErrorStatus::Error, message); - } - - void BuilderBase::SetErrorCallback(dawn::BuilderErrorCallback callback, - dawn::CallbackUserdata userdata1, - dawn::CallbackUserdata userdata2) { - mCallback = callback; - mUserdata1 = userdata1; - mUserdata2 = userdata2; - } - - BuilderBase::BuilderBase(DeviceBase* device) : ObjectBase(device) { - } - - BuilderBase::~BuilderBase() { - if (!mIsConsumed && mCallback != nullptr) { - mCallback(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, "Builder destroyed before GetResult", - mUserdata1, mUserdata2); - } - } - - void BuilderBase::SetStatus(dawn::BuilderErrorStatus status, const char* message) { - ASSERT(status != dawn::BuilderErrorStatus::Success); - ASSERT(status != dawn::BuilderErrorStatus::Unknown); - ASSERT(!mGotStatus); // This is not strictly necessary but something to strive for. - mGotStatus = true; - - mStoredStatus = status; - mStoredMessage = message; - } - - bool BuilderBase::HandleResult(ObjectBase* result) { - // GetResult can only be called once. - ASSERT(!mIsConsumed); - mIsConsumed = true; - - // result == nullptr implies there was an error which implies we should have a status set. - ASSERT(result != nullptr || mGotStatus); - - // If we have any error, then we have to return nullptr - if (mGotStatus) { - ASSERT(mStoredStatus != dawn::BuilderErrorStatus::Success); - - // The application will never see "result" so we need to remove the - // external ref here. - if (result != nullptr) { - result->Release(); - result = nullptr; - } - - // Unhandled builder errors are promoted to device errors - if (!mCallback) - GetDevice()->HandleError(("Unhandled builder error: " + mStoredMessage).c_str()); - } else { - ASSERT(mStoredStatus == dawn::BuilderErrorStatus::Success); - ASSERT(mStoredMessage.empty()); - } - - if (mCallback != nullptr) { - mCallback(static_cast(mStoredStatus), mStoredMessage.c_str(), - mUserdata1, mUserdata2); - } - - return result != nullptr; - } - -} // namespace dawn_native diff --git a/src/dawn_native/Builder.h b/src/dawn_native/Builder.h deleted file mode 100644 index 19fef8e2bc..0000000000 --- a/src/dawn_native/Builder.h +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright 2017 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_BUILDER_H_ -#define DAWNNATIVE_BUILDER_H_ - -#include "dawn_native/Forward.h" -#include "dawn_native/ObjectBase.h" - -#include "dawn_native/dawn_platform.h" - -#include - -namespace dawn_native { - - // This class implements behavior shared by all builders: - // - Tracking whether GetResult has been called already, needed by the autogenerated code to - // prevent operations on "consumed" builders. - // - The error status callback of the API. The callback is guaranteed to be called exactly once - // with an error, a success, or "unknown" if the builder is destroyed; also the builder - // callback cannot be called before either the object is destroyed or GetResult is called. - // - // It is possible for error to be generated before the error callback is registered when a - // builder "set" function performance validation inline. Because of this we have to store the - // status in the builder and defer calling the callback to GetResult. - - class BuilderBase : public ObjectBase { - public: - // Used by the auto-generated validation to prevent usage of the builder - // after GetResult or an error. - bool CanBeUsed() const; - - // Set the status of the builder to an error. - void HandleError(const char* message); - - // Internal API, to be used by builder and BackendProcTable only. - // Returns true for success cases, and calls the callback with appropriate status. - bool HandleResult(ObjectBase* result); - - // Dawn API - void SetErrorCallback(dawn::BuilderErrorCallback callback, - dawn::CallbackUserdata userdata1, - dawn::CallbackUserdata userdata2); - - protected: - BuilderBase(DeviceBase* device); - ~BuilderBase(); - - bool mGotStatus = false; - - private: - void SetStatus(dawn::BuilderErrorStatus status, const char* message); - - dawn::BuilderErrorCallback mCallback = nullptr; - dawn::CallbackUserdata mUserdata1 = 0; - dawn::CallbackUserdata mUserdata2 = 0; - - dawn::BuilderErrorStatus mStoredStatus = dawn::BuilderErrorStatus::Success; - std::string mStoredMessage; - - bool mIsConsumed = false; - }; - - // This builder base class is used to capture the calls to GetResult and make sure that either: - // - There was an error, callback is called with an error and nullptr is returned. - // - There was no error, callback is called with success and a non-null T* is returned. - template - class Builder : public BuilderBase { - public: - // Dawn API - T* GetResult(); - - protected: - using BuilderBase::BuilderBase; - - private: - virtual T* GetResultImpl() = 0; - }; - - template - T* Builder::GetResult() { - T* result = GetResultImpl(); - // An object can have been returned but failed its initialization, so if an error happened, - // return nullptr instead of result. - if (HandleResult(result)) { - return result; - } else { - return nullptr; - } - } - -} // namespace dawn_native - -#endif // DAWNNATIVE_BUILDER_H_ diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index 188dd6c365..82b01b5e8e 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -30,7 +30,6 @@ namespace dawn_native { using ErrorCallback = void (*)(const char* errorMessage, void* userData); class AdapterBase; - class BufferBuilder; class FenceSignalTracker; class DynamicUploader; class StagingBufferBase; @@ -106,10 +105,6 @@ namespace dawn_native { void Reference(); void Release(); - BufferBuilder* CreateBufferBuilderForTesting() { - return nullptr; - } - virtual ResultOrError> CreateStagingBuffer( size_t size) = 0; virtual MaybeError CopyFromStagingToBuffer(StagingBufferBase* source, diff --git a/src/dawn_native/Pipeline.h b/src/dawn_native/Pipeline.h index cc1c542833..1f141f9906 100644 --- a/src/dawn_native/Pipeline.h +++ b/src/dawn_native/Pipeline.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_PIPELINE_H_ #define DAWNNATIVE_PIPELINE_H_ -#include "dawn_native/Builder.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" #include "dawn_native/PerStage.h" diff --git a/src/dawn_native/Queue.h b/src/dawn_native/Queue.h index 3da2f0220c..7b1031eab5 100644 --- a/src/dawn_native/Queue.h +++ b/src/dawn_native/Queue.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_QUEUE_H_ #define DAWNNATIVE_QUEUE_H_ -#include "dawn_native/Builder.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index dbeeb7be27..b8020f9b7b 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -16,7 +16,6 @@ #define DAWNNATIVE_SHADERMODULE_H_ #include "common/Constants.h" -#include "dawn_native/Builder.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" diff --git a/src/dawn_native/SwapChain.h b/src/dawn_native/SwapChain.h index a17fc0c067..c8479c6a15 100644 --- a/src/dawn_native/SwapChain.h +++ b/src/dawn_native/SwapChain.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_SWAPCHAIN_H_ #define DAWNNATIVE_SWAPCHAIN_H_ -#include "dawn_native/Builder.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index db613cf98a..af91a639bd 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_TEXTURE_H_ #define DAWNNATIVE_TEXTURE_H_ -#include "dawn_native/Builder.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/ObjectBase.h" diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index 8ed66c8e28..10dbd89356 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/src/tests/unittests/wire/WireArgumentTests.cpp @@ -21,7 +21,7 @@ using namespace dawn_wire; class WireArgumentTests : public WireTest { public: - WireArgumentTests() : WireTest(true) { + WireArgumentTests() { } ~WireArgumentTests() override = default; }; diff --git a/src/tests/unittests/wire/WireBasicTests.cpp b/src/tests/unittests/wire/WireBasicTests.cpp index b7c8e29792..38bff0bc7c 100644 --- a/src/tests/unittests/wire/WireBasicTests.cpp +++ b/src/tests/unittests/wire/WireBasicTests.cpp @@ -19,7 +19,7 @@ using namespace dawn_wire; class WireBasicTests : public WireTest { public: - WireBasicTests() : WireTest(true) { + WireBasicTests() { } ~WireBasicTests() override = default; }; diff --git a/src/tests/unittests/wire/WireBufferMappingTests.cpp b/src/tests/unittests/wire/WireBufferMappingTests.cpp index 6a3126a967..e1037ff541 100644 --- a/src/tests/unittests/wire/WireBufferMappingTests.cpp +++ b/src/tests/unittests/wire/WireBufferMappingTests.cpp @@ -63,7 +63,7 @@ namespace { class WireBufferMappingTests : public WireTest { public: - WireBufferMappingTests() : WireTest(true) { + WireBufferMappingTests() { } ~WireBufferMappingTests() override = default; diff --git a/src/tests/unittests/wire/WireCallbackTests.cpp b/src/tests/unittests/wire/WireCallbackTests.cpp deleted file mode 100644 index 75258397a2..0000000000 --- a/src/tests/unittests/wire/WireCallbackTests.cpp +++ /dev/null @@ -1,251 +0,0 @@ -// 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/wire/WireTest.h" - -using namespace testing; -using namespace dawn_wire; - -namespace { - - // Mock classes to add expectations on the wire calling callbacks - class MockDeviceErrorCallback { - public: - MOCK_METHOD2(Call, void(const char* message, DawnCallbackUserdata userdata)); - }; - - std::unique_ptr> mockDeviceErrorCallback; - void ToMockDeviceErrorCallback(const char* message, DawnCallbackUserdata userdata) { - mockDeviceErrorCallback->Call(message, userdata); - } - - class MockBuilderErrorCallback { - public: - MOCK_METHOD4(Call, - void(DawnBuilderErrorStatus status, - const char* message, - DawnCallbackUserdata userdata1, - DawnCallbackUserdata userdata2)); - }; - - std::unique_ptr> mockBuilderErrorCallback; - void ToMockBuilderErrorCallback(DawnBuilderErrorStatus status, - const char* message, - DawnCallbackUserdata userdata1, - DawnCallbackUserdata userdata2) { - mockBuilderErrorCallback->Call(status, message, userdata1, userdata2); - } - -} // anonymous namespace - -class WireCallbackTests : public WireTest { - public: - WireCallbackTests() : WireTest(true) { - } - ~WireCallbackTests() override = default; - - void SetUp() override { - WireTest::SetUp(); - - mockDeviceErrorCallback = std::make_unique>(); - mockBuilderErrorCallback = std::make_unique>(); - } - - void TearDown() override { - WireTest::TearDown(); - - mockDeviceErrorCallback = nullptr; - mockBuilderErrorCallback = nullptr; - } - - void FlushServer() { - WireTest::FlushServer(); - - Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback); - Mock::VerifyAndClearExpectations(&mockBuilderErrorCallback); - } -}; - -// Test that we get a success builder error status when no error happens -TEST_F(WireCallbackTests, SuccessCallbackOnBuilderSuccess) { - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderSetErrorCallback(bufferBuilder, ToMockBuilderErrorCallback, 1, 2); - dawnBufferBuilderGetResult(bufferBuilder); - - DawnBufferBuilder apiBufferBuilder = api.GetNewBufferBuilder(); - EXPECT_CALL(api, DeviceCreateBufferBuilderForTesting(apiDevice)) - .WillOnce(Return(apiBufferBuilder)); - - DawnBuffer apiBuffer = api.GetNewBuffer(); - EXPECT_CALL(api, BufferBuilderGetResult(apiBufferBuilder)) - .WillOnce(InvokeWithoutArgs([&]() -> DawnBuffer { - api.CallBuilderErrorCallback(apiBufferBuilder, DAWN_BUILDER_ERROR_STATUS_SUCCESS, - "I like cheese"); - return apiBuffer; - })); - - FlushClient(); - - EXPECT_CALL(*mockBuilderErrorCallback, Call(DAWN_BUILDER_ERROR_STATUS_SUCCESS, _, 1, 2)); - - FlushServer(); -} - -// Test that the client calls the builder callback with unknown when it HAS to fire the callback but -// can't know the status yet. -TEST_F(WireCallbackTests, UnknownBuilderErrorStatusCallback) { - // The builder is destroyed before the object is built - { - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderSetErrorCallback(bufferBuilder, ToMockBuilderErrorCallback, 1, 2); - - EXPECT_CALL(*mockBuilderErrorCallback, Call(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, _, 1, 2)) - .Times(1); - - dawnBufferBuilderRelease(bufferBuilder); - } - - // If the builder has been consumed, it doesn't fire the callback with unknown - { - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderSetErrorCallback(bufferBuilder, ToMockBuilderErrorCallback, 3, 4); - dawnBufferBuilderGetResult(bufferBuilder); - - EXPECT_CALL(*mockBuilderErrorCallback, Call(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, _, 3, 4)) - .Times(0); - - dawnBufferBuilderRelease(bufferBuilder); - } - - // If the builder has been consumed, and the object is destroyed before the result comes from - // the server, then the callback is fired with unknown - { - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderSetErrorCallback(bufferBuilder, ToMockBuilderErrorCallback, 5, 6); - DawnBuffer buffer = dawnBufferBuilderGetResult(bufferBuilder); - - EXPECT_CALL(*mockBuilderErrorCallback, Call(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, _, 5, 6)) - .Times(1); - - dawnBufferRelease(buffer); - } -} - -// Test that a builder success status doesn't get forwarded to the device -TEST_F(WireCallbackTests, SuccessCallbackNotForwardedToDevice) { - dawnDeviceSetErrorCallback(device, ToMockDeviceErrorCallback, 0); - - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderGetResult(bufferBuilder); - - DawnBufferBuilder apiBufferBuilder = api.GetNewBufferBuilder(); - EXPECT_CALL(api, DeviceCreateBufferBuilderForTesting(apiDevice)) - .WillOnce(Return(apiBufferBuilder)); - - DawnBuffer apiBuffer = api.GetNewBuffer(); - EXPECT_CALL(api, BufferBuilderGetResult(apiBufferBuilder)) - .WillOnce(InvokeWithoutArgs([&]() -> DawnBuffer { - api.CallBuilderErrorCallback(apiBufferBuilder, DAWN_BUILDER_ERROR_STATUS_SUCCESS, - "I like cheese"); - return apiBuffer; - })); - - FlushClient(); - FlushServer(); -} - -// Test that a builder error status gets forwarded to the device -TEST_F(WireCallbackTests, ErrorCallbackForwardedToDevice) { - uint64_t userdata = 30495; - dawnDeviceSetErrorCallback(device, ToMockDeviceErrorCallback, userdata); - - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - dawnBufferBuilderGetResult(bufferBuilder); - - DawnBufferBuilder apiBufferBuilder = api.GetNewBufferBuilder(); - EXPECT_CALL(api, DeviceCreateBufferBuilderForTesting(apiDevice)) - .WillOnce(Return(apiBufferBuilder)); - - EXPECT_CALL(api, BufferBuilderGetResult(apiBufferBuilder)) - .WillOnce(InvokeWithoutArgs([&]() -> DawnBuffer { - api.CallBuilderErrorCallback(apiBufferBuilder, DAWN_BUILDER_ERROR_STATUS_ERROR, - "Error :("); - return nullptr; - })); - - FlushClient(); - - EXPECT_CALL(*mockDeviceErrorCallback, Call(_, userdata)).Times(1); - - FlushServer(); -} - -// Test the return wire for device error callbacks -TEST_F(WireCallbackTests, DeviceErrorCallback) { - uint64_t userdata = 3049785; - dawnDeviceSetErrorCallback(device, ToMockDeviceErrorCallback, userdata); - - // Setting the error callback should stay on the client side and do nothing - FlushClient(); - - // Calling the callback on the server side will result in the callback being called on the - // client side - api.CallDeviceErrorCallback(apiDevice, "Some error message"); - - EXPECT_CALL(*mockDeviceErrorCallback, Call(StrEq("Some error message"), userdata)).Times(1); - - FlushServer(); -} - -// Test the return wire for device error callbacks -TEST_F(WireCallbackTests, BuilderErrorCallback) { - uint64_t userdata1 = 982734; - uint64_t userdata2 = 982734239028; - - // Create the buffer builder, the callback is set immediately on the server side - DawnBufferBuilder bufferBuilder = dawnDeviceCreateBufferBuilderForTesting(device); - - DawnBufferBuilder apiBufferBuilder = api.GetNewBufferBuilder(); - EXPECT_CALL(api, DeviceCreateBufferBuilderForTesting(apiDevice)) - .WillOnce(Return(apiBufferBuilder)); - - EXPECT_CALL(api, OnBuilderSetErrorCallback(apiBufferBuilder, _, _, _)).Times(1); - - FlushClient(); - - // Setting the callback on the client side doesn't do anything on the server side - dawnBufferBuilderSetErrorCallback(bufferBuilder, ToMockBuilderErrorCallback, userdata1, - userdata2); - FlushClient(); - - // Create an object so that it is a valid case to call the error callback - dawnBufferBuilderGetResult(bufferBuilder); - - DawnBuffer apiBuffer = api.GetNewBuffer(); - EXPECT_CALL(api, BufferBuilderGetResult(apiBufferBuilder)) - .WillOnce(InvokeWithoutArgs([&]() -> DawnBuffer { - api.CallBuilderErrorCallback(apiBufferBuilder, DAWN_BUILDER_ERROR_STATUS_SUCCESS, - "Success!"); - return apiBuffer; - })); - - FlushClient(); - - // The error callback gets called on the client side - EXPECT_CALL(*mockBuilderErrorCallback, - Call(DAWN_BUILDER_ERROR_STATUS_SUCCESS, StrEq("Success!"), userdata1, userdata2)) - .Times(1); - - FlushServer(); -} diff --git a/src/tests/unittests/wire/WireErrorCallbackTests.cpp b/src/tests/unittests/wire/WireErrorCallbackTests.cpp new file mode 100644 index 0000000000..d92b83c742 --- /dev/null +++ b/src/tests/unittests/wire/WireErrorCallbackTests.cpp @@ -0,0 +1,75 @@ +// 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/wire/WireTest.h" + +using namespace testing; +using namespace dawn_wire; + +namespace { + + // Mock classes to add expectations on the wire calling callbacks + class MockDeviceErrorCallback { + public: + MOCK_METHOD2(Call, void(const char* message, DawnCallbackUserdata userdata)); + }; + + std::unique_ptr> mockDeviceErrorCallback; + void ToMockDeviceErrorCallback(const char* message, DawnCallbackUserdata userdata) { + mockDeviceErrorCallback->Call(message, userdata); + } + +} // anonymous namespace + +class WireErrorCallbackTests : public WireTest { + public: + WireErrorCallbackTests() { + } + ~WireErrorCallbackTests() override = default; + + void SetUp() override { + WireTest::SetUp(); + + mockDeviceErrorCallback = std::make_unique>(); + } + + void TearDown() override { + WireTest::TearDown(); + + mockDeviceErrorCallback = nullptr; + } + + void FlushServer() { + WireTest::FlushServer(); + + Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback); + } +}; + +// Test the return wire for device error callbacks +TEST_F(WireErrorCallbackTests, DeviceErrorCallback) { + uint64_t userdata = 3049785; + dawnDeviceSetErrorCallback(device, ToMockDeviceErrorCallback, userdata); + + // Setting the error callback should stay on the client side and do nothing + FlushClient(); + + // Calling the callback on the server side will result in the callback being called on the + // client side + api.CallDeviceErrorCallback(apiDevice, "Some error message"); + + EXPECT_CALL(*mockDeviceErrorCallback, Call(StrEq("Some error message"), userdata)).Times(1); + + FlushServer(); +} diff --git a/src/tests/unittests/wire/WireFenceTests.cpp b/src/tests/unittests/wire/WireFenceTests.cpp index a5275a32d1..f8d415f2f9 100644 --- a/src/tests/unittests/wire/WireFenceTests.cpp +++ b/src/tests/unittests/wire/WireFenceTests.cpp @@ -45,7 +45,7 @@ namespace { class WireFenceTests : public WireTest { public: - WireFenceTests() : WireTest(true) { + WireFenceTests() { } ~WireFenceTests() override = default; diff --git a/src/tests/unittests/wire/WireInjectTextureTests.cpp b/src/tests/unittests/wire/WireInjectTextureTests.cpp index 7c3a83bd2f..b8e0cbe7fd 100644 --- a/src/tests/unittests/wire/WireInjectTextureTests.cpp +++ b/src/tests/unittests/wire/WireInjectTextureTests.cpp @@ -22,7 +22,7 @@ using namespace dawn_wire; class WireInjectTextureTests : public WireTest { public: - WireInjectTextureTests() : WireTest(true) { + WireInjectTextureTests() { } ~WireInjectTextureTests() override = default; }; diff --git a/src/tests/unittests/wire/WireOptionalTests.cpp b/src/tests/unittests/wire/WireOptionalTests.cpp index 59b5206dbf..55b67ed727 100644 --- a/src/tests/unittests/wire/WireOptionalTests.cpp +++ b/src/tests/unittests/wire/WireOptionalTests.cpp @@ -19,7 +19,7 @@ using namespace dawn_wire; class WireOptionalTests : public WireTest { public: - WireOptionalTests() : WireTest(true) { + WireOptionalTests() { } ~WireOptionalTests() override = default; }; diff --git a/src/tests/unittests/wire/WireTest.cpp b/src/tests/unittests/wire/WireTest.cpp index 7bc94e0b3d..94aa7cde47 100644 --- a/src/tests/unittests/wire/WireTest.cpp +++ b/src/tests/unittests/wire/WireTest.cpp @@ -21,7 +21,7 @@ using namespace testing; using namespace dawn_wire; -WireTest::WireTest(bool ignoreSetCallbackCalls) : mIgnoreSetCallbackCalls(ignoreSetCallbackCalls) { +WireTest::WireTest() { } WireTest::~WireTest() { @@ -87,8 +87,5 @@ void WireTest::DeleteServer() { } void WireTest::SetupIgnoredCallExpectations() { - if (mIgnoreSetCallbackCalls) { - EXPECT_CALL(api, OnBuilderSetErrorCallback(_, _, _, _)).Times(AnyNumber()); - } EXPECT_CALL(api, DeviceTick(_)).Times(AnyNumber()); } diff --git a/src/tests/unittests/wire/WireTest.h b/src/tests/unittests/wire/WireTest.h index 3d3243766b..3173292acf 100644 --- a/src/tests/unittests/wire/WireTest.h +++ b/src/tests/unittests/wire/WireTest.h @@ -77,7 +77,7 @@ namespace utils { class WireTest : public testing::Test { protected: - WireTest(bool ignoreSetCallbackCalls); + WireTest(); ~WireTest() override; void SetUp() override; @@ -96,7 +96,6 @@ class WireTest : public testing::Test { private: void SetupIgnoredCallExpectations(); - bool mIgnoreSetCallbackCalls = false; std::unique_ptr mWireServer; std::unique_ptr mWireClient;