Merge r243820 - Crash in HTMLCanvasElement::createContext2d after the…

… element got adopted to a new document https://bugs.webkit.org/show_bug.cgi?id=196527 Reviewed by Antti Koivisto. We need to update CanvasBase::m_scriptExecutionContext when HTMLCanvasElement moves from one document to another. Fixed the bug by making CanvasBase::scriptExecutionContext make a virtual function call instead of directly storing a raw pointer. In HTMLCanvasElement, we use Node::scriptExecutionContext(). Use ContextDestructionObserver in CustomPaintCanvas and OffscreenCanvas instead of a raw pointer. Unfortunately, no new tests since there is no reproducible test case. * html/CanvasBase.cpp: (WebCore::CanvasBase::CanvasBase): * html/CanvasBase.h: (WebCore::CanvasBase::scriptExecutionContext const): * html/CustomPaintCanvas.cpp: (WebCore::CustomPaintCanvas::CustomPaintCanvas): * html/CustomPaintCanvas.h: * html/HTMLCanvasElement.cpp: (WebCore::HTMLCanvasElement::HTMLCanvasElement): * html/HTMLCanvasElement.h: * html/OffscreenCanvas.cpp: (WebCore::OffscreenCanvas::OffscreenCanvas): * html/OffscreenCanvas.h:
WebKit · Apr 8, 2019 · 523a2ae · 523a2ae
1 parent 12dfc06
commit 523a2ae
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 11 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2019-04-02  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in HTMLCanvasElement::createContext2d after the element got adopted to a new document
+        https://bugs.webkit.org/show_bug.cgi?id=196527
+
+        Reviewed by Antti Koivisto.
+
+        We need to update CanvasBase::m_scriptExecutionContext when HTMLCanvasElement moves from
+        one document to another. Fixed the bug by making CanvasBase::scriptExecutionContext make
+        a virtual function call instead of directly storing a raw pointer. In HTMLCanvasElement,
+        we use Node::scriptExecutionContext(). Use ContextDestructionObserver in CustomPaintCanvas
+        and OffscreenCanvas instead of a raw pointer.
+
+        Unfortunately, no new tests since there is no reproducible test case.
+
+        * html/CanvasBase.cpp:
+        (WebCore::CanvasBase::CanvasBase):
+        * html/CanvasBase.h:
+        (WebCore::CanvasBase::scriptExecutionContext const):
+        * html/CustomPaintCanvas.cpp:
+        (WebCore::CustomPaintCanvas::CustomPaintCanvas):
+        * html/CustomPaintCanvas.h:
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::HTMLCanvasElement):
+        * html/HTMLCanvasElement.h:
+        * html/OffscreenCanvas.cpp:
+        (WebCore::OffscreenCanvas::OffscreenCanvas):
+        * html/OffscreenCanvas.h:
+
 2019-03-26  Dean Jackson  <dino@apple.com>
 
         vertexAttribPointer must restrict offset parameter

diff --git a/Source/WebCore/html/CanvasBase.cpp b/Source/WebCore/html/CanvasBase.cpp
@@ -34,8 +34,7 @@
 
 namespace WebCore {
 
-CanvasBase::CanvasBase(ScriptExecutionContext* scriptExecutionContext)
-    : m_scriptExecutionContext(scriptExecutionContext)
+CanvasBase::CanvasBase()
 {
 }
 

diff --git a/Source/WebCore/html/CanvasBase.h b/Source/WebCore/html/CanvasBase.h
@@ -74,7 +74,7 @@ class CanvasBase {
     bool originClean() const { return m_originClean; }
 
     virtual SecurityOrigin* securityOrigin() const { return nullptr; }
-    ScriptExecutionContext* scriptExecutionContext() const { return m_scriptExecutionContext; }
+    ScriptExecutionContext* scriptExecutionContext() const { return canvasBaseScriptExecutionContext();  }
 
     CanvasRenderingContext* renderingContext() const;
 
@@ -98,7 +98,9 @@ class CanvasBase {
     bool callTracingActive() const;
 
 protected:
-    CanvasBase(ScriptExecutionContext*);
+    CanvasBase();
+
+    virtual ScriptExecutionContext* canvasBaseScriptExecutionContext() const = 0;
 
     std::unique_ptr<CanvasRenderingContext> m_context;
 
@@ -107,7 +109,6 @@ class CanvasBase {
 #ifndef NDEBUG
     bool m_didNotifyObserversCanvasDestroyed { false };
 #endif
-    ScriptExecutionContext* m_scriptExecutionContext;
     HashSet<CanvasObserver*> m_observers;
 };
 

diff --git a/Source/WebCore/html/CustomPaintCanvas.cpp b/Source/WebCore/html/CustomPaintCanvas.cpp
@@ -39,7 +39,7 @@ Ref<CustomPaintCanvas> CustomPaintCanvas::create(ScriptExecutionContext& context
 }
 
 CustomPaintCanvas::CustomPaintCanvas(ScriptExecutionContext& context, unsigned width, unsigned height)
-    : CanvasBase(&context)
+    : ContextDestructionObserver(&context)
     , m_size(width, height)
 {
 }

diff --git a/Source/WebCore/html/CustomPaintCanvas.h b/Source/WebCore/html/CustomPaintCanvas.h
@@ -44,7 +44,7 @@ namespace WebCore {
 class ImageBitmap;
 class PaintRenderingContext2D;
 
-class CustomPaintCanvas final : public RefCounted<CustomPaintCanvas>, public CanvasBase {
+class CustomPaintCanvas final : public RefCounted<CustomPaintCanvas>, public CanvasBase, private ContextDestructionObserver {
     WTF_MAKE_FAST_ALLOCATED;
 public:
 
@@ -80,6 +80,7 @@ class CustomPaintCanvas final : public RefCounted<CustomPaintCanvas>, public Can
 
     void refCanvasBase() final { ref(); }
     void derefCanvasBase() final { deref(); }
+    ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
 
     mutable GraphicsContext* m_destinationGraphicsContext = nullptr;
     mutable IntSize m_size;

diff --git a/Source/WebCore/html/HTMLCanvasElement.cpp b/Source/WebCore/html/HTMLCanvasElement.cpp
@@ -120,7 +120,6 @@ static size_t activePixelMemory = 0;
 
 HTMLCanvasElement::HTMLCanvasElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
-    , CanvasBase(&document)
     , m_size(defaultWidth, defaultHeight)
 {
     ASSERT(hasTagName(canvasTag));

diff --git a/Source/WebCore/html/HTMLCanvasElement.h b/Source/WebCore/html/HTMLCanvasElement.h
@@ -185,6 +185,8 @@ class HTMLCanvasElement final : public HTMLElement, public CanvasBase {
     void refCanvasBase() final { HTMLElement::ref(); }
     void derefCanvasBase() final { HTMLElement::deref(); }
 
+    ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return HTMLElement::scriptExecutionContext(); }
+
     FloatRect m_dirtyRect;
     mutable IntSize m_size;
 

diff --git a/Source/WebCore/html/OffscreenCanvas.cpp b/Source/WebCore/html/OffscreenCanvas.cpp
@@ -38,7 +38,7 @@ Ref<OffscreenCanvas> OffscreenCanvas::create(ScriptExecutionContext& context, un
 }
 
 OffscreenCanvas::OffscreenCanvas(ScriptExecutionContext& context, unsigned width, unsigned height)
-    : CanvasBase(&context)
+    : ContextDestructionObserver(&context)
     , m_size(width, height)
 {
 }

diff --git a/Source/WebCore/html/OffscreenCanvas.h b/Source/WebCore/html/OffscreenCanvas.h
@@ -45,7 +45,7 @@ class WebGLRenderingContext;
 using OffscreenRenderingContext = RefPtr<WebGLRenderingContext>;
 #endif
 
-class OffscreenCanvas final : public RefCounted<OffscreenCanvas>, public CanvasBase, public EventTargetWithInlineData {
+class OffscreenCanvas final : public RefCounted<OffscreenCanvas>, public CanvasBase, public EventTargetWithInlineData, private ContextDestructionObserver {
     WTF_MAKE_FAST_ALLOCATED;
 public:
 
@@ -94,7 +94,8 @@ class OffscreenCanvas final : public RefCounted<OffscreenCanvas>, public CanvasB
 
     bool isOffscreenCanvas() const final { return true; }
 
-    ScriptExecutionContext* scriptExecutionContext() const final { return CanvasBase::scriptExecutionContext(); }
+    ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
+    ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
 
     EventTargetInterface eventTargetInterface() const final { return OffscreenCanvasEventTargetInterfaceType; }
     void refEventTarget() final { ref(); }