From 192732c660eb5d1c4caafb83ae12e864df5fc6b9 Mon Sep 17 00:00:00 2001 From: "oliver@apple.com" Date: Wed, 2 Jul 2008 07:35:27 +0000 Subject: [PATCH] CanvasRenderingContext2D becomes invalid when source canvas element is collected 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 | 11 ++++++ .../canvas/canvas-longlived-context-expected.txt | 10 ++++++ .../fast/canvas/canvas-longlived-context.html | 6 ++++ .../fast/canvas/canvas-longlived-context.js | 42 ++++++++++++++++++++++ WebCore/ChangeLog | 23 ++++++++++++ WebCore/html/CanvasRenderingContext2D.cpp | 31 ++++++++-------- WebCore/html/CanvasRenderingContext2D.h | 13 +++---- WebCore/html/HTMLCanvasElement.cpp | 4 +-- WebCore/html/HTMLCanvasElement.h | 2 +- 9 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 LayoutTests/fast/canvas/canvas-longlived-context-expected.txt create mode 100644 LayoutTests/fast/canvas/canvas-longlived-context.html create mode 100644 LayoutTests/fast/canvas/canvas-longlived-context.js diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index ee46330..b0c81df 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,14 @@ +2008-07-01 Oliver Hunt + + 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 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 index 0000000..2af9411 --- /dev/null +++ b/LayoutTests/fast/canvas/canvas-longlived-context-expected.txt @@ -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 index 0000000..ad3ece4 --- /dev/null +++ b/LayoutTests/fast/canvas/canvas-longlived-context.html @@ -0,0 +1,6 @@ + + +

+
+ + diff --git a/LayoutTests/fast/canvas/canvas-longlived-context.js b/LayoutTests/fast/canvas/canvas-longlived-context.js new file mode 100644 index 0000000..45a1bdb --- /dev/null +++ b/LayoutTests/fast/canvas/canvas-longlived-context.js @@ -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; diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 72c94d0..d3abb0c 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2008-07-01 Oliver Hunt + + Reviewed by Geoff Garen. + + 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 Reviewed by Darin. diff --git a/WebCore/html/CanvasRenderingContext2D.cpp b/WebCore/html/CanvasRenderingContext2D.cpp index 62b6805..fd17e19 100644 --- a/WebCore/html/CanvasRenderingContext2D.cpp +++ b/WebCore/html/CanvasRenderingContext2D.cpp @@ -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 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 createEmptyImageData(const IntSize& size) PassRefPtr 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 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; diff --git a/WebCore/html/CanvasRenderingContext2D.h b/WebCore/html/CanvasRenderingContext2D.h index a6c88ff..4077344 100644 --- a/WebCore/html/CanvasRenderingContext2D.h +++ b/WebCore/html/CanvasRenderingContext2D.h @@ -51,12 +51,12 @@ namespace WebCore { typedef int ExceptionCode; - class CanvasRenderingContext2D : public RefCounted { + class CanvasRenderingContext2D : Noncopyable { public: - static PassRefPtr 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(); diff --git a/WebCore/html/HTMLCanvasElement.cpp b/WebCore/html/HTMLCanvasElement.cpp index 48ee226..907cccc 100644 --- a/WebCore/html/HTMLCanvasElement.cpp +++ b/WebCore/html/HTMLCanvasElement.cpp @@ -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; diff --git a/WebCore/html/HTMLCanvasElement.h b/WebCore/html/HTMLCanvasElement.h index ee2c250..a0b7cc5c 100644 --- a/WebCore/html/HTMLCanvasElement.h +++ b/WebCore/html/HTMLCanvasElement.h @@ -131,7 +131,7 @@ private: bool m_rendererIsCanvas; - RefPtr m_2DContext; + OwnPtr m_2DContext; IntSize m_size; CanvasObserver* m_observer; -- 1.8.3.1