Add 'reason' argument to device lost callback

Breaking change, but it should only require small changes in any project
that relies on it, so just doing this instead of a two-stage deprecation.
Will require a manual roll into (at least) Chromium.

Bug: dawn:1080, chromium:1253721
Change-Id: I6699e0629c3b2fe63e7f9d5ba0a928f00316a588
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/64520
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Kai Ninomiya 2021-09-28 11:52:17 +00:00 committed by Corentin Wallez
parent affb7a3ab9
commit 51791e0409
16 changed files with 67 additions and 39 deletions

View File

@ -1004,6 +1004,7 @@
"device lost callback": { "device lost callback": {
"category": "callback", "category": "callback",
"args": [ "args": [
{"name": "reason", "type": "device lost reason"},
{"name": "message", "type": "char", "annotation": "const*"}, {"name": "message", "type": "char", "annotation": "const*"},
{"name": "userdata", "type": "void", "annotation": "*"} {"name": "userdata", "type": "void", "annotation": "*"}
] ]
@ -1012,8 +1013,8 @@
"category": "enum", "category": "enum",
"emscripten_no_enum_table": true, "emscripten_no_enum_table": true,
"values": [ "values": [
{"name": "undefined", "value": 0, "jsrepr": "undefined"}, {"value": 0, "name": "undefined", "jsrepr": "undefined"},
{"name": "destroyed", "value": 1} {"value": 1, "name": "destroyed"}
] ]
}, },
"device properties": { "device properties": {

View File

@ -118,6 +118,7 @@
], ],
"device lost callback" : [ "device lost callback" : [
{ "name": "device", "type": "ObjectHandle", "handle_type": "device" }, { "name": "device", "type": "ObjectHandle", "handle_type": "device" },
{ "name": "reason", "type": "device lost reason" },
{ "name": "message", "type": "char", "annotation": "const*", "length": "strlen" } { "name": "message", "type": "char", "annotation": "const*", "length": "strlen" }
], ],
"device pop error scope callback": [ "device pop error scope callback": [

View File

@ -209,7 +209,7 @@ namespace dawn_native {
} }
}; };
mDeviceLostCallback = [](char const*, void*) { mDeviceLostCallback = [](WGPUDeviceLostReason, char const*, void*) {
static bool calledOnce = false; static bool calledOnce = false;
if (!calledOnce) { if (!calledOnce) {
calledOnce = true; calledOnce = true;
@ -365,7 +365,9 @@ namespace dawn_native {
if (type == InternalErrorType::DeviceLost) { if (type == InternalErrorType::DeviceLost) {
// The device was lost, call the application callback. // The device was lost, call the application callback.
if (mDeviceLostCallback != nullptr) { if (mDeviceLostCallback != nullptr) {
mDeviceLostCallback(message, mDeviceLostUserdata); // TODO(crbug.com/dawn/628): Make sure the "Destroyed" reason is passed if
// the device was destroyed.
mDeviceLostCallback(WGPUDeviceLostReason_Undefined, message, mDeviceLostUserdata);
mDeviceLostCallback = nullptr; mDeviceLostCallback = nullptr;
} }

View File

@ -138,7 +138,8 @@ namespace dawn_wire { namespace client {
{ {
for (LinkNode<ObjectBase>* device = deviceList.head(); device != deviceList.end(); for (LinkNode<ObjectBase>* device = deviceList.head(); device != deviceList.end();
device = device->next()) { device = device->next()) {
static_cast<Device*>(device->value())->HandleDeviceLost("GPU connection lost"); static_cast<Device*>(device->value())
->HandleDeviceLost(WGPUDeviceLostReason_Undefined, "GPU connection lost");
} }
} }
for (auto& objectList : mObjects) { for (auto& objectList : mObjects) {

View File

@ -52,12 +52,14 @@ namespace dawn_wire { namespace client {
return true; return true;
} }
bool Client::DoDeviceLostCallback(Device* device, char const* message) { bool Client::DoDeviceLostCallback(Device* device,
WGPUDeviceLostReason reason,
char const* message) {
if (device == nullptr) { if (device == nullptr) {
// The device might have been deleted or recreated so this isn't an error. // The device might have been deleted or recreated so this isn't an error.
return true; return true;
} }
device->HandleDeviceLost(message); device->HandleDeviceLost(reason, message);
return true; return true;
} }

View File

@ -35,7 +35,7 @@ namespace dawn_wire { namespace client {
} }
}; };
mDeviceLostCallback = [](char const*, void*) { mDeviceLostCallback = [](WGPUDeviceLostReason, char const*, void*) {
static bool calledOnce = false; static bool calledOnce = false;
if (!calledOnce) { if (!calledOnce) {
calledOnce = true; calledOnce = true;
@ -80,10 +80,10 @@ namespace dawn_wire { namespace client {
} }
} }
void Device::HandleDeviceLost(const char* message) { void Device::HandleDeviceLost(WGPUDeviceLostReason reason, const char* message) {
if (mDeviceLostCallback && !mDidRunLostCallback) { if (mDeviceLostCallback && !mDidRunLostCallback) {
mDidRunLostCallback = true; mDidRunLostCallback = true;
mDeviceLostCallback(message, mDeviceLostUserdata); mDeviceLostCallback(reason, message, mDeviceLostUserdata);
} }
} }

View File

@ -53,7 +53,7 @@ namespace dawn_wire { namespace client {
void HandleError(WGPUErrorType errorType, const char* message); void HandleError(WGPUErrorType errorType, const char* message);
void HandleLogging(WGPULoggingType loggingType, const char* message); void HandleLogging(WGPULoggingType loggingType, const char* message);
void HandleDeviceLost(const char* message); void HandleDeviceLost(WGPUDeviceLostReason reason, const char* message);
bool OnPopErrorScopeCallback(uint64_t requestSerial, bool OnPopErrorScopeCallback(uint64_t requestSerial,
WGPUErrorType type, WGPUErrorType type,
const char* message); const char* message);

View File

@ -144,9 +144,9 @@ namespace dawn_wire { namespace server {
data->info.get()); data->info.get());
mProcs.deviceSetDeviceLostCallback( mProcs.deviceSetDeviceLostCallback(
device, device,
[](const char* message, void* userdata) { [](WGPUDeviceLostReason reason, const char* message, void* userdata) {
DeviceInfo* info = static_cast<DeviceInfo*>(userdata); DeviceInfo* info = static_cast<DeviceInfo*>(userdata);
info->server->OnDeviceLost(info->self, message); info->server->OnDeviceLost(info->self, reason, message);
}, },
data->info.get()); data->info.get());

View File

@ -196,7 +196,7 @@ namespace dawn_wire { namespace server {
// Error callbacks // Error callbacks
void OnUncapturedError(ObjectHandle device, WGPUErrorType type, const char* message); void OnUncapturedError(ObjectHandle device, WGPUErrorType type, const char* message);
void OnDeviceLost(ObjectHandle device, const char* message); void OnDeviceLost(ObjectHandle device, WGPUDeviceLostReason reason, const char* message);
void OnLogging(ObjectHandle device, WGPULoggingType type, const char* message); void OnLogging(ObjectHandle device, WGPULoggingType type, const char* message);
void OnDevicePopErrorScope(WGPUErrorType type, void OnDevicePopErrorScope(WGPUErrorType type,
const char* message, const char* message,

View File

@ -59,9 +59,12 @@ namespace dawn_wire { namespace server {
SerializeCommand(cmd); SerializeCommand(cmd);
} }
void Server::OnDeviceLost(ObjectHandle device, const char* message) { void Server::OnDeviceLost(ObjectHandle device,
WGPUDeviceLostReason reason,
const char* message) {
ReturnDeviceLostCallbackCmd cmd; ReturnDeviceLostCallbackCmd cmd;
cmd.device = device; cmd.device = device;
cmd.reason = reason;
cmd.message = message; cmd.message = message;
SerializeCommand(cmd); SerializeCommand(cmd);

View File

@ -1010,7 +1010,7 @@ void DawnTestBase::OnDeviceError(WGPUErrorType type, const char* message, void*
self->mError = true; self->mError = true;
} }
void DawnTestBase::OnDeviceLost(const char* message, void* userdata) { void DawnTestBase::OnDeviceLost(WGPUDeviceLostReason reason, const char* message, void* userdata) {
// Using ADD_FAILURE + ASSERT instead of FAIL to prevent the current test from continuing with a // Using ADD_FAILURE + ASSERT instead of FAIL to prevent the current test from continuing with a
// corrupt state. // corrupt state.
ADD_FAILURE() << "Device Lost during test: " << message; ADD_FAILURE() << "Device Lost during test: " << message;

View File

@ -492,7 +492,7 @@ class DawnTestBase {
// Tracking for validation errors // Tracking for validation errors
static void OnDeviceError(WGPUErrorType type, const char* message, void* userdata); static void OnDeviceError(WGPUErrorType type, const char* message, void* userdata);
static void OnDeviceLost(const char* message, void* userdata); static void OnDeviceLost(WGPUDeviceLostReason reason, const char* message, void* userdata);
bool mExpectError = false; bool mExpectError = false;
bool mError = false; bool mError = false;

View File

@ -25,12 +25,14 @@ using namespace testing;
class MockDeviceLostCallback { class MockDeviceLostCallback {
public: public:
MOCK_METHOD(void, Call, (const char* message, void* userdata)); MOCK_METHOD(void, Call, (WGPUDeviceLostReason reason, const char* message, void* userdata));
}; };
static std::unique_ptr<MockDeviceLostCallback> mockDeviceLostCallback; static std::unique_ptr<MockDeviceLostCallback> mockDeviceLostCallback;
static void ToMockDeviceLostCallback(const char* message, void* userdata) { static void ToMockDeviceLostCallback(WGPUDeviceLostReason reason,
mockDeviceLostCallback->Call(message, userdata); const char* message,
void* userdata) {
mockDeviceLostCallback->Call(reason, message, userdata);
DawnTestBase* self = static_cast<DawnTestBase*>(userdata); DawnTestBase* self = static_cast<DawnTestBase*>(userdata);
self->StartExpectDeviceError(); self->StartExpectDeviceError();
} }
@ -67,7 +69,8 @@ class DeviceLostTest : public DawnTest {
} }
void LoseForTesting() { void LoseForTesting() {
EXPECT_CALL(*mockDeviceLostCallback, Call(_, this)).Times(1); EXPECT_CALL(*mockDeviceLostCallback, Call(WGPUDeviceLostReason_Undefined, _, this))
.Times(1);
device.LoseForTesting(); device.LoseForTesting();
} }
@ -427,13 +430,13 @@ TEST_P(DeviceLostTest, QueueOnSubmittedWorkDoneBeforeLossFails) {
// Test that LostForTesting can only be called on one time // Test that LostForTesting can only be called on one time
TEST_P(DeviceLostTest, LoseForTestingOnce) { TEST_P(DeviceLostTest, LoseForTestingOnce) {
// First LoseForTesting call should occur normally. The callback is already set in SetUp. // First LoseForTesting call should occur normally. The callback is already set in SetUp.
EXPECT_CALL(*mockDeviceLostCallback, Call(_, this)).Times(1); EXPECT_CALL(*mockDeviceLostCallback, Call(WGPUDeviceLostReason_Undefined, _, this)).Times(1);
device.LoseForTesting(); device.LoseForTesting();
// Second LoseForTesting call should result in no callbacks. The LoseForTesting will return // Second LoseForTesting call should result in no callbacks. The LoseForTesting will return
// without doing anything when it sees that device has already been lost. // without doing anything when it sees that device has already been lost.
device.SetDeviceLostCallback(ToMockDeviceLostCallback, this); device.SetDeviceLostCallback(ToMockDeviceLostCallback, this);
EXPECT_CALL(*mockDeviceLostCallback, Call(_, this)).Times(0); EXPECT_CALL(*mockDeviceLostCallback, Call(_, _, this)).Times(0);
device.LoseForTesting(); device.LoseForTesting();
} }

View File

@ -319,7 +319,9 @@ TEST_P(SwapChainValidationTests, SwapChainIsInvalidAfterSurfaceDestruction_After
} }
// Test that after Device is Lost, all swap chain operations fail // Test that after Device is Lost, all swap chain operations fail
static void ToMockDeviceLostCallback(const char* message, void* userdata) { static void ToMockDeviceLostCallback(WGPUDeviceLostReason reason,
const char* message,
void* userdata) {
DawnTest* self = static_cast<DawnTest*>(userdata); DawnTest* self = static_cast<DawnTest*>(userdata);
self->StartExpectDeviceError(); self->StartExpectDeviceError();
} }

View File

@ -69,7 +69,8 @@ TEST_F(WireDisconnectTests, CallsDeviceLostCallback) {
mockDeviceLostCallback.MakeUserdata(this)); mockDeviceLostCallback.MakeUserdata(this));
// Disconnect the wire client. We should receive device lost only once. // Disconnect the wire client. We should receive device lost only once.
EXPECT_CALL(mockDeviceLostCallback, Call(_, this)).Times(Exactly(1)); EXPECT_CALL(mockDeviceLostCallback, Call(WGPUDeviceLostReason_Undefined, _, this))
.Times(Exactly(1));
GetWireClient()->Disconnect(); GetWireClient()->Disconnect();
GetWireClient()->Disconnect(); GetWireClient()->Disconnect();
} }
@ -80,14 +81,17 @@ TEST_F(WireDisconnectTests, ServerLostThenDisconnect) {
wgpuDeviceSetDeviceLostCallback(device, mockDeviceLostCallback.Callback(), wgpuDeviceSetDeviceLostCallback(device, mockDeviceLostCallback.Callback(),
mockDeviceLostCallback.MakeUserdata(this)); mockDeviceLostCallback.MakeUserdata(this));
api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, "some reason"); api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, WGPUDeviceLostReason_Undefined,
"some reason");
// Flush the device lost return command. // Flush the device lost return command.
EXPECT_CALL(mockDeviceLostCallback, Call(StrEq("some reason"), this)).Times(Exactly(1)); EXPECT_CALL(mockDeviceLostCallback,
Call(WGPUDeviceLostReason_Undefined, StrEq("some reason"), this))
.Times(Exactly(1));
FlushServer(); FlushServer();
// Disconnect the client. We shouldn't see the lost callback again. // Disconnect the client. We shouldn't see the lost callback again.
EXPECT_CALL(mockDeviceLostCallback, Call(_, _)).Times(Exactly(0)); EXPECT_CALL(mockDeviceLostCallback, Call(_, _, _)).Times(Exactly(0));
GetWireClient()->Disconnect(); GetWireClient()->Disconnect();
} }
@ -98,13 +102,15 @@ TEST_F(WireDisconnectTests, ServerLostThenDisconnectInCallback) {
wgpuDeviceSetDeviceLostCallback(device, mockDeviceLostCallback.Callback(), wgpuDeviceSetDeviceLostCallback(device, mockDeviceLostCallback.Callback(),
mockDeviceLostCallback.MakeUserdata(this)); mockDeviceLostCallback.MakeUserdata(this));
api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, "lost reason"); api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, WGPUDeviceLostReason_Undefined,
"lost reason");
// Disconnect the client inside the lost callback. We should see the callback // Disconnect the client inside the lost callback. We should see the callback
// only once. // only once.
EXPECT_CALL(mockDeviceLostCallback, Call(StrEq("lost reason"), this)) EXPECT_CALL(mockDeviceLostCallback,
Call(WGPUDeviceLostReason_Undefined, StrEq("lost reason"), this))
.WillOnce(InvokeWithoutArgs([&]() { .WillOnce(InvokeWithoutArgs([&]() {
EXPECT_CALL(mockDeviceLostCallback, Call(_, _)).Times(Exactly(0)); EXPECT_CALL(mockDeviceLostCallback, Call(_, _, _)).Times(Exactly(0));
GetWireClient()->Disconnect(); GetWireClient()->Disconnect();
})); }));
FlushServer(); FlushServer();
@ -117,13 +123,15 @@ TEST_F(WireDisconnectTests, DisconnectThenServerLost) {
mockDeviceLostCallback.MakeUserdata(this)); mockDeviceLostCallback.MakeUserdata(this));
// Disconnect the client. We should see the callback once. // Disconnect the client. We should see the callback once.
EXPECT_CALL(mockDeviceLostCallback, Call(_, this)).Times(Exactly(1)); EXPECT_CALL(mockDeviceLostCallback, Call(WGPUDeviceLostReason_Undefined, _, this))
.Times(Exactly(1));
GetWireClient()->Disconnect(); GetWireClient()->Disconnect();
// Lose the device on the server. The client callback shouldn't be // Lose the device on the server. The client callback shouldn't be
// called again. // called again.
api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, "lost reason"); api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, WGPUDeviceLostReason_Undefined,
EXPECT_CALL(mockDeviceLostCallback, Call(_, _)).Times(Exactly(0)); "lost reason");
EXPECT_CALL(mockDeviceLostCallback, Call(_, _, _)).Times(Exactly(0));
FlushServer(); FlushServer();
} }

View File

@ -56,12 +56,14 @@ namespace {
class MockDeviceLostCallback { class MockDeviceLostCallback {
public: public:
MOCK_METHOD(void, Call, (const char* message, void* userdata)); MOCK_METHOD(void, Call, (WGPUDeviceLostReason reason, const char* message, void* userdata));
}; };
std::unique_ptr<StrictMock<MockDeviceLostCallback>> mockDeviceLostCallback; std::unique_ptr<StrictMock<MockDeviceLostCallback>> mockDeviceLostCallback;
void ToMockDeviceLostCallback(const char* message, void* userdata) { void ToMockDeviceLostCallback(WGPUDeviceLostReason reason,
mockDeviceLostCallback->Call(message, userdata); const char* message,
void* userdata) {
mockDeviceLostCallback->Call(reason, message, userdata);
} }
} // anonymous namespace } // anonymous namespace
@ -319,9 +321,12 @@ TEST_F(WireErrorCallbackTests, DeviceLostCallback) {
// Calling the callback on the server side will result in the callback being called on the // Calling the callback on the server side will result in the callback being called on the
// client side // client side
api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, "Some error message"); api.CallDeviceSetDeviceLostCallbackCallback(apiDevice, WGPUDeviceLostReason_Undefined,
"Some error message");
EXPECT_CALL(*mockDeviceLostCallback, Call(StrEq("Some error message"), this)).Times(1); EXPECT_CALL(*mockDeviceLostCallback,
Call(WGPUDeviceLostReason_Undefined, StrEq("Some error message"), this))
.Times(1);
FlushServer(); FlushServer();
} }