Allow eviction of ImageBitmaps that are created from ImageElements.

Source/core/page/ImageBitmap.cpp

Index: Source/core/page/ImageBitmap.cpp
diff --git a/Source/core/page/ImageBitmap.cpp b/Source/core/page/ImageBitmap.cpp
index fabe6847b5aabfeb1a3482cb5754b9cbb20caddd..dd23338db66c7f2f11e65ceb082cdaead878ab4e 100644
--- a/Source/core/page/ImageBitmap.cpp
+++ b/Source/core/page/ImageBitmap.cpp
@@ -9,7 +9,6 @@
 #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"
 
@@ -17,7 +16,7 @@ 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()),
@@ -25,26 +24,32 @@ static inline IntRect normalizeRect(const IntRect rect)
         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);
@@ -55,14 +60,16 @@ ImageBitmap::ImageBitmap(HTMLVideoElement* video, IntRect cropRect)
     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));
@@ -70,14 +77,16 @@ ImageBitmap::ImageBitmap(HTMLCanvasElement* canvas, IntRect cropRect)
 
     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()));
 
@@ -85,55 +94,76 @@ ImageBitmap::ImageBitmap(ImageData* data, IntRect cropRect)
     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;
+}
+
 }