Bug 23212

Summary: Crash in HTMLCanvasElement
Product: WebKit Reporter: Mike Belshe <mbelshe>
Component: DOMAssignee: Mike Belshe <mbelshe>
Status: RESOLVED FIXED    
Severity: Critical CC: darin, eric
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
The test page which can crash
none
Patch eric: review+

Mike Belshe
Reported 2009-01-09 13:03:07 PST
Attached is a simple web page which can crash webkit. The problem is a null-dereference problem when we can't create the ImageBuffer. On Chromium, this is fairly easy to reproduce.
Attachments
The test page which can crash (958 bytes, text/html)
2009-01-09 13:04 PST, Mike Belshe 		no flags
Details
Patch (1.42 KB, patch)
2009-01-09 13:13 PST, Mike Belshe 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mike Belshe
Comment 1 2009-01-09 13:04:06 PST
Created attachment 26571 [details] The test page which can crash Crashing is somewhat platform dependent, as it requires a case where the ImageBuffer::create() call fails.
Mike Belshe
Comment 2 2009-01-09 13:13:44 PST
Created attachment 26572 [details] Patch
Eric Seidel (no email)
Comment 3 2009-01-09 13:17:10 PST
Comment on attachment 26572 [details] Patch The change looks great. Is there a way to make a LayoutTest for this for run-webkit-tests?
Darin Adler
Comment 4 2009-01-09 13:27:24 PST
Comment on attachment 26572 [details] Patch Code change looks good. How did you discover this bug? Can you write a regression test? Normally we require a regression test for every bug fix unless there's a reason that's impractical.
Darin Adler
Comment 5 2009-01-10 14:36:27 PST
Comment on attachment 26572 [details] Patch For now, review- because of the lack of a regression test. If it's not possible to make one, please explain why in the change log and put a new patch up for review.
Jon@Chromium
Comment 6 2009-01-12 14:44:59 PST
Filed in Chromium as http://code.google.com/p/chromium/issues/detail?id=5452
Mike Belshe
Comment 7 2009-02-12 10:36:54 PST
Sorry for not having had time to get the regression test yet; the attached page reproduces the problem. I'll get to this eventually; I think the fix is simple enough it could go as-is. If someone else wants to take a shot at the layout test in the interim, please do.
Eric Seidel (no email)
Comment 8 2009-02-27 14:27:11 PST
*** Bug 24209 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
Comment 9 2009-04-06 01:22:57 PDT
Comment on attachment 26572 [details] Patch I recommend we land this w/o test case. I just attempted to make one and found bug 25055. The test case I've attached to bug 25055 could possibly catch this on some platforms. This exact crash is sorta protected against by IntSize HTMLCanvasElement::convertLogicalToDevice(const FloatSize& logicalSize) const which makes sure that logicalSize is < MaxCanvasSize. however testing max canvas size (as I did in bug 25055) reveals that we fail in other parts of the code (making it difficult to test for a crash here on platforms that might fail earlier than CG does. Mike, since I can never remember if you have commit-bit or not, I'll land this, with additional information in the ChangeLog about why a test case could not be created.
Eric Seidel (no email)
Comment 10 2009-04-07 06:39:09 PDT
http://trac.webkit.org/changeset/42236
Note You need to log in before you can comment on or make changes to this bug.