<rdar://problem/5974306> CanvasRenderingContext2D becomes invalid when source canvas...
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Jul 2008 07:35:27 +0000 (07:35 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Jul 2008 07:35:27 +0000 (07:35 +0000)
Reviewed by Geoff Garen.

In order to fix this we now make the rendering context and the canvas element
share the same reference count, ensuring that references to the rendering
context will force the canvas element to remain live as well.

Test: fast/canvas/canvas-longlived-context.html

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

LayoutTests/ChangeLog
LayoutTests/fast/canvas/canvas-longlived-context-expected.txt [new file with mode: 0644]
LayoutTests/fast/canvas/canvas-longlived-context.html [new file with mode: 0644]
LayoutTests/fast/canvas/canvas-longlived-context.js [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/CanvasRenderingContext2D.cpp
WebCore/html/CanvasRenderingContext2D.h
WebCore/html/HTMLCanvasElement.cpp
WebCore/html/HTMLCanvasElement.h

index ee46330..b0c81df 100644 (file)
@@ -1,3 +1,14 @@
+2008-07-01  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Testcase to cover a CanvasRenderingContext2D reference outlasting any
+        references to the underlying canvas element.
+
+        * fast/canvas/canvas-longlived-context-expected.txt: Added.
+        * fast/canvas/canvas-longlived-context.html: Added.
+        * fast/canvas/canvas-longlived-context.js: Added.
+
 2008-07-01  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
 
         Reviewed by Darin.
diff --git a/LayoutTests/fast/canvas/canvas-longlived-context-expected.txt b/LayoutTests/fast/canvas/canvas-longlived-context-expected.txt
new file mode 100644 (file)
index 0000000..2af9411
--- /dev/null
@@ -0,0 +1,10 @@
+This test ensures that Canvas and CanvasRenderingContext2D work correctly if the rendering context outlives the canvas element
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getPixel(50,50) is [0,128,0,255]
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/canvas/canvas-longlived-context.html b/LayoutTests/fast/canvas/canvas-longlived-context.html
new file mode 100644 (file)
index 0000000..ad3ece4
--- /dev/null
@@ -0,0 +1,6 @@
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+<p id="description"></p>
+<div id="console"></div>
+<script src="canvas-longlived-context.js"></script>
+<script src="../js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/canvas/canvas-longlived-context.js b/LayoutTests/fast/canvas/canvas-longlived-context.js
new file mode 100644 (file)
index 0000000..45a1bdb
--- /dev/null
@@ -0,0 +1,42 @@
+description("This test ensures that Canvas and CanvasRenderingContext2D work correctly if the rendering context outlives the canvas element");
+
+function dataToArray(data) {
+    var result = new Array(data.length)
+    for (var i = 0; i < data.length; i++)
+        result[i] = data[i];
+    return result;
+}
+
+function getPixel(x, y) {
+    var data = context.getImageData(x,y,1,1);
+    if (!data) // getImageData failed, which should never happen
+        return [-1,-1,-1,-1];
+    return dataToArray(data.data);
+}
+
+function pixelShouldBe(x, y, colour) {
+    shouldBe("getPixel(" + [x, y] +")", "["+colour+"]");
+}
+
+function prepareCanvas() {
+    var context = document.createElement("canvas").getContext("2d");
+    context.fillStyle = "green";
+    context.fillRect(0,0,100,100);
+    return context;
+}
+
+function clobberGC(count) {
+    for (var i = 0; i < 10000; ++i)
+        ({a: i*i*i*0.5+"str", b: i/Math.sqrt(i)});
+    if (count > 0)
+        clobberGC(count-1);
+}
+
+function test() {
+    context = prepareCanvas();
+    clobberGC(50);
+    pixelShouldBe(50, 50, [0, 128, 0, 255]);
+}
+test();
+
+var successfullyParsed = true;
index 72c94d0..d3abb0c 100644 (file)
@@ -1,3 +1,26 @@
+2008-07-01  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        <rdar://problem/5974306> CanvasRenderingContext2D becomes invalid when source canvas element is collected
+
+        In order to fix this we now make the rendering context and the canvas element
+        share the same reference count, ensuring that references to the rendering
+        context will force the canvas element to remain live as well.
+
+        Test: fast/canvas/canvas-longlived-context.html
+
+        * html/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::clearPathForDashboardBackwardCompatibilityMode):
+        (WebCore::CanvasRenderingContext2D::createImageData):
+        (WebCore::CanvasRenderingContext2D::getImageData):
+        (WebCore::CanvasRenderingContext2D::putImageData):
+        * html/CanvasRenderingContext2D.h:
+        (WebCore::CanvasRenderingContext2D::create):
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::getContext):
+        * html/HTMLCanvasElement.h:
+
 2008-07-01  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Darin.
index 62b6805..fd17e19 100644 (file)
@@ -74,6 +74,16 @@ CanvasRenderingContext2D::CanvasRenderingContext2D(HTMLCanvasElement* canvas)
 {
 }
 
+void CanvasRenderingContext2D::ref()
+{
+m_canvas->ref();
+}
+
+void CanvasRenderingContext2D::deref()
+{
+    m_canvas->deref(); 
+}
+
 void CanvasRenderingContext2D::reset()
 {
     m_stateStack.resize(1);
@@ -520,10 +530,9 @@ void CanvasRenderingContext2D::rect(float x, float y, float width, float height)
 #if ENABLE(DASHBOARD_SUPPORT)
 void CanvasRenderingContext2D::clearPathForDashboardBackwardCompatibilityMode()
 {
-    if (m_canvas)
-        if (Settings* settings = m_canvas->document()->settings())
-            if (settings->usesDashboardBackwardCompatibilityMode())
-                m_path.clear();
+    if (Settings* settings = m_canvas->document()->settings())
+        if (settings->usesDashboardBackwardCompatibilityMode())
+            m_path.clear();
 }
 #endif
 
@@ -1151,8 +1160,6 @@ PassRefPtr<CanvasPattern> CanvasRenderingContext2D::createPattern(HTMLCanvasElem
 
 void CanvasRenderingContext2D::willDraw(const FloatRect& r)
 {
-    if (!m_canvas)
-        return;
     GraphicsContext* c = drawingContext();
     if (!c)
         return;
@@ -1162,8 +1169,6 @@ void CanvasRenderingContext2D::willDraw(const FloatRect& r)
 
 GraphicsContext* CanvasRenderingContext2D::drawingContext() const
 {
-    if (!m_canvas)
-        return 0;
     return m_canvas->drawingContext();
 }
 
@@ -1279,11 +1284,7 @@ static PassRefPtr<ImageData> createEmptyImageData(const IntSize& size)
 PassRefPtr<ImageData> CanvasRenderingContext2D::createImageData(float sw, float sh) const
 {
     FloatSize unscaledSize(sw, sh);
-    IntSize scaledSize;
-    if (m_canvas)
-        scaledSize = m_canvas->convertLogicalToDevice(unscaledSize);
-    else
-        scaledSize = IntSize(ceilf(sw), ceilf(sh));
+    IntSize scaledSize = m_canvas->convertLogicalToDevice(unscaledSize);
     if (scaledSize.width() < 1)
         scaledSize.setWidth(1);
     if (scaledSize.height() < 1)
@@ -1300,7 +1301,7 @@ PassRefPtr<ImageData> CanvasRenderingContext2D::getImageData(float sx, float sy,
     }
     
     FloatRect unscaledRect(sx, sy, sw, sh);
-    IntRect scaledRect = m_canvas ? m_canvas->convertLogicalToDevice(unscaledRect) : enclosingIntRect(unscaledRect);
+    IntRect scaledRect = m_canvas->convertLogicalToDevice(unscaledRect);
     if (scaledRect.width() < 1)
         scaledRect.setWidth(1);
     if (scaledRect.height() < 1)
@@ -1333,7 +1334,7 @@ void CanvasRenderingContext2D::putImageData(ImageData* data, float dx, float dy,
         return;
     }
 
-    ImageBuffer* buffer = m_canvas ? m_canvas->buffer() : 0;
+    ImageBuffer* buffer = m_canvas->buffer();
     if (!buffer)
         return;
 
index a6c88ff..4077344 100644 (file)
@@ -51,12 +51,12 @@ namespace WebCore {
 
     typedef int ExceptionCode;
 
-    class CanvasRenderingContext2D : public RefCounted<CanvasRenderingContext2D> {
+    class CanvasRenderingContext2D : Noncopyable {
     public:
-        static PassRefPtr<CanvasRenderingContext2D> create(HTMLCanvasElement* canvas) 
-        {
-            return adoptRef(new CanvasRenderingContext2D(canvas));
-        }
+        CanvasRenderingContext2D(HTMLCanvasElement*);
+        
+        void ref();
+        void deref();
         
         HTMLCanvasElement* canvas() const { return m_canvas; }
 
@@ -175,11 +175,8 @@ namespace WebCore {
         void putImageData(ImageData*, float dx, float dy, float dirtyX, float dirtyY, float dirtyWidth, float dirtyHeight, ExceptionCode&);
         
         void reset();
-        void detachCanvas() { m_canvas = 0; }
 
     private:
-        CanvasRenderingContext2D(HTMLCanvasElement*);
-
         struct State {
             State();
 
index 48ee226..907cccc 100644 (file)
@@ -78,8 +78,6 @@ HTMLCanvasElement::HTMLCanvasElement(Document* doc)
 
 HTMLCanvasElement::~HTMLCanvasElement()
 {
-    if (m_2DContext)
-        m_2DContext->detachCanvas();
 }
 
 #if ENABLE(DASHBOARD_SUPPORT)
@@ -154,7 +152,7 @@ CanvasRenderingContext* HTMLCanvasElement::getContext(const String& type)
 {
     if (type == "2d") {
         if (!m_2DContext)
-            m_2DContext = CanvasRenderingContext2D::create(this);
+            m_2DContext.set(new CanvasRenderingContext2D(this));
         return m_2DContext.get();
     }
     return 0;
index ee2c250..a0b7cc5 100644 (file)
@@ -131,7 +131,7 @@ private:
 
     bool m_rendererIsCanvas;
 
-    RefPtr<CanvasRenderingContext2D> m_2DContext;
+    OwnPtr<CanvasRenderingContext2D> m_2DContext;
     IntSize m_size;    
     CanvasObserver* m_observer;