Improve Memory Management of Result class

The way in which the Result class is used in Dawn can be fragile
with respect to memory management because the caller of AcquireError
must know they need to delete the returned pointer or a memory leak
will occur. We've had a couple of instances where developers have
accidentally left out the delete call and managed to get past code
review.

This CL changes the Result class so that it assumes the error is
allocated on the heap and forces the caller to use unique_ptr when
calling AcquireError.

Bug:dawn:320
Change-Id: I13ec953b0c37eaafbd6ce93c2f719b4743676acb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14960
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
diff --git a/BUILD.gn b/BUILD.gn
index 5287bee..c7df582 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -204,7 +204,6 @@
     "src/dawn_native/DynamicUploader.h",
     "src/dawn_native/EncodingContext.cpp",
     "src/dawn_native/EncodingContext.h",
-    "src/dawn_native/Error.cpp",
     "src/dawn_native/Error.h",
     "src/dawn_native/ErrorData.cpp",
     "src/dawn_native/ErrorData.h",
diff --git a/src/common/Result.h b/src/common/Result.h
index 3e33052..7dc5e2d 100644
--- a/src/common/Result.h
+++ b/src/common/Result.h
@@ -20,6 +20,7 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <memory>
 #include <type_traits>
 #include <utility>
 
@@ -38,10 +39,10 @@
 template <typename T, typename E>
 class Result;
 
-// The interface of Result<T, E> shoud look like the following.
+// The interface of Result<T, E> should look like the following.
 //  public:
 //    Result(T&& success);
-//    Result(E&& error);
+//    Result(std::unique_ptr<E> error);
 //
 //    Result(Result<T, E>&& other);
 //    Result<T, E>& operator=(Result<T, E>&& other);
@@ -52,18 +53,18 @@
 //    bool IsSuccess() const;
 //
 //    T&& AcquireSuccess();
-//    E&& AcquireError();
+//    std::unique_ptr<E> AcquireError();
 
 // Specialization of Result for returning errors only via pointers. It is basically a pointer
 // where nullptr is both Success and Empty.
 template <typename E>
-class DAWN_NO_DISCARD Result<void, E*> {
+class DAWN_NO_DISCARD Result<void, E> {
   public:
     Result();
-    Result(E* error);
+    Result(std::unique_ptr<E> error);
 
-    Result(Result<void, E*>&& other);
-    Result<void, E*>& operator=(Result<void, E>&& other);
+    Result(Result<void, E>&& other);
+    Result<void, E>& operator=(Result<void, E>&& other);
 
     ~Result();
 
@@ -71,10 +72,10 @@
     bool IsSuccess() const;
 
     void AcquireSuccess();
-    E* AcquireError();
+    std::unique_ptr<E> AcquireError();
 
   private:
-    E* mError = nullptr;
+    std::unique_ptr<E> mError;
 };
 
 // Uses SFINAE to try to get alignof(T) but fallback to Default if T isn't defined.
@@ -108,7 +109,7 @@
 }  // namespace detail
 
 template <typename T, typename E>
-class DAWN_NO_DISCARD Result<T*, E*> {
+class DAWN_NO_DISCARD Result<T*, E> {
   public:
     static_assert(alignof_if_defined_else_default<T, 4> >= 4,
                   "Result<T*, E*> reserves two bits for tagging pointers");
@@ -116,13 +117,13 @@
                   "Result<T*, E*> reserves two bits for tagging pointers");
 
     Result(T* success);
-    Result(E* error);
+    Result(std::unique_ptr<E> error);
 
     // Support returning a Result<T*, E*> from a Result<TChild*, E*>
     template <typename TChild>
-    Result(Result<TChild*, E*>&& other);
+    Result(Result<TChild*, E>&& other);
     template <typename TChild>
-    Result<T*, E*>& operator=(Result<TChild*, E>&& other);
+    Result<T*, E>& operator=(Result<TChild*, E>&& other);
 
     ~Result();
 
@@ -130,7 +131,7 @@
     bool IsSuccess() const;
 
     T* AcquireSuccess();
-    E* AcquireError();
+    std::unique_ptr<E> AcquireError();
 
   private:
     template <typename T2, typename E2>
@@ -140,7 +141,7 @@
 };
 
 template <typename T, typename E>
-class DAWN_NO_DISCARD Result<const T*, E*> {
+class DAWN_NO_DISCARD Result<const T*, E> {
   public:
     static_assert(alignof_if_defined_else_default<T, 4> >= 4,
                   "Result<T*, E*> reserves two bits for tagging pointers");
@@ -148,10 +149,10 @@
                   "Result<T*, E*> reserves two bits for tagging pointers");
 
     Result(const T* success);
-    Result(E* error);
+    Result(std::unique_ptr<E> error);
 
-    Result(Result<const T*, E*>&& other);
-    Result<const T*, E*>& operator=(Result<const T*, E>&& other);
+    Result(Result<const T*, E>&& other);
+    Result<const T*, E>& operator=(Result<const T*, E>&& other);
 
     ~Result();
 
@@ -159,7 +160,7 @@
     bool IsSuccess() const;
 
     const T* AcquireSuccess();
-    E* AcquireError();
+    std::unique_ptr<E> AcquireError();
 
   private:
     intptr_t mPayload = detail::kEmptyPayload;
@@ -172,7 +173,7 @@
 class DAWN_NO_DISCARD Result {
   public:
     Result(T&& success);
-    Result(E&& error);
+    Result(std::unique_ptr<E> error);
 
     Result(Result<T, E>&& other);
     Result<T, E>& operator=(Result<T, E>&& other);
@@ -183,7 +184,7 @@
     bool IsSuccess() const;
 
     T&& AcquireSuccess();
-    E&& AcquireError();
+    std::unique_ptr<E> AcquireError();
 
   private:
     enum PayloadType {
@@ -193,56 +194,52 @@
     };
     PayloadType mType;
 
-    E mError;
+    std::unique_ptr<E> mError;
     T mSuccess;
 };
 
-// Implementation of Result<void, E*>
+// Implementation of Result<void, E>
 template <typename E>
-Result<void, E*>::Result() {
+Result<void, E>::Result() {
 }
 
 template <typename E>
-Result<void, E*>::Result(E* error) : mError(error) {
+Result<void, E>::Result(std::unique_ptr<E> error) : mError(std::move(error)) {
 }
 
 template <typename E>
-Result<void, E*>::Result(Result<void, E*>&& other) : mError(other.mError) {
-    other.mError = nullptr;
+Result<void, E>::Result(Result<void, E>&& other) : mError(std::move(other.mError)) {
 }
 
 template <typename E>
-Result<void, E*>& Result<void, E*>::operator=(Result<void, E>&& other) {
+Result<void, E>& Result<void, E>::operator=(Result<void, E>&& other) {
     ASSERT(mError == nullptr);
-    mError = other.mError;
-    other.mError = nullptr;
+    mError = std::move(other.mError);
     return *this;
 }
 
 template <typename E>
-Result<void, E*>::~Result() {
+Result<void, E>::~Result() {
     ASSERT(mError == nullptr);
 }
 
 template <typename E>
-bool Result<void, E*>::IsError() const {
+bool Result<void, E>::IsError() const {
     return mError != nullptr;
 }
 
 template <typename E>
-bool Result<void, E*>::IsSuccess() const {
+bool Result<void, E>::IsSuccess() const {
     return mError == nullptr;
 }
 
 template <typename E>
-void Result<void, E*>::AcquireSuccess() {
+void Result<void, E>::AcquireSuccess() {
 }
 
 template <typename E>
-E* Result<void, E*>::AcquireError() {
-    E* error = mError;
-    mError = nullptr;
-    return error;
+std::unique_ptr<E> Result<void, E>::AcquireError() {
+    return std::move(mError);
 }
 
 // Implementation details of the tagged pointer Results
@@ -262,25 +259,26 @@
 
 }  // namespace detail
 
-// Implementation of Result<T*, E*>
+// Implementation of Result<T*, E>
 template <typename T, typename E>
-Result<T*, E*>::Result(T* success) : mPayload(detail::MakePayload(success, detail::Success)) {
+Result<T*, E>::Result(T* success) : mPayload(detail::MakePayload(success, detail::Success)) {
 }
 
 template <typename T, typename E>
-Result<T*, E*>::Result(E* error) : mPayload(detail::MakePayload(error, detail::Error)) {
+Result<T*, E>::Result(std::unique_ptr<E> error)
+    : mPayload(detail::MakePayload(error.release(), detail::Error)) {
 }
 
 template <typename T, typename E>
 template <typename TChild>
-Result<T*, E*>::Result(Result<TChild*, E*>&& other) : mPayload(other.mPayload) {
+Result<T*, E>::Result(Result<TChild*, E>&& other) : mPayload(other.mPayload) {
     other.mPayload = detail::kEmptyPayload;
     static_assert(std::is_same<T, TChild>::value || std::is_base_of<T, TChild>::value, "");
 }
 
 template <typename T, typename E>
 template <typename TChild>
-Result<T*, E*>& Result<T*, E*>::operator=(Result<TChild*, E>&& other) {
+Result<T*, E>& Result<T*, E>::operator=(Result<TChild*, E>&& other) {
     ASSERT(mPayload == detail::kEmptyPayload);
     static_assert(std::is_same<T, TChild>::value || std::is_base_of<T, TChild>::value, "");
     mPayload = other.mPayload;
@@ -289,51 +287,52 @@
 }
 
 template <typename T, typename E>
-Result<T*, E*>::~Result() {
+Result<T*, E>::~Result() {
     ASSERT(mPayload == detail::kEmptyPayload);
 }
 
 template <typename T, typename E>
-bool Result<T*, E*>::IsError() const {
+bool Result<T*, E>::IsError() const {
     return detail::GetPayloadType(mPayload) == detail::Error;
 }
 
 template <typename T, typename E>
-bool Result<T*, E*>::IsSuccess() const {
+bool Result<T*, E>::IsSuccess() const {
     return detail::GetPayloadType(mPayload) == detail::Success;
 }
 
 template <typename T, typename E>
-T* Result<T*, E*>::AcquireSuccess() {
+T* Result<T*, E>::AcquireSuccess() {
     T* success = detail::GetSuccessFromPayload<T>(mPayload);
     mPayload = detail::kEmptyPayload;
     return success;
 }
 
 template <typename T, typename E>
-E* Result<T*, E*>::AcquireError() {
-    E* error = detail::GetErrorFromPayload<E>(mPayload);
+std::unique_ptr<E> Result<T*, E>::AcquireError() {
+    std::unique_ptr<E> error(detail::GetErrorFromPayload<E>(mPayload));
     mPayload = detail::kEmptyPayload;
-    return error;
+    return std::move(error);
 }
 
 // Implementation of Result<const T*, E*>
 template <typename T, typename E>
-Result<const T*, E*>::Result(const T* success)
+Result<const T*, E>::Result(const T* success)
     : mPayload(detail::MakePayload(success, detail::Success)) {
 }
 
 template <typename T, typename E>
-Result<const T*, E*>::Result(E* error) : mPayload(detail::MakePayload(error, detail::Error)) {
+Result<const T*, E>::Result(std::unique_ptr<E> error)
+    : mPayload(detail::MakePayload(error.release(), detail::Error)) {
 }
 
 template <typename T, typename E>
-Result<const T*, E*>::Result(Result<const T*, E*>&& other) : mPayload(other.mPayload) {
+Result<const T*, E>::Result(Result<const T*, E>&& other) : mPayload(other.mPayload) {
     other.mPayload = detail::kEmptyPayload;
 }
 
 template <typename T, typename E>
-Result<const T*, E*>& Result<const T*, E*>::operator=(Result<const T*, E>&& other) {
+Result<const T*, E>& Result<const T*, E>::operator=(Result<const T*, E>&& other) {
     ASSERT(mPayload == detail::kEmptyPayload);
     mPayload = other.mPayload;
     other.mPayload = detail::kEmptyPayload;
@@ -341,32 +340,32 @@
 }
 
 template <typename T, typename E>
-Result<const T*, E*>::~Result() {
+Result<const T*, E>::~Result() {
     ASSERT(mPayload == detail::kEmptyPayload);
 }
 
 template <typename T, typename E>
-bool Result<const T*, E*>::IsError() const {
+bool Result<const T*, E>::IsError() const {
     return detail::GetPayloadType(mPayload) == detail::Error;
 }
 
 template <typename T, typename E>
-bool Result<const T*, E*>::IsSuccess() const {
+bool Result<const T*, E>::IsSuccess() const {
     return detail::GetPayloadType(mPayload) == detail::Success;
 }
 
 template <typename T, typename E>
-const T* Result<const T*, E*>::AcquireSuccess() {
+const T* Result<const T*, E>::AcquireSuccess() {
     T* success = detail::GetSuccessFromPayload<T>(mPayload);
     mPayload = detail::kEmptyPayload;
     return success;
 }
 
 template <typename T, typename E>
-E* Result<const T*, E*>::AcquireError() {
-    E* error = detail::GetErrorFromPayload<E>(mPayload);
+std::unique_ptr<E> Result<const T*, E>::AcquireError() {
+    std::unique_ptr<E> error(detail::GetErrorFromPayload<E>(mPayload));
     mPayload = detail::kEmptyPayload;
-    return error;
+    return std::move(error);
 }
 
 // Implementation of Result<T, E>
@@ -375,7 +374,7 @@
 }
 
 template <typename T, typename E>
-Result<T, E>::Result(E&& error) : mType(Error), mError(std::move(error)) {
+Result<T, E>::Result(std::unique_ptr<E> error) : mType(Error), mError(std::move(error)) {
 }
 
 template <typename T, typename E>
@@ -415,7 +414,7 @@
 }
 
 template <typename T, typename E>
-E&& Result<T, E>::AcquireError() {
+std::unique_ptr<E> Result<T, E>::AcquireError() {
     ASSERT(mType == Error);
     mType = Acquired;
     return std::move(mError);
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 3926a89..f5609dc 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -118,10 +118,9 @@
         HandleError(type, message);
     }
 
-    void DeviceBase::ConsumeError(ErrorData* error) {
+    void DeviceBase::ConsumeError(std::unique_ptr<ErrorData> error) {
         ASSERT(error != nullptr);
         HandleError(error->GetType(), error->GetMessage().c_str());
-        delete error;
     }
 
     void DeviceBase::SetUncapturedErrorCallback(wgpu::ErrorCallback callback, void* userdata) {
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index aa6471f..9d0947d 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -251,7 +251,7 @@
 
         void SetDefaultToggles();
 
-        void ConsumeError(ErrorData* error);
+        void ConsumeError(std::unique_ptr<ErrorData> error);
 
         // Destroy is used to clean up and release resources used by device, does not wait for GPU
         // or check errors.
diff --git a/src/dawn_native/EncodingContext.h b/src/dawn_native/EncodingContext.h
index c16d544..1eef6c5 100644
--- a/src/dawn_native/EncodingContext.h
+++ b/src/dawn_native/EncodingContext.h
@@ -41,9 +41,8 @@
         // Functions to handle encoder errors
         void HandleError(wgpu::ErrorType type, const char* message);
 
-        inline void ConsumeError(ErrorData* error) {
+        inline void ConsumeError(std::unique_ptr<ErrorData> error) {
             HandleError(error->GetType(), error->GetMessage().c_str());
-            delete error;
         }
 
         inline bool ConsumedError(MaybeError maybeError) {
diff --git a/src/dawn_native/Error.cpp b/src/dawn_native/Error.cpp
deleted file mode 100644
index 195afb2..0000000
--- a/src/dawn_native/Error.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2018 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/Error.h"
-
-#include "dawn_native/ErrorData.h"
-
-namespace dawn_native {
-
-    ErrorData* MakeError(InternalErrorType type,
-                         std::string message,
-                         const char* file,
-                         const char* function,
-                         int line) {
-        ErrorData* error = new ErrorData(type, message);
-        error->AppendBacktrace(file, function, line);
-        return error;
-    }
-
-    void AppendBacktrace(ErrorData* error, const char* file, const char* function, int line) {
-        error->AppendBacktrace(file, function, line);
-    }
-
-}  // namespace dawn_native
diff --git a/src/dawn_native/Error.h b/src/dawn_native/Error.h
index b4b968a..7f9dbcb 100644
--- a/src/dawn_native/Error.h
+++ b/src/dawn_native/Error.h
@@ -16,23 +16,20 @@
 #define DAWNNATIVE_ERROR_H_
 
 #include "common/Result.h"
+#include "dawn_native/ErrorData.h"
 
 #include <string>
 
 namespace dawn_native {
 
-    // This is the content of an error value for MaybeError or ResultOrError, split off to its own
-    // file to avoid having all files including headers like <string> and <vector>
-    class ErrorData;
-
     enum class InternalErrorType : uint32_t { Validation, DeviceLost, Unimplemented, OutOfMemory };
 
     // MaybeError and ResultOrError are meant to be used as return value for function that are not
     // expected to, but might fail. The handling of error is potentially much slower than successes.
-    using MaybeError = Result<void, ErrorData*>;
+    using MaybeError = Result<void, ErrorData>;
 
     template <typename T>
-    using ResultOrError = Result<T, ErrorData*>;
+    using ResultOrError = Result<T, ErrorData>;
 
     // Returning a success is done like so:
     //   return {}; // for Error
@@ -44,7 +41,7 @@
     // but shorthand version for specific error types are preferred:
     //   return DAWN_VALIDATION_ERROR("My error message");
 #define DAWN_MAKE_ERROR(TYPE, MESSAGE) \
-    ::dawn_native::MakeError(TYPE, MESSAGE, __FILE__, __func__, __LINE__)
+    ::dawn_native::ErrorData::Create(TYPE, MESSAGE, __FILE__, __func__, __LINE__)
 #define DAWN_VALIDATION_ERROR(MESSAGE) DAWN_MAKE_ERROR(InternalErrorType::Validation, MESSAGE)
 #define DAWN_DEVICE_LOST_ERROR(MESSAGE) DAWN_MAKE_ERROR(InternalErrorType::DeviceLost, MESSAGE)
 #define DAWN_UNIMPLEMENTED_ERROR(MESSAGE) DAWN_MAKE_ERROR(InternalErrorType::Unimplemented, MESSAGE)
@@ -57,43 +54,33 @@
     // When Errors aren't handled explicitly, calls to functions returning errors should be
     // wrapped in an DAWN_TRY. It will return the error if any, otherwise keep executing
     // the current function.
-#define DAWN_TRY(EXPR)                                                           \
-    {                                                                            \
-        auto DAWN_LOCAL_VAR = EXPR;                                              \
-        if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) {                           \
-            ::dawn_native::ErrorData* error = DAWN_LOCAL_VAR.AcquireError();     \
-            ::dawn_native::AppendBacktrace(error, __FILE__, __func__, __LINE__); \
-            return {std::move(error)};                                           \
-        }                                                                        \
-    }                                                                            \
-    for (;;)                                                                     \
+#define DAWN_TRY(EXPR)                                                                       \
+    {                                                                                        \
+        auto DAWN_LOCAL_VAR = EXPR;                                                          \
+        if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) {                                       \
+            std::unique_ptr<::dawn_native::ErrorData> error = DAWN_LOCAL_VAR.AcquireError(); \
+            error->AppendBacktrace(__FILE__, __func__, __LINE__);                            \
+            return {std::move(error)};                                                       \
+        }                                                                                    \
+    }                                                                                        \
+    for (;;)                                                                                 \
     break
 
     // DAWN_TRY_ASSIGN is the same as DAWN_TRY for ResultOrError and assigns the success value, if
     // any, to VAR.
-#define DAWN_TRY_ASSIGN(VAR, EXPR)                                               \
-    {                                                                            \
-        auto DAWN_LOCAL_VAR = EXPR;                                              \
-        if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) {                           \
-            ErrorData* error = DAWN_LOCAL_VAR.AcquireError();                    \
-            ::dawn_native::AppendBacktrace(error, __FILE__, __func__, __LINE__); \
-            return {std::move(error)};                                           \
-        }                                                                        \
-        VAR = DAWN_LOCAL_VAR.AcquireSuccess();                                   \
-    }                                                                            \
-    for (;;)                                                                     \
+#define DAWN_TRY_ASSIGN(VAR, EXPR)                                            \
+    {                                                                         \
+        auto DAWN_LOCAL_VAR = EXPR;                                           \
+        if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) {                        \
+            std::unique_ptr<ErrorData> error = DAWN_LOCAL_VAR.AcquireError(); \
+            error->AppendBacktrace(__FILE__, __func__, __LINE__);             \
+            return {std::move(error)};                                        \
+        }                                                                     \
+        VAR = DAWN_LOCAL_VAR.AcquireSuccess();                                \
+    }                                                                         \
+    for (;;)                                                                  \
     break
 
-    // Implementation detail of DAWN_TRY and DAWN_TRY_ASSIGN's adding to the Error's backtrace.
-    void AppendBacktrace(ErrorData* error, const char* file, const char* function, int line);
-
-    // Implementation detail of DAWN_MAKE_ERROR
-    ErrorData* MakeError(InternalErrorType type,
-                         std::string message,
-                         const char* file,
-                         const char* function,
-                         int line);
-
 }  // namespace dawn_native
 
 #endif  // DAWNNATIVE_ERROR_H_
diff --git a/src/dawn_native/ErrorData.cpp b/src/dawn_native/ErrorData.cpp
index 2cd01da..bdfed7f 100644
--- a/src/dawn_native/ErrorData.cpp
+++ b/src/dawn_native/ErrorData.cpp
@@ -19,7 +19,15 @@
 
 namespace dawn_native {
 
-    ErrorData::ErrorData() = default;
+    std::unique_ptr<ErrorData> ErrorData::Create(InternalErrorType type,
+                                                 std::string message,
+                                                 const char* file,
+                                                 const char* function,
+                                                 int line) {
+        std::unique_ptr<ErrorData> error = std::make_unique<ErrorData>(type, message);
+        error->AppendBacktrace(file, function, line);
+        return error;
+    }
 
     ErrorData::ErrorData(InternalErrorType type, std::string message)
         : mType(type), mMessage(std::move(message)) {
diff --git a/src/dawn_native/ErrorData.h b/src/dawn_native/ErrorData.h
index a73d90d..27004de 100644
--- a/src/dawn_native/ErrorData.h
+++ b/src/dawn_native/ErrorData.h
@@ -33,7 +33,11 @@
 
     class ErrorData {
       public:
-        ErrorData();
+        static std::unique_ptr<ErrorData> Create(InternalErrorType type,
+                                                 std::string message,
+                                                 const char* file,
+                                                 const char* function,
+                                                 int line);
         ErrorData(InternalErrorType type, std::string message);
 
         struct BacktraceRecord {
diff --git a/src/dawn_native/Instance.cpp b/src/dawn_native/Instance.cpp
index 7219ab6..735c617 100644
--- a/src/dawn_native/Instance.cpp
+++ b/src/dawn_native/Instance.cpp
@@ -177,11 +177,10 @@
 
     bool InstanceBase::ConsumedError(MaybeError maybeError) {
         if (maybeError.IsError()) {
-            ErrorData* error = maybeError.AcquireError();
+            std::unique_ptr<ErrorData> error = maybeError.AcquireError();
 
             ASSERT(error != nullptr);
             dawn::InfoLog() << error->GetMessage();
-            delete error;
 
             return true;
         }
diff --git a/src/tests/unittests/ErrorTests.cpp b/src/tests/unittests/ErrorTests.cpp
index f7d5bc8..514c740 100644
--- a/src/tests/unittests/ErrorTests.cpp
+++ b/src/tests/unittests/ErrorTests.cpp
@@ -43,9 +43,8 @@
     MaybeError result = ReturnError();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check returning a success ResultOrError with an implicit conversion
@@ -68,9 +67,8 @@
     ResultOrError<int*> result = ReturnError();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check DAWN_TRY handles successes correctly.
@@ -109,9 +107,8 @@
     MaybeError result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check DAWN_TRY adds to the backtrace.
@@ -136,13 +133,10 @@
     MaybeError doubleResult = DoubleTry();
     ASSERT_TRUE(doubleResult.IsError());
 
-    ErrorData* singleData = singleResult.AcquireError();
-    ErrorData* doubleData = doubleResult.AcquireError();
+    std::unique_ptr<ErrorData> singleData = singleResult.AcquireError();
+    std::unique_ptr<ErrorData> doubleData = doubleResult.AcquireError();
 
     ASSERT_EQ(singleData->GetBacktrace().size() + 1, doubleData->GetBacktrace().size());
-
-    delete singleData;
-    delete doubleData;
 }
 
 // Check DAWN_TRY_ASSIGN handles successes correctly.
@@ -188,9 +182,8 @@
     ResultOrError<int*> result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check DAWN_TRY_ASSIGN adds to the backtrace.
@@ -215,13 +208,10 @@
     ResultOrError<int*> doubleResult = DoubleTry();
     ASSERT_TRUE(doubleResult.IsError());
 
-    ErrorData* singleData = singleResult.AcquireError();
-    ErrorData* doubleData = doubleResult.AcquireError();
+    std::unique_ptr<ErrorData> singleData = singleResult.AcquireError();
+    std::unique_ptr<ErrorData> doubleData = doubleResult.AcquireError();
 
     ASSERT_EQ(singleData->GetBacktrace().size() + 1, doubleData->GetBacktrace().size());
-
-    delete singleData;
-    delete doubleData;
 }
 
 // Check a ResultOrError can be DAWN_TRY_ASSIGNED in a function that returns an Error
@@ -241,9 +231,8 @@
     MaybeError result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check a ResultOrError can be DAWN_TRY_ASSIGNED in a function that returns an Error
@@ -264,9 +253,8 @@
     MaybeError result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check a MaybeError can be DAWN_TRIED in a function that returns an ResultOrError
@@ -284,9 +272,8 @@
     ResultOrError<int*> result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 // Check a MaybeError can be DAWN_TRIED in a function that returns an ResultOrError
@@ -304,9 +291,8 @@
     ResultOrError<int> result = Try();
     ASSERT_TRUE(result.IsError());
 
-    ErrorData* errorData = result.AcquireError();
+    std::unique_ptr<ErrorData> errorData = result.AcquireError();
     ASSERT_EQ(errorData->GetMessage(), dummyErrorMessage);
-    delete errorData;
 }
 
 }  // anonymous namespace
diff --git a/src/tests/unittests/ResultTests.cpp b/src/tests/unittests/ResultTests.cpp
index d991462..dd87e22 100644
--- a/src/tests/unittests/ResultTests.cpp
+++ b/src/tests/unittests/ResultTests.cpp
@@ -23,8 +23,8 @@
     ASSERT_TRUE(result->IsError());
     ASSERT_FALSE(result->IsSuccess());
 
-    E storedError = result->AcquireError();
-    ASSERT_EQ(storedError, expectedError);
+    std::unique_ptr<E> storedError = result->AcquireError();
+    ASSERT_EQ(*storedError, expectedError);
 }
 
 template<typename T, typename E>
@@ -42,94 +42,88 @@
 
 // Result<void, E*>
 
-// Test constructing an error Result<void, E*>
+// Test constructing an error Result<void, E>
 TEST(ResultOnlyPointerError, ConstructingError) {
-    Result<void, int*> result(&dummyError);
-    TestError(&result, &dummyError);
+    Result<void, int> result(std::make_unique<int>(dummyError));
+    TestError(&result, dummyError);
 }
 
-// Test moving an error Result<void, E*>
+// Test moving an error Result<void, E>
 TEST(ResultOnlyPointerError, MovingError) {
-    Result<void, int*> result(&dummyError);
-    Result<void, int*> movedResult(std::move(result));
-    TestError(&movedResult, &dummyError);
+    Result<void, int> result(std::make_unique<int>(dummyError));
+    Result<void, int> movedResult(std::move(result));
+    TestError(&movedResult, dummyError);
 }
 
-// Test returning an error Result<void, E*>
+// Test returning an error Result<void, E>
 TEST(ResultOnlyPointerError, ReturningError) {
-    auto CreateError = []() -> Result<void, int*> {
-        return {&dummyError};
-    };
+    auto CreateError = []() -> Result<void, int> { return {std::make_unique<int>(dummyError)}; };
 
-    Result<void, int*> result = CreateError();
-    TestError(&result, &dummyError);
+    Result<void, int> result = CreateError();
+    TestError(&result, dummyError);
 }
 
-// Test constructing a success Result<void, E*>
+// Test constructing a success Result<void, E>
 TEST(ResultOnlyPointerError, ConstructingSuccess) {
-    Result<void, int*> result;
+    Result<void, int> result;
     ASSERT_TRUE(result.IsSuccess());
     ASSERT_FALSE(result.IsError());
 }
 
-// Test moving a success Result<void, E*>
+// Test moving a success Result<void, E>
 TEST(ResultOnlyPointerError, MovingSuccess) {
-    Result<void, int*> result;
-    Result<void, int*> movedResult(std::move(result));
+    Result<void, int> result;
+    Result<void, int> movedResult(std::move(result));
     ASSERT_TRUE(movedResult.IsSuccess());
     ASSERT_FALSE(movedResult.IsError());
 }
 
-// Test returning a success Result<void, E*>
+// Test returning a success Result<void, E>
 TEST(ResultOnlyPointerError, ReturningSuccess) {
-    auto CreateError = []() -> Result<void, int*> {
-        return {};
-    };
+    auto CreateError = []() -> Result<void, int> { return {}; };
 
-    Result<void, int*> result = CreateError();
+    Result<void, int> result = CreateError();
     ASSERT_TRUE(result.IsSuccess());
     ASSERT_FALSE(result.IsError());
 }
 
 // Result<T*, E*>
 
-// Test constructing an error Result<T*, E*>
+// Test constructing an error Result<T*, E>
 TEST(ResultBothPointer, ConstructingError) {
-    Result<float*, int*> result(&dummyError);
-    TestError(&result, &dummyError);
+    Result<float*, int> result(std::make_unique<int>(dummyError));
+    TestError(&result, dummyError);
 }
 
-// Test moving an error Result<T*, E*>
+// Test moving an error Result<T*, E>
 TEST(ResultBothPointer, MovingError) {
-    Result<float*, int*> result(&dummyError);
-    Result<float*, int*> movedResult(std::move(result));
-    TestError(&movedResult, &dummyError);
+    Result<float*, int> result(std::make_unique<int>(dummyError));
+    Result<float*, int> movedResult(std::move(result));
+    TestError(&movedResult, dummyError);
 }
 
-// Test returning an error Result<T*, E*>
+// Test returning an error Result<T*, E>
 TEST(ResultBothPointer, ReturningError) {
-    auto CreateError = []() -> Result<float*, int*> {
-        return {&dummyError};
-    };
+    auto CreateError = []() -> Result<float*, int> { return {std::make_unique<int>(dummyError)}; };
 
-    Result<float*, int*> result = CreateError();
-    TestError(&result, &dummyError);
+    Result<float*, int> result = CreateError();
+    TestError(&result, dummyError);
 }
 
-// Test constructing a success Result<T*, E*>
+// Test constructing a success Result<T*, E>
 TEST(ResultBothPointer, ConstructingSuccess) {
-    Result<float*, int*> result(&dummySuccess);
+    Result<float*, int> result(&dummySuccess);
     TestSuccess(&result, &dummySuccess);
 }
 
-// Test moving a success Result<T*, E*>
+// Test moving a success Result<T*, E>
 TEST(ResultBothPointer, MovingSuccess) {
-    Result<float*, int*> result(&dummySuccess);
-    Result<float*, int*> movedResult(std::move(result));
+    Result<float*, int> result(&dummySuccess);
+    Result<float*, int> movedResult(std::move(result));
     TestSuccess(&movedResult, &dummySuccess);
 }
 
-// Test returning a success Result<T*, E*>
+// Test returning a success Result<T*, E>
 TEST(ResultBothPointer, ReturningSuccess) {
     auto CreateSuccess = []() -> Result<float*, int*> {
         return {&dummySuccess};
@@ -139,7 +133,7 @@
     TestSuccess(&result, &dummySuccess);
 }
 
-// Tests converting from a Result<TChild*, E*>
+// Tests converting from a Result<TChild*, E>
 TEST(ResultBothPointer, ConversionFromChildClass) {
     struct T {
         int a;
@@ -149,62 +143,64 @@
     TChild child;
     T* childAsT = &child;
     {
-        Result<T*, int*> result(&child);
+        Result<T*, int> result(&child);
         TestSuccess(&result, childAsT);
     }
     {
-        Result<TChild*, int*> resultChild(&child);
-        Result<T*, int*> result(std::move(resultChild));
+        Result<TChild*, int> resultChild(&child);
+        Result<T*, int> result(std::move(resultChild));
         TestSuccess(&result, childAsT);
     }
     {
-        Result<TChild*, int*> resultChild(&child);
-        Result<T*, int*> result = std::move(resultChild);
+        Result<TChild*, int> resultChild(&child);
+        Result<T*, int> result = std::move(resultChild);
         TestSuccess(&result, childAsT);
     }
 }
 
-// Result<const T*, E*>
+// Result<const T*, E>
 
-// Test constructing an error Result<const T*, E*>
+// Test constructing an error Result<const T*, E>
 TEST(ResultBothPointerWithConstResult, ConstructingError) {
-    Result<const float*, int*> result(&dummyError);
-    TestError(&result, &dummyError);
+    Result<const float*, int> result(std::make_unique<int>(dummyError));
+    TestError(&result, dummyError);
 }
 
-// Test moving an error Result<const T*, E*>
+// Test moving an error Result<const T*, E>
 TEST(ResultBothPointerWithConstResult, MovingError) {
-    Result<const float*, int*> result(&dummyError);
-    Result<const float*, int*> movedResult(std::move(result));
-    TestError(&movedResult, &dummyError);
+    Result<const float*, int> result(std::make_unique<int>(dummyError));
+    Result<const float*, int> movedResult(std::move(result));
+    TestError(&movedResult, dummyError);
 }
 
 // Test returning an error Result<const T*, E*>
 TEST(ResultBothPointerWithConstResult, ReturningError) {
-    auto CreateError = []() -> Result<const float*, int*> { return {&dummyError}; };
+    auto CreateError = []() -> Result<const float*, int> {
+        return {std::make_unique<int>(dummyError)};
+    };
 
-    Result<const float*, int*> result = CreateError();
-    TestError(&result, &dummyError);
+    Result<const float*, int> result = CreateError();
+    TestError(&result, dummyError);
 }
 
 // Test constructing a success Result<const T*, E*>
 TEST(ResultBothPointerWithConstResult, ConstructingSuccess) {
-    Result<const float*, int*> result(&dummyConstSuccess);
+    Result<const float*, int> result(&dummyConstSuccess);
     TestSuccess(&result, &dummyConstSuccess);
 }
 
 // Test moving a success Result<const T*, E*>
 TEST(ResultBothPointerWithConstResult, MovingSuccess) {
-    Result<const float*, int*> result(&dummyConstSuccess);
-    Result<const float*, int*> movedResult(std::move(result));
+    Result<const float*, int> result(&dummyConstSuccess);
+    Result<const float*, int> movedResult(std::move(result));
     TestSuccess(&movedResult, &dummyConstSuccess);
 }
 
 // Test returning a success Result<const T*, E*>
 TEST(ResultBothPointerWithConstResult, ReturningSuccess) {
-    auto CreateSuccess = []() -> Result<const float*, int*> { return {&dummyConstSuccess}; };
+    auto CreateSuccess = []() -> Result<const float*, int> { return {&dummyConstSuccess}; };
 
-    Result<const float*, int*> result = CreateSuccess();
+    Result<const float*, int> result = CreateSuccess();
     TestSuccess(&result, &dummyConstSuccess);
 }
 
@@ -212,47 +208,45 @@
 
 // Test constructing an error Result<T, E>
 TEST(ResultGeneric, ConstructingError) {
-    Result<std::vector<float>, int*> result(&dummyError);
-    TestError(&result, &dummyError);
+    Result<std::vector<float>, int> result(std::make_unique<int>(dummyError));
+    TestError(&result, dummyError);
 }
 
 // Test moving an error Result<T, E>
 TEST(ResultGeneric, MovingError) {
-    Result<std::vector<float>, int*> result(&dummyError);
-    Result<std::vector<float>, int*> movedResult(std::move(result));
-    TestError(&movedResult, &dummyError);
+    Result<std::vector<float>, int> result(std::make_unique<int>(dummyError));
+    Result<std::vector<float>, int> movedResult(std::move(result));
+    TestError(&movedResult, dummyError);
 }
 
 // Test returning an error Result<T, E>
 TEST(ResultGeneric, ReturningError) {
-    auto CreateError = []() -> Result<std::vector<float>, int*> {
-        return {&dummyError};
+    auto CreateError = []() -> Result<std::vector<float>, int> {
+        return {std::make_unique<int>(dummyError)};
     };
 
-    Result<std::vector<float>, int*> result = CreateError();
-    TestError(&result, &dummyError);
+    Result<std::vector<float>, int> result = CreateError();
+    TestError(&result, dummyError);
 }
 
 // Test constructing a success Result<T, E>
 TEST(ResultGeneric, ConstructingSuccess) {
-    Result<std::vector<float>, int*> result({1.0f});
+    Result<std::vector<float>, int> result({1.0f});
     TestSuccess(&result, {1.0f});
 }
 
 // Test moving a success Result<T, E>
 TEST(ResultGeneric, MovingSuccess) {
-    Result<std::vector<float>, int*> result({1.0f});
-    Result<std::vector<float>, int*> movedResult(std::move(result));
+    Result<std::vector<float>, int> result({1.0f});
+    Result<std::vector<float>, int> movedResult(std::move(result));
     TestSuccess(&movedResult, {1.0f});
 }
 
 // Test returning a success Result<T, E>
 TEST(ResultGeneric, ReturningSuccess) {
-    auto CreateSuccess = []() -> Result<std::vector<float>, int*> {
-        return {{1.0f}};
-    };
+    auto CreateSuccess = []() -> Result<std::vector<float>, int> { return {{1.0f}}; };
 
-    Result<std::vector<float>, int*> result = CreateSuccess();
+    Result<std::vector<float>, int> result = CreateSuccess();
     TestSuccess(&result, {1.0f});
 }
 
diff --git a/src/tests/white_box/VulkanErrorInjectorTests.cpp b/src/tests/white_box/VulkanErrorInjectorTests.cpp
index 938bd5d..bd4a49e 100644
--- a/src/tests/white_box/VulkanErrorInjectorTests.cpp
+++ b/src/tests/white_box/VulkanErrorInjectorTests.cpp
@@ -61,7 +61,7 @@
         if (err.IsError()) {
             // The handle should never be written to, even for mock failures.
             EXPECT_EQ(buffer, VK_NULL_HANDLE);
-            delete err.AcquireError();
+            err.AcquireError();
             return false;
         }
         EXPECT_NE(buffer, VK_NULL_HANDLE);