diff options
author | reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-12-12 16:27:12 +0000 |
---|---|---|
committer | reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2013-12-12 16:27:12 +0000 |
commit | 772d8524cef44f973abde368a69f8d17b55c6d74 (patch) | |
tree | cb16ab90ff6f74885ec858e61d74bff749bf609b | |
parent | 728f2a62526c8643ba5d234e6f570a4ebd06271a (diff) |
ensure that we call onUnlock only when we onLock succeeded
BUG=
R=halcanary@google.com, scroggo@google.com
Review URL: https://codereview.chromium.org/112783004
git-svn-id: http://skia.googlecode.com/svn/trunk@12642 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | include/core/SkPixelRef.h | 13 | ||||
-rw-r--r-- | src/core/SkPixelRef.cpp | 15 | ||||
-rw-r--r-- | src/core/SkScaledImageCache.cpp | 13 | ||||
-rw-r--r-- | src/images/SkImageRef_ashmem.cpp | 2 | ||||
-rw-r--r-- | src/lazy/SkCachingPixelRef.cpp | 8 | ||||
-rw-r--r-- | src/lazy/SkDiscardablePixelRef.cpp | 4 |
6 files changed, 29 insertions, 26 deletions
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index ec4937bea0..d4c35323bb 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -244,10 +244,15 @@ protected: acquire a mutex for thread safety, so this method need not do that. */ virtual void* onLockPixels(SkColorTable**) = 0; - /** 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. - */ + + /** + * 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. + * + * If the previous call to onLockPixels failed (i.e. returned NULL), then + * the onUnlockPixels will NOT be called. + */ virtual void onUnlockPixels() = 0; /** Default impl returns true */ diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index e93882a465..da88ca5c71 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -182,6 +182,10 @@ void SkPixelRef::lockPixels() { if (1 == ++fLockCount) { fPixels = this->onLockPixels(&fColorTable); + // If onLockPixels failed, it will return NULL + if (NULL == fPixels) { + fColorTable = NULL; + } } } } @@ -194,9 +198,14 @@ void SkPixelRef::unlockPixels() { SkASSERT(fLockCount > 0); if (0 == --fLockCount) { - this->onUnlockPixels(); - fPixels = NULL; - fColorTable = NULL; + // don't call onUnlockPixels unless onLockPixels succeeded + if (fPixels) { + this->onUnlockPixels(); + fPixels = NULL; + fColorTable = NULL; + } else { + SkASSERT(NULL == fColorTable); + } } } } diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp index b3956f4a8d..1b8ad52462 100644 --- a/src/core/SkScaledImageCache.cpp +++ b/src/core/SkScaledImageCache.cpp @@ -209,7 +209,6 @@ private: SkDiscardableMemory* fDM; size_t fRB; bool fFirstTime; - bool fIsLocked; typedef SkPixelRef INHERITED; }; @@ -225,7 +224,6 @@ SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& in SkASSERT(dm->data()); fFirstTime = true; - fIsLocked = false; } SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() { @@ -235,21 +233,16 @@ SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() { void* SkOneShotDiscardablePixelRef::onLockPixels(SkColorTable** ctable) { if (fFirstTime) { // we're already locked + SkASSERT(fDM->data()); fFirstTime = false; return fDM->data(); } - - SkASSERT(!fIsLocked); - fIsLocked = fDM->lock(); - return fIsLocked ? fDM->data() : NULL; + return fDM->lock() ? fDM->data() : NULL; } void SkOneShotDiscardablePixelRef::onUnlockPixels() { SkASSERT(!fFirstTime); - if (fIsLocked) { - fIsLocked = false; - fDM->unlock(); - } + fDM->unlock(); } size_t SkOneShotDiscardablePixelRef::getAllocatedSizeInBytes() const { diff --git a/src/images/SkImageRef_ashmem.cpp b/src/images/SkImageRef_ashmem.cpp index 0dba1d1191..9933ca9b4b 100644 --- a/src/images/SkImageRef_ashmem.cpp +++ b/src/images/SkImageRef_ashmem.cpp @@ -1,10 +1,10 @@ - /* * Copyright 2011 Google Inc. * * Use of this source code is governed by a BSD-style license that can be * found in the LICENSE file. */ + #include "SkImageRef_ashmem.h" #include "SkImageDecoder.h" #include "SkFlattenableBuffers.h" diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp index 667a94931b..b7eaf574aa 100644 --- a/src/lazy/SkCachingPixelRef.cpp +++ b/src/lazy/SkCachingPixelRef.cpp @@ -90,9 +90,7 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable** colorTable) { } void SkCachingPixelRef::onUnlockPixels() { - if (fScaledCacheId != NULL) { - SkScaledImageCache::Unlock( - static_cast<SkScaledImageCache::ID*>(fScaledCacheId)); - fScaledCacheId = NULL; - } + SkASSERT(fScaledCacheId != NULL); + SkScaledImageCache::Unlock( static_cast<SkScaledImageCache::ID*>(fScaledCacheId)); + fScaledCacheId = NULL; } diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp index 6a9507c8c7..2528a994d1 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -58,9 +58,7 @@ void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) { return pixels; } void SkDiscardablePixelRef::onUnlockPixels() { - if (fDiscardableMemory != NULL) { - fDiscardableMemory->unlock(); - } + fDiscardableMemory->unlock(); } bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, |