Make canvas code a bit more resilient to crashes....

Make canvas code a bit more resilient to crashes.

* Initialize PAINTSTRUCT to prevent crash if BeginPaint fails.
* Sanitize width-height for empty bitmaps.
* Return failure from initialize instead of crashing at that point.

BUG=3795
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=4914

[brettw, 2008-11-06 18:55:23]

http://codereview.chromium.org/9459/diff/1/2
File base/gfx/bitmap_platform_device_win.cc (left):

http://codereview.chromium.org/9459/diff/1/2#oldcode286
Line 286: CrashForBitmapAllocationFailure(width, height);
I think you can now remove this function from the file. Or maybe it should be
public somehow so your new crash in the canvasPaint can use it?

[amit, 2008-11-06 19:11:17]

Done, please take another look.

On 2008/11/06 18:55:23, brettw wrote:
> http://codereview.chromium.org/9459/diff/1/2
> File base/gfx/bitmap_platform_device_win.cc (left):
> 
> http://codereview.chromium.org/9459/diff/1/2#oldcode286
> Line 286: CrashForBitmapAllocationFailure(width, height);
> I think you can now remove this function from the file. Or maybe it should be
> public somehow so your new crash in the canvasPaint can use it?

[brettw, 2008-11-06 21:08:20]

LGTM.

Sorry for the duplicate comments, Reitveld is on crack.

http://codereview.chromium.org/9459/diff/20/21
File base/gfx/bitmap_platform_device_win.cc (right):

http://codereview.chromium.org/9459/diff/20/21#newcode287
Line 287: NOTREACHED() << "CreateDIBSection Failed. Error: " << error << "\n";
How about using LOG(ERROR) instead. This will give some data in release mode
(normally turned off, but runtime switchable), and NOTREACHED() is debug-only.

http://codereview.chromium.org/9459/diff/20/21#newcode287
Line 287: NOTREACHED() << "CreateDIBSection Failed. Error: " << error << "\n";
How about LOG(ERROR) here which will work in release mode as well as debug
(NOTREACHED is debug-only)

http://codereview.chromium.org/9459/diff/209/29
File base/gfx/platform_canvas_win.h (right):

http://codereview.chromium.org/9459/diff/209/29#newcode172
Line 172: // Cause a deliberate crash;
; -> .