Bug 47241 - [chromium] DRT crash in fast/canvas tests w/accel 2D canvas due to NULL DrawingBuffer
Summary: [chromium] DRT crash in fast/canvas tests w/accel 2D canvas due to NULL Drawi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-05 19:24 PDT by Kenneth Russell
Modified: 2010-10-11 15:10 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.61 KB, patch)
2010-10-05 20:31 PDT, James Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-10-05 19:24:00 PDT
The following test cases in fast/canvas are failing in DRT when run with --enable-accelerated-compositing --enable-accelerated-2d-canvas due to a NULL m_drawingBuffer in CanvasRenderingContext2D. The reason is that drawingContext() is NULL when the CanvasRenderingContext2D is created, so the drawing buffer is never initialized.

fast/canvas/access-zero-sized-canvas.html
fast/canvas/canvas-skia-excessive-size.html
fast/canvas/toDataURL-noData.html
Comment 1 James Robinson 2010-10-05 20:31:00 PDT
Created attachment 69880 [details]
Patch
Comment 2 James Robinson 2010-10-05 20:32:19 PDT
Patch flips around the logic so we don't touch m_context3d or m_drawingBuffer if there's no drawingContext(), which makes sense since if there is no drawingBuffer() we won't be doing any software draws either.  This also means that <canvas> elements that are zero sized or excessively large will not get composited layers since canvas layers are only set if CanvasRenderingContext2D::isAccelerated() is true which checks m_context3d.
Comment 3 Stephen White 2010-10-06 10:32:48 PDT
Comment on attachment 69880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69880&action=review

Looks good.

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:171
> +        if (m_context3D) {

Nit:  Could use && and a single if (although it'd probably need more parens around the =; your call).
Comment 4 Kenneth Russell 2010-10-06 10:38:05 PDT
Comment on attachment 69880 [details]
Patch

Looks fine to me.
Comment 5 James Robinson 2010-10-11 15:04:21 PDT
Comment on attachment 69880 [details]
Patch

I thought this had already landed - whoops!
Comment 6 James Robinson 2010-10-11 15:10:10 PDT
Comment on attachment 69880 [details]
Patch

Clearing flags on attachment: 69880

Committed r69529: <http://trac.webkit.org/changeset/69529>
Comment 7 James Robinson 2010-10-11 15:10:15 PDT
All reviewed patches have been landed.  Closing bug.