Special-case GetDefaultQueue in the wire

Reland with a fix where commands only start being serialized by the
device after the first GetDevice() is called, not in the constructor.

This makes it so calling GetDefaultQueue always returns the same
object. It required updating various WireTests to account for the
additional wire calls.

Bug: dawn:22
Change-Id: Ibe43d84b25100f58a9ec5029a9341e400aec97f6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19982
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

dawn_wire.json

@@ -101,6 +101,7 @@
             "BufferUnmap",
             "DeviceCreateBuffer",
             "DeviceCreateBufferMapped",
+            "DeviceGetDefaultQueue",
             "DevicePushErrorScope",
             "QueueCreateFence",
             "QueueSignal"

src/dawn_wire/client/ApiProcs.cpp

@@ -370,6 +370,11 @@ namespace dawn_wire { namespace client {
     void ClientDeviceRelease(WGPUDevice) {
     }
 
+    WGPUQueue ClientHandwrittenDeviceGetDefaultQueue(WGPUDevice cSelf) {
+        Device* device = reinterpret_cast<Device*>(cSelf);
+        return device->GetDefaultQueue();
+    }
+
     void ClientHandwrittenDeviceSetUncapturedErrorCallback(WGPUDevice cSelf,
                                                            WGPUErrorCallback callback,
                                                            void* userdata) {

src/dawn_wire/client/Client.cpp

@@ -21,7 +21,6 @@ namespace dawn_wire { namespace client {
 
     Client::Client(CommandSerializer* serializer, MemoryTransferService* memoryTransferService)
         : ClientBase(),
-          mDevice(DeviceAllocator().New(this)->object.get()),
           mSerializer(serializer),
           mMemoryTransferService(memoryTransferService) {
         if (mMemoryTransferService == nullptr) {
@@ -32,7 +31,16 @@ namespace dawn_wire { namespace client {
     }
 
     Client::~Client() {
-        DeviceAllocator().Free(mDevice);
+        if (mDevice != nullptr) {
+            DeviceAllocator().Free(mDevice);
+        }
+    }
+
+    WGPUDevice Client::GetDevice() {
+        if (mDevice == nullptr) {
+            mDevice = DeviceAllocator().New(this)->object.get();
+        }
+        return reinterpret_cast<WGPUDeviceImpl*>(mDevice);
     }
 
     ReservedTexture Client::ReserveTexture(WGPUDevice cDevice) {
@@ -57,8 +65,12 @@ namespace dawn_wire { namespace client {
     }
 
     void Client::Disconnect() {
-        if (!mIsDisconnected) {
+        if (mIsDisconnected) {
-            mIsDisconnected = true;
+            return;
+        }
+
+        mIsDisconnected = true;
+        if (mDevice != nullptr) {
             mDevice->HandleDeviceLost("GPU connection lost");
         }
     }

src/dawn_wire/client/Client.h

@@ -33,9 +33,7 @@ namespace dawn_wire { namespace client {
         Client(CommandSerializer* serializer, MemoryTransferService* memoryTransferService);
         ~Client();
 
-        WGPUDevice GetDevice() const {
+        WGPUDevice GetDevice();
-            return reinterpret_cast<WGPUDeviceImpl*>(mDevice);
-        }
 
         MemoryTransferService* GetMemoryTransferService() const {
             return mMemoryTransferService;

src/dawn_wire/client/Device.cpp

@@ -16,20 +16,46 @@
 
 #include "common/Assert.h"
 #include "dawn_wire/WireCmd_autogen.h"
+#include "dawn_wire/client/ApiObjects_autogen.h"
 #include "dawn_wire/client/Client.h"
+#include "dawn_wire/client/ObjectAllocator.h"
 
 namespace dawn_wire { namespace client {
 
     Device::Device(Client* client, uint32_t initialRefcount, uint32_t initialId)
         : ObjectBase(this, initialRefcount, initialId), mClient(client) {
         this->device = this;
+
+        // Get the default queue for this device.
+        ObjectAllocator<Queue>::ObjectAndSerial* allocation = mClient->QueueAllocator().New(this);
+        mDefaultQueue = allocation->object.get();
+
+        DeviceGetDefaultQueueCmd cmd;
+        cmd.self = reinterpret_cast<WGPUDevice>(this);
+        cmd.result = ObjectHandle{allocation->object->id, allocation->generation};
+
+        size_t requiredSize = cmd.GetRequiredSize();
+        char* allocatedBuffer = static_cast<char*>(mClient->GetCmdSpace(requiredSize));
+        cmd.Serialize(allocatedBuffer, *mClient);
     }
 
     Device::~Device() {
+        // Fire pending error scopes
         auto errorScopes = std::move(mErrorScopes);
         for (const auto& it : errorScopes) {
             it.second.callback(WGPUErrorType_Unknown, "Device destroyed", it.second.userdata);
         }
+
+        // Destroy the default queue
+        DestroyObjectCmd cmd;
+        cmd.objectType = ObjectType::Queue;
+        cmd.objectId = mDefaultQueue->id;
+
+        size_t requiredSize = cmd.GetRequiredSize();
+        char* allocatedBuffer = static_cast<char*>(mClient->GetCmdSpace(requiredSize));
+        cmd.Serialize(allocatedBuffer);
+
+        mClient->QueueAllocator().Free(mDefaultQueue);
     }
 
     Client* Device::GetClient() {
@@ -119,4 +145,9 @@ namespace dawn_wire { namespace client {
         return true;
     }
 
+    WGPUQueue Device::GetDefaultQueue() {
+        mDefaultQueue->refcount++;
+        return reinterpret_cast<WGPUQueue>(mDefaultQueue);
+    }
+
 }}  // namespace dawn_wire::client

src/dawn_wire/client/Device.h

@@ -24,6 +24,7 @@
 namespace dawn_wire { namespace client {
 
     class Client;
+    struct Queue;
 
     class Device : public ObjectBase {
       public:
@@ -40,6 +41,8 @@ namespace dawn_wire { namespace client {
         bool RequestPopErrorScope(WGPUErrorCallback callback, void* userdata);
         bool PopErrorScope(uint64_t requestSerial, WGPUErrorType type, const char* message);
 
+        WGPUQueue GetDefaultQueue();
+
       private:
         struct ErrorScopeData {
             WGPUErrorCallback callback = nullptr;
@@ -53,8 +56,10 @@ namespace dawn_wire { namespace client {
         WGPUErrorCallback mErrorCallback = nullptr;
         WGPUDeviceLostCallback mDeviceLostCallback = nullptr;
         bool mDidRunLostCallback = false;
-        void* mErrorUserdata;
+        void* mErrorUserdata = nullptr;
-        void* mDeviceLostUserdata;
+        void* mDeviceLostUserdata = nullptr;
+
+        Queue* mDefaultQueue = nullptr;
     };
 
 }}  // namespace dawn_wire::client

src/tests/end2end/QueueTests.cpp

@@ -25,7 +25,7 @@ class QueueTests : public DawnTest {};
 TEST_P(QueueTests, GetDefaultQueueSameObject) {
     wgpu::Queue q1 = device.GetDefaultQueue();
     wgpu::Queue q2 = device.GetDefaultQueue();
-    EXPECT_EQ(q1.Get() == q2.Get(), !UsesWire());
+    EXPECT_EQ(q1.Get(), q2.Get());
 }
 
 DAWN_INSTANTIATE_TEST(QueueTests,

src/tests/unittests/wire/WireArgumentTests.cpp

@@ -231,11 +231,6 @@ TEST_F(WireArgumentTests, ObjectsAsPointerArgument) {
             .WillOnce(Return(apiCmdBufs[i]));
     }
 
-    // Create queue
-    WGPUQueue queue = wgpuDeviceGetDefaultQueue(device);
-    WGPUQueue apiQueue = api.GetNewQueue();
-    EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue));
-
     // Submit command buffer and check we got a call with both API-side command buffers
     wgpuQueueSubmit(queue, 2, cmdBufs);
 

src/tests/unittests/wire/WireFenceTests.cpp

@@ -43,12 +43,6 @@ class WireFenceTests : public WireTest {
         mockFenceOnCompletionCallback =
             std::make_unique<StrictMock<MockFenceOnCompletionCallback>>();
 
-        {
-            queue = wgpuDeviceGetDefaultQueue(device);
-            apiQueue = api.GetNewQueue();
-            EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue));
-            FlushClient();
-        }
         {
             WGPUFenceDescriptor descriptor = {};
             descriptor.initialValue = 1;
@@ -89,9 +83,6 @@ class WireFenceTests : public WireTest {
     // A successfully created fence
     WGPUFence fence;
     WGPUFence apiFence;
-
-    WGPUQueue queue;
-    WGPUQueue apiQueue;
 };
 
 // Check that signaling a fence succeeds
@@ -227,7 +218,8 @@ TEST_F(WireFenceTests, DestroyBeforeOnCompletionEnd) {
 }
 
 // Test that signaling a fence on a wrong queue is invalid
-TEST_F(WireFenceTests, SignalWrongQueue) {
+// DISABLED until we have support for multiple queues.
+TEST_F(WireFenceTests, DISABLED_SignalWrongQueue) {
     WGPUQueue queue2 = wgpuDeviceGetDefaultQueue(device);
     WGPUQueue apiQueue2 = api.GetNewQueue();
     EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue2));
@@ -240,7 +232,8 @@ TEST_F(WireFenceTests, SignalWrongQueue) {
 }
 
 // Test that signaling a fence on a wrong queue does not update fence signaled value
-TEST_F(WireFenceTests, SignalWrongQueueDoesNotUpdateValue) {
+// DISABLED until we have support for multiple queues.
+TEST_F(WireFenceTests, DISABLED_SignalWrongQueueDoesNotUpdateValue) {
     WGPUQueue queue2 = wgpuDeviceGetDefaultQueue(device);
     WGPUQueue apiQueue2 = api.GetNewQueue();
     EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue2));

src/tests/unittests/wire/WireMultipleDeviceTests.cpp

@@ -69,6 +69,12 @@ class WireMultipleDeviceTests : public testing::Test {
             mS2cBuf->SetHandler(mWireClient.get());
 
             mClientDevice = mWireClient->GetDevice();
+
+            // The GetDefaultQueue is done on WireClient startup so we expect it now.
+            mClientQueue = wgpuDeviceGetDefaultQueue(mClientDevice);
+            mServerQueue = mApi.GetNewQueue();
+            EXPECT_CALL(mApi, DeviceGetDefaultQueue(mServerDevice)).WillOnce(Return(mServerQueue));
+            FlushClient();
         }
 
         ~WireHolder() {
@@ -97,6 +103,14 @@ class WireMultipleDeviceTests : public testing::Test {
             return mServerDevice;
         }
 
+        WGPUQueue ClientQueue() {
+            return mClientQueue;
+        }
+
+        WGPUQueue ServerQueue() {
+            return mServerQueue;
+        }
+
       private:
         testing::StrictMock<MockProcTable> mApi;
         std::unique_ptr<dawn_wire::WireServer> mWireServer;
@@ -105,6 +119,8 @@ class WireMultipleDeviceTests : public testing::Test {
         std::unique_ptr<utils::TerribleCommandBuffer> mC2sBuf;
         WGPUDevice mServerDevice;
         WGPUDevice mClientDevice;
+        WGPUQueue mServerQueue;
+        WGPUQueue mClientQueue;
     };
 
     void ExpectInjectedError(WireHolder* wire) {
@@ -134,20 +150,12 @@ TEST_F(WireMultipleDeviceTests, ValidatesSameDevice) {
     WireHolder wireA;
     WireHolder wireB;
 
-    // Create the objects
+    // Create the fence
-    WGPUQueue queueA = wgpuDeviceGetDefaultQueue(wireA.ClientDevice());
-    WGPUQueue queueB = wgpuDeviceGetDefaultQueue(wireB.ClientDevice());
-
     WGPUFenceDescriptor desc = {};
-    WGPUFence fenceA = wgpuQueueCreateFence(queueA, &desc);
+    WGPUFence fenceA = wgpuQueueCreateFence(wireA.ClientQueue(), &desc);
-
-    // Flush on wire B. We should see the queue created.
-    EXPECT_CALL(*wireB.Api(), DeviceGetDefaultQueue(wireB.ServerDevice()))
-        .WillOnce(Return(wireB.Api()->GetNewQueue()));
-    wireB.FlushClient();
 
     // Signal with a fence from a different wire.
-    wgpuQueueSignal(queueB, fenceA, 1u);
+    wgpuQueueSignal(wireB.ClientQueue(), fenceA, 1u);
 
     // We should inject an error into the server.
     ExpectInjectedError(&wireB);

src/tests/unittests/wire/WireTest.cpp

@@ -70,6 +70,12 @@ void WireTest::SetUp() {
     dawnProcSetProcs(&clientProcs);
 
     apiDevice = mockDevice;
+
+    // The GetDefaultQueue is done on WireClient startup so we expect it now.
+    queue = wgpuDeviceGetDefaultQueue(device);
+    apiQueue = api.GetNewQueue();
+    EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue));
+    FlushClient();
 }
 
 void WireTest::TearDown() {
@@ -104,6 +110,7 @@ dawn_wire::WireClient* WireTest::GetWireClient() {
 }
 
 void WireTest::DeleteServer() {
+    EXPECT_CALL(api, QueueRelease(apiQueue)).Times(1);
     mWireServer = nullptr;
 }
 

src/tests/unittests/wire/WireTest.h

@@ -123,7 +123,9 @@ class WireTest : public testing::Test {
 
     testing::StrictMock<MockProcTable> api;
     WGPUDevice apiDevice;
+    WGPUQueue apiQueue;
     WGPUDevice device;
+    WGPUQueue queue;
 
     dawn_wire::WireServer* GetWireServer();
     dawn_wire::WireClient* GetWireClient();