This leads to bugs since code in WebKit typically assumes that if a function returns true to isFoo() it is safe to static_cast<Foo*> and call functions. See https://bugs.webkit.org/show_bug.cgi?id=65063 for an example of where this bug caused real crashes.
I don't like the changes made for bug 65063, so I welcome cleanup here.
For the record, I don't either. It was either that or turn off your optimization for all skia ports, though :/.
The original motivation for BitmapImageSingleFrameSkia is to avoid having all of the animation overhead for simple 1-frame images, which seems like a useful optimization for all ports. I think the option 3 from https://bugs.webkit.org/show_bug.cgi?id=65063#c18 is the best way to go here - we should turn BitmapImage into a simple class that doesn't handle animation concerns, and use an AnimatedBitmapImage subclass to handle the animation case. This should have the nice benefit of reducing the memory use for non-animated images for all platforms and clean up the class hierarchy a lot for the skia case.
It feels like a stretch for me, but it looks like the original class of bug may be triggering again in the Chromium port, so I'm going to try to do the refactoring James endorsed this week.
Created attachment 140089 [details] Patch
In the discussions around https://bugs.webkit.org/show_bug.cgi?id=65063#c18, one thing we didn't realize is not only does the Skia-specific single-frame class avoid all the implementation for multiple frames, it *also* uses a different type to store the image, so there doesn't seem to be any implementation that can be shared between the two classes. This patch introduces a lightweight class SimpleBitmapImage that does nothing more than mark that an object is some sort of Bitmap (rather than SVG). I've gone through the WebCore fixing up call sites that were using isBitmapImage() to use isSimpleBitmapImage() where that seems reasonable. If we're going to do something more comprehensive, I'll need pointers from people familiar with this part of the code.
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12545388
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12607303
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12545391
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12625094
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12612385
Comment on attachment 140089 [details] Patch Attachment 140089 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12611334
Created attachment 140244 [details] Patch
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12543344
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12632170
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12633176
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12631189
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12627184
Comment on attachment 140244 [details] Patch Attachment 140244 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12629213
Created attachment 140324 [details] Patch
Comment on attachment 140324 [details] Patch Attachment 140324 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12631284
Comment on attachment 140324 [details] Patch Attachment 140324 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12631286
Comment on attachment 140324 [details] Patch Attachment 140324 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12627265
Comment on attachment 140324 [details] Patch Attachment 140324 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12531283
Created attachment 142772 [details] Patch
Comment on attachment 142772 [details] Patch Attachment 142772 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12720968
Comment on attachment 142772 [details] Patch Attachment 142772 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12722888
Comment on attachment 142772 [details] Patch Attachment 142772 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12728332
Comment on attachment 142772 [details] Patch Attachment 142772 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12728344