Skip to content

Commit

Permalink
Bug 1751718 - Reject WebGPU device promise on wrong features and limi…
Browse files Browse the repository at this point in the history
…ts r=jimb

Differential Revision: https://phabricator.services.mozilla.com/D136770
  • Loading branch information
kvark committed Jan 28, 2022
1 parent 377caf3 commit e7c5d38
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 29 deletions.
31 changes: 26 additions & 5 deletions dom/webgpu/Adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,41 @@ already_AddRefed<dom::Promise> Adapter::RequestDevice(
}

ffi::WGPULimits limits = {};
Maybe<RawId> id = mBridge->AdapterRequestDevice(mId, aDesc, &limits);
if (id.isSome()) {
auto request = mBridge->AdapterRequestDevice(mId, aDesc, &limits);
if (request) {
RefPtr<Device> device =
new Device(this, id.value(), MakeUnique<ffi::WGPULimits>(limits));
new Device(this, request->mId, MakeUnique<ffi::WGPULimits>(limits));
// copy over the features
for (const auto& feature : aDesc.mRequiredFeatures) {
NS_ConvertASCIItoUTF16 string(
dom::GPUFeatureNameValues::GetString(feature));
dom::GPUSupportedFeatures_Binding::SetlikeHelpers::Add(device->mFeatures,
string, aRv);
}
promise->MaybeResolve(device);

request->mPromise->Then(
GetCurrentSerialEventTarget(), __func__,
[promise, device](bool aSuccess) {
if (aSuccess) {
promise->MaybeResolve(device);
} else {
// In this path, request->mId has an error entry in the wgpu
// registry, so let Device::~Device clean things up on both the
// child and parent side.
promise->MaybeRejectWithInvalidStateError(
"Unable to fulfill requested features and limits");
}
},
[promise, device](const ipc::ResponseRejectReason& aReason) {
// We can't be sure how far along the WebGPUParent got in handling
// our AdapterRequestDevice message, but we can't communicate with it,
// so clear up our client state for this Device without trying to
// communicate with the parent about it.
device->CleanupUnregisteredInParent();
promise->MaybeRejectWithNotSupportedError("IPC error");
});
} else {
promise->MaybeRejectWithNotSupportedError("Unable to instanciate a Device");
promise->MaybeRejectWithNotSupportedError("Unable to instantiate a Device");
}

return promise.forget();
Expand Down
9 changes: 8 additions & 1 deletion dom/webgpu/Device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Device::Device(Adapter* const aParent, RawId aId,
mLimits(new SupportedLimits(aParent, std::move(aRawLimits))),
mBridge(aParent->mBridge),
mQueue(new class Queue(this, aParent->mBridge, aId)) {
mBridge->RegisterDevice(mId, this);
mBridge->RegisterDevice(this);
}

Device::~Device() { Cleanup(); }
Expand All @@ -75,6 +75,13 @@ void Device::Cleanup() {
}
}

void Device::CleanupUnregisteredInParent() {
if (mBridge) {
mBridge->FreeUnregisteredInParentDevice(mId);
}
mValid = false;
}

void Device::GetLabel(nsAString& aValue) const { aValue = mLabel; }
void Device::SetLabel(const nsAString& aLabel) { mLabel = aLabel; }

Expand Down
2 changes: 2 additions & 0 deletions dom/webgpu/Device.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class Device final : public DOMEventTargetHelper, public SupportsWeakPtr {
gfx::IntSize* aDefaultSize);
bool CheckNewWarning(const nsACString& aMessage);

void CleanupUnregisteredInParent();

private:
~Device();
void Cleanup();
Expand Down
2 changes: 1 addition & 1 deletion dom/webgpu/ipc/PWebGPU.ipdl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ parent:
async BumpImplicitBindGroupLayout(RawId pipelineId, bool isCompute, uint32_t index, RawId assignId);

async InstanceRequestAdapter(GPURequestAdapterOptions options, RawId[] ids) returns (ByteBuf byteBuf);
async AdapterRequestDevice(RawId selfId, ByteBuf buf, RawId newId);
async AdapterRequestDevice(RawId selfId, ByteBuf buf, RawId newId) returns (bool success);
async AdapterDestroy(RawId selfId);
async BufferReturnShmem(RawId selfId, Shmem shmem);
async BufferMap(RawId selfId, WGPUHostMap hostMap, uint64_t offset, uint64_t size) returns (Shmem sm);
Expand Down
30 changes: 18 additions & 12 deletions dom/webgpu/ipc/WebGPUChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,15 @@ RefPtr<AdapterPromise> WebGPUChild::InstanceRequestAdapter(
});
}

Maybe<RawId> WebGPUChild::AdapterRequestDevice(
Maybe<DeviceRequest> WebGPUChild::AdapterRequestDevice(
RawId aSelfId, const dom::GPUDeviceDescriptor& aDesc,
ffi::WGPULimits* aLimits) {
RawId id = ffi::wgpu_client_make_device_id(mClient, aSelfId);

ffi::WGPUDeviceDescriptor desc = {};
ffi::wgpu_client_fill_default_limits(&desc.limits);

const auto featureBits = Adapter::MakeFeatureBits(aDesc.mRequiredFeatures);
if (!featureBits) {
return {};
return Nothing();
}
desc.features = *featureBits;

Expand Down Expand Up @@ -283,14 +281,17 @@ Maybe<RawId> WebGPUChild::AdapterRequestDevice(
}
}

RawId id = ffi::wgpu_client_make_device_id(mClient, aSelfId);

ByteBuf bb;
ffi::wgpu_client_serialize_device_descriptor(&desc, ToFFI(&bb));
if (SendAdapterRequestDevice(aSelfId, std::move(bb), id)) {
*aLimits = desc.limits;
return Some(id);
}
ffi::wgpu_client_kill_device_id(mClient, id);
return Nothing();

DeviceRequest request;
request.mId = id;
request.mPromise = SendAdapterRequestDevice(aSelfId, std::move(bb), id);
*aLimits = desc.limits;

return Some(std::move(request));
}

RawId WebGPUChild::DeviceCreateBuffer(RawId aSelfId,
Expand Down Expand Up @@ -979,8 +980,8 @@ void WebGPUChild::SwapChainPresent(wr::ExternalImageId aExternalImageId,
SendSwapChainPresent(aExternalImageId, aTextureId, encoderId);
}

void WebGPUChild::RegisterDevice(RawId aId, Device* aDevice) {
mDeviceMap.insert({aId, aDevice});
void WebGPUChild::RegisterDevice(Device* const aDevice) {
mDeviceMap.insert({aDevice->mId, aDevice});
}

void WebGPUChild::UnregisterDevice(RawId aId) {
Expand All @@ -990,5 +991,10 @@ void WebGPUChild::UnregisterDevice(RawId aId) {
}
}

void WebGPUChild::FreeUnregisteredInParentDevice(RawId aId) {
ffi::wgpu_client_kill_device_id(mClient, aId);
mDeviceMap.erase(aId);
}

} // namespace webgpu
} // namespace mozilla
17 changes: 13 additions & 4 deletions dom/webgpu/ipc/WebGPUChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,21 @@ struct WGPUTextureViewDescriptor;
using AdapterPromise =
MozPromise<ipc::ByteBuf, Maybe<ipc::ResponseRejectReason>, true>;
using PipelinePromise = MozPromise<RawId, ipc::ResponseRejectReason, true>;
using DevicePromise = MozPromise<bool, ipc::ResponseRejectReason, true>;

struct PipelineCreationContext {
RawId mParentId = 0;
RawId mImplicitPipelineLayoutId = 0;
nsTArray<RawId> mImplicitBindGroupLayoutIds;
};

struct DeviceRequest {
RawId mId = 0;
RefPtr<DevicePromise> mPromise;
// Note: we could put `ffi::WGPULimits` in here as well,
// but we don't want to #include ffi stuff in this header
};

ffi::WGPUByteBuf* ToFFI(ipc::ByteBuf* x);

class WebGPUChild final : public PWebGPUChild, public SupportsWeakPtr {
Expand All @@ -50,9 +58,9 @@ class WebGPUChild final : public PWebGPUChild, public SupportsWeakPtr {

RefPtr<AdapterPromise> InstanceRequestAdapter(
const dom::GPURequestAdapterOptions& aOptions);
Maybe<RawId> AdapterRequestDevice(RawId aSelfId,
const dom::GPUDeviceDescriptor& aDesc,
ffi::WGPULimits* aLimtis);
Maybe<DeviceRequest> AdapterRequestDevice(
RawId aSelfId, const dom::GPUDeviceDescriptor& aDesc,
ffi::WGPULimits* aLimits);
RawId DeviceCreateBuffer(RawId aSelfId,
const dom::GPUBufferDescriptor& aDesc);
RawId DeviceCreateTexture(RawId aSelfId,
Expand Down Expand Up @@ -95,8 +103,9 @@ class WebGPUChild final : public PWebGPUChild, public SupportsWeakPtr {
wr::ExternalImageId aExternalImageId);
void SwapChainPresent(wr::ExternalImageId aExternalImageId, RawId aTextureId);

void RegisterDevice(RawId aId, Device* aDevice);
void RegisterDevice(Device* const aDevice);
void UnregisterDevice(RawId aId);
void FreeUnregisteredInParentDevice(RawId aId);

static void ConvertTextureFormatRef(const dom::GPUTextureFormat& aInput,
ffi::WGPUTextureFormat& aOutput);
Expand Down
11 changes: 8 additions & 3 deletions dom/webgpu/ipc/WebGPUParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,17 @@ ipc::IPCResult WebGPUParent::RecvInstanceRequestAdapter(
}

ipc::IPCResult WebGPUParent::RecvAdapterRequestDevice(
RawId aSelfId, const ipc::ByteBuf& aByteBuf, RawId aNewId) {
RawId aSelfId, const ipc::ByteBuf& aByteBuf, RawId aNewId,
AdapterRequestDeviceResolver&& resolver) {
ErrorBuffer error;
ffi::wgpu_server_adapter_request_device(mContext, aSelfId, ToFFI(&aByteBuf),
aNewId, error.ToFFI());
mErrorScopeMap.insert({aSelfId, ErrorScopeStack()});
ForwardError(0, error);
if (ForwardError(0, error)) {
resolver(false);
} else {
mErrorScopeMap.insert({aSelfId, ErrorScopeStack()});
resolver(true);
}
return IPC_OK();
}

Expand Down
6 changes: 3 additions & 3 deletions dom/webgpu/ipc/WebGPUParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ class WebGPUParent final : public PWebGPUParent {
const dom::GPURequestAdapterOptions& aOptions,
const nsTArray<RawId>& aTargetIds,
InstanceRequestAdapterResolver&& resolver);
ipc::IPCResult RecvAdapterRequestDevice(RawId aSelfId,
const ipc::ByteBuf& aByteBuf,
RawId aNewId);
ipc::IPCResult RecvAdapterRequestDevice(
RawId aSelfId, const ipc::ByteBuf& aByteBuf, RawId aNewId,
AdapterRequestDeviceResolver&& resolver);
ipc::IPCResult RecvAdapterDestroy(RawId aSelfId);
ipc::IPCResult RecvDeviceDestroy(RawId aSelfId);
ipc::IPCResult RecvBufferReturnShmem(RawId aSelfId, Shmem&& aShmem);
Expand Down

0 comments on commit e7c5d38

Please sign in to comment.