Make WireTest::instance a wgpu::Instance instead of WGPUInstance.
Switching the client-side objects in WireTest to be the C++ version
helps both do proper refcounting (now that the client doesn't
automatically destroy objects on ~Client) and separate the client-side
from the server-side more in tests.
Also fixes an issue where the client::Instance would try to access the
Client even if it has been already destroyed.
Bug: 353294052
Change-Id: Ic436b712663fc86c5e6ba50211ec8eb8c7061902
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/198674
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/tests/unittests/wire/WireFutureTest.h b/src/dawn/tests/unittests/wire/WireFutureTest.h
index 5b26c93..7112739 100644
--- a/src/dawn/tests/unittests/wire/WireFutureTest.h
+++ b/src/dawn/tests/unittests/wire/WireFutureTest.h
@@ -139,14 +139,14 @@
if (mFutureIDs.empty()) {
return;
}
- std::vector<WGPUFutureWaitInfo> waitInfos;
+ std::vector<wgpu::FutureWaitInfo> waitInfos;
for (auto futureID : mFutureIDs) {
waitInfos.push_back({{futureID}, false});
}
- EXPECT_EQ(wgpuInstanceWaitAny(instance, mFutureIDs.size(), waitInfos.data(), 0),
- WGPUWaitStatus_Success);
+ EXPECT_EQ(instance.WaitAny(mFutureIDs.size(), waitInfos.data(), 0),
+ wgpu::WaitStatus::Success);
} else if (callbackMode == CallbackMode::ProcessEvents) {
- wgpuInstanceProcessEvents(instance);
+ instance.ProcessEvents();
}
}
diff --git a/src/dawn/tests/unittests/wire/WireInjectInstanceTests.cpp b/src/dawn/tests/unittests/wire/WireInjectInstanceTests.cpp
index d5e0ede..f7bd243 100644
--- a/src/dawn/tests/unittests/wire/WireInjectInstanceTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireInjectInstanceTests.cpp
@@ -27,13 +27,16 @@
#include "dawn/tests/unittests/wire/WireTest.h"
+#include "dawn/tests/MockCallback.h"
#include "dawn/wire/WireClient.h"
#include "dawn/wire/WireServer.h"
namespace dawn::wire {
namespace {
+using testing::_;
using testing::Mock;
+using testing::MockCallback;
using testing::NotNull;
using testing::Return;
@@ -52,12 +55,18 @@
EXPECT_CALL(api, InstanceAddRef(serverInstance));
ASSERT_TRUE(GetWireServer()->InjectInstance(serverInstance, reserved.handle));
- WGPUSurfaceDescriptor surfaceDesc = {};
- wgpuInstanceCreateSurface(reserved.instance, &surfaceDesc);
- WGPUSurface serverSurface = api.GetNewSurface();
- EXPECT_CALL(api, InstanceCreateSurface(serverInstance, NotNull()))
- .WillOnce(Return(serverSurface));
+ MockCallback<void (*)(wgpu::RequestAdapterStatus, wgpu::Adapter, const char*, void*)> adapterCb;
+ instance.RequestAdapter(nullptr, wgpu::CallbackMode::AllowSpontaneous, adapterCb.Callback(),
+ adapterCb.MakeUserdata(this));
+
+ EXPECT_CALL(api, OnInstanceRequestAdapter2(apiInstance, _, _)).WillOnce([&]() {
+ api.CallInstanceRequestAdapter2Callback(apiInstance, WGPURequestAdapterStatus_Error,
+ nullptr, "Some error message.");
+ });
FlushClient();
+
+ EXPECT_CALL(adapterCb, Call(wgpu::RequestAdapterStatus::Error, _, _, this));
+ FlushServer();
}
// Test that reserve correctly returns different IDs each time.
diff --git a/src/dawn/tests/unittests/wire/WireInstanceTests.cpp b/src/dawn/tests/unittests/wire/WireInstanceTests.cpp
index 465ea6a..69b7283 100644
--- a/src/dawn/tests/unittests/wire/WireInstanceTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireInstanceTests.cpp
@@ -76,10 +76,10 @@
protected:
// Overriden version of wgpuInstanceRequestAdapter that defers to the API call based on the
// test callback mode.
- void InstanceRequestAdapter(WGPUInstance i,
- WGPURequestAdapterOptions const* options,
+ void InstanceRequestAdapter(const wgpu::Instance& i,
+ wgpu::RequestAdapterOptions const* options,
void* userdata = nullptr) {
- CallImpl(userdata, i, options);
+ CallImpl(userdata, i.Get(), reinterpret_cast<const WGPURequestAdapterOptions*>(options));
}
};
@@ -87,9 +87,9 @@
// Test that RequestAdapterOptions are passed from the client to the server.
TEST_P(WireInstanceTests, RequestAdapterPassesOptions) {
- for (WGPUPowerPreference powerPreference :
- {WGPUPowerPreference_LowPower, WGPUPowerPreference_HighPerformance}) {
- WGPURequestAdapterOptions options = {};
+ for (auto powerPreference :
+ {wgpu::PowerPreference::LowPower, wgpu::PowerPreference::HighPerformance}) {
+ wgpu::RequestAdapterOptions options = {};
options.powerPreference = powerPreference;
InstanceRequestAdapter(instance, &options, nullptr);
@@ -115,7 +115,7 @@
// Test that RequestAdapter forwards the adapter information to the client.
TEST_P(WireInstanceTests, RequestAdapterSuccess) {
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
WGPUAdapterInfo fakeInfo = {};
@@ -222,7 +222,7 @@
// Test that RequestAdapter forwards all chained properties to the client.
TEST_P(WireInstanceTests, RequestAdapterPassesChainedProperties) {
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
WGPUMemoryHeapInfo fakeHeapInfo[3] = {
@@ -366,7 +366,7 @@
// Test that features returned by the implementation that aren't supported in the wire are not
// exposed.
TEST_P(WireInstanceTests, RequestAdapterWireLacksFeatureSupport) {
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
std::initializer_list<WGPUFeatureName> fakeFeatures = {
@@ -429,7 +429,7 @@
// Test that RequestAdapter errors forward to the client.
TEST_P(WireInstanceTests, RequestAdapterError) {
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
// Expect the server to receive the message. Then, mock an error.
@@ -458,21 +458,21 @@
// TODO(crbug.com/dawn/2061) This test does not currently pass because the callbacks aren't
// actually triggered by the destruction of the instance at the moment. Once we move the
// EventManager to be per-Instance, this test should pass.
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
ExpectWireCallbacksWhen([&](auto& mockCb) {
EXPECT_CALL(mockCb, Call(WGPURequestAdapterStatus_Unknown, nullptr, NotNull(), nullptr))
.Times(1);
- wgpuInstanceRelease(instance);
+ instance = nullptr;
});
}
// Test that RequestAdapter receives unknown status if the wire is disconnected
// before the callback happens.
TEST_P(WireInstanceTests, RequestAdapterWireDisconnectBeforeCallback) {
- WGPURequestAdapterOptions options = {};
+ wgpu::RequestAdapterOptions options = {};
InstanceRequestAdapter(instance, &options, nullptr);
ExpectWireCallbacksWhen([&](auto& mockCb) {
diff --git a/src/dawn/tests/unittests/wire/WireTest.cpp b/src/dawn/tests/unittests/wire/WireTest.cpp
index eb687ec..3bf47b8 100644
--- a/src/dawn/tests/unittests/wire/WireTest.cpp
+++ b/src/dawn/tests/unittests/wire/WireTest.cpp
@@ -83,19 +83,18 @@
dawnProcSetProcs(&dawn::wire::client::GetProcs());
auto reservedInstance = GetWireClient()->ReserveInstance();
- instance = reservedInstance.instance;
+ instance = wgpu::Instance::Acquire(reservedInstance.instance);
apiInstance = api.GetNewInstance();
EXPECT_CALL(api, InstanceAddRef(apiInstance));
EXPECT_TRUE(GetWireServer()->InjectInstance(apiInstance, reservedInstance.handle));
// Create the adapter for testing.
apiAdapter = api.GetNewAdapter();
- WGPURequestAdapterOptions adapterOpts = {};
- MockCallback<WGPURequestAdapterCallback2> adapterCb;
- wgpuInstanceRequestAdapter2(instance, &adapterOpts,
- {nullptr, WGPUCallbackMode_AllowSpontaneous, adapterCb.Callback(),
- nullptr, adapterCb.MakeUserdata(this)});
- EXPECT_CALL(api, OnInstanceRequestAdapter2(apiInstance, NotNull(), _)).WillOnce([&]() {
+ MockCallback<void (*)(wgpu::RequestAdapterStatus, wgpu::Adapter, const char*, void*)> adapterCb;
+ instance.RequestAdapter(nullptr, wgpu::CallbackMode::AllowSpontaneous, adapterCb.Callback(),
+ adapterCb.MakeUserdata(this));
+
+ EXPECT_CALL(api, OnInstanceRequestAdapter2(apiInstance, _, _)).WillOnce([&]() {
EXPECT_CALL(api, AdapterHasFeature(apiAdapter, _)).WillRepeatedly(Return(false));
EXPECT_CALL(api, AdapterGetInfo(apiAdapter, NotNull()))
@@ -122,13 +121,10 @@
apiAdapter, nullptr);
});
FlushClient();
- WGPUAdapter cAdapter = nullptr;
- EXPECT_CALL(adapterCb,
- Call(WGPURequestAdapterStatus_Success, NotNull(), nullptr, nullptr, this))
- .WillOnce(SaveArg<1>(&cAdapter));
+ EXPECT_CALL(adapterCb, Call(wgpu::RequestAdapterStatus::Success, NotNull(), nullptr, this))
+ .WillOnce(SaveArg<1>(&adapter));
FlushServer();
- EXPECT_NE(cAdapter, nullptr);
- adapter = wgpu::Adapter::Acquire(cAdapter);
+ EXPECT_NE(adapter, nullptr);
// Create the device for testing.
apiDevice = api.GetNewDevice();
@@ -195,6 +191,7 @@
adapter.MoveToCHandle();
}
+ instance = nullptr;
dawnProcSetProcs(nullptr);
// Derived classes should call the base TearDown() first. The client must
diff --git a/src/dawn/tests/unittests/wire/WireTest.h b/src/dawn/tests/unittests/wire/WireTest.h
index 1680a64..8fae353 100644
--- a/src/dawn/tests/unittests/wire/WireTest.h
+++ b/src/dawn/tests/unittests/wire/WireTest.h
@@ -153,7 +153,7 @@
testing::MockCallback<WGPUDeviceLostCallback2> deviceLostCallback;
testing::MockCallback<WGPUUncapturedErrorCallback> uncapturedErrorCallback;
- WGPUInstance instance;
+ wgpu::Instance instance;
WGPUInstance apiInstance;
wgpu::Adapter adapter;
WGPUAdapter apiAdapter;
diff --git a/src/dawn/wire/client/Instance.cpp b/src/dawn/wire/client/Instance.cpp
index 5c760cd..2e8a43a 100644
--- a/src/dawn/wire/client/Instance.cpp
+++ b/src/dawn/wire/client/Instance.cpp
@@ -150,7 +150,9 @@
: RefCountedWithExternalCount<ObjectWithEventsBase>(params, params.handle) {}
void Instance::WillDropLastExternalRef() {
- GetEventManager().TransitionTo(EventManager::State::InstanceDropped);
+ if (IsRegistered()) {
+ GetEventManager().TransitionTo(EventManager::State::InstanceDropped);
+ }
Unregister();
}
