[ASAN] Fix WebGPU tests after r250258
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Oct 2019 00:44:28 +0000 (00:44 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Oct 2019 00:44:28 +0000 (00:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203133
<rdar://problem/56379008>

Reviewed by Joseph Pecoraro.

* platform/graphics/gpu/GPUObjectBase.h:
(WebCore::GPUObjectBase::~GPUObjectBase): Added.
Add a default virtual destructor since this class class is the one that is `RefCounted` and
the `WebGPUPipeline` subclass is virtual.

* Modules/webgpu/WebGPUPipeline.h:
(WebCore::WebGPUPipeline::scriptExecutionContext const): Deleted.
* Modules/webgpu/WebGPUPipeline.cpp:
(WebCore::WebGPUPipeline::WebGPUPipeline):
(WebCore::WebGPUPipeline::contextDestroyed): Added.
Make `WebGPUPipeline` a subclass of `ContextDestructionObserver` so that the raw pointer to
the associated `ScriptExecutionContext` is properly cleared and isn't UAFd.

* Modules/webgpu/WebGPUComputePipeline.h:
* Modules/webgpu/WebGPUComputePipeline.cpp:
(WebCore::WebGPUComputePipeline::create):
(WebCore::WebGPUComputePipeline::WebGPUComputePipeline):
(WebCore::WebGPUComputePipeline::cloneShaderModules):
(WebCore::WebGPUComputePipeline::recompile):
* Modules/webgpu/WebGPURenderPipeline.h:
* Modules/webgpu/WebGPURenderPipeline.cpp:
(WebCore::WebGPURenderPipeline::create):
(WebCore::WebGPURenderPipeline::WebGPURenderPipeline):
(WebCore::WebGPURenderPipeline::cloneShaderModules):
(WebCore::WebGPURenderPipeline::recompile):
* Modules/webgpu/WebGPUDevice.cpp:
(WebCore::WebGPUDevice::createRenderPipeline):
(WebCore::WebGPUDevice::createComputePipeline):
Rework how Web Inspector preserves related shader modules so that there isn't as much wasted
space in the class layout, as we can use the `RefPtr` itself instead of an `Optional`.

* Modules/webgpu/WebGPUBuffer.idl:
Now that `GPUObjectBase` has a virtual destructor, it has a vtable, which means that this
object also does and therefore cannot be marked with `ImplementationLacksVTable`.

* inspector/InspectorShaderProgram.cpp:
(WebCore::shaderForType):
(WebCore::InspectorShaderProgram::requestShaderSource):
(WebCore::InspectorShaderProgram::updateShader):
(WebCore::InspectorShaderProgram::buildObjectForShaderProgram):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@251315 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/webgpu/WebGPUBuffer.idl
Source/WebCore/Modules/webgpu/WebGPUComputePipeline.cpp
Source/WebCore/Modules/webgpu/WebGPUComputePipeline.h
Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
Source/WebCore/Modules/webgpu/WebGPUPipeline.cpp
Source/WebCore/Modules/webgpu/WebGPUPipeline.h
Source/WebCore/Modules/webgpu/WebGPURenderPipeline.cpp
Source/WebCore/Modules/webgpu/WebGPURenderPipeline.h
Source/WebCore/inspector/InspectorShaderProgram.cpp
Source/WebCore/platform/graphics/gpu/GPUObjectBase.h

index 9069ab6..fba828a 100644 (file)
@@ -1,3 +1,52 @@
+2019-10-18  Devin Rousso  <drousso@apple.com>
+
+        [ASAN] Fix WebGPU tests after r250258
+        https://bugs.webkit.org/show_bug.cgi?id=203133
+        <rdar://problem/56379008>
+
+        Reviewed by Joseph Pecoraro.
+
+        * platform/graphics/gpu/GPUObjectBase.h:
+        (WebCore::GPUObjectBase::~GPUObjectBase): Added.
+        Add a default virtual destructor since this class class is the one that is `RefCounted` and
+        the `WebGPUPipeline` subclass is virtual.
+
+        * Modules/webgpu/WebGPUPipeline.h:
+        (WebCore::WebGPUPipeline::scriptExecutionContext const): Deleted.
+        * Modules/webgpu/WebGPUPipeline.cpp:
+        (WebCore::WebGPUPipeline::WebGPUPipeline):
+        (WebCore::WebGPUPipeline::contextDestroyed): Added.
+        Make `WebGPUPipeline` a subclass of `ContextDestructionObserver` so that the raw pointer to
+        the associated `ScriptExecutionContext` is properly cleared and isn't UAFd.
+
+        * Modules/webgpu/WebGPUComputePipeline.h:
+        * Modules/webgpu/WebGPUComputePipeline.cpp:
+        (WebCore::WebGPUComputePipeline::create):
+        (WebCore::WebGPUComputePipeline::WebGPUComputePipeline):
+        (WebCore::WebGPUComputePipeline::cloneShaderModules):
+        (WebCore::WebGPUComputePipeline::recompile):
+        * Modules/webgpu/WebGPURenderPipeline.h:
+        * Modules/webgpu/WebGPURenderPipeline.cpp:
+        (WebCore::WebGPURenderPipeline::create):
+        (WebCore::WebGPURenderPipeline::WebGPURenderPipeline):
+        (WebCore::WebGPURenderPipeline::cloneShaderModules):
+        (WebCore::WebGPURenderPipeline::recompile):
+        * Modules/webgpu/WebGPUDevice.cpp:
+        (WebCore::WebGPUDevice::createRenderPipeline):
+        (WebCore::WebGPUDevice::createComputePipeline):
+        Rework how Web Inspector preserves related shader modules so that there isn't as much wasted
+        space in the class layout, as we can use the `RefPtr` itself instead of an `Optional`.
+
+        * Modules/webgpu/WebGPUBuffer.idl:
+        Now that `GPUObjectBase` has a virtual destructor, it has a vtable, which means that this
+        object also does and therefore cannot be marked with `ImplementationLacksVTable`.
+
+        * inspector/InspectorShaderProgram.cpp:
+        (WebCore::shaderForType):
+        (WebCore::InspectorShaderProgram::requestShaderSource):
+        (WebCore::InspectorShaderProgram::updateShader):
+        (WebCore::InspectorShaderProgram::buildObjectForShaderProgram):
+
 2019-10-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Refactor AbstractEventLoop out of WindowEventLoop
index 4b8d62e..e133dd7 100644 (file)
@@ -29,7 +29,6 @@ typedef unsigned long long u64;
 [
     Conditional=WEBGPU,
     EnabledAtRuntime=WebGPU,
-    ImplementationLacksVTable,
     InterfaceName=GPUBuffer
 ] interface WebGPUBuffer {
     Promise<ArrayBuffer> mapReadAsync();
index 8a8cf0d..6b89945 100644 (file)
 
 namespace WebCore {
 
-Ref<WebGPUComputePipeline> WebGPUComputePipeline::create(WebGPUDevice& device, RefPtr<GPUComputePipeline>&& pipeline, GPUErrorScopes& errorScopes, Optional<WebGPUPipeline::ShaderData> computeShader)
+Ref<WebGPUComputePipeline> WebGPUComputePipeline::create(WebGPUDevice& device, RefPtr<GPUComputePipeline>&& pipeline, GPUErrorScopes& errorScopes, WebGPUPipeline::ShaderData&& computeShader)
 {
-    return adoptRef(*new WebGPUComputePipeline(device, WTFMove(pipeline), errorScopes, computeShader));
+    return adoptRef(*new WebGPUComputePipeline(device, WTFMove(pipeline), errorScopes, WTFMove(computeShader)));
 }
 
-WebGPUComputePipeline::WebGPUComputePipeline(WebGPUDevice& device, RefPtr<GPUComputePipeline>&& pipeline, GPUErrorScopes& errorScopes, Optional<WebGPUPipeline::ShaderData> computeShader)
+WebGPUComputePipeline::WebGPUComputePipeline(WebGPUDevice& device, RefPtr<GPUComputePipeline>&& pipeline, GPUErrorScopes& errorScopes, WebGPUPipeline::ShaderData&& computeShader)
     : WebGPUPipeline(device, errorScopes)
     , m_computePipeline(WTFMove(pipeline))
-    , m_computeShader(computeShader)
+    , m_computeShader(WTFMove(computeShader))
 {
 }
 
@@ -57,27 +57,20 @@ WebGPUComputePipeline::~WebGPUComputePipeline() = default;
 
 bool WebGPUComputePipeline::cloneShaderModules(const WebGPUDevice& device)
 {
-    if (m_computeShader) {
-        if (auto& webGPUComputeShaderModule = m_computeShader.value().module) {
-            const auto& computeSource = webGPUComputeShaderModule->source();
-            webGPUComputeShaderModule = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { computeSource }), computeSource);
-            return true;
-        }
+    if (m_computeShader.module) {
+        const auto& computeSource = m_computeShader.module->source();
+        m_computeShader.module = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { computeSource }), computeSource);
+        return true;
     }
     return false;
 }
 
 bool WebGPUComputePipeline::recompile(const WebGPUDevice& device)
 {
-    if (m_computePipeline && m_computeShader) {
-        if (auto& webGPUComputeShaderModule = m_computeShader.value().module) {
-            // Recreate the shader module so that modifications to it via this pipeline don't affect
-            // other pipelines that also use the same shader module.
-
-            if (auto* gpuComputeShaderModule = webGPUComputeShaderModule->module()) {
-                GPUProgrammableStageDescriptor computeStage(makeRef(*gpuComputeShaderModule), { m_computeShader.value().entryPoint });
-                return m_computePipeline->recompile(device.device(), WTFMove(computeStage));
-            }
+    if (m_computePipeline && m_computeShader.module) {
+        if (auto* gpuComputeShaderModule = m_computeShader.module->module()) {
+            GPUProgrammableStageDescriptor computeStage(makeRef(*gpuComputeShaderModule), { m_computeShader.entryPoint });
+            return m_computePipeline->recompile(device.device(), WTFMove(computeStage));
         }
     }
     return false;
index 65ae411..b847135 100644 (file)
@@ -36,29 +36,30 @@ class GPUComputePipeline;
 class GPUPipeline;
 class GPUErrorScopes;
 class WebGPUDevice;
+class WebGPUShaderModule;
 
 class WebGPUComputePipeline final : public WebGPUPipeline {
 public:
     virtual ~WebGPUComputePipeline();
 
-    static Ref<WebGPUComputePipeline> create(WebGPUDevice&, RefPtr<GPUComputePipeline>&&, GPUErrorScopes&, Optional<WebGPUPipeline::ShaderData> computeShader);
+    static Ref<WebGPUComputePipeline> create(WebGPUDevice&, RefPtr<GPUComputePipeline>&&, GPUErrorScopes&, WebGPUPipeline::ShaderData&& computeShader);
 
     bool isComputePipeline() const { return true; }
 
     bool isValid() const { return computePipeline(); }
     const GPUComputePipeline* computePipeline() const { return m_computePipeline.get(); }
-    Optional<WebGPUPipeline::ShaderData> computeShader() const { return m_computeShader; }
+    RefPtr<WebGPUShaderModule> computeShader() const { return m_computeShader.module; }
 
     bool cloneShaderModules(const WebGPUDevice&);
     bool recompile(const WebGPUDevice&);
 
 private:
-    WebGPUComputePipeline(WebGPUDevice&, RefPtr<GPUComputePipeline>&&, GPUErrorScopes&, Optional<WebGPUPipeline::ShaderData> computeShader);
+    WebGPUComputePipeline(WebGPUDevice&, RefPtr<GPUComputePipeline>&&, GPUErrorScopes&, WebGPUPipeline::ShaderData&& computeShader);
 
     RefPtr<GPUComputePipeline> m_computePipeline;
 
     // Preserved for Web Inspector recompilation.
-    Optional<WebGPUPipeline::ShaderData> m_computeShader;
+    WebGPUPipeline::ShaderData m_computeShader;
 };
 
 } // namespace WebCore
index 6dc8db3..c1b375d 100644 (file)
@@ -233,17 +233,17 @@ Ref<WebGPURenderPipeline> WebGPUDevice::createRenderPipeline(const WebGPURenderP
 
     auto gpuDescriptor = descriptor.tryCreateGPURenderPipelineDescriptor(m_errorScopes);
     if (!gpuDescriptor)
-        return WebGPURenderPipeline::create(*this, nullptr, m_errorScopes, WTF::nullopt, WTF::nullopt);
+        return WebGPURenderPipeline::create(*this, nullptr, m_errorScopes, { }, { });
 
     auto gpuPipeline = m_device->tryCreateRenderPipeline(*gpuDescriptor, m_errorScopes);
 
     WebGPUPipeline::ShaderData vertexShader = { descriptor.vertexStage.module, descriptor.vertexStage.entryPoint };
 
-    Optional<WebGPUPipeline::ShaderData> fragmentShader;
+    WebGPUPipeline::ShaderData fragmentShader;
     if (descriptor.fragmentStage)
-        fragmentShader = { { descriptor.fragmentStage.value().module, descriptor.fragmentStage.value().entryPoint } };
+        fragmentShader = { descriptor.fragmentStage.value().module, descriptor.fragmentStage.value().entryPoint };
 
-    auto webGPUPipeline = WebGPURenderPipeline::create(*this, WTFMove(gpuPipeline), m_errorScopes, vertexShader, fragmentShader);
+    auto webGPUPipeline = WebGPURenderPipeline::create(*this, WTFMove(gpuPipeline), m_errorScopes, WTFMove(vertexShader), WTFMove(fragmentShader));
     if (webGPUPipeline->isValid())
         InspectorInstrumentation::didCreateWebGPUPipeline(*this, webGPUPipeline.get());
     return webGPUPipeline;
@@ -255,13 +255,13 @@ Ref<WebGPUComputePipeline> WebGPUDevice::createComputePipeline(const WebGPUCompu
 
     auto gpuDescriptor = descriptor.tryCreateGPUComputePipelineDescriptor(m_errorScopes);
     if (!gpuDescriptor)
-        return WebGPUComputePipeline::create(*this, nullptr, m_errorScopes, WTF::nullopt);
+        return WebGPUComputePipeline::create(*this, nullptr, m_errorScopes, { });
 
     auto gpuPipeline = m_device->tryCreateComputePipeline(*gpuDescriptor, m_errorScopes);
 
     WebGPUPipeline::ShaderData computeShader = { descriptor.computeStage.module, descriptor.computeStage.entryPoint };
 
-    auto webGPUPipeline = WebGPUComputePipeline::create(*this, WTFMove(gpuPipeline), m_errorScopes, computeShader);
+    auto webGPUPipeline = WebGPUComputePipeline::create(*this, WTFMove(gpuPipeline), m_errorScopes, WTFMove(computeShader));
     if (webGPUPipeline->isValid())
         InspectorInstrumentation::didCreateWebGPUPipeline(*this, webGPUPipeline.get());
     return webGPUPipeline;
index 7a12159..f33baf4 100644 (file)
@@ -57,9 +57,9 @@ Lock& WebGPUPipeline::instancesMutex()
 
 WebGPUPipeline::WebGPUPipeline(WebGPUDevice& device, GPUErrorScopes& errorScopes)
     : GPUObjectBase(makeRef(errorScopes))
-    , m_scriptExecutionContext(device.scriptExecutionContext())
+    , ContextDestructionObserver(device.scriptExecutionContext())
 {
-    ASSERT(m_scriptExecutionContext);
+    ASSERT(scriptExecutionContext());
 
     {
         LockHolder lock(instancesMutex());
@@ -78,6 +78,13 @@ WebGPUPipeline::~WebGPUPipeline()
     }
 }
 
+void WebGPUPipeline::contextDestroyed()
+{
+    InspectorInstrumentation::willDestroyWebGPUPipeline(*this);
+
+    ContextDestructionObserver::contextDestroyed();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(WEBGPU)
index c41a1a9..d7f11a3 100644 (file)
@@ -27,6 +27,7 @@
 
 #if ENABLE(WEBGPU)
 
+#include "ContextDestructionObserver.h"
 #include "GPUObjectBase.h"
 #include "WebGPUShaderModule.h"
 #include <wtf/Forward.h>
@@ -38,7 +39,7 @@ class ScriptExecutionContext;
 class GPUErrorScopes;
 class WebGPUDevice;
 
-class WebGPUPipeline : public GPUObjectBase {
+class WebGPUPipeline : public GPUObjectBase, public ContextDestructionObserver {
 public:
     virtual ~WebGPUPipeline();
 
@@ -48,7 +49,6 @@ public:
     virtual bool isRenderPipeline() const { return false; }
     virtual bool isComputePipeline() const { return false; }
 
-    ScriptExecutionContext* scriptExecutionContext() const { return m_scriptExecutionContext; }
     virtual bool isValid() const = 0;
 
     struct ShaderData {
@@ -59,10 +59,10 @@ public:
     virtual bool cloneShaderModules(const WebGPUDevice&) = 0;
     virtual bool recompile(const WebGPUDevice&) = 0;
 
+    void contextDestroyed() final;
+
 protected:
     WebGPUPipeline(WebGPUDevice&, GPUErrorScopes&);
-
-    ScriptExecutionContext* m_scriptExecutionContext;
 };
 
 } // namespace WebCore
index 02aabdb..e929c54 100644 (file)
 
 namespace WebCore {
 
-Ref<WebGPURenderPipeline> WebGPURenderPipeline::create(WebGPUDevice& device, RefPtr<GPURenderPipeline>&& pipeline, GPUErrorScopes& errorScopes, Optional<WebGPUPipeline::ShaderData> vertexShader, Optional<WebGPUPipeline::ShaderData> fragmentShader)
+Ref<WebGPURenderPipeline> WebGPURenderPipeline::create(WebGPUDevice& device, RefPtr<GPURenderPipeline>&& pipeline, GPUErrorScopes& errorScopes, WebGPUPipeline::ShaderData&& vertexShader, WebGPUPipeline::ShaderData&& fragmentShader)
 {
-    return adoptRef(*new WebGPURenderPipeline(device, WTFMove(pipeline), errorScopes, vertexShader, fragmentShader));
+    return adoptRef(*new WebGPURenderPipeline(device, WTFMove(pipeline), errorScopes, WTFMove(vertexShader), WTFMove(fragmentShader)));
 }
 
-WebGPURenderPipeline::WebGPURenderPipeline(WebGPUDevice& device, RefPtr<GPURenderPipeline>&& pipeline, GPUErrorScopes& errorScopes, Optional<WebGPUPipeline::ShaderData> vertexShader, Optional<WebGPUPipeline::ShaderData> fragmentShader)
+WebGPURenderPipeline::WebGPURenderPipeline(WebGPUDevice& device, RefPtr<GPURenderPipeline>&& pipeline, GPUErrorScopes& errorScopes, WebGPUPipeline::ShaderData&& vertexShader, WebGPUPipeline::ShaderData&& fragmentShader)
     : WebGPUPipeline(device, errorScopes)
     , m_renderPipeline(WTFMove(pipeline))
-    , m_vertexShader(vertexShader)
-    , m_fragmentShader(fragmentShader)
+    , m_vertexShader(WTFMove(vertexShader))
+    , m_fragmentShader(WTFMove(fragmentShader))
 {
 }
 
@@ -58,45 +58,37 @@ WebGPURenderPipeline::~WebGPURenderPipeline() = default;
 
 bool WebGPURenderPipeline::cloneShaderModules(const WebGPUDevice& device)
 {
-    if (m_vertexShader) {
-        if (auto& webGPUVertexShaderModule = m_vertexShader.value().module) {
-            bool sharesVertexFragmentShaderModule = m_fragmentShader && m_fragmentShader.value().module == webGPUVertexShaderModule;
+    if (m_vertexShader.module) {
+        bool sharesVertexFragmentShaderModule = m_fragmentShader.module == m_vertexShader.module;
 
-            const auto& vertexSource = webGPUVertexShaderModule->source();
-            webGPUVertexShaderModule = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { vertexSource }), vertexSource);
+        const auto& vertexSource = m_vertexShader.module->source();
+        m_vertexShader.module = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { vertexSource }), vertexSource);
 
-            if (!m_fragmentShader)
-                return true;
-
-            if (auto& webGPUFragmentShaderModule = m_fragmentShader.value().module) {
-                if (sharesVertexFragmentShaderModule)
-                    webGPUFragmentShaderModule = webGPUVertexShaderModule;
-                else {
-                    const auto& fragmentSource = webGPUFragmentShaderModule->source();
-                    webGPUFragmentShaderModule = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { fragmentSource }), fragmentSource);
-                }
-                return true;
+        if (m_fragmentShader.module) {
+            if (sharesVertexFragmentShaderModule)
+                m_fragmentShader.module = m_vertexShader.module;
+            else {
+                const auto& fragmentSource = m_fragmentShader.module->source();
+                m_fragmentShader.module = WebGPUShaderModule::create(GPUShaderModule::tryCreate(device.device(), { fragmentSource }), fragmentSource);
             }
         }
+
+        return true;
     }
     return false;
 }
 
 bool WebGPURenderPipeline::recompile(const WebGPUDevice& device)
 {
-    if (m_renderPipeline && m_vertexShader) {
-        if (auto& webGPUVertexShaderModule = m_vertexShader.value().module) {
-            if (auto* gpuVertexShaderModule = webGPUVertexShaderModule->module()) {
-                GPUProgrammableStageDescriptor vertexStage(makeRef(*gpuVertexShaderModule), { m_vertexShader.value().entryPoint });
-                Optional<GPUProgrammableStageDescriptor> fragmentStage;
-                if (m_fragmentShader) {
-                    if (auto& webGPUFragmentShaderModule = m_fragmentShader.value().module) {
-                        if (auto* gpuFragmentShaderModule = webGPUFragmentShaderModule->module())
-                            fragmentStage = GPUProgrammableStageDescriptor(makeRef(*gpuFragmentShaderModule), { m_fragmentShader.value().entryPoint });
-                    }
-                }
-                return m_renderPipeline->recompile(device.device(), WTFMove(vertexStage), WTFMove(fragmentStage));
+    if (m_renderPipeline && m_vertexShader.module) {
+        if (auto* gpuVertexShaderModule = m_vertexShader.module->module()) {
+            GPUProgrammableStageDescriptor vertexStage(makeRef(*gpuVertexShaderModule), { m_vertexShader.entryPoint });
+            Optional<GPUProgrammableStageDescriptor> fragmentStage;
+            if (m_fragmentShader.module) {
+                if (auto* gpuFragmentShaderModule = m_fragmentShader.module->module())
+                    fragmentStage = GPUProgrammableStageDescriptor(makeRef(*gpuFragmentShaderModule), { m_fragmentShader.entryPoint });
             }
+            return m_renderPipeline->recompile(device.device(), WTFMove(vertexStage), WTFMove(fragmentStage));
         }
     }
     return false;
index 62a70d1..b460b13 100644 (file)
@@ -36,31 +36,32 @@ class GPUPipeline;
 class GPURenderPipeline;
 class GPUErrorScopes;
 class WebGPUDevice;
+class WebGPUShaderModule;
 
 class WebGPURenderPipeline final : public WebGPUPipeline {
 public:
     virtual ~WebGPURenderPipeline();
 
-    static Ref<WebGPURenderPipeline> create(WebGPUDevice&, RefPtr<GPURenderPipeline>&&, GPUErrorScopes&, Optional<WebGPUPipeline::ShaderData> vertexShader, Optional<WebGPUPipeline::ShaderData> fragmentShader);
+    static Ref<WebGPURenderPipeline> create(WebGPUDevice&, RefPtr<GPURenderPipeline>&&, GPUErrorScopes&, WebGPUPipeline::ShaderData&& vertexShader, WebGPUPipeline::ShaderData&& fragmentShader);
 
     bool isRenderPipeline() const { return true; }
 
     bool isValid() const { return renderPipeline(); }
     const GPURenderPipeline* renderPipeline() const { return m_renderPipeline.get(); }
-    Optional<WebGPUPipeline::ShaderData> vertexShader() const { return m_vertexShader; }
-    Optional<WebGPUPipeline::ShaderData> fragmentShader() const { return m_fragmentShader; }
+    RefPtr<WebGPUShaderModule> vertexShader() const { return m_vertexShader.module; }
+    RefPtr<WebGPUShaderModule> fragmentShader() const { return m_fragmentShader.module; }
 
     bool cloneShaderModules(const WebGPUDevice&);
     bool recompile(const WebGPUDevice&);
 
 private:
-    WebGPURenderPipeline(WebGPUDevice&, RefPtr<GPURenderPipeline>&&, GPUErrorScopes&, Optional<WebGPUPipeline::ShaderData> vertexShader, Optional<WebGPUPipeline::ShaderData> fragmentShader);
+    WebGPURenderPipeline(WebGPUDevice&, RefPtr<GPURenderPipeline>&&, GPUErrorScopes&, WebGPUPipeline::ShaderData&& vertexShader, WebGPUPipeline::ShaderData&& fragmentShader);
 
     RefPtr<GPURenderPipeline> m_renderPipeline;
 
     // Preserved for Web Inspector recompilation.
-    Optional<WebGPUPipeline::ShaderData> m_vertexShader;
-    Optional<WebGPUPipeline::ShaderData> m_fragmentShader;
+    WebGPUPipeline::ShaderData m_vertexShader;
+    WebGPUPipeline::ShaderData m_fragmentShader;
 };
 
 } // namespace WebCore
index 1151bf2..e94238b 100644 (file)
@@ -126,27 +126,27 @@ static WebGLShader* shaderForType(WebGLProgram& program, Inspector::Protocol::Ca
 #endif
 
 #if ENABLE(WEBGPU)
-static Optional<WebGPUPipeline::ShaderData> shaderForType(WebGPUPipeline& pipeline, Inspector::Protocol::Canvas::ShaderType shaderType)
+static RefPtr<WebGPUShaderModule> shaderForType(WebGPUPipeline& pipeline, Inspector::Protocol::Canvas::ShaderType shaderType)
 {
     switch (shaderType) {
     case Inspector::Protocol::Canvas::ShaderType::Compute:
         if (is<WebGPUComputePipeline>(pipeline))
             return downcast<WebGPUComputePipeline>(pipeline).computeShader();
-        return WTF::nullopt;
+        return nullptr;
 
     case Inspector::Protocol::Canvas::ShaderType::Fragment:
         if (is<WebGPURenderPipeline>(pipeline))
             return downcast<WebGPURenderPipeline>(pipeline).fragmentShader();
-        return WTF::nullopt;
+        return nullptr;
 
     case Inspector::Protocol::Canvas::ShaderType::Vertex:
         if (is<WebGPURenderPipeline>(pipeline))
             return downcast<WebGPURenderPipeline>(pipeline).vertexShader();
-        return WTF::nullopt;
+        return nullptr;
     }
 
     ASSERT_NOT_REACHED();
-    return WTF::nullopt;
+    return nullptr;
 }
 #endif
 
@@ -168,10 +168,8 @@ String InspectorShaderProgram::requestShaderSource(Inspector::Protocol::Canvas::
 #if ENABLE(WEBGPU)
         [&] (std::reference_wrapper<WebGPUPipeline> pipelineWrapper) {
             auto& pipeline = pipelineWrapper.get();
-            if (auto shaderData = shaderForType(pipeline, shaderType)) {
-                if (auto module = shaderData.value().module)
-                    return module->source();
-            }
+            if (auto shader = shaderForType(pipeline, shaderType))
+                return shader->source();
             return String();
         },
 #endif
@@ -216,12 +214,10 @@ bool InspectorShaderProgram::updateShader(Inspector::Protocol::Canvas::ShaderTyp
             auto& pipeline = pipelineWrapper.get();
             if (auto* device = m_canvas.deviceContext()) {
                 if (pipeline.cloneShaderModules(*device)) {
-                    if (auto shaderData = shaderForType(pipeline, shaderType)) {
-                        if (auto module = shaderData.value().module) {
-                            module->update(*device, source);
-                            if (pipeline.recompile(*device))
-                                return true;
-                        }
+                    if (auto shader = shaderForType(pipeline, shaderType)) {
+                        shader->update(*device, source);
+                        if (pipeline.recompile(*device))
+                            return true;
                     }
                 }
             }
@@ -255,9 +251,7 @@ Ref<Inspector::Protocol::Canvas::ShaderProgram> InspectorShaderProgram::buildObj
                 return Inspector::Protocol::Canvas::ProgramType::Compute;
             if (is<WebGPURenderPipeline>(pipeline)) {
                 auto& renderPipeline = downcast<WebGPURenderPipeline>(pipeline);
-                auto vertexShader = renderPipeline.vertexShader();
-                auto fragmentShader = renderPipeline.fragmentShader();
-                if (vertexShader && fragmentShader && vertexShader.value().module == fragmentShader.value().module)
+                if (renderPipeline.vertexShader() == renderPipeline.fragmentShader())
                     sharesVertexFragmentShader = true;
                 return Inspector::Protocol::Canvas::ProgramType::Render;
             }
index 6577057..94c1f52 100644 (file)
@@ -33,6 +33,9 @@
 namespace WebCore {
 
 class GPUObjectBase : public RefCounted<GPUObjectBase> {
+public:
+    virtual ~GPUObjectBase() = default;
+
 protected:
     GPUObjectBase(Ref<GPUErrorScopes>&& reporter)
         : m_errorScopes(WTFMove(reporter)) { }