RESOLVED FIXED 103207
Be consistent in handling of *Image::frameAtIndex (and related) return values 
https://bugs.webkit.org/show_bug.cgi?id=103207
Summary Be consistent in handling of *Image::frameAtIndex (and related) return values
Brent Fulgham
Reported 2012-11-25 16:23:53 PST
Under certain conditions, the various image object "frameAtIndex" methods return null. This means that 'nativeImageForCurrentFrame" can also return null. This has resulted in a set of patches to ImageSVG (Bug 14531), BitmapImage (Bug 68753), ImageCG (Bug 61684), GraphicsLayerChromium (Bug 98456) and others (Bug 39797) to deal with the null return. A number of classes check for null return and exit early (ImageWx.cpp, parts of ImageWinCE.cpp, ImageSkia.cpp, ImageCairoWin.cpp, ImageQt.cpp, ImageMac.mm, BitmapImageCG.cpp, BitmapImageCairo.cpp) However, several others take the return value and use without validating (ImageCGWin.cpp, ImageCairoWin.cpp, parts of ImageWinCE.cpp) This bug applies the same null checking used elsewhere in the codebase to these missed locations.
Attachments
Patch (9.99 KB, patch)
2012-11-25 16:49 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (10.40 KB, patch)
2012-11-25 17:08 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (9.71 KB, patch)
2012-11-28 14:37 PST, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2012-11-25 16:49:32 PST
Created attachment 175898 [details] Patch
WebKit Review Bot
Comment 2 2012-11-25 16:55:13 PST
Comment on attachment 175898 [details] Patch Attachment 175898 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14982425
Peter Beverloo (cr-android ews)
Comment 3 2012-11-25 16:59:41 PST
Comment on attachment 175898 [details] Patch Attachment 175898 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14989360
Brent Fulgham
Comment 4 2012-11-25 17:05:34 PST
These kinds of bugs are likely to creep in again. I wonder if any of C++ method declarations could help in the future?
Brent Fulgham
Comment 5 2012-11-25 17:08:54 PST
Created attachment 175901 [details] Patch
WebKit Review Bot
Comment 6 2012-11-25 17:51:57 PST
Comment on attachment 175901 [details] Patch Attachment 175901 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14985439 New failing tests: svg/W3C-SVG-1.1/animate-elem-52-t.svg
Brent Fulgham
Comment 7 2012-11-25 19:20:39 PST
I don't think the cr-linux failure is real, based on a review of the test bot output, and the fact that the cr-android test seems to succeed. Can any Chromium people confirm for me if the patch causes a failure for them?
Brent Fulgham
Comment 8 2012-11-28 14:37:19 PST
Created attachment 176581 [details] Patch
kov's GTK+ EWS bot
Comment 9 2012-11-28 14:48:04 PST
Comment on attachment 176581 [details] Patch Attachment 176581 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/15023580
Dave Hyatt
Comment 10 2012-11-29 11:42:17 PST
Comment on attachment 176581 [details] Patch r=me
WebKit Review Bot
Comment 11 2012-11-29 12:13:45 PST
Comment on attachment 176581 [details] Patch Clearing flags on attachment: 176581 Committed r136147: <http://trac.webkit.org/changeset/136147>
WebKit Review Bot
Comment 12 2012-11-29 12:13:50 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.