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/skia/SkiaUtils.h"
@@ skipping 27 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 144 matching lines @@
     return newSkImageFromRaster(info, std::move(dstPixels), static_cast<size_t>(input->width()) * info.bytesPerPixel());
 }
 
 sk_sp<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))
-        return nullptr;
-    return SkImage::MakeFromBitmap(bitmap);
+    DCHECK(!frame->bitmap().isNull() && !frame->bitmap().empty());
+    return (frame->bitmap().colorType() == kN32_SkColorType) ? frame->finalizePixelsAndGetImage() : nullptr;

[dcheng, 2016/09/27 22:44:15]

Just curious: what does that mean if we get here and the color type doesn't
match what we expect? Does that mean an error of some sort occurred earlier in
the decoding pipeline?

[aleksandar.stojiljkovic, 2016/09/28 18:51:24]

kUnknown_SkColorType:
In initial state, SkBitmap has kUnknown_SkColorType. This happens on error but
also when file is partially received, the frame data is not yet received and the
frame not initialized.
In this case ASSERT/DCHECK(!isNull && !isEmpty) is covering the same as the
condition.

some other bitmap type:
Not an error but someone implements a decoder producing different bitmap type.
Don't think there is such case now - I'm working on a patch of such kind for
chromium [1]. Don't know if there are decoders in different chromium flavors
doing it (probably no need to care about it).
[1]
https://codereview.chromium.org/1460523002/


Otherwise, the change in this file is refactoring only. We're not using
isImmutable but it had the same meaning as getStatus() ==
ImageFrame::FrameComplete.

[dcheng, 2016/09/28 21:16:52]

I understand that this is a refactoring only change, but I don't really
understand it either. Is it possible to add a comment about why we'd only return
an SkImage if the color type matches exactly? How is it handled if it doesn't
match? Do we just not return an image at all? That doesn't seem completely right
either?

(I'm sure this makes more sense to someone who's already familiar with the
decoding pipeline, but I'm not, and so more comments seem like they'd be useful
here)

[aleksandar.stojiljkovic, 2016/09/28 22:10:22]

Yes, given the explanation I provided above, there is really no sense in doing
the color check. Removed it.
There are no decoders that produce other color types than N32.
If the frame's bitmap is empty (colortype kUnknown_SkColorType) but frame marked
as complete which cannot happen now, but let's open possibility for future, it
is still perfectly fine to call SkImage::MakeFromBitmap(m_bitmap) on empty
m_bitmap - it returns null.
So, no need to check color type - all cases are handled.

Related to your other concern, returning nullptr from this method is correct and
expected in case of error. All the clients of the call are handling it.

 }
 
 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 477 matching lines @@
 FloatSize ImageBitmap::elementSize(const FloatSize&) const
 {
     return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap)
 {
 }
 
 } // namespace blink