Add onNewLockPixels, that returns rowbytes and relies on info in pixelref

This reverts commit 890a6ec633c1f54891104a072a8964b4c2c81af9.

BUG=
R=scroggo@google.com

Review URL: https://codereview.chromium.org/110593003

git-svn-id: http://skia.googlecode.com/svn/trunk@12883 2bbb7eff-a529-9590-31e7-b0007b416f81

15 files changed, 211 insertions, 119 deletions

diff --git a/include/core/SkBitmapDevice.h b/include/core/SkBitmapDevice.h
index 83f480c60c..f3d40d0ce8 100644
--- a/include/core/SkBitmapDevice.h
+++ b/include/core/SkBitmapDevice.h
@@ -258,6 +258,8 @@ private:
 
     friend class SkSurface_Raster;
 
+    void init(SkBitmap::Config config, int width, int height, bool isOpaque);
+
     // used to change the backend's pixels (and possibly config/rowbytes)
     // but cannot change the width/height, so there should be no change to
     // any clip information.
diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h
index 272dc21fd8..c40afc433a 100644
--- a/include/core/SkMallocPixelRef.h
+++ b/include/core/SkMallocPixelRef.h
@@ -32,11 +32,9 @@ public:
 
     /**
      *  Return a new SkMallocPixelRef, automatically allocating storage for the
-     *  pixels.
-     *
-     *  If rowBytes is 0, an optimal value will be chosen automatically.
-     *  If rowBytes is > 0, then it will be used, unless it is invald for the
-     *  specified info, in which case NULL will be returned (failure).
+     *  pixels. If rowBytes are 0, an optimal value will be chosen automatically.
+     *  If rowBytes is > 0, then it will be respected, or NULL will be returned
+     *  if rowBytes is invalid for the specified info.
      *
      *  This pixelref will ref() the specified colortable (if not NULL).
      *
@@ -90,7 +88,7 @@ protected:
     SkMallocPixelRef(SkFlattenableReadBuffer& buffer);
     virtual ~SkMallocPixelRef();
 
-    virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
     virtual void onUnlockPixels() SK_OVERRIDE;
     virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE;
     virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE;
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index e611dc0ef8..a332b76a5d 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -14,8 +14,11 @@
 #include "SkRefCnt.h"
 #include "SkString.h"
 #include "SkFlattenable.h"
+#include "SkImageInfo.h"
 #include "SkTDArray.h"
 
+//#define SK_SUPPORT_LEGACY_ONLOCKPIXELS
+
 #ifdef SK_DEBUG
     /**
      *  Defining SK_IGNORE_PIXELREF_SETPRELOCKED will force all pixelref
@@ -60,23 +63,48 @@ public:
     /** Return the pixel memory returned from lockPixels, or null if the
         lockCount is 0.
     */
-    void* pixels() const { return fPixels; }
+    void* pixels() const { return fRec.fPixels; }
 
     /** Return the current colorTable (if any) if pixels are locked, or null.
     */
-    SkColorTable* colorTable() const { return fColorTable; }
+    SkColorTable* colorTable() const { return fRec.fColorTable; }
 
     /**
+     *  To access the actual pixels of a pixelref, it must be "locked".
+     *  Calling lockPixels returns a LockRec struct (on success).
+     */
+    struct LockRec {
+        void*           fPixels;
+        SkColorTable*   fColorTable;
+        size_t          fRowBytes;
+        
+        void zero() { sk_bzero(this, sizeof(*this)); }
+
+        bool isZero() const {
+            return NULL == fPixels && NULL == fColorTable && 0 == fRowBytes;
+        }
+    };
+    
+    /**
      *  Returns true if the lockcount > 0
      */
     bool isLocked() const { return fLockCount > 0; }
 
     SkDEBUGCODE(int getLockCount() const { return fLockCount; })
 
-    /** Call to access the pixel memory, which is returned. Balance with a call
-        to unlockPixels().
-    */
-    void lockPixels();
+    /**
+     *  Call to access the pixel memory. Return true on success. Balance this
+     *  with a call to unlockPixels().
+     */
+    bool lockPixels();
+
+    /**
+     *  Call to access the pixel memory. On success, return true and fill out
+     *  the specified rec. On failure, return false and ignore the rec parameter.
+     *  Balance this with a call to unlockPixels().
+     */
+    bool lockPixels(LockRec* rec);
+
     /** Call to balanace a previous call to lockPixels(). Returns the pixels
         (or null) after the unlock. NOTE: lock calls can be nested, but the
         matching number of unlock calls must be made in order to free the
@@ -233,18 +261,27 @@ public:
     void addGenIDChangeListener(GenIDChangeListener* listener);
 
 protected:
-    /** Called when the lockCount goes from 0 to 1. The caller will have already
-        acquire a mutex for thread safety, so this method need not do that.
-    */
-    virtual void* onLockPixels(SkColorTable**) = 0;
+#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS
+    virtual void* onLockPixels(SkColorTable**);
+    virtual bool onNewLockPixels(LockRec*);
+#else
+    /**
+     *  On success, returns true and fills out the LockRec for the pixels. On
+     *  failure returns false and ignores the LockRec parameter.
+     *
+     *  The caller will have already acquired a mutex for thread safety, so this
+     *  method need not do that.
+     */
+    virtual bool onNewLockPixels(LockRec*) = 0;
+#endif
 
     /**
-     *  Called when the lock count goes from 1 to 0. The caller will have
-     *  already acquire a mutex for thread safety, so this method need not do
-     *  that.
+     *  Balancing the previous successful call to onNewLockPixels. The locked
+     *  pixel address will no longer be referenced, so the subclass is free to
+     *  move or discard that memory.
      *
-     *  If the previous call to onLockPixels failed (i.e. returned NULL), then
-     *  the onUnlockPixels will NOT be called.
+     *  The caller will have already acquired a mutex for thread safety, so this
+     *  method need not do that.
      */
     virtual void onUnlockPixels() = 0;
 
@@ -289,15 +326,15 @@ protected:
     // only call from constructor. Flags this to always be locked, removing
     // the need to grab the mutex and call onLockPixels/onUnlockPixels.
     // Performance tweak to avoid those calls (esp. in multi-thread use case).
-    void setPreLocked(void* pixels, SkColorTable* ctable);
+    void setPreLocked(void*, size_t rowBytes, SkColorTable*);
 
 private:
     SkBaseMutex*    fMutex; // must remain in scope for the life of this object
 
     const SkImageInfo fInfo;
 
-    void*           fPixels;
-    SkColorTable*   fColorTable;    // we do not track ownership, subclass does
+    // LockRec is only valid if we're in a locked state (isLocked())
+    LockRec         fRec;
     int             fLockCount;
 
     mutable uint32_t fGenerationID;
diff --git a/include/gpu/SkGrPixelRef.h b/include/gpu/SkGrPixelRef.h
index d8933724fb..63e975673f 100644
--- a/include/gpu/SkGrPixelRef.h
+++ b/include/gpu/SkGrPixelRef.h
@@ -24,10 +24,9 @@ public:
     virtual ~SkROLockPixelsPixelRef();
 
 protected:
-    // override from SkPixelRef
-    virtual void* onLockPixels(SkColorTable** ptr);
-    virtual void onUnlockPixels();
-    virtual bool onLockPixelsAreWritable() const;   // return false;
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
+    virtual void onUnlockPixels() SK_OVERRIDE;
+    virtual bool onLockPixelsAreWritable() const SK_OVERRIDE;   // return false;
 
 private:
     SkBitmap    fBitmap;
diff --git a/include/images/SkImageRef.h b/include/images/SkImageRef.h
index 30b1562c98..36f95e64b2 100644
--- a/include/images/SkImageRef.h
+++ b/include/images/SkImageRef.h
@@ -72,9 +72,9 @@ protected:
         When these are called, we will have already acquired the mutex!
      */
 
-    virtual void* onLockPixels(SkColorTable**);
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
     // override this in your subclass to clean up when we're unlocking pixels
-    virtual void onUnlockPixels() {}
+    virtual void onUnlockPixels() SK_OVERRIDE {}
 
     SkImageRef(SkFlattenableReadBuffer&, SkBaseMutex* mutex = NULL);
     virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE;
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 1668618cf7..368c807511 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -24,31 +24,30 @@ SkBitmapDevice::SkBitmapDevice(const SkBitmap& bitmap, const SkDeviceProperties&
     , fBitmap(bitmap) {
 }
 
-SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) {
+void SkBitmapDevice::init(SkBitmap::Config config, int width, int height, bool isOpaque) {
     fBitmap.setConfig(config, width, height, 0, isOpaque ?
                       kOpaque_SkAlphaType : kPremul_SkAlphaType);
-    if (!fBitmap.allocPixels()) {
-        fBitmap.setConfig(config, 0, 0, 0, isOpaque ?
-                          kOpaque_SkAlphaType : kPremul_SkAlphaType);
-    }
-    if (!isOpaque) {
-        fBitmap.eraseColor(SK_ColorTRANSPARENT);
+    
+    if (SkBitmap::kNo_Config != config) {
+        if (!fBitmap.allocPixels()) {
+            // indicate failure by zeroing our bitmap
+            fBitmap.setConfig(config, 0, 0, 0, isOpaque ?
+                              kOpaque_SkAlphaType : kPremul_SkAlphaType);
+        } else if (!isOpaque) {
+            fBitmap.eraseColor(SK_ColorTRANSPARENT);
+        }
     }
 }
 
+SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) {
+    this->init(config, width, height, isOpaque);
+}
+
 SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque,
                                const SkDeviceProperties& deviceProperties)
-    : SkBaseDevice(deviceProperties) {
-
-    fBitmap.setConfig(config, width, height, 0, isOpaque ?
-                      kOpaque_SkAlphaType : kPremul_SkAlphaType);
-    if (!fBitmap.allocPixels()) {
-        fBitmap.setConfig(config, 0, 0, 0, isOpaque ?
-                          kOpaque_SkAlphaType : kPremul_SkAlphaType);
-    }
-    if (!isOpaque) {
-        fBitmap.eraseColor(SK_ColorTRANSPARENT);
-    }
+    : SkBaseDevice(deviceProperties)
+{
+    this->init(config, width, height, isOpaque);
 }
 
 SkBitmapDevice::~SkBitmapDevice() {
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index c3e605c358..c86d33bf18 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -152,7 +152,7 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
     fRB = rowBytes;
     SkSafeRef(ctable);
 
-    this->setPreLocked(fStorage, fCTable);
+    this->setPreLocked(fStorage, fRB, fCTable);
 }
 
 SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
@@ -174,8 +174,8 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
     fCTable = ctable;
     fRB = rowBytes;
     SkSafeRef(ctable);
-
-    this->setPreLocked(fStorage, fCTable);
+    
+    this->setPreLocked(fStorage, fRB, fCTable);
 }
 
 
@@ -186,9 +186,11 @@ SkMallocPixelRef::~SkMallocPixelRef() {
     }
 }
 
-void* SkMallocPixelRef::onLockPixels(SkColorTable** ctable) {
-    *ctable = fCTable;
-    return fStorage;
+bool SkMallocPixelRef::onNewLockPixels(LockRec* rec) {
+    rec->fPixels = fStorage;
+    rec->fRowBytes = fRB;
+    rec->fColorTable = fCTable;
+    return true;
 }
 
 void SkMallocPixelRef::onUnlockPixels() {
@@ -234,5 +236,5 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer)
         fCTable = NULL;
     }
 
-    this->setPreLocked(fStorage, fCTable);
+    this->setPreLocked(fStorage, fRB, fCTable);
 }
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index fedbb5ac7d..7ec5929669 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -82,20 +82,19 @@ void SkPixelRef::setMutex(SkBaseMutex* mutex) {
 // just need a > 0 value, so pick a funny one to aid in debugging
 #define SKPIXELREF_PRELOCKED_LOCKCOUNT     123456789
 
-SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) {
-    this->setMutex(mutex);
-    fPixels = NULL;
-    fColorTable = NULL; // we do not track ownership of this
+SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) {
+    this->setMutex(NULL);
+    fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
     fPreLocked = false;
 }
 
-SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) {
-    this->setMutex(NULL);
-    fPixels = NULL;
-    fColorTable = NULL; // we do not track ownership of this
+
+SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) {
+    this->setMutex(mutex);
+    fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
@@ -113,8 +112,7 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
         , fInfo(read_info(buffer))
 {
     this->setMutex(mutex);
-    fPixels = NULL;
-    fColorTable = NULL; // we do not track ownership of this
+    fRec.zero();
     fLockCount = 0;
     fIsImmutable = buffer.readBool();
     fGenerationID = buffer.readUInt();
@@ -138,12 +136,13 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) {
     that.fUniqueGenerationID = false;
 }
 
-void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
+void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) {
 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
     // only call me in your constructor, otherwise fLockCount tracking can get
     // out of sync.
-    fPixels = pixels;
-    fColorTable = ctable;
+    fRec.fPixels = pixels;
+    fRec.fColorTable = ctable;
+    fRec.fRowBytes = rowBytes;
     fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
     fPreLocked = true;
 #endif
@@ -166,20 +165,30 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
     }
 }
 
-void SkPixelRef::lockPixels() {
+bool SkPixelRef::lockPixels(LockRec* rec) {
     SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
-
+    
     if (!fPreLocked) {
         SkAutoMutexAcquire  ac(*fMutex);
-
+        
         if (1 == ++fLockCount) {
-            fPixels = this->onLockPixels(&fColorTable);
-            // If onLockPixels failed, it will return NULL
-            if (NULL == fPixels) {
-                fColorTable = NULL;
+            SkASSERT(fRec.isZero());
+
+            LockRec rec;
+            if (!this->onNewLockPixels(&rec)) {
+                return false;
             }
+            SkASSERT(!rec.isZero());    // else why did onNewLock return true?
+            fRec = rec;
         }
     }
+    *rec = fRec;
+    return true;
+}
+
+bool SkPixelRef::lockPixels() {
+    LockRec rec;
+    return this->lockPixels(&rec);
 }
 
 void SkPixelRef::unlockPixels() {
@@ -191,12 +200,11 @@ void SkPixelRef::unlockPixels() {
         SkASSERT(fLockCount > 0);
         if (0 == --fLockCount) {
             // don't call onUnlockPixels unless onLockPixels succeeded
-            if (fPixels) {
+            if (fRec.fPixels) {
                 this->onUnlockPixels();
-                fPixels = NULL;
-                fColorTable = NULL;
+                fRec.zero();
             } else {
-                SkASSERT(NULL == fColorTable);
+                SkASSERT(fRec.isZero());
             }
         }
     }
@@ -278,6 +286,29 @@ size_t SkPixelRef::getAllocatedSizeInBytes() const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS
+
+void* SkPixelRef::onLockPixels(SkColorTable** ctable) {
+    return NULL;
+}
+
+bool SkPixelRef::onNewLockPixels(LockRec* rec) {
+    SkColorTable* ctable;
+    void* pixels = this->onLockPixels(&ctable);
+    if (!pixels) {
+        return false;
+    }
+
+    rec->fPixels = pixels;
+    rec->fColorTable = ctable;
+    rec->fRowBytes = 0; // callers don't currently need this (thank goodness)
+    return true;
+}
+
+#endif
+
+///////////////////////////////////////////////////////////////////////////////
+
 #ifdef SK_BUILD_FOR_ANDROID
 void SkPixelRef::globalRef(void* data) {
     this->ref();
diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp
index 45a5684638..5a772a7cd5 100644
--- a/src/core/SkScaledImageCache.cpp
+++ b/src/core/SkScaledImageCache.cpp
@@ -199,13 +199,11 @@ public:
     SK_DECLARE_UNFLATTENABLE_OBJECT()
 
 protected:
-    virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
     virtual void onUnlockPixels() SK_OVERRIDE;
     virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE;
 
 private:
-    SkImageInfo fInfo;  // remove when SkPixelRef gets this in baseclass
-
     SkDiscardableMemory* fDM;
     size_t               fRB;
     bool                 fFirstTime;
@@ -220,8 +218,6 @@ SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& in
     , fDM(dm)
     , fRB(rowBytes)
 {
-    fInfo = info;   // remove this redundant field when SkPixelRef has info
-
     SkASSERT(dm->data());
     fFirstTime = true;
 }
@@ -230,26 +226,31 @@ SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() {
     SkDELETE(fDM);
 }
 
-void* SkOneShotDiscardablePixelRef::onLockPixels(SkColorTable** ctable) {
+bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
     if (fFirstTime) {
         // we're already locked
         SkASSERT(fDM->data());
         fFirstTime = false;
-        return fDM->data();
+        goto SUCCESS;
     }
 
     // A previous call to onUnlock may have deleted our DM, so check for that
     if (NULL == fDM) {
-        return NULL;
+        return false;
     }
 
     if (!fDM->lock()) {
         // since it failed, we delete it now, to free-up the resource
         delete fDM;
         fDM = NULL;
-        return NULL;
+        return false;
     }
-    return fDM->data();
+
+SUCCESS:
+    rec->fPixels = fDM->data();
+    rec->fColorTable = NULL;
+    rec->fRowBytes = fRB;
+    return true;
 }
 
 void SkOneShotDiscardablePixelRef::onUnlockPixels() {
@@ -258,7 +259,7 @@ void SkOneShotDiscardablePixelRef::onUnlockPixels() {
 }
 
 size_t SkOneShotDiscardablePixelRef::getAllocatedSizeInBytes() const {
-    return fInfo.fHeight * fRB;
+    return this->info().getSafeSize(fRB);
 }
 
 class SkScaledImageCacheDiscardableAllocator : public SkBitmap::Allocator {
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index a068d8dcc1..8a16437780 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -23,18 +23,22 @@ SkROLockPixelsPixelRef::SkROLockPixelsPixelRef(const SkImageInfo& info)
 
 SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {}
 
-void* SkROLockPixelsPixelRef::onLockPixels(SkColorTable** ctable) {
-    if (ctable) {
-        *ctable = NULL;
-    }
+bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) {
     fBitmap.reset();
 //    SkDebugf("---------- calling readpixels in support of lockpixels\n");
     if (!this->onReadPixels(&fBitmap, NULL)) {
         SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n");
-        return NULL;
+        return false;
     }
     fBitmap.lockPixels();
-    return fBitmap.getPixels();
+    if (NULL == fBitmap.getPixels()) {
+        return false;
+    }
+
+    rec->fPixels = fBitmap.getPixels();
+    rec->fColorTable = NULL;
+    rec->fRowBytes = fBitmap.rowBytes();
+    return true;
 }
 
 void SkROLockPixelsPixelRef::onUnlockPixels() {
diff --git a/src/images/SkImageRef.cpp b/src/images/SkImageRef.cpp
index 843f4c01f9..2c1ec38116 100644
--- a/src/images/SkImageRef.cpp
+++ b/src/images/SkImageRef.cpp
@@ -15,12 +15,12 @@
 
 //#define DUMP_IMAGEREF_LIFECYCLE
 
-
 ///////////////////////////////////////////////////////////////////////////////
 
 SkImageRef::SkImageRef(const SkImageInfo& info, SkStreamRewindable* stream,
                        int sampleSize, SkBaseMutex* mutex)
-        : SkPixelRef(info, mutex), fErrorInDecoding(false) {
+        : INHERITED(info, mutex), fErrorInDecoding(false)
+{
     SkASSERT(stream);
     stream->ref();
     fStream = stream;
@@ -39,7 +39,7 @@ SkImageRef::~SkImageRef() {
 
 #ifdef DUMP_IMAGEREF_LIFECYCLE
     SkDebugf("delete ImageRef %p [%d] data=%d\n",
-              this, fConfig, (int)fStream->getLength());
+              this, this->info().fColorType, (int)fStream->getLength());
 #endif
 
     fStream->unref();
@@ -134,15 +134,18 @@ bool SkImageRef::prepareBitmap(SkImageDecoder::Mode mode) {
     return false;
 }
 
-void* SkImageRef::onLockPixels(SkColorTable** ct) {
+bool SkImageRef::onNewLockPixels(LockRec* rec) {
     if (NULL == fBitmap.getPixels()) {
         (void)this->prepareBitmap(SkImageDecoder::kDecodePixels_Mode);
     }
 
-    if (ct) {
-        *ct = fBitmap.getColorTable();
+    if (NULL == fBitmap.getPixels()) {
+        return false;
     }
-    return fBitmap.getPixels();
+    rec->fPixels = fBitmap.getPixels();
+    rec->fColorTable = NULL;
+    rec->fRowBytes = fBitmap.rowBytes();
+    return true;
 }
 
 size_t SkImageRef::ramUsed() const {
diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp
index fb30d051e1..033a6b5ebb 100644
--- a/src/lazy/SkCachingPixelRef.cpp
+++ b/src/lazy/SkCachingPixelRef.cpp
@@ -8,7 +8,6 @@
 #include "SkCachingPixelRef.h"
 #include "SkScaledImageCache.h"
 
-
 bool SkCachingPixelRef::Install(SkImageGenerator* generator,
                                 SkBitmap* dst) {
     SkImageInfo info;
@@ -41,12 +40,12 @@ SkCachingPixelRef::~SkCachingPixelRef() {
     // Assert always unlock before unref.
 }
 
-void* SkCachingPixelRef::onLockPixels(SkColorTable**) {
-    const SkImageInfo& info = this->info();
-
+bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) {
     if (fErrorInDecoding) {
-        return NULL;  // don't try again.
+        return false;  // don't try again.
     }
+    
+    const SkImageInfo& info = this->info();
     SkBitmap bitmap;
     SkASSERT(NULL == fScaledCacheId);
     fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(),
@@ -57,12 +56,12 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) {
         // Cache has been purged, must re-decode.
         if ((!bitmap.setConfig(info, fRowBytes)) || !bitmap.allocPixels()) {
             fErrorInDecoding = true;
-            return NULL;
+            return false;
         }
         SkAutoLockPixels autoLockPixels(bitmap);
         if (!fImageGenerator->getPixels(info, bitmap.getPixels(), fRowBytes)) {
             fErrorInDecoding = true;
-            return NULL;
+            return false;
         }
         fScaledCacheId = SkScaledImageCache::AddAndLock(this->getGenerationID(),
                                                         info.fWidth,
@@ -76,6 +75,7 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) {
     SkAutoLockPixels autoLockPixels(bitmap);
     void* pixels = bitmap.getPixels();
     SkASSERT(pixels != NULL);
+    
     // At this point, the autoLockPixels will unlockPixels()
     // to remove bitmap's lock on the pixels.  We will then
     // destroy bitmap.  The *only* guarantee that this pointer
@@ -84,7 +84,10 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) {
     // bitmap (SkScaledImageCache::Rec.fBitmap) that holds a
     // reference to the concrete PixelRef while this record is
     // locked.
-    return pixels;
+    rec->fPixels = pixels;
+    rec->fColorTable = NULL;
+    rec->fRowBytes = bitmap.rowBytes();
+    return true;
 }
 
 void SkCachingPixelRef::onUnlockPixels() {
diff --git a/src/lazy/SkCachingPixelRef.h b/src/lazy/SkCachingPixelRef.h
index b1f2fcd669..905ee9bf0d 100644
--- a/src/lazy/SkCachingPixelRef.h
+++ b/src/lazy/SkCachingPixelRef.h
@@ -40,7 +40,7 @@ public:
 
 protected:
     virtual ~SkCachingPixelRef();
-    virtual void* onLockPixels(SkColorTable** colorTable) SK_OVERRIDE;
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
     virtual void onUnlockPixels() SK_OVERRIDE;
     virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; }
 
diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp
index 2886156102..3f960d3c47 100644
--- a/src/lazy/SkDiscardablePixelRef.cpp
+++ b/src/lazy/SkDiscardablePixelRef.cpp
@@ -36,15 +36,18 @@ SkDiscardablePixelRef::~SkDiscardablePixelRef() {
     SkDELETE(fGenerator);
 }
 
-void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) {
+bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
     if (fDiscardableMemory != NULL) {
         if (fDiscardableMemory->lock()) {
-            return fDiscardableMemory->data();
+            rec->fPixels = fDiscardableMemory->data();
+            rec->fColorTable = NULL;
+            rec->fRowBytes = fRowBytes;
+            return true;
         }
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
     }
-
+    
     const size_t size = this->info().getSafeSize(fRowBytes);
 
     if (fDMFactory != NULL) {
@@ -53,17 +56,23 @@ void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) {
         fDiscardableMemory = SkDiscardableMemory::Create(size);
     }
     if (NULL == fDiscardableMemory) {
-        return NULL;  // Memory allocation failed.
+        return false;  // Memory allocation failed.
     }
+
     void* pixels = fDiscardableMemory->data();
     if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) {
         fDiscardableMemory->unlock();
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
-        return NULL;
+        return false;
     }
-    return pixels;
+
+    rec->fPixels = pixels;
+    rec->fColorTable = NULL;
+    rec->fRowBytes = fRowBytes;
+    return true;
 }
+
 void SkDiscardablePixelRef::onUnlockPixels() {
     fDiscardableMemory->unlock();
 }
diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h
index 3367096c26..4a013fda03 100644
--- a/src/lazy/SkDiscardablePixelRef.h
+++ b/src/lazy/SkDiscardablePixelRef.h
@@ -28,7 +28,8 @@ public:
 
 protected:
     ~SkDiscardablePixelRef();
-    virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
+
+    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
     virtual void onUnlockPixels() SK_OVERRIDE;
     virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; }
 
@@ -49,9 +50,12 @@ private:
     SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*,
                           size_t rowBytes,
                           SkDiscardableMemory::Factory* factory);
+
     friend bool SkInstallDiscardablePixelRef(SkImageGenerator*,
                                              SkBitmap*,
                                              SkDiscardableMemory::Factory*);
+
     typedef SkPixelRef INHERITED;
 };
+
 #endif  // SkDiscardablePixelRef_DEFINED