Changeset 250381 in webkit

Timestamp:

Sep 26, 2019 1:33:35 AM (5 years ago)

Author:

Devin Rousso

Message:

Flaky Test: inspector/canvas/updateShader.html
​https://bugs.webkit.org/show_bug.cgi?id=202186
<rdar://problem/55716053>

Reviewed by Joseph Pecoraro.

If the WebGPUPipeline/WebGLProgram outlives it's WebGPUDevice/WebGLRenderingContext,
the ScriptExecutionContext* that was provided in the constructor won't be invalidated
leading to the bad access crash.

Rather than pass the ScriptExecutionContext* directly, have WebGPUPipeline/WebGLProgram
extend from ContextDestructionObserver so that it can propertly invalidate (and notify Web
Inspector) when the related context is about to be destroyed.

Test: inspector/canvas/updateShader.html

• Modules/webgpu/WebGPUPipeline.h:

(WebCore::WebGPUPipeline::scriptExecutionContext const): Deleted.

• Modules/webgpu/WebGPUPipeline.cpp:

(WebCore::WebGPUPipeline::WebGPUPipeline):
(WebCore::WebGPUPipeline::contextDestroyed): Added.

• html/canvas/WebGLProgram.h:

(WebCore::WebGLProgram::scriptExecutionContext const): Deleted.

• html/canvas/WebGLProgram.cpp:

(WebCore::WebGLProgram::WebGLProgram):
(WebCore::WebGLProgram::contextDestroyed): Added.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webgpu/WebGPUPipeline.cpp (modified) (2 diffs)
• Modules/webgpu/WebGPUPipeline.h (modified) (4 diffs)
• html/canvas/WebGLProgram.cpp (modified) (2 diffs)
• html/canvas/WebGLProgram.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-09-26  Devin Rousso  <drousso@apple.com>
+
+        Flaky Test: inspector/canvas/updateShader.html
+        https://bugs.webkit.org/show_bug.cgi?id=202186
+        <rdar://problem/55716053>
+
+        Reviewed by Joseph Pecoraro.
+
+        If the `WebGPUPipeline`/`WebGLProgram` outlives it's `WebGPUDevice`/`WebGLRenderingContext`,
+        the `ScriptExecutionContext*` that was provided in the constructor won't be invalidated
+        leading to the bad access crash.
+
+        Rather than pass the `ScriptExecutionContext*` directly, have `WebGPUPipeline`/`WebGLProgram`
+        extend from `ContextDestructionObserver` so that it can propertly invalidate (and notify Web
+        Inspector) when the related context is about to be destroyed.
+
+        Test: inspector/canvas/updateShader.html
+
+        * Modules/webgpu/WebGPUPipeline.h:
+        (WebCore::WebGPUPipeline::scriptExecutionContext const): Deleted.
+        * Modules/webgpu/WebGPUPipeline.cpp:
+        (WebCore::WebGPUPipeline::WebGPUPipeline):
+        (WebCore::WebGPUPipeline::contextDestroyed): Added.
+
+        * html/canvas/WebGLProgram.h:
+        (WebCore::WebGLProgram::scriptExecutionContext const): Deleted.
+        * html/canvas/WebGLProgram.cpp:
+        (WebCore::WebGLProgram::WebGLProgram):
+        (WebCore::WebGLProgram::contextDestroyed): Added.
+
 2019-09-25  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/Modules/webgpu/WebGPUPipeline.cpp

@@ -58 +58 @@
 WebGPUPipeline::WebGPUPipeline(WebGPUDevice& device, GPUErrorScopes& errorScopes)
     : GPUObjectBase(makeRef(errorScopes))
-    , m_scriptExecutionContext(device.scriptExecutionContext())
+    , ContextDestructionObserver(device.scriptExecutionContext())
 {
-    ASSERT(m_scriptExecutionContext);
+    ASSERT(scriptExecutionContext());
 
     {
@@ -79 +79 @@
 }
 
+void WebGPUPipeline::contextDestroyed()
+{
+    InspectorInstrumentation::willDestroyWebGPUPipeline(*this);
+
+    ContextDestructionObserver::contextDestroyed();
+}
+
 } // namespace WebCore
 

trunk/Source/WebCore/Modules/webgpu/WebGPUPipeline.h

@@ -28 +28 @@
 #if ENABLE(WEBGPU)
 
+#include "ContextDestructionObserver.h"
 #include "GPUObjectBase.h"
 #include "WebGPUShaderModule.h"
@@ -38 +39 @@
 class WebGPUDevice;
 
-class WebGPUPipeline : public GPUObjectBase {
+class WebGPUPipeline : public GPUObjectBase, public ContextDestructionObserver {
 public:
     virtual ~WebGPUPipeline();
@@ -45 +46 @@
     static Lock& instancesMutex();
 
+    void contextDestroyed() final;
+
     virtual bool isRenderPipeline() const { return false; }
     virtual bool isComputePipeline() const { return false; }
 
-    ScriptExecutionContext* scriptExecutionContext() const { return m_scriptExecutionContext; }
     virtual bool isValid() const = 0;
 
@@ -60 +62 @@
 protected:
     WebGPUPipeline(WebGPUDevice&, GPUErrorScopes&);
-
-    ScriptExecutionContext* m_scriptExecutionContext;
 };
 

trunk/Source/WebCore/html/canvas/WebGLProgram.cpp

@@ -63 +63 @@
 WebGLProgram::WebGLProgram(WebGLRenderingContextBase& ctx)
     : WebGLSharedObject(ctx)
-    , m_scriptExecutionContext(ctx.scriptExecutionContext())
-{
-    ASSERT(m_scriptExecutionContext);
+    , ContextDestructionObserver(ctx.scriptExecutionContext())
+{
+    ASSERT(scriptExecutionContext());
 
     {
@@ -86 +86 @@
         instances(lock).remove(this);
     }
+}
+
+void WebGLProgram::contextDestroyed()
+{
+    InspectorInstrumentation::willDestroyWebGLProgram(*this);
+
+    ContextDestructionObserver::contextDestroyed();
 }
 

trunk/Source/WebCore/html/canvas/WebGLProgram.h

@@ -28 +28 @@
 #if ENABLE(WEBGL)
 
+#include "ContextDestructionObserver.h"
 #include "WebGLSharedObject.h"
 #include <wtf/Forward.h>
@@ -37 +38 @@
 class WebGLShader;
 
-class WebGLProgram final : public WebGLSharedObject {
+class WebGLProgram final : public WebGLSharedObject, public ContextDestructionObserver {
 public:
     static Ref<WebGLProgram> create(WebGLRenderingContextBase&);
@@ -45 +46 @@
     static Lock& instancesMutex();
 
-    ScriptExecutionContext* scriptExecutionContext() const { return m_scriptExecutionContext; }
+    void contextDestroyed() final;
 
     unsigned numActiveAttribLocations();
@@ -76 +77 @@
     void cacheInfoIfNeeded();
 
-    ScriptExecutionContext* m_scriptExecutionContext;
-
     Vector<GC3Dint> m_activeAttribLocations;