aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>2013-12-12 16:27:12 +0000
committerGravatar reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>2013-12-12 16:27:12 +0000
commit772d8524cef44f973abde368a69f8d17b55c6d74 (patch)
treecb16ab90ff6f74885ec858e61d74bff749bf609b
parent728f2a62526c8643ba5d234e6f570a4ebd06271a (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.h13
-rw-r--r--src/core/SkPixelRef.cpp15
-rw-r--r--src/core/SkScaledImageCache.cpp13
-rw-r--r--src/images/SkImageRef_ashmem.cpp2
-rw-r--r--src/lazy/SkCachingPixelRef.cpp8
-rw-r--r--src/lazy/SkDiscardablePixelRef.cpp4
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,