Crash in HTMLCanvasElement::createContext2d after the element got adopted to a new...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Apr 2019 20:04:37 +0000 (20:04 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Apr 2019 20:04:37 +0000 (20:04 +0000)
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:

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

Source/WebCore/ChangeLog
Source/WebCore/html/CanvasBase.cpp
Source/WebCore/html/CanvasBase.h
Source/WebCore/html/CustomPaintCanvas.cpp
Source/WebCore/html/CustomPaintCanvas.h
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/HTMLCanvasElement.h
Source/WebCore/html/OffscreenCanvas.cpp
Source/WebCore/html/OffscreenCanvas.h

index 369ba3c..cec56c1 100644 (file)
@@ -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-04-03  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Remove support for -apple-trailing-word
index bb22039..9f166ef 100644 (file)
@@ -35,8 +35,7 @@
 
 namespace WebCore {
 
-CanvasBase::CanvasBase(ScriptExecutionContext* scriptExecutionContext)
-    : m_scriptExecutionContext(scriptExecutionContext)
+CanvasBase::CanvasBase()
 {
 }
 
index f9409d1..9d8041c 100644 (file)
@@ -74,7 +74,7 @@ public:
     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 @@ public:
     bool callTracingActive() const;
 
 protected:
-    CanvasBase(ScriptExecutionContext*);
+    CanvasBase();
+
+    virtual ScriptExecutionContext* canvasBaseScriptExecutionContext() const = 0;
 
     std::unique_ptr<CanvasRenderingContext> m_context;
 
@@ -107,7 +109,6 @@ private:
 #ifndef NDEBUG
     bool m_didNotifyObserversCanvasDestroyed { false };
 #endif
-    ScriptExecutionContext* m_scriptExecutionContext;
     HashSet<CanvasObserver*> m_observers;
 };
 
index aa97704..62034ba 100644 (file)
@@ -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)
 {
 }
index e1fa62b..21d2d0c 100644 (file)
@@ -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 @@ private:
 
     void refCanvasBase() final { ref(); }
     void derefCanvasBase() final { deref(); }
+    ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
 
     mutable GraphicsContext* m_destinationGraphicsContext = nullptr;
     mutable IntSize m_size;
index a269eae..7df4d2b 100644 (file)
@@ -116,7 +116,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));
index 0efc1f2..7b52b8f 100644 (file)
@@ -179,6 +179,8 @@ private:
     void refCanvasBase() final { HTMLElement::ref(); }
     void derefCanvasBase() final { HTMLElement::deref(); }
 
+    ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return HTMLElement::scriptExecutionContext(); }
+
     FloatRect m_dirtyRect;
     mutable IntSize m_size;
 
index a036515..c80d504 100644 (file)
@@ -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)
 {
 }
index fc863df..36d2e46 100644 (file)
@@ -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 @@ private:
 
     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(); }