[webkit-reviews] review denied: [Bug 68753] [WinCairo] BitmapImage::drawFrameMatchingSourceSize causes access violation if BitmapImage::frameAtIndex() returns NULL : [Attachment 108581] Check for zero cairo_surface_t * pointer to avoid null pointer exception

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has denied David Delaune
<david.delaune at hotmail.com>'s request for review:
Bug 68753: [WinCairo] BitmapImage::drawFrameMatchingSourceSize causes access
violation if BitmapImage::frameAtIndex() returns NULL
https://bugs.webkit.org/show_bug.cgi?id=68753

Attachment 108581: Check for zero cairo_surface_t * pointer to avoid null
pointer exception
https://bugs.webkit.org/attachment.cgi?id=108581&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=108581&action=review


Is it possible to add a regression test for this? Perhaps via TestWebKitAPI?

> Source/WebCore/platform/graphics/win/ImageCairoWin.cpp:100
>      for (size_t i = 0; i < frames; ++i) {
>	   cairo_surface_t* image = frameAtIndex(i);
> -	   if (cairo_image_surface_get_height(image) ==
static_cast<size_t>(srcSize.height()) && cairo_image_surface_get_width(image)
== static_cast<size_t>(srcSize.width())) {
> +	   if (image && cairo_image_surface_get_height(image) ==
static_cast<size_t>(srcSize.height()) && cairo_image_surface_get_width(image)
== static_cast<size_t>(srcSize.width())) {

I think this would be a little clearer using an early continue:

cairo_surface_t* image = frameAtIndex(i);
if (!image)
    continue;