refractoring ImageBitmap class

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

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index bb45b3f009b97649d272ffb1158c13e864aee62a..12c5ee538ec5048572857ab385775e6e1ce90c9d 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -25,40 +25,24 @@ static inline IntRect normalizeRect(const IntRect& rect)
         std::max(rect.height(), -rect.height()));
 }
 
-static inline PassRefPtr<Image> cropImage(PassRefPtr<Image> image, const IntRect& cropRect)
+static inline PassRefPtr<SkImage> cropImage(PassRefPtr<SkImage> image, const IntRect& cropRect)
 {
     ASSERT(image);
 
-    const SkIRect srcRect = intersection(image->rect(), cropRect);
+    IntRect imgRect = IntRect(IntPoint(), IntSize(image->width(), image->height()));
+    const SkIRect srcRect = intersection(imgRect, cropRect);
     if (srcRect.isEmpty())
         return nullptr;
 
-    RefPtr<SkImage> skImage = image->imageForCurrentFrame();
-    if (!skImage)
-        return nullptr;
-
-    return StaticBitmapImage::create(adoptRef(skImage->newSubset(srcRect)));
+    return adoptRef(image->newSubset(srcRect));

[Justin Novosad, 2015/11/02 15:53:19]

Not so fast! In the case where the intersection operation clips the crop rect,
you have to add padding to the result image.

Basically in the case where cropRect != srcRect, you need to create an SkSurface
the size of cropRect, fill it with "transparent", the draw the srcRect to it
with an offset of (srcRect.x() - cropRect.x(), srcRect.y() - cropRect.y())

This will allow you to produce results equivalent to the old m_bitmapRect logic.
Not doing this is what is causing the layout test failures you are seeing in the
try jobs.

 }
 
 ImageBitmap::ImageBitmap(HTMLImageElement* image, const IntRect& cropRect)
-    : m_imageElement(image)
-    , m_bitmap(nullptr)
-    , m_cropRect(cropRect)
 {
-    IntRect srcRect = intersection(cropRect, IntRect(0, 0, image->width(), image->height()));
-    m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())), srcRect.size());
-    m_bitmapOffset = srcRect.location();
-
-    if (!srcRect.width() || !srcRect.height())
-        m_imageElement = nullptr;
-    else
-        m_imageElement->addClient(this);
+    m_image = cropImage(image->cachedImage()->image()->imageForCurrentFrame(), cropRect);
 }
 
 ImageBitmap::ImageBitmap(HTMLVideoElement* video, const IntRect& cropRect)
-    : m_imageElement(nullptr)
-    , m_cropRect(cropRect)
-    , m_bitmapOffset(IntPoint())
 {
     IntSize playerSize;
 
@@ -77,27 +61,19 @@ ImageBitmap::ImageBitmap(HTMLVideoElement* video, const IntRect& cropRect)
     buffer->canvas()->translate(-srcRect.x(), -srcRect.y());
 
     video->paintCurrentFrame(buffer->canvas(), videoRect, nullptr);
-    m_bitmap = buffer->newImageSnapshot();
-    m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())), srcRect.size());
+    m_image = buffer->newSkImageSnapshot(PreferNoAcceleration);
 }
 
 ImageBitmap::ImageBitmap(HTMLCanvasElement* canvas, const IntRect& cropRect)
-    : m_imageElement(nullptr)
-    , m_cropRect(cropRect)
-    , m_bitmapOffset(IntPoint())
 {
-    IntRect srcRect = intersection(cropRect, IntRect(IntPoint(), canvas->size()));
-    m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())), srcRect.size());
     ASSERT(canvas->isPaintable());
-    m_bitmap = cropImage(canvas->copiedImage(BackBuffer, PreferAcceleration), cropRect);
+    m_image = cropImage(canvas->copiedImage(BackBuffer, PreferAcceleration)->imageForCurrentFrame(), cropRect);
 }
 
 ImageBitmap::ImageBitmap(ImageData* data, const IntRect& cropRect)
-    : m_imageElement(nullptr)
-    , m_cropRect(cropRect)
-    , m_bitmapOffset(IntPoint())
 {
     IntRect srcRect = intersection(cropRect, IntRect(IntPoint(), data->size()));
+    // TODO(xidachen): buffer should be with size of srcRect->size(), not data->size().
     OwnPtr<ImageBuffer> buffer = ImageBuffer::create(data->size(), NonOpaque, DoNotInitializeImagePixels);
     if (!buffer)
         return;
@@ -105,44 +81,21 @@ ImageBitmap::ImageBitmap(ImageData* data, const IntRect& cropRect)
     if (srcRect.width() > 0 && srcRect.height() > 0)
         buffer->putByteArray(Unmultiplied, data->data()->data(), data->size(), srcRect, IntPoint(std::min(0, -cropRect.x()), std::min(0, -cropRect.y())));
 
-    m_bitmap = buffer->newImageSnapshot();
-    m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())),  srcRect.size());
+    m_image = buffer->newSkImageSnapshot(PreferNoAcceleration);
 }
 
 ImageBitmap::ImageBitmap(ImageBitmap* bitmap, const IntRect& cropRect)
-    : m_imageElement(bitmap->imageElement())
-    , m_bitmap(nullptr)
-    , m_cropRect(cropRect)
-    , m_bitmapOffset(IntPoint())
 {
-    IntRect oldBitmapRect = bitmap->bitmapRect();
-    IntRect srcRect = intersection(cropRect, oldBitmapRect);
-    m_bitmapRect = IntRect(IntPoint(std::max(0, oldBitmapRect.x() - cropRect.x()), std::max(0, oldBitmapRect.y() - cropRect.y())), srcRect.size());
-
-    if (m_imageElement) {
-        m_imageElement->addClient(this);
-        m_bitmapOffset = srcRect.location();
-    } else if (bitmap->bitmapImage()) {
-        IntRect adjustedCropRect(IntPoint(cropRect.x() -oldBitmapRect.x(), cropRect.y() - oldBitmapRect.y()), cropRect.size());
-        m_bitmap = cropImage(bitmap->bitmapImage(), adjustedCropRect);
-    }
+    m_image = cropImage(bitmap->skImage(), cropRect);
 }
 
 ImageBitmap::ImageBitmap(Image* image, const IntRect& cropRect)
-    : m_imageElement(nullptr)
-    , m_cropRect(cropRect)
 {
-    IntRect srcRect = intersection(cropRect, image->rect());
-    m_bitmap = cropImage(image, cropRect);
-    m_bitmapRect = IntRect(IntPoint(std::max(0, -cropRect.x()), std::max(0, -cropRect.y())),  srcRect.size());
+    m_image = cropImage(image->imageForCurrentFrame(), cropRect);
 }
 
 ImageBitmap::~ImageBitmap()
 {
-#if !ENABLE(OILPAN)
-    if (m_imageElement)
-        m_imageElement->removeClient(this);
-#endif
 }
 
 PassRefPtrWillBeRawPtr<ImageBitmap> ImageBitmap::create(HTMLImageElement* image, const IntRect& cropRect)
@@ -183,33 +136,21 @@ PassRefPtrWillBeRawPtr<ImageBitmap> ImageBitmap::create(Image* image, const IntR
 
 void ImageBitmap::notifyImageSourceChanged()
 {
-    m_bitmap = cropImage(m_imageElement->cachedImage()->image(), m_cropRect);
-    m_bitmapOffset = IntPoint();
-    m_imageElement = nullptr;
-}
-
-PassRefPtr<Image> ImageBitmap::bitmapImage() const
-{
-    ASSERT((m_imageElement || m_bitmap || !m_bitmapRect.width() || !m_bitmapRect.height()) && (!m_imageElement || !m_bitmap));
-    if (m_imageElement)
-        return m_imageElement->cachedImage()->image();
-    return m_bitmap;
 }
 
 PassRefPtr<Image> ImageBitmap::getSourceImageForCanvas(SourceImageStatus* status, AccelerationHint) const
 {
     *status = NormalSourceImageStatus;
-    return bitmapImage();
+    return StaticBitmapImage::create(m_image);
 }
 
 void ImageBitmap::adjustDrawRects(FloatRect* srcRect, FloatRect* dstRect) const
 {
-    FloatRect intersectRect = intersection(m_bitmapRect, *srcRect);
+    FloatRect intersectRect = intersection(IntRect(0, 0, width(), height()), *srcRect);
     FloatRect newSrcRect = intersectRect;
-    newSrcRect.move(m_bitmapOffset - m_bitmapRect.location());
-    FloatRect newDstRect(FloatPoint(intersectRect.location() - srcRect->location()), m_bitmapRect.size());
-    newDstRect.scale(dstRect->width() / srcRect->width() * intersectRect.width() / m_bitmapRect.width(),
-        dstRect->height() / srcRect->height() * intersectRect.height() / m_bitmapRect.height());
+    FloatRect newDstRect(FloatPoint(intersectRect.location() - srcRect->location()), size());
+    newDstRect.scale(dstRect->width() / srcRect->width() * intersectRect.width() / width(),
+        dstRect->height() / srcRect->height() * intersectRect.height() / height());
     newDstRect.moveBy(dstRect->location());
     *srcRect = newSrcRect;
     *dstRect = newDstRect;
@@ -222,7 +163,6 @@ FloatSize ImageBitmap::elementSize() const
 
 DEFINE_TRACE(ImageBitmap)
 {
-    visitor->trace(m_imageElement);
     ImageLoaderClient::trace(visitor);
 }