ImagePixelLocker now manually allocates SkPixmap

third_party/WebKit/Source/platform/graphics/skia/ImagePixelLocker.cpp

 // Copyright 2015 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 "platform/graphics/skia/ImagePixelLocker.h"
 
 #include "third_party/skia/include/core/SkImage.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 
 namespace blink {
@@ skipping 11 matching lines @@
     return info.alphaType() == alphaType || info.alphaType() == kOpaque_SkAlphaType;
 }
 
 } // anonymous namespace
 
 ImagePixelLocker::ImagePixelLocker(PassRefPtr<const SkImage> image, SkAlphaType alphaType,
     SkColorType colorType)
     : m_image(image)
 {
     SkImageInfo info;
-    size_t imageRowBytes;
+    size_t rowBytes;
 
     // If the image has in-RAM pixels and their format matches, use them directly.
     // TODO(fmalita): All current clients expect packed pixel rows.  Maybe we could update them
     // to support arbitrary rowBytes, and relax the check below.
-    m_pixels = m_image->peekPixels(&info, &imageRowBytes);
+    m_pixels = m_image->peekPixels(&info, &rowBytes);
     if (m_pixels
         && infoIsCompatible(info, alphaType, colorType)
-        && imageRowBytes == info.minRowBytes()) {
+        && rowBytes == info.minRowBytes()) {
         return;
     }
 
+    m_pixels = nullptr;
+
     // No luck, we need to read the pixels into our local buffer.
     info = SkImageInfo::Make(m_image->width(), m_image->height(), colorType, alphaType);
-    if (!m_pixelStorage.tryAlloc(info) || !m_image->readPixels(m_pixelStorage, 0, 0)) {
-        m_pixels = nullptr;
+    rowBytes = info.minRowBytes();
+    size_t size = info.getSafeSize(rowBytes);
+    if (0 == size) {
+        return;
+    }
+
+    void* pixels = sk_malloc_flags(size, 0);

[f(malita), 2016/03/16 19:28:29]

Would it make sense to use a separate SkAutoMalloc m_storage to avoid manual
memory management?

That would likely explode on allocation failures, but I don't think the intent
here is to fail gracefully when OOM (it's just behavior inherited from
SkAutoPixmapStorage).

[robertphillips, 2016/03/16 19:59:10]

Done.

+    if (!pixels) {
+        return;
+    }
+
+    m_pixelStorage.reset(info, pixels, rowBytes);
+
+    if (!m_image->readPixels(m_pixelStorage, 0, 0)) {
+        sk_free(pixels);
+        m_pixelStorage.reset();
         return;
     }
 
     m_pixels = m_pixelStorage.addr();
 }
 
+ImagePixelLocker::~ImagePixelLocker()
+{
+    if (m_pixels == m_pixelStorage.addr()) {
+        // We had to make our own copy in the constructor
+        sk_free(const_cast<void*>(m_pixelStorage.addr()));
+    }
+}
+
 } // namespace blink