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

third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
index e294d1ffcbcc80f7936e30bb4d45f16b7e0f5584..f338a9d305c1a272844e8fe59d8ffeaafd374723 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
@@ -1924,14 +1924,6 @@ void FormatConverter::convert()
     return;
 }
 
-bool frameIsValid(const SkBitmap& frameBitmap)
-{
-    return !frameBitmap.isNull()
-        && !frameBitmap.empty()
-        && frameBitmap.isImmutable()
-        && frameBitmap.colorType() == kN32_SkColorType;
-}
-
 } // anonymous namespace
 
 WebGLImageConversion::PixelStoreParams::PixelStoreParams()
@@ -2150,18 +2142,17 @@ void WebGLImageConversion::ImageExtractor::extractImage(bool premultiplyAlpha, b
         if (!decoder->frameCount())
             return;
         ImageFrame* frame = decoder->frameBufferAtIndex(0);
+
+        // TODO(fmalita): Partial frames are not supported currently: only fully
+        // decoded frames make it through.  We could potentially relax this and
+        // allow ImageFrame::finalizePixelsAndGetImage to make a copy.

[Peter Kasting, 2016/08/30 06:59:40]

Isn't it already allowed to make a copy?  Does this comment mean something like
"We could potentially relax this if we decided it was OK for the conversion from
bitmap to image to make a copy in this case."?

[aleksandar.stojiljkovic, 2016/09/21 20:56:57]

By early return if "frame->getStatus() != ImageFrame::FrameComplete" we ensure
that there is no copy done by finalizePixelsAndGetImage. This means that if
don't want to prevent copy by finalizePixelsAndGetImage, we can relax the check
to e.g.:
if (!frame)
    return;

         if (!frame || frame->getStatus() != ImageFrame::FrameComplete)
             return;
         hasAlpha = frame->hasAlpha();
         SkBitmap bitmap = frame->bitmap();
-        if (!frameIsValid(bitmap))
-            return;
 
-        // TODO(fmalita): Partial frames are not supported currently: frameIsValid ensures that
-        // only immutable/fully decoded frames make it through.  We could potentially relax this
-        // and allow SkImage::NewFromBitmap to make a copy.
-        ASSERT(bitmap.isImmutable());
-        skiaImage = fromSkSp(SkImage::MakeFromBitmap(bitmap));
+        DCHECK_EQ(bitmap.colorType(), kN32_SkColorType);

[Peter Kasting, 2016/08/30 06:59:40]

Same comment as in ImageBitmap.cpp.

[aleksandar.stojiljkovic, 2016/09/21 20:56:57]

Done.

+        skiaImage = frame->finalizePixelsAndGetImage();
         info = bitmap.info();
 
         if (hasAlpha && premultiplyAlpha)