diff options
author | reed <reed@google.com> | 2016-03-06 13:54:00 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-06 13:54:00 -0800 |
commit | 941da9d66171bd8efd2f6c5e25ff90c8c69885c1 (patch) | |
tree | a8ab867b53ab24493a2010cdd71224afaceeb791 | |
parent | 992854d62e179a589aa7366e443246e3672c3248 (diff) |
Fix behavior of sk_sp::reset(T*) and add unittest.
Previously, sk_sp::reset(T* t) did not release its own reference
if its internal pointer was the same as 't'. This leaks a reference.
Now always release the current reference when non-nullptr.
Review URL: https://codereview.chromium.org/1767983002
-rw-r--r-- | include/core/SkRefCnt.h | 6 | ||||
-rw-r--r-- | tests/RefCntTest.cpp | 18 |
2 files changed, 20 insertions, 4 deletions
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h index 9b05ad4969..d33117751e 100644 --- a/include/core/SkRefCnt.h +++ b/include/core/SkRefCnt.h @@ -338,10 +338,8 @@ public: * No call to ref() will be made. */ void reset(T* ptr = nullptr) { - if (fPtr != ptr) { - SkSafeUnref(fPtr); - fPtr = ptr; - } + SkSafeUnref(fPtr); + fPtr = ptr; } /** diff --git a/tests/RefCntTest.cpp b/tests/RefCntTest.cpp index 8914ccff49..f5345b6a54 100644 --- a/tests/RefCntTest.cpp +++ b/tests/RefCntTest.cpp @@ -290,3 +290,21 @@ static sk_sp<FooAbstract> make_foo() { DEF_TEST(sk_make_sp, r) { auto x = make_foo(); } + +// Test that reset() "adopts" ownership from the caller, even if we are given the same ptr twice +// +DEF_TEST(sk_sp_reset, r) { + SkRefCnt* rc = new SkRefCnt; + REPORTER_ASSERT(r, rc->unique()); + + sk_sp<SkRefCnt> sp; + sp.reset(rc); + // We have transfered our ownership over to sp + REPORTER_ASSERT(r, rc->unique()); + + rc->ref(); // now "rc" is also an owner + REPORTER_ASSERT(r, !rc->unique()); + + sp.reset(rc); // this should transfer our ownership over to sp + REPORTER_ASSERT(r, rc->unique()); +} |