Allow eviction of ImageBitmaps that are created from ImageElements.

Source/core/page/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 "config.h"
 #include "core/page/ImageBitmap.h"
 
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
-#include "core/page/ImageBitmapCallback.h"
 #include "core/platform/graphics/GraphicsContext.h"
 #include "wtf/RefPtr.h"
 
 using namespace std;
 
 namespace WebCore {
 
-static inline IntRect normalizeRect(const IntRect rect)
+static inline IntRect normalizeRect(const IntRect& rect)
 {
     return IntRect(min(rect.x(), rect.maxX()),
         min(rect.y(), rect.maxY()),
         max(rect.width(), -rect.width()),
         max(rect.height(), -rect.height()));
 }
 
-static inline PassRefPtr<BitmapImage> cropImage(Image* image, IntRect cropRect)
+static inline PassRefPtr<BitmapImage> cropImage(Image* image, const IntRect& cropRect)
 {
     SkBitmap cropped;
     image->nativeImageForCurrentFrame()->bitmap().extractSubset(&cropped, cropRect);
     return BitmapImage::create(NativeImageSkia::create(cropped));
 }
 
-ImageBitmap::ImageBitmap(HTMLImageElement* image, IntRect cropRect)
-    : m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
-    , m_size(cropRect.size())
+ImageBitmap::ImageBitmap(HTMLImageElement* image, const IntRect& cropRect)
+    : m_imageElement(image)
+    , m_bitmap(0)
+    , m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
+    , m_cropRect(cropRect)
 {
-    Image* bitmapImage = image->cachedImage()->image();
-    m_bitmap = cropImage(bitmapImage, cropRect).get();
+    CachedImage* cachedImage = m_imageElement->cachedImage();
+    // this will also elevate the decode cache priority of the original HTMLImageElement

[Justin Novosad, 2013/07/19 21:09:53]

This is fine, no need to write this comment.

+    // which is probably undesired behavior
+    cachedImage->setDecodeCachePriority(CachedResource::DecodeCachePriorityHigh);

[Justin Novosad, 2013/07/19 21:09:53]

I don't think this code works.  The unit test you wrote for the cache is fine,
but you need an additional test that exercises this code.  When you set the
cache priority on an item that is already inserted in the cache, its position in
the cache's linked list remains unchanged. So this probably has no effect.

+    m_bitmapSize = intersection(cropRect, IntRect(IntPoint(), cachedImage->image()->size())).size();
 
     ScriptWrappable::init(this);
 }
 
-ImageBitmap::ImageBitmap(HTMLVideoElement* video, IntRect cropRect)
-    : m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
-    , m_size(cropRect.size())
+ImageBitmap::ImageBitmap(HTMLVideoElement* video, const IntRect& cropRect)
+    : m_imageElement(0)
+    , m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
+    , m_cropRect(cropRect)
 {
     IntRect videoRect = IntRect(IntPoint(), video->player()->naturalSize());
     IntRect srcRect = intersection(cropRect, videoRect);
     IntRect dstRect(IntPoint(), srcRect.size());
 
     m_buffer = ImageBuffer::create(videoRect.size());
     GraphicsContext* c = m_buffer->context();
     c->clip(dstRect);
     c->translate(-srcRect.x(), -srcRect.y());
     video->paintCurrentFrameInContext(c, videoRect);
-    m_bitmap = static_cast<BitmapImage*>(m_buffer->copyImage(DontCopyBackingStore).get());
+    m_bitmap = m_buffer->copyImage(DontCopyBackingStore);
+    m_bitmapSize = IntSize(m_bitmap->size());
 
     ScriptWrappable::init(this);
 }
 
-ImageBitmap::ImageBitmap(HTMLCanvasElement* canvas, IntRect cropRect)
-    : m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
-    , m_size(cropRect.size())
+ImageBitmap::ImageBitmap(HTMLCanvasElement* canvas, const IntRect& cropRect)
+    : m_imageElement(0)
+    , m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
+    , m_cropRect(cropRect)
 {
     IntSize canvasSize = canvas->size();
     IntRect srcRect = intersection(cropRect, IntRect(IntPoint(), canvasSize));
     IntRect dstRect(IntPoint(), srcRect.size());
 
     m_buffer = ImageBuffer::create(canvasSize);
     m_buffer->context()->drawImageBuffer(canvas->buffer(), dstRect, srcRect);
-    m_bitmap = static_cast<BitmapImage*>(m_buffer->copyImage(DontCopyBackingStore).get());
+    m_bitmap = m_buffer->copyImage(DontCopyBackingStore);
+    m_bitmapSize = IntSize(m_bitmap->size());
 
     ScriptWrappable::init(this);
 }
 
-ImageBitmap::ImageBitmap(ImageData* data, IntRect cropRect)
-    : m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
-    , m_size(cropRect.size())
+ImageBitmap::ImageBitmap(ImageData* data, const IntRect& cropRect)
+    : m_imageElement(0)
+    , m_bitmapOffset(max(0, -cropRect.x()), max(0, -cropRect.y()))
+    , m_cropRect(cropRect)
 {
     IntRect srcRect = intersection(cropRect, IntRect(IntPoint(), data->size()));
 
     m_buffer = ImageBuffer::create(data->size());
     if (srcRect.width() > 0 && srcRect.height() > 0)
         m_buffer->putByteArray(Unmultiplied, data->data(), data->size(), srcRect, IntPoint(min(0, -cropRect.x()), min(0, -cropRect.y())));
 
-    m_bitmap = static_cast<BitmapImage*>(m_buffer->copyImage(DontCopyBackingStore).get());
+    m_bitmap = m_buffer->copyImage(DontCopyBackingStore);
+    m_bitmapSize = IntSize(m_bitmap->size());
 
     ScriptWrappable::init(this);
 }
 
-ImageBitmap::ImageBitmap(ImageBitmap* bitmap, IntRect cropRect)
+ImageBitmap::ImageBitmap(ImageBitmap* bitmap, const IntRect& cropRect)
     : m_bitmapOffset(max(0, bitmap->bitmapOffset().x() - cropRect.x()), max(0, bitmap->bitmapOffset().y() - cropRect.y()))
-    , m_size(cropRect.size())
+    , m_cropRect(cropRect)
 {
-    Image* bitmapImage = bitmap->bitmapImage();
-    cropRect.moveBy(IntPoint(-bitmap->bitmapOffset().x(), -bitmap->bitmapOffset().y()));
-    m_bitmap = cropImage(bitmapImage, cropRect).get();
+    IntRect adjustedCropRect(IntPoint(cropRect.x() - bitmap->bitmapOffset().x(), cropRect.y() - bitmap->bitmapOffset().y()), cropRect.size());
+
+    // hold a reference to the old ImageBitmap's HTMLImageElement if possible
+    if (m_imageElement = bitmap->imageElement()) {
+        m_bitmap = 0;
+        m_bitmapSize = intersection(adjustedCropRect, IntRect(IntPoint(), m_imageElement->cachedImage()->image()->size())).size();
+    } else {
+        RefPtr<Image> bitmapImage = bitmap->bitmapImage();
+        m_bitmap = cropImage(bitmapImage.get(), adjustedCropRect);
+        m_bitmapSize = IntSize(m_bitmap->size());
+    }
 
     ScriptWrappable::init(this);
 }
 
-PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLImageElement* image, IntRect cropRect)
+PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLImageElement* image, const IntRect& cropRect)
 {
     IntRect normalizedCropRect = normalizeRect(cropRect);
-    RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap(image, normalizedCropRect)));
+    RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap( image, normalizedCropRect)));
     return imageBitmap.release();
 }
 
-PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLVideoElement* video, IntRect cropRect)
+PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLVideoElement* video, const IntRect& cropRect)
 {
     IntRect normalizedCropRect = normalizeRect(cropRect);
     RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap(video, normalizedCropRect)));
     return imageBitmap.release();
 }
 
-PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLCanvasElement* canvas, IntRect cropRect)
+PassRefPtr<ImageBitmap> ImageBitmap::create(HTMLCanvasElement* canvas, const IntRect& cropRect)
 {
     IntRect normalizedCropRect = normalizeRect(cropRect);
     RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap(canvas, normalizedCropRect)));
     return imageBitmap.release();
 }
 
-PassRefPtr<ImageBitmap> ImageBitmap::create(ImageData* data, IntRect cropRect)
+PassRefPtr<ImageBitmap> ImageBitmap::create(ImageData* data, const IntRect& cropRect)
 {
     IntRect normalizedCropRect = normalizeRect(cropRect);
     RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap(data, normalizedCropRect)));
     return imageBitmap.release();
 }
 
-PassRefPtr<ImageBitmap> ImageBitmap::create(ImageBitmap* bitmap, IntRect cropRect)
+PassRefPtr<ImageBitmap> ImageBitmap::create(ImageBitmap* bitmap, const IntRect& cropRect)
 {
     IntRect normalizedCropRect = normalizeRect(cropRect);
     RefPtr<ImageBitmap> imageBitmap(adoptRef(new ImageBitmap(bitmap, normalizedCropRect)));
     return imageBitmap.release();
 }
 
+PassRefPtr<Image> ImageBitmap::bitmapImage() const
+{
+    ASSERT(!m_imageElement || !m_bitmap);

[Justin Novosad, 2013/07/19 21:09:53]

To be more accurate, this should be an exclusive or.  There is no xor logical
operator in C++, so you could write it as
ASSERT(m_imageElement || m_bitmap && (!m_imageElement || !m_bitmap));

+    if (m_imageElement) {
+        Image* oldImage = m_imageElement->cachedImage()->image();
+        RefPtr<Image> image = cropImage(oldImage, m_cropRect);
+        image->setImageObserver(oldImage->imageObserver());
+        return image;
+    }
+    return m_bitmap;
 }
+
+}