Bug 23212 – Crash in HTMLCanvasElement

• Component: DOM
• Status: RESOLVED FIXED

[Mike Belshe]

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.

[Mike Belshe]

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]

Created attachment 26572 [details]
Patch

[Eric Seidel (no email)]

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 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 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]

Filed in Chromium as http://code.google.com/p/chromium/issues/detail?id=5452

[Mike Belshe]

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)]

*** Bug 24209 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

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)]

http://trac.webkit.org/changeset/42236