Save a bitmap copy when advancing to dependent GIF and WebP animation frames

third_party/WebKit/Source/core/frame/ImageBitmap.cpp

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index 990fd97df9293e470145e6e37e5d0c9368ae8726..e239ae8a081e808a0bfbbadb356eaa5720b4295d 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -42,12 +42,6 @@ static inline IntRect normalizeRect(const IntRect& rect)
         std::max(rect.height(), -rect.height()));
 }
 
-static bool frameIsValid(const SkBitmap& frameBitmap)
-{
-    ASSERT(!frameBitmap.isNull() && !frameBitmap.empty() && frameBitmap.isImmutable());
-    return frameBitmap.colorType() == kN32_SkColorType;
-}
-
 ParsedOptions parseOptions(const ImageBitmapOptions& options, Optional<IntRect> cropRect, IntSize sourceSize)
 {
     ParsedOptions parsedOptions;
@@ -180,10 +174,10 @@ PassRefPtr<SkImage> ImageBitmap::getSkImageFromDecoder(std::unique_ptr<ImageDeco
     ImageFrame* frame = decoder->frameBufferAtIndex(0);
     if (!frame || frame->getStatus() != ImageFrame::FrameComplete)
         return nullptr;
-    SkBitmap bitmap = frame->bitmap();
-    if (!frameIsValid(bitmap))
+    DCHECK(!frame->bitmap().isNull() && !frame->bitmap().empty());
+    if (frame->bitmap().colorType() != kN32_SkColorType)

[Peter Kasting, 2016/09/22 21:44:08]

So, what I meant by "is this safe" was not so much "leave this as a conditional"
but simply "can you justify that this does or does not happen"?

It sounded like you don't think this can ever fail, so maybe we _should_ DCHECK
it.  An intentional behavior change is OK, I just didn't want an accidental one.

If we don't DCHECK it, it deserves a comment about when it can fail.

Another question to ask is "do we even care"?  Why bother checking the color
type at all?  Can't SkImage handle other color types anyway?  I guess I don't
really know what the code is trying to do.

Applies in the other place that checks the color type too.

[aleksandar.stojiljkovic, 2016/09/27 18:08:28]

I didn't see how it could happen with current set of decoders in public Chromium
base. Reverted it back because, after reducing workload I plan to resume work on
Index8 and RGB565 (for Android when target surface is 565) and then to handle
this constraints. 

         return nullptr;
-    return fromSkSp(SkImage::MakeFromBitmap(bitmap));
+    return fromSkSp(frame->finalizePixelsAndGetImage());

[Peter Kasting, 2016/09/22 21:44:08]

Nit: Shorter:

  return (frame->bitmap().colorType() == kN32_SkColorType) ?
fromSkSp(frame->finalizePixelsAndGetImage()) : nullptr;

[aleksandar.stojiljkovic, 2016/09/27 18:08:28]

Done.

 }
 
 bool ImageBitmap::isResizeOptionValid(const ImageBitmapOptions& options, ExceptionState& exceptionState)