Nuke Builders Part 2: remove all builder code from wire

This removes blocks of code that were obviously builder-specific but
also removes the ObjectStorage::valid member that was used to implement
the maybe monad on the wire server side. This is no longer needed since
dawn_native handles the maybe monad internally now.

BUG=dawn:125

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

BUILD.gn

@@ -396,18 +396,15 @@ dawn_component("libdawn_native") {
 dawn_generator("libdawn_wire_gen") {
   target = "dawn_wire"
   outputs = [
-    "dawn_wire/TypeTraits_autogen.h",
     "dawn_wire/WireCmd_autogen.h",
     "dawn_wire/WireCmd_autogen.cpp",
     "dawn_wire/client/ApiObjects_autogen.h",
     "dawn_wire/client/ApiProcs_autogen.cpp",
     "dawn_wire/client/ApiProcs_autogen.h",
     "dawn_wire/client/ClientBase_autogen.h",
-    "dawn_wire/client/ClientDoers_autogen.cpp",
     "dawn_wire/client/ClientHandlers_autogen.cpp",
     "dawn_wire/client/ClientPrototypes_autogen.inl",
     "dawn_wire/server/ServerBase_autogen.h",
-    "dawn_wire/server/ServerCallbacks_autogen.cpp",
     "dawn_wire/server/ServerDoers_autogen.cpp",
     "dawn_wire/server/ServerHandlers_autogen.cpp",
     "dawn_wire/server/ServerPrototypes_autogen.inl",

generator/main.py

@@ -386,18 +386,15 @@ def get_renders_for_targets(api_params, wire_json, targets):
             },
             additional_params
         ]
-        renders.append(FileRender('dawn_wire/TypeTraits.h', 'dawn_wire/TypeTraits_autogen.h', wire_params))
         renders.append(FileRender('dawn_wire/WireCmd.h', 'dawn_wire/WireCmd_autogen.h', wire_params))
         renders.append(FileRender('dawn_wire/WireCmd.cpp', 'dawn_wire/WireCmd_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/client/ApiObjects.h', 'dawn_wire/client/ApiObjects_autogen.h', wire_params))
         renders.append(FileRender('dawn_wire/client/ApiProcs.cpp', 'dawn_wire/client/ApiProcs_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/client/ApiProcs.h', 'dawn_wire/client/ApiProcs_autogen.h', wire_params))
         renders.append(FileRender('dawn_wire/client/ClientBase.h', 'dawn_wire/client/ClientBase_autogen.h', wire_params))
-        renders.append(FileRender('dawn_wire/client/ClientDoers.cpp', 'dawn_wire/client/ClientDoers_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/client/ClientHandlers.cpp', 'dawn_wire/client/ClientHandlers_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/client/ClientPrototypes.inl', 'dawn_wire/client/ClientPrototypes_autogen.inl', wire_params))
         renders.append(FileRender('dawn_wire/server/ServerBase.h', 'dawn_wire/server/ServerBase_autogen.h', wire_params))
-        renders.append(FileRender('dawn_wire/server/ServerCallbacks.cpp', 'dawn_wire/server/ServerCallbacks_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/server/ServerDoers.cpp', 'dawn_wire/server/ServerDoers_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/server/ServerHandlers.cpp', 'dawn_wire/server/ServerHandlers_autogen.cpp', wire_params))
         renders.append(FileRender('dawn_wire/server/ServerPrototypes.inl', 'dawn_wire/server/ServerPrototypes_autogen.inl', wire_params))

generator/templates/dawn_wire/TypeTraits.h

@@ -1,35 +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.
-
-#ifndef DAWNWIRE_TYPETRAITS_AUTOGEN_H_
-#define DAWNWIRE_TYPETRAITS_AUTOGEN_H_
-
-#include <dawn/dawn.h>
-
-//* This file can be removed when WebGPU error handling is implemented
-namespace dawn_wire {
-    template <typename T>
-    struct IsBuilderType {
-        static constexpr bool value = false;
-    };
-
-    {% for type in by_category["object"] if type.is_builder %}
-        template<>
-        struct IsBuilderType<{{as_cType(type.name)}}> {
-            static constexpr bool value = true;
-        };
-    {% endfor %}
-}
-
-#endif  // DAWNWIRE_TYPETRAITS_AUTOGEN_H_

generator/templates/dawn_wire/WireCmd.cpp

@@ -83,7 +83,7 @@
 //* Methods are very similar to structures that have one member corresponding to each arguments.
 //* This macro takes advantage of the similarity to output [de]serialization code for a record
 //* that is either a structure or a method, with some special cases for each.
-{% macro write_record_serialization_helpers(record, name, members, is_cmd=False, is_method=False, is_return_command=False) %}
+{% macro write_record_serialization_helpers(record, name, members, is_cmd=False, is_return_command=False) %}
     {% set Return = "Return" if is_return_command else "" %}
     {% set Cmd = "Cmd" if is_cmd else "" %}
 
@@ -218,53 +218,14 @@
             ASSERT(transfer->commandId == {{Return}}WireCmd::{{name}});
         {% endif %}
 
-        //* First assign result ObjectHandles:
-        //* Deserialize guarantees they are filled even if there is an ID for an error object
-        //* for the Maybe monad mechanism.
-        //* TODO(enga): This won't need to be done first once we have "WebGPU error handling".
-        {% set return_handles = members
-          |selectattr("is_return_value")
-          |selectattr("annotation", "equalto", "value")
-          |selectattr("type.dict_name", "equalto", "ObjectHandle")
-          |list %}
-
-        //* Strip return_handles so we don't deserialize it again
-        {% set members = members|reject("in", return_handles)|list %}
-
-        {% for member in return_handles %}
-            {% set memberName = as_varName(member.name) %}
-            {{deserialize_member(member, "transfer->" + memberName, "record->" + memberName)}}
-        {% endfor %}
-
-        //* Handle special transfer members for methods
-        {% if is_method %}
-            //* First assign selfId:
-            //* Deserialize guarantees they are filled even if there is an ID for an error object
-            //* for the Maybe monad mechanism.
-            //* TODO(enga): This won't need to be done first once we have "WebGPU error handling".
-            //*             We can also remove is_method
-            record->selfId = transfer->self;
-            //* This conversion is done after the copying of result* and selfId: Deserialize
-            //* guarantees they are filled even if there is an ID for an error object for the
-            //* Maybe monad mechanism.
-            DESERIALIZE_TRY(resolver.GetFromId(record->selfId, &record->self));
-
-            //* Strip self so we don't deserialize it again
-            {% set members = members|rejectattr("name.chunks", "equalto", ["self"])|list %}
-
-            //* The object resolver returns a success even if the object is null because the
-            //* frontend is responsible to validate that (null objects sometimes have special
-            //* meanings). However it is never valid to call a method on a null object so we
-            //* can error out in that case.
-            if (record->self == nullptr) {
-                return DeserializeResult::FatalError;
-            }
-        {% endif %}
-
         {% if record.extensible %}
             record->nextInChain = nullptr;
         {% endif %}
 
+        {% if record.derived_method %}
+            record->selfId = transfer->self;
+        {% endif %}
+
         //* Value types are directly in the transfer record, objects being replaced with their IDs.
         {% for member in members if member.annotation == "value" %}
             {% set memberName = as_varName(member.name) %}
@@ -428,15 +389,14 @@ namespace dawn_wire {
         {% for command in cmd_records["command"] %}
             {% set name = command.name.CamelCase() %}
             {{write_record_serialization_helpers(command, name, command.members,
-              is_cmd=True, is_method=command.derived_method != None)}}
+              is_cmd=True)}}
         {% endfor %}
 
         //* Output [de]serialization helpers for return commands
         {% for command in cmd_records["return command"] %}
             {% set name = command.name.CamelCase() %}
             {{write_record_serialization_helpers(command, name, command.members,
-              is_cmd=True, is_method=command.derived_method != None,
+              is_cmd=True, is_return_command=True)}}
-              is_return_command=True)}}
         {% endfor %}
     }  // anonymous namespace
 

generator/templates/dawn_wire/WireCmd.h

@@ -29,7 +29,6 @@ namespace dawn_wire {
     enum class DeserializeResult {
         Success,
         FatalError,
-        ErrorObject,
     };
 
     // Interface to allocate more space to deserialize pointed-to data.
@@ -40,8 +39,7 @@ namespace dawn_wire {
     };
 
     // Interface to convert an ID to a server object, if possible.
-    // Methods return FatalError if the ID is for a non-existent object, ErrorObject if the
+    // Methods return FatalError if the ID is for a non-existent object and Success otherwise.
-    // object is an error value and Success otherwise.
     class ObjectIdResolver {
         public:
             {% for type in by_category["object"] %}
@@ -101,10 +99,6 @@ namespace dawn_wire {
         //* Deserialize returns:
         //*  - Success if everything went well (yay!)
         //*  - FatalError is something bad happened (buffer too small for example)
-        //*  - ErrorObject if one if the deserialized object is an error value, for the implementation
-        //*    of the Maybe monad.
-        //* If the return value is not FatalError, selfId, resultId and resultSerial (if present) are
-        //* filled.
         DeserializeResult Deserialize(const char** buffer, size_t* size, DeserializeAllocator* allocator
             {%- if command.has_dawn_object -%}
                 , const ObjectIdResolver& resolver

generator/templates/dawn_wire/client/ApiProcs.cpp

@@ -42,15 +42,6 @@ namespace dawn_wire { namespace client {
                     //* For object creation, store the object ID the client will use for the result.
                     {% if method.return_type.category == "object" %}
                         auto* allocation = self->device->GetClient()->{{method.return_type.name.CamelCase()}}Allocator().New(self->device);
-
-                        {% if type.is_builder %}
-                            //* We are in GetResult, so the callback that should be called is the
-                            //* currently set one. Copy it over to the created object and prevent the
-                            //* builder from calling the callback on destruction.
-                            allocation->object->builderCallback = self->builderCallback;
-                            self->builderCallback.canCall = false;
-                        {% endif %}
-
                         cmd.result = ObjectHandle{allocation->object->id, allocation->serial};
                     {% endif %}
 
@@ -70,18 +61,6 @@ namespace dawn_wire { namespace client {
             {% endif %}
         {% endfor %}
 
-        {% if type.is_builder %}
-            void Client{{as_MethodSuffix(type.name, Name("set error callback"))}}({{cType}} cSelf,
-                                                                                  DawnBuilderErrorCallback callback,
-                                                                                  DawnCallbackUserdata userdata1,
-                                                                                  DawnCallbackUserdata userdata2) {
-                {{Type}}* self = reinterpret_cast<{{Type}}*>(cSelf);
-                self->builderCallback.callback = callback;
-                self->builderCallback.userdata1 = userdata1;
-                self->builderCallback.userdata2 = userdata2;
-            }
-        {% endif %}
-
         {% if not type.name.canonical_case() == "device" %}
             //* When an object's refcount reaches 0, notify the server side of it and delete it.
             void Client{{as_MethodSuffix(type.name, Name("release"))}}({{cType}} cObj) {
@@ -92,8 +71,6 @@ namespace dawn_wire { namespace client {
                     return;
                 }
 
-                obj->builderCallback.Call(DAWN_BUILDER_ERROR_STATUS_UNKNOWN, "Unknown");
-
                 DestroyObjectCmd cmd;
                 cmd.objectType = ObjectType::{{type.name.CamelCase()}};
                 cmd.objectId = obj->id;

generator/templates/dawn_wire/client/ClientDoers.cpp

@@ -1,39 +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 "dawn_wire/client/Client.h"
-
-#include <string>
-
-namespace dawn_wire {
-    namespace client {
-        {% for type in by_category["object"] if type.is_builder %}
-            {% set Type = type.name.CamelCase() %}
-            bool Client::Do{{Type}}ErrorCallback({{type.built_type.name.CamelCase()}}* object, uint32_t status, const char* message) {
-                //* The object might have been deleted or a new object created with the same ID.
-                if (object == nullptr) {
-                    return true;
-                }
-                bool called = object->builderCallback.Call(static_cast<DawnBuilderErrorStatus>(status), message);
-
-                //* Unhandled builder errors are forwarded to the device
-                if (!called && status != DAWN_BUILDER_ERROR_STATUS_SUCCESS && status != DAWN_BUILDER_ERROR_STATUS_UNKNOWN) {
-                    mDevice->HandleError(("Unhandled builder error: " + std::string(message)).c_str());
-                }
-
-                return true;
-            }
-        {% endfor %}
-    }
-}

generator/templates/dawn_wire/server/ServerBase.h

@@ -29,7 +29,7 @@ namespace dawn_wire { namespace server {
 
       protected:
         void DestroyAllObjects(const DawnProcTable& procs) {
-          //* Free all objects when the server is destroyed
+            //* Free all objects when the server is destroyed
             {% for type in by_category["object"] if type.name.canonical_case() != "device" %}
                 {
                     std::vector<{{as_cType(type.name)}}> handles = mKnown{{type.name.CamelCase()}}.AcquireAllHandles();
@@ -68,11 +68,7 @@ namespace dawn_wire { namespace server {
                 }
 
                 *out = data->handle;
-                if (data->valid) {
+                return DeserializeResult::Success;
-                    return DeserializeResult::Success;
-                } else {
-                    return DeserializeResult::ErrorObject;
-                }
             }
 
             DeserializeResult GetOptionalFromId(ObjectId id, {{as_cType(type.name)}}* out) const final {

generator/templates/dawn_wire/server/ServerCallbacks.cpp

@@ -1,58 +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 "common/Assert.h"
-#include "dawn_wire/server/Server.h"
-
-namespace dawn_wire { namespace server {
-    {% for type in by_category["object"] if type.is_builder%}
-        void Server::Forward{{type.name.CamelCase()}}(DawnBuilderErrorStatus status, const char* message, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2) {
-            auto server = reinterpret_cast<Server*>(static_cast<uintptr_t>(userdata1));
-            uint32_t id = userdata2 & 0xFFFFFFFFu;
-            uint32_t serial = userdata2 >> uint64_t(32);
-            server->On{{type.name.CamelCase()}}Error(status, message, id, serial);
-        }
-    {% endfor %}
-
-    {% for type in by_category["object"] if type.is_builder%}
-        {% set Type = type.name.CamelCase() %}
-        void Server::On{{Type}}Error(DawnBuilderErrorStatus status, const char* message, uint32_t id, uint32_t serial) {
-            auto* builder = {{Type}}Objects().Get(id);
-
-            if (builder == nullptr || builder->serial != serial) {
-                return;
-            }
-
-            if (status != DAWN_BUILDER_ERROR_STATUS_SUCCESS) {
-                builder->valid = false;
-            }
-
-            if (status != DAWN_BUILDER_ERROR_STATUS_UNKNOWN) {
-                //* Unknown is the only status that can be returned without a call to GetResult
-                //* so we are guaranteed to have created an object.
-                ASSERT(builder->builtObject.id != 0);
-
-                Return{{Type}}ErrorCallbackCmd cmd;
-                cmd.builtObject = builder->builtObject;
-                cmd.status = status;
-                cmd.message = message;
-
-                size_t requiredSize = cmd.GetRequiredSize();
-                char* allocatedBuffer = static_cast<char*>(GetCmdSpace(requiredSize));
-                cmd.Serialize(allocatedBuffer);
-            }
-        }
-    {% endfor %}
-
-}}  // namespace dawn_wire::server

generator/templates/dawn_wire/server/ServerDoers.cpp

@@ -53,6 +53,11 @@ namespace dawn_wire { namespace server {
                             {%- if not loop.last -%}, {% endif %}
                         {%- endfor -%}
                     );
+                    {% if ret|length == 1 %}
+                        //* WebGPU error handling guarantees that no null object can be returned by
+                        //* object creation functions.
+                        ASSERT(*{{as_varName(ret[0].name)}} != nullptr);
+                    {% endif %}
                     return true;
                 }
             {% endif %}

generator/templates/dawn_wire/server/ServerHandlers.cpp

@@ -43,12 +43,6 @@ namespace dawn_wire { namespace server {
                     }
                 {% endif %}
 
-                {% if is_method %}
-                    //* Unpack 'self'
-                    auto* selfData = {{type.name.CamelCase()}}Objects().Get(cmd.selfId);
-                    ASSERT(selfData != nullptr);
-                {% endif %}
-
                 //* Allocate any result objects
                 {%- for member in command.members if member.is_return_value -%}
                     {{ assert(member.handle_type) }}
@@ -60,24 +54,8 @@ namespace dawn_wire { namespace server {
                         return false;
                     }
                     {{name}}Data->serial = cmd.{{name}}.serial;
-
-                    {% if type.is_builder %}
-                        selfData->builtObject = cmd.{{name}};
-                    {% endif %}
                 {% endfor %}
 
-                //* After the data is allocated, apply the argument error propagation mechanism
-                if (deserializeResult == DeserializeResult::ErrorObject) {
-                    {% if type.is_builder %}
-                        selfData->valid = false;
-                        //* If we are in GetResult, fake an error callback
-                        {% if returns %}
-                            On{{type.name.CamelCase()}}Error(DAWN_BUILDER_ERROR_STATUS_ERROR, "Maybe monad", cmd.selfId, selfData->serial);
-                        {% endif %}
-                    {% endif %}
-                    return true;
-                }
-
                 //* Do command
                 bool success = Do{{Suffix}}(
                     {%- for member in command.members -%}
@@ -94,12 +72,6 @@ namespace dawn_wire { namespace server {
                     {%- endfor -%}
                 );
 
-                //* Mark output object handles as valid/invalid
-                {%- for member in command.members if member.is_return_value and member.handle_type -%}
-                    {% set name = as_varName(member.name) %}
-                    {{name}}Data->valid = {{name}}Data->handle != nullptr;
-                {% endfor %}
-
                 if (!success) {
                     return false;
                 }
@@ -110,19 +82,7 @@ namespace dawn_wire { namespace server {
 
                     {% if Type in server_reverse_lookup_objects %}
                         //* For created objects, store a mapping from them back to their client IDs
-                        if ({{name}}Data->valid) {
+                        {{Type}}ObjectIdTable().Store({{name}}Data->handle, cmd.{{name}}.id);
-                            {{Type}}ObjectIdTable().Store({{name}}Data->handle, cmd.{{name}}.id);
-                        }
-                    {% endif %}
-
-                    //* builders remember the ID of the object they built so that they can send it
-                    //* in the callback to the client.
-                    {% if member.handle_type.is_builder %}
-                        if ({{name}}Data->valid) {
-                            uint64_t userdata1 = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(this));
-                            uint64_t userdata2 = (uint64_t({{name}}Data->serial) << uint64_t(32)) + cmd.{{name}}.id;
-                            mProcs.{{as_varName(member.handle_type.name, Name("set error callback"))}}({{name}}Data->handle, Forward{{Type}}, userdata1, userdata2);
-                        }
                     {% endif %}
                 {% endfor %}
 

generator/templates/dawn_wire/server/ServerPrototypes.inl

@@ -12,17 +12,6 @@
 //* See the License for the specific language governing permissions and
 //* limitations under the License.
 
-// Forwarding callbacks
-{% for type in by_category["object"] if type.is_builder%}
-    static void Forward{{type.name.CamelCase()}}(DawnBuilderErrorStatus status, const char* message, DawnCallbackUserdata userdata1, DawnCallbackUserdata userdata2);
-{% endfor %}
-
-// Error callbacks
-{% for type in by_category["object"] if type.is_builder%}
-    {% set Type = type.name.CamelCase() %}
-    void On{{Type}}Error(DawnBuilderErrorStatus status, const char* message, uint32_t id, uint32_t serial);
-{% endfor %}
-
 // Command handlers & doers
 {% for command in cmd_records["command"] if command.name.CamelCase() not in client_side_commands %}
     {% set Suffix = command.name.CamelCase() %}

generator/wire_cmd.py

@@ -57,20 +57,6 @@ def compute_wire_params(api_params, wire_json):
             command.derived_method = method
             commands.append(command)
 
-        # Create builder return ErrorCallback commands
-        # This can be removed when WebGPU error handling is implemented
-        if api_object.is_builder:
-            command_name = concat_names(api_object.name, Name('error callback'))
-            built_object = common.RecordMember(Name('built object'), types['ObjectHandle'], 'value', False, False)
-            built_object.set_handle_type(api_object.built_type)
-            command = common.Command(command_name, [
-                built_object,
-                callback_status_member,
-                string_message_member,
-            ])
-            command.derived_object = api_object
-            return_commands.append(command)
-
     for (name, json_data) in wire_json['commands'].items():
         commands.append(common.Command(name, common.linked_record_members(json_data, types)))
 

src/dawn_wire/client/ObjectAllocator.h

@@ -25,8 +25,6 @@ namespace dawn_wire { namespace client {
     class Client;
     class Device;
 
-    // TODO(cwallez@chromium.org): Do something with objects before they are destroyed ?
-    //  - Call still uncalled builder callbacks
     template <typename T>
     class ObjectAllocator {
         using ObjectOwner =

src/dawn_wire/client/ObjectBase.h

@@ -21,24 +21,6 @@ namespace dawn_wire { namespace client {
 
     class Device;
 
-    struct BuilderCallbackData {
-        bool Call(DawnBuilderErrorStatus status, const char* message) {
-            if (canCall && callback != nullptr) {
-                canCall = true;
-                callback(status, message, userdata1, userdata2);
-                return true;
-            }
-
-            return false;
-        }
-
-        // For help with development, prints all builder errors by default.
-        DawnBuilderErrorCallback callback = nullptr;
-        DawnCallbackUserdata userdata1 = 0;
-        DawnCallbackUserdata userdata2 = 0;
-        bool canCall = true;
-    };
-
     // All non-Device objects of the client side have:
     //  - A pointer to the device to get where to serialize commands
     //  - The external reference count
@@ -51,8 +33,6 @@ namespace dawn_wire { namespace client {
         Device* device;
         uint32_t refcount;
         uint32_t id;
-
-        BuilderCallbackData builderCallback;
     };
 
 }}  // namespace dawn_wire::client

src/dawn_wire/server/ObjectStorage.h

@@ -15,7 +15,6 @@
 #ifndef DAWNWIRE_SERVER_OBJECTSTORAGE_H_
 #define DAWNWIRE_SERVER_OBJECTSTORAGE_H_
 
-#include "dawn_wire/TypeTraits_autogen.h"
 #include "dawn_wire/WireCmd_autogen.h"
 
 #include <algorithm>
@@ -29,26 +28,17 @@ namespace dawn_wire { namespace server {
         T handle;
         uint32_t serial = 0;
 
-        // Used by the error-propagation mechanism to know if this object is an error.
-        // TODO(cwallez@chromium.org): this is doubling the memory usage of
-        // std::vector<ObjectDataBase> consider making it a special marker value in handle instead.
-        bool valid;
         // Whether this object has been allocated, used by the KnownObjects queries
         // TODO(cwallez@chromium.org): make this an internal bit vector in KnownObjects.
         bool allocated;
     };
 
     // Stores what the backend knows about the type.
-    template <typename T, bool IsBuilder = IsBuilderType<T>::value>
+    template <typename T>
     struct ObjectData : public ObjectDataBase<T> {};
 
-    template <typename T>
-    struct ObjectData<T, true> : public ObjectDataBase<T> {
-        ObjectHandle builtObject = ObjectHandle{0, 0};
-    };
-
     template <>
-    struct ObjectData<DawnBuffer, false> : public ObjectDataBase<DawnBuffer> {
+    struct ObjectData<DawnBuffer> : public ObjectDataBase<DawnBuffer> {
         void* mappedData = nullptr;
         size_t mappedDataSize = 0;
     };
@@ -65,7 +55,6 @@ namespace dawn_wire { namespace server {
             // KnownObjects for ID 0.
             Data reservation;
             reservation.handle = nullptr;
-            reservation.valid = false;
             reservation.allocated = false;
             mKnown.push_back(reservation);
         }
@@ -109,7 +98,6 @@ namespace dawn_wire { namespace server {
 
             Data data;
             data.allocated = true;
-            data.valid = false;
             data.handle = nullptr;
 
             if (id >= mKnown.size()) {
@@ -136,7 +124,6 @@ namespace dawn_wire { namespace server {
             for (Data& data : mKnown) {
                 if (data.allocated && data.handle != nullptr) {
                     objects.push_back(data.handle);
-                    data.valid = false;
                     data.allocated = false;
                     data.handle = nullptr;
                 }

src/dawn_wire/server/Server.cpp

@@ -21,7 +21,6 @@ namespace dawn_wire { namespace server {
         // The client-server knowledge is bootstrapped with device 1.
         auto* deviceData = DeviceObjects().Allocate(1);
         deviceData->handle = device;
-        deviceData->valid = true;
 
         auto userdata = static_cast<DawnCallbackUserdata>(reinterpret_cast<intptr_t>(this));
         mProcs.deviceSetErrorCallback(device, ForwardDeviceError, userdata);
@@ -43,7 +42,6 @@ namespace dawn_wire { namespace server {
 
         data->handle = texture;
         data->serial = generation;
-        data->valid = true;
         data->allocated = true;
 
         // The texture is externally owned so it shouldn't be destroyed when we receive a destroy

src/dawn_wire/server/ServerBuffer.cpp

@@ -52,17 +52,6 @@ namespace dawn_wire { namespace server {
 
         auto userdata = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(data));
 
-        if (!buffer->valid) {
-            // Fake the buffer returning a failure, data will be freed in this call.
-            if (isWrite) {
-                ForwardBufferMapWriteAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0,
-                                           userdata);
-            } else {
-                ForwardBufferMapReadAsync(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata);
-            }
-            return true;
-        }
-
         if (isWrite) {
             mProcs.bufferMapWriteAsync(buffer->handle, ForwardBufferMapWriteAsync, userdata);
         } else {
@@ -79,8 +68,7 @@ namespace dawn_wire { namespace server {
         }
 
         auto* buffer = BufferObjects().Get(bufferId);
-        if (buffer == nullptr || !buffer->valid || buffer->mappedData == nullptr ||
+        if (buffer == nullptr || buffer->mappedData == nullptr || buffer->mappedDataSize != count) {
-            buffer->mappedDataSize != count) {
             return false;
         }
 

src/tests/unittests/wire/WireArgumentTests.cpp

@@ -158,12 +158,14 @@ TEST_F(WireArgumentTests, CStringArgument) {
     pipelineDescriptor.depthStencilState = &depthStencilState;
 
     dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor);
+
+    DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline();
     EXPECT_CALL(api,
                 DeviceCreateRenderPipeline(
                     apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool {
                         return desc->vertexStage->entryPoint == std::string("main");
                     })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummyPipeline));
 
     FlushClient();
 }
@@ -247,6 +249,8 @@ TEST_F(WireArgumentTests, StructureOfValuesArgument) {
     descriptor.borderColor = DAWN_BORDER_COLOR_TRANSPARENT_BLACK;
 
     dawnDeviceCreateSampler(device, &descriptor);
+
+    DawnSampler apiDummySampler = api.GetNewSampler();
     EXPECT_CALL(api, DeviceCreateSampler(
                          apiDevice, MatchesLambda([](const DawnSamplerDescriptor* desc) -> bool {
                              return desc->nextInChain == nullptr &&
@@ -260,7 +264,7 @@ TEST_F(WireArgumentTests, StructureOfValuesArgument) {
                                     desc->borderColor == DAWN_BORDER_COLOR_TRANSPARENT_BLACK &&
                                     desc->lodMinClamp == kLodMin && desc->lodMaxClamp == kLodMax;
                          })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummySampler));
 
     FlushClient();
 }
@@ -281,6 +285,8 @@ TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) {
     descriptor.bindGroupLayouts = &bgl;
 
     dawnDeviceCreatePipelineLayout(device, &descriptor);
+
+    DawnPipelineLayout apiDummyLayout = api.GetNewPipelineLayout();
     EXPECT_CALL(api, DeviceCreatePipelineLayout(
                          apiDevice,
                          MatchesLambda([apiBgl](const DawnPipelineLayoutDescriptor* desc) -> bool {
@@ -288,7 +294,7 @@ TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) {
                                     desc->bindGroupLayoutCount == 1 &&
                                     desc->bindGroupLayouts[0] == apiBgl;
                          })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummyLayout));
 
     FlushClient();
 }

src/tests/unittests/wire/WireBufferMappingTests.cpp

@@ -73,29 +73,16 @@ class WireBufferMappingTests : public WireTest {
         mockBufferMapReadCallback = std::make_unique<StrictMock<MockBufferMapReadCallback>>();
         mockBufferMapWriteCallback = std::make_unique<StrictMock<MockBufferMapWriteCallback>>();
 
-        {
+        DawnBufferDescriptor descriptor;
-            DawnBufferDescriptor descriptor;
+        descriptor.nextInChain = nullptr;
-            descriptor.nextInChain = nullptr;
 
-            apiBuffer = api.GetNewBuffer();
+        apiBuffer = api.GetNewBuffer();
-            buffer = dawnDeviceCreateBuffer(device, &descriptor);
+        buffer = dawnDeviceCreateBuffer(device, &descriptor);
 
-            EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
+        EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
-                .WillOnce(Return(apiBuffer))
+            .WillOnce(Return(apiBuffer))
-                .RetiresOnSaturation();
+            .RetiresOnSaturation();
-            FlushClient();
+        FlushClient();
-        }
-        {
-            DawnBufferDescriptor descriptor;
-            descriptor.nextInChain = nullptr;
-
-            errorBuffer = dawnDeviceCreateBuffer(device, &descriptor);
-
-            EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
-                .WillOnce(Return(nullptr))
-                .RetiresOnSaturation();
-            FlushClient();
-        }
     }
 
     void TearDown() override {
@@ -117,9 +104,6 @@ class WireBufferMappingTests : public WireTest {
     // A successfully created buffer
     DawnBuffer buffer;
     DawnBuffer apiBuffer;
-
-    // An buffer that wasn't created on the server side
-    DawnBuffer errorBuffer;
 };
 
 // MapRead-specific tests
@@ -171,35 +155,24 @@ TEST_F(WireBufferMappingTests, ErrorWhileMappingForRead) {
     FlushServer();
 }
 
-// Check mapping for reading a buffer that didn't get created on the server side
-TEST_F(WireBufferMappingTests, MappingForReadErrorBuffer) {
-    DawnCallbackUserdata userdata = 8655;
-    dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata);
-
-    FlushClient();
-
-    EXPECT_CALL(*mockBufferMapReadCallback,
-                Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata))
-        .Times(1);
-
-    FlushServer();
-
-    dawnBufferUnmap(errorBuffer);
-
-    FlushClient();
-}
-
 // Check that the map read callback is called with UNKNOWN when the buffer is destroyed before the
 // request is finished
 TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) {
     DawnCallbackUserdata userdata = 8656;
-    dawnBufferMapReadAsync(errorBuffer, ToMockBufferMapReadCallback, userdata);
+    dawnBufferMapReadAsync(buffer, ToMockBufferMapReadCallback, userdata);
 
+    // Return success
+    EXPECT_CALL(api, OnBufferMapReadAsyncCallback(apiBuffer, _, _))
+        .WillOnce(InvokeWithoutArgs([&]() {
+            api.CallMapReadCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0);
+        }));
+
+    // Destroy before the client gets the success, so the callback is called with unknown.
     EXPECT_CALL(*mockBufferMapReadCallback,
                 Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata))
         .Times(1);
-
+    dawnBufferRelease(buffer);
-    dawnBufferRelease(errorBuffer);
+    EXPECT_CALL(api, BufferRelease(apiBuffer));
 
     FlushClient();
     FlushServer();
@@ -381,35 +354,27 @@ TEST_F(WireBufferMappingTests, ErrorWhileMappingForWrite) {
     FlushServer();
 }
 
-// Check mapping for writing a buffer that didn't get created on the server side
-TEST_F(WireBufferMappingTests, MappingForWriteErrorBuffer) {
-    DawnCallbackUserdata userdata = 8655;
-    dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata);
-
-    FlushClient();
-
-    EXPECT_CALL(*mockBufferMapWriteCallback,
-                Call(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, 0, userdata))
-        .Times(1);
-
-    FlushServer();
-
-    dawnBufferUnmap(errorBuffer);
-
-    FlushClient();
-}
-
 // Check that the map write callback is called with UNKNOWN when the buffer is destroyed before the
 // request is finished
 TEST_F(WireBufferMappingTests, DestroyBeforeWriteRequestEnd) {
     DawnCallbackUserdata userdata = 8656;
-    dawnBufferMapWriteAsync(errorBuffer, ToMockBufferMapWriteCallback, userdata);
+    dawnBufferMapWriteAsync(buffer, ToMockBufferMapWriteCallback, userdata);
 
+    // Return success
+    EXPECT_CALL(api, OnBufferMapWriteAsyncCallback(apiBuffer, _, _))
+        .WillOnce(InvokeWithoutArgs([&]() {
+            api.CallMapWriteCallback(apiBuffer, DAWN_BUFFER_MAP_ASYNC_STATUS_SUCCESS, nullptr, 0);
+        }));
+
+    // Destroy before the client gets the success, so the callback is called with unknown.
     EXPECT_CALL(*mockBufferMapWriteCallback,
                 Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, 0, userdata))
         .Times(1);
+    dawnBufferRelease(buffer);
+    EXPECT_CALL(api, BufferRelease(apiBuffer));
 
-    dawnBufferRelease(errorBuffer);
+    FlushClient();
+    FlushServer();
 }
 
 // Check the map read callback is called with UNKNOWN when the map request would have worked, but

src/tests/unittests/wire/WireInjectTextureTests.cpp

@@ -37,7 +37,8 @@ TEST_F(WireInjectTextureTests, CallAfterReserveInject) {
     ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id, reservation.generation));
 
     dawnTextureCreateDefaultTextureView(reservation.texture);
-    EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(nullptr));
+    DawnTextureView apiDummyView = api.GetNewTextureView();
+    EXPECT_CALL(api, TextureCreateDefaultTextureView(apiTexture)).WillOnce(Return(apiDummyView));
     FlushClient();
 }
 

src/tests/unittests/wire/WireOptionalTests.cpp

@@ -49,6 +49,8 @@ TEST_F(WireOptionalTests, OptionalObjectValue) {
     bgDesc.bindings = &binding;
 
     dawnDeviceCreateBindGroup(device, &bgDesc);
+
+    DawnBindGroup apiDummyBindGroup = api.GetNewBindGroup();
     EXPECT_CALL(api, DeviceCreateBindGroup(
                          apiDevice, MatchesLambda([](const DawnBindGroupDescriptor* desc) -> bool {
                              return desc->nextInChain == nullptr && desc->bindingCount == 1 &&
@@ -57,7 +59,7 @@ TEST_F(WireOptionalTests, OptionalObjectValue) {
                                     desc->bindings[0].buffer == nullptr &&
                                     desc->bindings[0].textureView == nullptr;
                          })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummyBindGroup));
 
     FlushClient();
 }
@@ -147,6 +149,8 @@ TEST_F(WireOptionalTests, OptionalStructPointer) {
     // First case: depthStencilState is not null.
     pipelineDescriptor.depthStencilState = &depthStencilState;
     dawnDeviceCreateRenderPipeline(device, &pipelineDescriptor);
+
+    DawnRenderPipeline apiDummyPipeline = api.GetNewRenderPipeline();
     EXPECT_CALL(
         api,
         DeviceCreateRenderPipeline(
@@ -172,7 +176,7 @@ TEST_F(WireOptionalTests, OptionalStructPointer) {
                        desc->depthStencilState->stencilReadMask == 0xff &&
                        desc->depthStencilState->stencilWriteMask == 0xff;
             })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummyPipeline));
 
     FlushClient();
 
@@ -184,7 +188,7 @@ TEST_F(WireOptionalTests, OptionalStructPointer) {
                     apiDevice, MatchesLambda([](const DawnRenderPipelineDescriptor* desc) -> bool {
                         return desc->depthStencilState == nullptr;
                     })))
-        .WillOnce(Return(nullptr));
+        .WillOnce(Return(apiDummyPipeline));
 
     FlushClient();
 }