Commit c3a3aa7c authored by Austin Eng's avatar Austin Eng Committed by Chromium LUCI CQ

WebGPU: Only disconnect, don't destroy the client context on page teardown

We cannot immediately destroy the context as JS code can still execute
after the iframe is removed. Instead we disconnect the context so that
new commands do nothing. Now, the context is not destroyed until all
WebGPU objects are GC'ed. A future change will improve this to tear down
the server GPU objects but keep the client context alive until GC.

Bug: 1160459
Change-Id: Iba58279a565d0099bf13420e1743003fc7c7aa23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613517
Auto-Submit: Austin Eng <enga@chromium.org>
Reviewed-by: default avatarKai Ninomiya <kainino@chromium.org>
Reviewed-by: default avatarCorentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845175}
parent fe0ad27c
<!--
Iframes can be tricky because even after they're removed from the page,
script can continue to run.
So, WebGPU cannot assume that WebGPU resources can be immediately
destroyed with the page.
This test tests a cases one after another by inserting an iframe into the page
with the test case script and waiting for the test to call
|removeIframe| which should remove itself from the DOM. After removal, we wait a bit
and then continue on to the next case.
-->
<html>
<head>
<script>
// |runTestCase| creates an iframe containing the stringified script source of |testFn|.
// It returns a Promise and saves the |resolve| function as |iframeRemovedCallback|.
// This callback is called in removeIframe, so when the Promise resolves, we know
// the iframe containing the test case page has been removed.
let iframeRemovedCallback;
function runTestCase(testFn) {
return new Promise(resolve => {
iframeRemovedCallback = resolve;
let iframe = document.createElement("iframe");
iframe.setAttribute("id", "iframe");
document.body.appendChild(iframe);
const head = iframe.contentWindow.document.getElementsByTagName('head')[0];
const script = iframe.contentWindow.document.createElement('script');
script.innerText = `(${testFn.toString()})().catch(parent.fail);`;
script.type = 'text/javascript';
head.appendChild(script);
});
}
// Called by the inner iframe to remove itself.
function removeIframe() {
const iframe = document.getElementById('iframe');
iframe.parentNode.removeChild(iframe);
// Wait a bit before signaling that the test case is finished.
setTimeout(iframeRemovedCallback, 100);
}
function fail(err) {
if (window.domAutomationController) {
console.log(err);
} else {
alert(err);
}
window.domAutomationController.send('FAILED');
}
async function runTests() {
// Test running basic WebGPU commands after the iframe is removed.
await runTestCase(async () => {
let device;
try {
const adapter = await navigator.gpu.requestAdapter();
device = await adapter.requestDevice();
} catch {
console.warn('WebGPU not supported');
parent.removeIframe();
return;
}
parent.removeIframe();
device.defaultQueue.submit([device.createCommandEncoder().finish()]);
});
// Test getting a swapchain texture after the iframe is removed.
// This may interact with the shared image interfaces.
await runTestCase(async () => {
const adapter = navigator.gpu && await navigator.gpu.requestAdapter();
if (!adapter) {
console.warn('WebGPU not supported');
parent.removeIframe();
return;
}
const device = await adapter.requestDevice();
const canvas = document.createElement('canvas');
const context = canvas.getContext('gpupresent');
const sc = context.configureSwapChain({
device,
format: 'bgra8unorm'
});
parent.removeIframe();
const texture = sc.getCurrentTexture();
});
// Test reconfiguring the swapchain after the iframe is removed.
// This may interact with the shared image interfaces.
// The expecation is that reconfiguring after removal does not crash, but
// does throw an exception. It is invalid to call |configureSwapChain| after
// the canvas is destroyed.
await runTestCase(async () => {
const adapter = navigator.gpu && await navigator.gpu.requestAdapter();
if (!adapter) {
console.warn('WebGPU not supported');
parent.removeIframe();
return;
}
const device = await adapter.requestDevice();
const canvas = document.createElement('canvas');
const context = canvas.getContext('gpupresent');
const sc = context.configureSwapChain({
device,
format: 'bgra8unorm'
});
const texture = sc.getCurrentTexture();
parent.removeIframe();
try {
/* Throws an exception since the canvas is destroyed */
context.configureSwapChain({
device,
format: 'bgra8unorm'
});
} catch (err) {
}
});
window.domAutomationController.send('FINISHED');
}
</script>
</head>
<body onload="runTests()"></body>
</html>
......@@ -119,7 +119,9 @@ class GpuProcessIntegrationTest(gpu_integration_test.GpuIntegrationTest):
'gpu/functional_blank.html'), ('GpuProcess_swiftshader_for_webgl',
'gpu/functional_webgl.html'),
('GpuProcess_webgl_disabled_extension',
'gpu/functional_webgl_disabled_extension.html'))
'gpu/functional_webgl_disabled_extension.html'),
('GpuProcess_webgpu_iframe_removed',
'gpu/webgpu-iframe-removed.html'))
for t in tests:
yield (t[0], t[1], ('_' + t[0]))
......@@ -587,6 +589,12 @@ class GpuProcessIntegrationTest(gpu_integration_test.GpuIntegrationTest):
'Backgrounded high-performance WebGL context did not release the '
'hold on the high-performance GPU')
def _GpuProcess_webgpu_iframe_removed(self, test_path):
self.RestartBrowserIfNecessaryWithArgs([
'--enable-unsafe-webgpu',
])
self._NavigateAndWait(test_path)
@classmethod
def ExpectationsFiles(cls):
return [
......
......@@ -615,6 +615,13 @@ void WebGPUImplementation::FlushAwaitingCommands(
#endif
}
void WebGPUImplementation::DisconnectContextAndDestroyServer() {
// Treat this like a context lost since the context is no longer usable.
// TODO(crbug.com/1160459): Also send a message to eagerly free server-side
// resources.
OnGpuControlLostContextMaybeReentrant();
}
WGPUDevice WebGPUImplementation::GetDevice(
DawnDeviceClientID device_client_id) {
#if BUILDFLAG(USE_DAWN)
......
......@@ -163,6 +163,7 @@ class WEBGPU_EXPORT WebGPUImplementation final : public WebGPUInterface,
void EnsureAwaitingFlush(DawnDeviceClientID device_client_id,
bool* needs_flush) override;
void FlushAwaitingCommands(DawnDeviceClientID device_client_id) override;
void DisconnectContextAndDestroyServer() override;
WGPUDevice GetDevice(DawnDeviceClientID device_client_id) override;
ReservedTexture ReserveTexture(DawnDeviceClientID device_client_id) override;
bool RequestAdapterAsync(
......
......@@ -46,6 +46,10 @@ class WebGPUInterface : public InterfaceBase {
// nothing.
virtual void FlushAwaitingCommands(DawnDeviceClientID device_client_id) = 0;
// Disconnect. All commands should become a no-op and server-side resources
// can be freed.
virtual void DisconnectContextAndDestroyServer() = 0;
virtual WGPUDevice GetDevice(DawnDeviceClientID device_client_id) = 0;
virtual ReservedTexture ReserveTexture(
DawnDeviceClientID device_client_id) = 0;
......
......@@ -29,6 +29,7 @@ void WebGPUInterfaceStub::EnsureAwaitingFlush(
bool* needs_flush) {}
void WebGPUInterfaceStub::FlushAwaitingCommands(
DawnDeviceClientID device_client_id) {}
void WebGPUInterfaceStub::DisconnectContextAndDestroyServer() {}
WGPUDevice WebGPUInterfaceStub::GetDevice(DawnDeviceClientID device_client_id) {
return nullptr;
}
......
......@@ -29,6 +29,7 @@ class WebGPUInterfaceStub : public WebGPUInterface {
void EnsureAwaitingFlush(DawnDeviceClientID device_client_id,
bool* needs_flush) override;
void FlushAwaitingCommands(DawnDeviceClientID device_client_id) override;
void DisconnectContextAndDestroyServer() override;
WGPUDevice GetDevice(DawnDeviceClientID device_client_id) override;
ReservedTexture ReserveTexture(DawnDeviceClientID device_client_id) override;
bool RequestAdapterAsync(
......
......@@ -19,10 +19,6 @@ DawnObjectBase::GetDawnControlClient() const {
return dawn_control_client_;
}
bool DawnObjectBase::IsDawnControlClientDestroyed() const {
return dawn_control_client_->IsDestroyed();
}
gpu::webgpu::WebGPUInterface* DawnObjectBase::GetInterface() const {
return dawn_control_client_->GetInterface();
}
......@@ -38,9 +34,6 @@ DawnDeviceClientSerializerHolder::DawnDeviceClientSerializerHolder(
device_client_id_(device_client_id) {}
DawnDeviceClientSerializerHolder::~DawnDeviceClientSerializerHolder() {
if (dawn_control_client_->IsDestroyed()) {
return;
}
dawn_control_client_->GetInterface()->RemoveDevice(device_client_id_);
}
......@@ -49,9 +42,6 @@ DeviceTreeObject::GetDawnControlClient() const {
return device_client_serializer_holder_->dawn_control_client_;
}
bool DeviceTreeObject::IsDawnControlClientDestroyed() const {
return GetDawnControlClient()->IsDestroyed();
}
gpu::webgpu::WebGPUInterface* DeviceTreeObject::GetInterface() const {
return GetDawnControlClient()->GetInterface();
}
......@@ -74,9 +64,6 @@ void DeviceTreeObject::EnsureFlush() {
}
Microtask::EnqueueMicrotask(WTF::Bind(
[](scoped_refptr<DawnDeviceClientSerializerHolder> holder) {
if (holder->dawn_control_client_->IsDestroyed()) {
return;
}
holder->dawn_control_client_->GetInterface()->FlushAwaitingCommands(
holder->device_client_id_);
},
......
......@@ -35,7 +35,6 @@ class DawnObjectBase {
scoped_refptr<DawnControlClientHolder> dawn_control_client);
const scoped_refptr<DawnControlClientHolder>& GetDawnControlClient() const;
bool IsDawnControlClientDestroyed() const;
gpu::webgpu::WebGPUInterface* GetInterface() const;
const DawnProcTable& GetProcs() const;
......@@ -82,7 +81,6 @@ class DeviceTreeObject {
std::move(device_client_seralizer_holder)) {}
const scoped_refptr<DawnControlClientHolder>& GetDawnControlClient() const;
bool IsDawnControlClientDestroyed() const;
gpu::webgpu::WebGPUInterface* GetInterface() const;
const DawnProcTable& GetProcs() const;
......
......@@ -78,9 +78,6 @@ GPUBindGroup::GPUBindGroup(GPUDevice* device, WGPUBindGroup bind_group)
: DawnObject<WGPUBindGroup>(device, bind_group) {}
GPUBindGroup::~GPUBindGroup() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().bindGroupRelease(GetHandle());
}
......
......@@ -135,9 +135,6 @@ GPUBindGroupLayout::GPUBindGroupLayout(GPUDevice* device,
: DawnObject<WGPUBindGroupLayout>(device, bind_group_layout) {}
GPUBindGroupLayout::~GPUBindGroupLayout() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().bindGroupLayoutRelease(GetHandle());
}
......
......@@ -80,9 +80,6 @@ GPUBuffer::GPUBuffer(GPUDevice* device,
}
GPUBuffer::~GPUBuffer() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().bufferRelease(GetHandle());
}
......
......@@ -13,9 +13,6 @@ GPUCommandBuffer::GPUCommandBuffer(GPUDevice* device,
: DawnObject<WGPUCommandBuffer>(device, command_buffer) {}
GPUCommandBuffer::~GPUCommandBuffer() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().commandBufferRelease(GetHandle());
}
......
......@@ -166,9 +166,6 @@ GPUCommandEncoder::GPUCommandEncoder(GPUDevice* device,
: DawnObject<WGPUCommandEncoder>(device, command_encoder) {}
GPUCommandEncoder::~GPUCommandEncoder() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().commandEncoderRelease(GetHandle());
}
......
......@@ -18,9 +18,6 @@ GPUComputePassEncoder::GPUComputePassEncoder(
: DawnObject<WGPUComputePassEncoder>(device, compute_pass_encoder) {}
GPUComputePassEncoder::~GPUComputePassEncoder() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().computePassEncoderRelease(GetHandle());
}
......
......@@ -59,9 +59,6 @@ GPUComputePipeline::GPUComputePipeline(GPUDevice* device,
: DawnObject<WGPUComputePipeline>(device, compute_pipeline) {}
GPUComputePipeline::~GPUComputePipeline() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().computePipelineRelease(GetHandle());
}
......
......@@ -85,9 +85,6 @@ GPUDevice::GPUDevice(ExecutionContext* execution_context,
}
GPUDevice::~GPUDevice() {
if (IsDawnControlClientDestroyed()) {
return;
}
queue_ = nullptr;
GetProcs().deviceRelease(GetHandle());
}
......
......@@ -18,9 +18,6 @@ GPUFence::GPUFence(GPUDevice* device, WGPUFence fence)
: DawnObject<WGPUFence>(device, fence) {}
GPUFence::~GPUFence() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().fenceRelease(GetHandle());
}
......
......@@ -45,9 +45,6 @@ GPUPipelineLayout::GPUPipelineLayout(GPUDevice* device,
: DawnObject<WGPUPipelineLayout>(device, pipeline_layout) {}
GPUPipelineLayout::~GPUPipelineLayout() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().pipelineLayoutRelease(GetHandle());
}
......
......@@ -46,9 +46,6 @@ GPUQuerySet::GPUQuerySet(GPUDevice* device, WGPUQuerySet querySet)
: DawnObject<WGPUQuerySet>(device, querySet) {}
GPUQuerySet::~GPUQuerySet() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().querySetRelease(GetHandle());
}
......
......@@ -134,10 +134,6 @@ GPUQueue::GPUQueue(GPUDevice* device, WGPUQueue queue)
GPUQueue::~GPUQueue() {
produce_dawn_texture_handler_ = nullptr;
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().queueRelease(GetHandle());
}
......
......@@ -13,9 +13,6 @@ GPURenderBundle::GPURenderBundle(GPUDevice* device,
: DawnObject<WGPURenderBundle>(device, render_bundle) {}
GPURenderBundle::~GPURenderBundle() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().renderBundleRelease(GetHandle());
}
......
......@@ -55,9 +55,6 @@ GPURenderBundleEncoder::GPURenderBundleEncoder(
: DawnObject<WGPURenderBundleEncoder>(device, render_bundle_encoder) {}
GPURenderBundleEncoder::~GPURenderBundleEncoder() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().renderBundleEncoderRelease(GetHandle());
}
......
......@@ -23,9 +23,6 @@ GPURenderPassEncoder::GPURenderPassEncoder(
: DawnObject<WGPURenderPassEncoder>(device, render_pass_encoder) {}
GPURenderPassEncoder::~GPURenderPassEncoder() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().renderPassEncoderRelease(GetHandle());
}
......
......@@ -291,9 +291,6 @@ GPURenderPipeline::GPURenderPipeline(GPUDevice* device,
: DawnObject<WGPURenderPipeline>(device, render_pipeline) {}
GPURenderPipeline::~GPURenderPipeline() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().renderPipelineRelease(GetHandle());
}
......
......@@ -61,9 +61,6 @@ GPUSampler::GPUSampler(GPUDevice* device, WGPUSampler sampler)
: DawnObject<WGPUSampler>(device, sampler) {}
GPUSampler::~GPUSampler() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().samplerRelease(GetHandle());
}
......
......@@ -65,9 +65,6 @@ GPUShaderModule::GPUShaderModule(GPUDevice* device,
: DawnObject<WGPUShaderModule>(device, shader_module) {}
GPUShaderModule::~GPUShaderModule() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().shaderModuleRelease(GetHandle());
}
......
......@@ -96,9 +96,6 @@ GPUTexture::GPUTexture(GPUDevice* device,
: DawnObject<WGPUTexture>(device, texture), format_(format) {}
GPUTexture::~GPUTexture() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().textureRelease(GetHandle());
}
......
......@@ -12,9 +12,6 @@ GPUTextureView::GPUTextureView(GPUDevice* device, WGPUTextureView texture_view)
: DawnObject<WGPUTextureView>(device, texture_view) {}
GPUTextureView::~GPUTextureView() {
if (IsDawnControlClientDestroyed()) {
return;
}
GetProcs().textureViewRelease(GetHandle());
}
......
......@@ -21,12 +21,8 @@ void DawnControlClientHolder::SetLostContextCallback() {
}
void DawnControlClientHolder::Destroy() {
interface_ = nullptr;
context_provider_.reset();
}
bool DawnControlClientHolder::IsDestroyed() const {
return !interface_;
SetContextLost();
interface_->DisconnectContextAndDestroyServer();
}
WebGraphicsContext3DProvider* DawnControlClientHolder::GetContextProvider()
......
......@@ -32,7 +32,6 @@ class PLATFORM_EXPORT DawnControlClientHolder
std::unique_ptr<WebGraphicsContext3DProvider> context_provider);
void Destroy();
bool IsDestroyed() const;
WebGraphicsContext3DProvider* GetContextProvider() const;
gpu::webgpu::WebGPUInterface* GetInterface() const;
......
......@@ -151,8 +151,6 @@ DawnTextureFromImageBitmap::~DawnTextureFromImageBitmap() {
WGPUTexture DawnTextureFromImageBitmap::ProduceDawnTextureFromImageBitmap(
scoped_refptr<StaticBitmapImage> image) {
DCHECK(!dawn_control_client_->IsDestroyed());
associated_resource_ = image->GetMailboxHolder().mailbox;
// Produce and inject image to WebGPU texture
......@@ -174,7 +172,6 @@ WGPUTexture DawnTextureFromImageBitmap::ProduceDawnTextureFromImageBitmap(
}
void DawnTextureFromImageBitmap::FinishDawnTextureFromImageBitmapAccess() {
DCHECK(!dawn_control_client_->IsDestroyed());
DCHECK_NE(wire_texture_id_, 0u);
gpu::webgpu::WebGPUInterface* webgpu = dawn_control_client_->GetInterface();
......
......@@ -78,7 +78,7 @@ void WebGPUSwapBufferProvider::Neuter() {
layer_ = nullptr;
}
if (current_swap_buffer_ && !dawn_control_client_->IsDestroyed()) {
if (current_swap_buffer_) {
// Ensure we wait for previous WebGPU commands before destroying the shared
// image.
gpu::webgpu::WebGPUInterface* webgpu = dawn_control_client_->GetInterface();
......@@ -92,7 +92,7 @@ void WebGPUSwapBufferProvider::Neuter() {
}
WGPUTexture WebGPUSwapBufferProvider::GetNewTexture(const IntSize& size) {
DCHECK(!current_swap_buffer_ && !dawn_control_client_->IsDestroyed());
DCHECK(!current_swap_buffer_);
gpu::webgpu::WebGPUInterface* webgpu = dawn_control_client_->GetInterface();
gpu::SharedImageInterface* sii =
......@@ -138,7 +138,7 @@ bool WebGPUSwapBufferProvider::PrepareTransferableResource(
cc::SharedBitmapIdRegistrar* bitmap_registrar,
viz::TransferableResource* out_resource,
std::unique_ptr<viz::SingleReleaseCallback>* out_release_callback) {
DCHECK(!neutered_ && !dawn_control_client_->IsDestroyed());
DCHECK(!neutered_);
if (!current_swap_buffer_ || neutered_) {
return false;
......@@ -213,9 +213,6 @@ WebGPUSwapBufferProvider::SwapBuffer::SwapBuffer(
access_finished_token(creation_token) {}
WebGPUSwapBufferProvider::SwapBuffer::~SwapBuffer() {
if (swap_buffers->dawn_control_client_->IsDestroyed()) {
return;
}
gpu::SharedImageInterface* sii =
swap_buffers->dawn_control_client_->GetContextProvider()
->SharedImageInterface();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment