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

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/frame/ImageBitmap.h"
 
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
 #include "platform/graphics/AcceleratedStaticBitmapImage.h"
@@ skipping 24 matching lines @@
 
 // The following two functions are helpers used in cropImage
 static inline IntRect normalizeRect(const IntRect& rect)
 {
     return IntRect(std::min(rect.x(), rect.maxX()),
         std::min(rect.y(), rect.maxY()),
         std::max(rect.width(), -rect.width()),
         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;
     if (options.imageOrientation() == imageOrientationFlipY) {
         parsedOptions.flipY = true;
     } else {
         parsedOptions.flipY = false;
         DCHECK(options.imageOrientation() == imageBitmapOptionNone);
     }
     if (options.premultiplyAlpha() == imageBitmapOptionNone) {
@@ skipping 112 matching lines @@
     return newSkImageFromRaster(info, std::move(dstPixels), input->width() * info.bytesPerPixel());
 }
 
 PassRefPtr<SkImage> ImageBitmap::getSkImageFromDecoder(std::unique_ptr<ImageDecoder> decoder)
 {
     if (!decoder->frameCount())
         return nullptr;
     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)
 {
     if ((options.hasResizeWidth() && options.resizeWidth() == 0) || (options.hasResizeHeight() && options.resizeHeight() == 0)) {
         exceptionState.throwDOMException(InvalidStateError, "The resizeWidth or/and resizeHeight is equal to 0.");
         return false;
     }
     return true;
 }
@@ skipping 455 matching lines @@
 FloatSize ImageBitmap::elementSize(const FloatSize&) const
 {
     return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap)
 {
 }
 
 } // namespace blink