Revert "Revert "Revert of https://codereview.chromium.org/110593003/""

This reverts commit 0fef787f33aa38109a0c8427e0098d997efdd5ff.

failed in chrome: https://codereview.chromium.org/124503002/

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

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

12 files changed, 78 insertions, 129 deletions

diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 0570fd9afd..953956abfc 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -27,7 +27,7 @@ SkBitmapDevice::SkBitmapDevice(const SkBitmap& bitmap, const SkDeviceProperties&
 void SkBitmapDevice::init(SkBitmap::Config config, int width, int height, bool isOpaque) {
     fBitmap.setConfig(config, width, height, 0, isOpaque ?
                       kOpaque_SkAlphaType : kPremul_SkAlphaType);
-
+    
     if (SkBitmap::kNo_Config != config) {
         if (!fBitmap.allocPixels()) {
             // indicate failure by zeroing our bitmap
@@ -45,7 +45,7 @@ SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, b
 
 SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque,
                                const SkDeviceProperties& deviceProperties)
-    : SkBaseDevice(deviceProperties)
+: SkBaseDevice(deviceProperties)
 {
     this->init(config, width, height, isOpaque);
 }
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 53042d263d..c3e605c358 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, fRB, fCTable);
+    this->setPreLocked(fStorage, fCTable);
 }
 
 SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
@@ -175,7 +175,7 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage,
     fRB = rowBytes;
     SkSafeRef(ctable);
 
-    this->setPreLocked(fStorage, fRB, fCTable);
+    this->setPreLocked(fStorage, fCTable);
 }
 
 
@@ -186,11 +186,9 @@ SkMallocPixelRef::~SkMallocPixelRef() {
     }
 }
 
-bool SkMallocPixelRef::onNewLockPixels(LockRec* rec) {
-    rec->fPixels = fStorage;
-    rec->fRowBytes = fRB;
-    rec->fColorTable = fCTable;
-    return true;
+void* SkMallocPixelRef::onLockPixels(SkColorTable** ctable) {
+    *ctable = fCTable;
+    return fStorage;
 }
 
 void SkMallocPixelRef::onUnlockPixels() {
@@ -236,5 +234,5 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer)
         fCTable = NULL;
     }
 
-    this->setPreLocked(fStorage, fRB, fCTable);
+    this->setPreLocked(fStorage, fCTable);
 }
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 422d5039fe..fedbb5ac7d 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -82,19 +82,20 @@ 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) : fInfo(info) {
-    this->setMutex(NULL);
-    fRec.zero();
+SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) {
+    this->setMutex(mutex);
+    fPixels = NULL;
+    fColorTable = NULL; // we do not track ownership of this
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
     fPreLocked = false;
 }
 
-
-SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) {
-    this->setMutex(mutex);
-    fRec.zero();
+SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) {
+    this->setMutex(NULL);
+    fPixels = NULL;
+    fColorTable = NULL; // we do not track ownership of this
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
@@ -112,7 +113,8 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
         , fInfo(read_info(buffer))
 {
     this->setMutex(mutex);
-    fRec.zero();
+    fPixels = NULL;
+    fColorTable = NULL; // we do not track ownership of this
     fLockCount = 0;
     fIsImmutable = buffer.readBool();
     fGenerationID = buffer.readUInt();
@@ -136,13 +138,12 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) {
     that.fUniqueGenerationID = false;
 }
 
-void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) {
+void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
     // only call me in your constructor, otherwise fLockCount tracking can get
     // out of sync.
-    fRec.fPixels = pixels;
-    fRec.fColorTable = ctable;
-    fRec.fRowBytes = rowBytes;
+    fPixels = pixels;
+    fColorTable = ctable;
     fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
     fPreLocked = true;
 #endif
@@ -165,30 +166,20 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
     }
 }
 
-bool SkPixelRef::lockPixels(LockRec* rec) {
+void SkPixelRef::lockPixels() {
     SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
 
     if (!fPreLocked) {
         SkAutoMutexAcquire  ac(*fMutex);
 
         if (1 == ++fLockCount) {
-            SkASSERT(fRec.isZero());
-
-            LockRec rec;
-            if (!this->onNewLockPixels(&rec)) {
-                return false;
+            fPixels = this->onLockPixels(&fColorTable);
+            // If onLockPixels failed, it will return NULL
+            if (NULL == fPixels) {
+                fColorTable = NULL;
             }
-            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() {
@@ -200,11 +191,12 @@ void SkPixelRef::unlockPixels() {
         SkASSERT(fLockCount > 0);
         if (0 == --fLockCount) {
             // don't call onUnlockPixels unless onLockPixels succeeded
-            if (fRec.fPixels) {
+            if (fPixels) {
                 this->onUnlockPixels();
-                fRec.zero();
+                fPixels = NULL;
+                fColorTable = NULL;
             } else {
-                SkASSERT(fRec.isZero());
+                SkASSERT(NULL == fColorTable);
             }
         }
     }
@@ -286,29 +278,6 @@ 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 5a772a7cd5..45a5684638 100644
--- a/src/core/SkScaledImageCache.cpp
+++ b/src/core/SkScaledImageCache.cpp
@@ -199,11 +199,13 @@ public:
     SK_DECLARE_UNFLATTENABLE_OBJECT()
 
 protected:
-    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
+    virtual void* onLockPixels(SkColorTable**) 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;
@@ -218,6 +220,8 @@ SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& in
     , fDM(dm)
     , fRB(rowBytes)
 {
+    fInfo = info;   // remove this redundant field when SkPixelRef has info
+
     SkASSERT(dm->data());
     fFirstTime = true;
 }
@@ -226,31 +230,26 @@ SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() {
     SkDELETE(fDM);
 }
 
-bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
+void* SkOneShotDiscardablePixelRef::onLockPixels(SkColorTable** ctable) {
     if (fFirstTime) {
         // we're already locked
         SkASSERT(fDM->data());
         fFirstTime = false;
-        goto SUCCESS;
+        return fDM->data();
     }
 
     // A previous call to onUnlock may have deleted our DM, so check for that
     if (NULL == fDM) {
-        return false;
+        return NULL;
     }
 
     if (!fDM->lock()) {
         // since it failed, we delete it now, to free-up the resource
         delete fDM;
         fDM = NULL;
-        return false;
+        return NULL;
     }
-
-SUCCESS:
-    rec->fPixels = fDM->data();
-    rec->fColorTable = NULL;
-    rec->fRowBytes = fRB;
-    return true;
+    return fDM->data();
 }
 
 void SkOneShotDiscardablePixelRef::onUnlockPixels() {
@@ -259,7 +258,7 @@ void SkOneShotDiscardablePixelRef::onUnlockPixels() {
 }
 
 size_t SkOneShotDiscardablePixelRef::getAllocatedSizeInBytes() const {
-    return this->info().getSafeSize(fRB);
+    return fInfo.fHeight * fRB;
 }
 
 class SkScaledImageCacheDiscardableAllocator : public SkBitmap::Allocator {
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index 8a16437780..a068d8dcc1 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -23,22 +23,18 @@ SkROLockPixelsPixelRef::SkROLockPixelsPixelRef(const SkImageInfo& info)
 
 SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {}
 
-bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) {
+void* SkROLockPixelsPixelRef::onLockPixels(SkColorTable** ctable) {
+    if (ctable) {
+        *ctable = NULL;
+    }
     fBitmap.reset();
 //    SkDebugf("---------- calling readpixels in support of lockpixels\n");
     if (!this->onReadPixels(&fBitmap, NULL)) {
         SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n");
-        return false;
+        return NULL;
     }
     fBitmap.lockPixels();
-    if (NULL == fBitmap.getPixels()) {
-        return false;
-    }
-
-    rec->fPixels = fBitmap.getPixels();
-    rec->fColorTable = NULL;
-    rec->fRowBytes = fBitmap.rowBytes();
-    return true;
+    return fBitmap.getPixels();
 }
 
 void SkROLockPixelsPixelRef::onUnlockPixels() {
diff --git a/src/images/SkImageRef.cpp b/src/images/SkImageRef.cpp
index 2c1ec38116..843f4c01f9 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)
-        : INHERITED(info, mutex), fErrorInDecoding(false)
-{
+        : SkPixelRef(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, this->info().fColorType, (int)fStream->getLength());
+              this, fConfig, (int)fStream->getLength());
 #endif
 
     fStream->unref();
@@ -134,18 +134,15 @@ bool SkImageRef::prepareBitmap(SkImageDecoder::Mode mode) {
     return false;
 }
 
-bool SkImageRef::onNewLockPixels(LockRec* rec) {
+void* SkImageRef::onLockPixels(SkColorTable** ct) {
     if (NULL == fBitmap.getPixels()) {
         (void)this->prepareBitmap(SkImageDecoder::kDecodePixels_Mode);
     }
 
-    if (NULL == fBitmap.getPixels()) {
-        return false;
+    if (ct) {
+        *ct = fBitmap.getColorTable();
     }
-    rec->fPixels = fBitmap.getPixels();
-    rec->fColorTable = NULL;
-    rec->fRowBytes = fBitmap.rowBytes();
-    return true;
+    return fBitmap.getPixels();
 }
 
 size_t SkImageRef::ramUsed() const {
diff --git a/src/images/SkImageRef_ashmem.cpp b/src/images/SkImageRef_ashmem.cpp
index 14dedf8bd3..269199faf8 100644
--- a/src/images/SkImageRef_ashmem.cpp
+++ b/src/images/SkImageRef_ashmem.cpp
@@ -159,7 +159,7 @@ bool SkImageRef_ashmem::onDecode(SkImageDecoder* codec, SkStreamRewindable* stre
     }
 }
 
-bool SkImageRef_ashmem::onNewLockPixels(LockRec* rec) {
+void* SkImageRef_ashmem::onLockPixels(SkColorTable** ct) {
     SkASSERT(fBitmap.getPixels() == NULL);
     SkASSERT(fBitmap.getColorTable() == NULL);
 
@@ -185,13 +185,17 @@ bool SkImageRef_ashmem::onNewLockPixels(LockRec* rec) {
 #endif
         } else {
             SkDebugf("===== ashmem pin_region(%d) returned %d\n", fRec.fFD, pin);
-            return false;
+            // return null result for failure
+            if (ct) {
+                *ct = NULL;
+            }
+            return NULL;
         }
     } else {
         // no FD, will create an ashmem region in allocator
     }
 
-    return this->INHERITED::onNewLockPixels(rec);
+    return this->INHERITED::onLockPixels(ct);
 }
 
 void SkImageRef_ashmem::onUnlockPixels() {
diff --git a/src/images/SkImageRef_ashmem.h b/src/images/SkImageRef_ashmem.h
index fa30baf99c..a2652fbc30 100644
--- a/src/images/SkImageRef_ashmem.h
+++ b/src/images/SkImageRef_ashmem.h
@@ -32,8 +32,8 @@ protected:
                           SkBitmap* bitmap, SkBitmap::Config config,
                           SkImageDecoder::Mode mode);
 
-    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
-    virtual void onUnlockPixels() SK_OVERRIDE;
+    virtual void* onLockPixels(SkColorTable**);
+    virtual void onUnlockPixels();
 
 private:
     void closeFD();
diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp
index f1510fb67c..452ea4ed03 100644
--- a/src/lazy/SkCachingPixelRef.cpp
+++ b/src/lazy/SkCachingPixelRef.cpp
@@ -8,6 +8,7 @@
 #include "SkCachingPixelRef.h"
 #include "SkScaledImageCache.h"
 
+
 bool SkCachingPixelRef::Install(SkImageGenerator* generator,
                                 SkBitmap* dst) {
     SkImageInfo info;
@@ -40,12 +41,13 @@ SkCachingPixelRef::~SkCachingPixelRef() {
     // Assert always unlock before unref.
 }
 
-bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) {
+void* SkCachingPixelRef::onLockPixels(SkColorTable**) {
     if (fErrorInDecoding) {
-        return false;  // don't try again.
+        return NULL;  // don't try again.
     }
-
+    
     const SkImageInfo& info = this->info();
+
     SkBitmap bitmap;
     SkASSERT(NULL == fScaledCacheId);
     fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(),
@@ -56,12 +58,12 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) {
         // Cache has been purged, must re-decode.
         if ((!bitmap.setConfig(info, fRowBytes)) || !bitmap.allocPixels()) {
             fErrorInDecoding = true;
-            return false;
+            return NULL;
         }
         SkAutoLockPixels autoLockPixels(bitmap);
         if (!fImageGenerator->getPixels(info, bitmap.getPixels(), fRowBytes)) {
             fErrorInDecoding = true;
-            return false;
+            return NULL;
         }
         fScaledCacheId = SkScaledImageCache::AddAndLock(this->getGenerationID(),
                                                         info.fWidth,
@@ -84,10 +86,7 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) {
     // bitmap (SkScaledImageCache::Rec.fBitmap) that holds a
     // reference to the concrete PixelRef while this record is
     // locked.
-    rec->fPixels = pixels;
-    rec->fColorTable = NULL;
-    rec->fRowBytes = bitmap.rowBytes();
-    return true;
+    return pixels;
 }
 
 void SkCachingPixelRef::onUnlockPixels() {
diff --git a/src/lazy/SkCachingPixelRef.h b/src/lazy/SkCachingPixelRef.h
index 905ee9bf0d..b1f2fcd669 100644
--- a/src/lazy/SkCachingPixelRef.h
+++ b/src/lazy/SkCachingPixelRef.h
@@ -40,7 +40,7 @@ public:
 
 protected:
     virtual ~SkCachingPixelRef();
-    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
+    virtual void* onLockPixels(SkColorTable** colorTable) 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 abd80f2e0a..2886156102 100644
--- a/src/lazy/SkDiscardablePixelRef.cpp
+++ b/src/lazy/SkDiscardablePixelRef.cpp
@@ -36,13 +36,10 @@ SkDiscardablePixelRef::~SkDiscardablePixelRef() {
     SkDELETE(fGenerator);
 }
 
-bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
+void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) {
     if (fDiscardableMemory != NULL) {
         if (fDiscardableMemory->lock()) {
-            rec->fPixels = fDiscardableMemory->data();
-            rec->fColorTable = NULL;
-            rec->fRowBytes = fRowBytes;
-            return true;
+            return fDiscardableMemory->data();
         }
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
@@ -56,23 +53,17 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
         fDiscardableMemory = SkDiscardableMemory::Create(size);
     }
     if (NULL == fDiscardableMemory) {
-        return false;  // Memory allocation failed.
+        return NULL;  // Memory allocation failed.
     }
-
     void* pixels = fDiscardableMemory->data();
     if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) {
         fDiscardableMemory->unlock();
         SkDELETE(fDiscardableMemory);
         fDiscardableMemory = NULL;
-        return false;
+        return NULL;
     }
-
-    rec->fPixels = pixels;
-    rec->fColorTable = NULL;
-    rec->fRowBytes = fRowBytes;
-    return true;
+    return pixels;
 }
-
 void SkDiscardablePixelRef::onUnlockPixels() {
     fDiscardableMemory->unlock();
 }
diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h
index 4a013fda03..3367096c26 100644
--- a/src/lazy/SkDiscardablePixelRef.h
+++ b/src/lazy/SkDiscardablePixelRef.h
@@ -28,8 +28,7 @@ public:
 
 protected:
     ~SkDiscardablePixelRef();
-
-    virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE;
+    virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
     virtual void onUnlockPixels() SK_OVERRIDE;
     virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; }
 
@@ -50,12 +49,9 @@ private:
     SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*,
                           size_t rowBytes,
                           SkDiscardableMemory::Factory* factory);
-
     friend bool SkInstallDiscardablePixelRef(SkImageGenerator*,
                                              SkBitmap*,
                                              SkDiscardableMemory::Factory*);
-
     typedef SkPixelRef INHERITED;
 };
-
 #endif  // SkDiscardablePixelRef_DEFINED