If Ashmem cache fails pinCache, do not reallocate.

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

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

4 files changed, 29 insertions, 19 deletions

diff --git a/include/lazy/SkImageCache.h b/include/lazy/SkImageCache.h
index 045ce2c384..6cd064ba5b 100644
--- a/include/lazy/SkImageCache.h
+++ b/include/lazy/SkImageCache.h
@@ -34,15 +34,16 @@ public:
      *  @param ID Unique ID for the memory block.
      *  @return Pointer: If non-NULL, points to the previously allocated memory, in which case
      *          this call must be balanced with a call to releaseCache. If NULL, the memory
-     *          has been reclaimed, so allocAndPinCache must be called again, and ID is no
-     *          longer valid (thus throwAwayCache need not be called).
+     *          has been reclaimed, so allocAndPinCache must be called again with a pointer to
+     *          the same ID.
      */
     virtual void* pinCache(intptr_t ID) = 0;
 
     /**
      *  Inform the cache that it is safe to free the block of memory corresponding to ID. After
-     *  calling this function, the pointer returnted by allocAndPinCache or pinCache must not be
-     *  used again. In order to access the same memory after this, pinCache must be called.
+     *  calling this function, the pointer returned by allocAndPinCache or pinCache must not be
+     *  used again. In order to access the same memory after this, pinCache must be called with
+     *  the same ID.
      *  @param ID Unique ID for the memory block which is now safe to age out of the cache.
      */
     virtual void releaseCache(intptr_t ID) = 0;
@@ -50,8 +51,7 @@ public:
     /**
      *  Inform the cache that the block of memory associated with ID will not be asked for again.
      *  After this call, ID is no longer valid. Must not be called while the associated memory is
-     *  pinned. Must be called to balance a successful allocAndPinCache, unless a later pinCache
-     *  returns NULL.
+     *  pinned. Must be called to balance a successful allocAndPinCache.
      */
     virtual void throwAwayCache(intptr_t ID) = 0;
 
diff --git a/src/lazy/SkLazyPixelRef.cpp b/src/lazy/SkLazyPixelRef.cpp
index a20b3c028d..9ae22f2207 100644
--- a/src/lazy/SkLazyPixelRef.cpp
+++ b/src/lazy/SkLazyPixelRef.cpp
@@ -88,6 +88,8 @@ void* SkLazyPixelRef::onLockPixels(SkColorTable**) {
     // FIXME: As an optimization, only do this part once.
     fErrorInDecoding = !fDecodeProc(fData->data(), fData->size(), &info, NULL);
     if (fErrorInDecoding) {
+        // In case a previous call to allocAndPinCache succeeded.
+        fImageCache->throwAwayCache(fCacheId);
         fCacheId = SkImageCache::UNINITIALIZED_ID;
         return NULL;
     }
diff --git a/src/lazy/SkLruImageCache.cpp b/src/lazy/SkLruImageCache.cpp
index 6beb8a479b..54f26fb5dc 100644
--- a/src/lazy/SkLruImageCache.cpp
+++ b/src/lazy/SkLruImageCache.cpp
@@ -133,7 +133,6 @@ void SkLruImageCache::releaseCache(intptr_t ID) {
 }
 
 void SkLruImageCache::throwAwayCache(intptr_t ID) {
-    SkASSERT(ID != SkImageCache::UNINITIALIZED_ID);
     SkAutoMutexAcquire ac(&fMutex);
     CachedPixels* pixels = this->findByID(ID);
     if (pixels != NULL) {
@@ -156,6 +155,9 @@ void SkLruImageCache::removePixels(CachedPixels* pixels) {
 
 CachedPixels* SkLruImageCache::findByID(intptr_t ID) const {
     // Mutex is already locked.
+    if (SkImageCache::UNINITIALIZED_ID == ID) {
+        return NULL;
+    }
     Iter iter;
     // Start from the head, most recently used.
     CachedPixels* pixels = iter.init(fLRU, Iter::kHead_IterStart);
diff --git a/src/ports/SkAshmemImageCache.cpp b/src/ports/SkAshmemImageCache.cpp
index a85542271d..b7c6c7058b 100644
--- a/src/ports/SkAshmemImageCache.cpp
+++ b/src/ports/SkAshmemImageCache.cpp
@@ -41,11 +41,26 @@ static size_t roundToPageSize(size_t size) {
 }
 
 void* SkAshmemImageCache::allocAndPinCache(size_t bytes, intptr_t* ID) {
-    AshmemRec rec;
-    rec.fSize = roundToPageSize(bytes);
+    SkASSERT(ID != NULL);
 
     SkAutoMutexAcquire ac(&gAshmemMutex);
 
+    if (*ID != SkImageCache::UNINITIALIZED_ID) {
+        // This rec was previously allocated, but pinCache subsequently
+        // failed.
+        AshmemRec* pRec = reinterpret_cast<AshmemRec*>(*ID);
+        SkASSERT(roundToPageSize(bytes) == pRec->fSize);
+        SkASSERT(pRec->fFD != -1);
+        (void) ashmem_pin_region(pRec->fFD, 0, 0);
+#ifdef SK_DEBUG
+        pRec->fPinned = true;
+#endif
+        return pRec->fAddr;
+    }
+
+    AshmemRec rec;
+    rec.fSize = roundToPageSize(bytes);
+
     rec.fFD = ashmem_create_region(NULL, rec.fSize);
     if (-1 == rec.fFD) {
         SkDebugf("ashmem_create_region failed\n");
@@ -70,7 +85,6 @@ void* SkAshmemImageCache::allocAndPinCache(size_t bytes, intptr_t* ID) {
     // In release mode, we do not keep a pointer to this object. It will be destroyed
     // either when pinCache returns NULL or when throwAwayCache is called.
     AshmemRec* pRec = SkNEW_ARGS(AshmemRec, (rec));
-    SkASSERT(ID != NULL);
     *ID = reinterpret_cast<intptr_t>(pRec);
 #ifdef SK_DEBUG
     this->appendRec(pRec);
@@ -89,8 +103,6 @@ void* SkAshmemImageCache::pinCache(intptr_t ID) {
 #endif
         return rec->fAddr;
     }
-    // Purged. Remove the associated AshmemRec:
-    this->removeRec(rec);
     ashmem_unpin_region(fd, 0, 0);
     return NULL;
 }
@@ -107,16 +119,10 @@ void SkAshmemImageCache::releaseCache(intptr_t ID) {
 void SkAshmemImageCache::throwAwayCache(intptr_t ID) {
     SkAutoMutexAcquire ac(&gAshmemMutex);
     AshmemRec* rec = reinterpret_cast<AshmemRec*>(ID);
-#ifdef SK_DEBUG
-    SkASSERT(!rec->fPinned);
-#endif
-    this->removeRec(rec);
-}
-
-void SkAshmemImageCache::removeRec(SkAshmemImageCache::AshmemRec* rec) {
     munmap(rec->fAddr, rec->fSize);
     close(rec->fFD);
 #ifdef SK_DEBUG
+    SkASSERT(!rec->fPinned);
     int index = this->findRec(rec);
     SkASSERT(index >= 0);
     fRecs.remove(index);