From 4ba37c6f6964f6a24fc4b8b48bc82c02edb70370 Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 28 Sep 2016 14:49:01 -0700 Subject: Implement weak pointers These will be a replacement for CFX_CountRef in future CLs, since CFX_CountRef is manually incremented and error-prone. Review-Url: https://codereview.chromium.org/2377143002 --- BUILD.gn | 2 + core/fxcrt/cfx_weak_ptr_unittest.cpp | 174 +++++++++++++++++++++++++++++++++++ core/fxcrt/include/cfx_weak_ptr.h | 80 ++++++++++++++++ 3 files changed, 256 insertions(+) create mode 100644 core/fxcrt/cfx_weak_ptr_unittest.cpp create mode 100644 core/fxcrt/include/cfx_weak_ptr.h diff --git a/BUILD.gn b/BUILD.gn index 22f018eadd..778e03829c 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -710,6 +710,7 @@ static_library("fxcrt") { "core/fxcrt/include/cfx_observable.h", "core/fxcrt/include/cfx_retain_ptr.h", "core/fxcrt/include/cfx_string_pool_template.h", + "core/fxcrt/include/cfx_weak_ptr.h", "core/fxcrt/include/fx_basic.h", "core/fxcrt/include/fx_coordinates.h", "core/fxcrt/include/fx_ext.h", @@ -1656,6 +1657,7 @@ test("pdfium_unittests") { "core/fxcrt/cfx_observable_unittest.cpp", "core/fxcrt/cfx_retain_ptr_unittest.cpp", "core/fxcrt/cfx_string_pool_template_unittest.cpp", + "core/fxcrt/cfx_weak_ptr_unittest.cpp", "core/fxcrt/fx_basic_bstring_unittest.cpp", "core/fxcrt/fx_basic_gcc_unittest.cpp", "core/fxcrt/fx_basic_memmgr_unittest.cpp", diff --git a/core/fxcrt/cfx_weak_ptr_unittest.cpp b/core/fxcrt/cfx_weak_ptr_unittest.cpp new file mode 100644 index 0000000000..c4fa514bef --- /dev/null +++ b/core/fxcrt/cfx_weak_ptr_unittest.cpp @@ -0,0 +1,174 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/include/cfx_weak_ptr.h" + +#include +#include + +#include "core/fxcrt/include/fx_memory.h" +#include "testing/fx_string_testhelpers.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class PseudoDeletable; +using WeakPtr = CFX_WeakPtr>; +using UniquePtr = + std::unique_ptr>; + +class PseudoDeletable { + public: + PseudoDeletable() : delete_count_(0) {} + void Release() { + ++delete_count_; + next_.Reset(); + } + void SetNext(const WeakPtr& next) { next_ = next; } + int delete_count() const { return delete_count_; } + + private: + int delete_count_; + WeakPtr next_; +}; + +} // namespace + +TEST(fxcrt, WeakPtrNull) { + WeakPtr ptr1; + EXPECT_FALSE(ptr1); + + WeakPtr ptr2; + EXPECT_TRUE(ptr1 == ptr2); + EXPECT_FALSE(ptr1 != ptr2); + + WeakPtr ptr3(ptr1); + EXPECT_TRUE(ptr1 == ptr3); + EXPECT_FALSE(ptr1 != ptr3); + + WeakPtr ptr4 = ptr1; + EXPECT_TRUE(ptr1 == ptr4); + EXPECT_FALSE(ptr1 != ptr4); +} + +TEST(fxcrt, WeakPtrNonNull) { + PseudoDeletable thing; + EXPECT_EQ(0, thing.delete_count()); + { + UniquePtr unique(&thing); + WeakPtr ptr1(std::move(unique)); + EXPECT_TRUE(ptr1); + EXPECT_EQ(&thing, ptr1.Get()); + + WeakPtr ptr2; + EXPECT_FALSE(ptr1 == ptr2); + EXPECT_TRUE(ptr1 != ptr2); + { + WeakPtr ptr3(ptr1); + EXPECT_TRUE(ptr1 == ptr3); + EXPECT_FALSE(ptr1 != ptr3); + EXPECT_EQ(&thing, ptr3.Get()); + { + WeakPtr ptr4 = ptr1; + EXPECT_TRUE(ptr1 == ptr4); + EXPECT_FALSE(ptr1 != ptr4); + EXPECT_EQ(&thing, ptr4.Get()); + } + } + EXPECT_EQ(0, thing.delete_count()); + } + EXPECT_EQ(1, thing.delete_count()); +} + +TEST(fxcrt, WeakPtrResetNull) { + PseudoDeletable thing; + { + UniquePtr unique(&thing); + WeakPtr ptr1(std::move(unique)); + WeakPtr ptr2 = ptr1; + ptr1.Reset(); + EXPECT_FALSE(ptr1); + EXPECT_EQ(nullptr, ptr1.Get()); + EXPECT_TRUE(ptr2); + EXPECT_EQ(&thing, ptr2.Get()); + EXPECT_FALSE(ptr1 == ptr2); + EXPECT_TRUE(ptr1 != ptr2); + EXPECT_EQ(0, thing.delete_count()); + } + EXPECT_EQ(1, thing.delete_count()); +} + +TEST(fxcrt, WeakPtrResetNonNull) { + PseudoDeletable thing1; + PseudoDeletable thing2; + { + UniquePtr unique1(&thing1); + WeakPtr ptr1(std::move(unique1)); + WeakPtr ptr2 = ptr1; + UniquePtr unique2(&thing2); + ptr2.Reset(std::move(unique2)); + EXPECT_TRUE(ptr1); + EXPECT_EQ(&thing1, ptr1.Get()); + EXPECT_TRUE(ptr2); + EXPECT_EQ(&thing2, ptr2.Get()); + EXPECT_FALSE(ptr1 == ptr2); + EXPECT_TRUE(ptr1 != ptr2); + EXPECT_EQ(0, thing1.delete_count()); + EXPECT_EQ(0, thing2.delete_count()); + } + EXPECT_EQ(1, thing1.delete_count()); + EXPECT_EQ(1, thing2.delete_count()); +} + +TEST(fxcrt, WeakPtrClear) { + PseudoDeletable thing; + { + UniquePtr unique(&thing); + WeakPtr ptr1(std::move(unique)); + WeakPtr ptr2 = ptr1; + ptr1.Clear(); + EXPECT_FALSE(ptr1); + EXPECT_EQ(nullptr, ptr1.Get()); + EXPECT_FALSE(ptr2); + EXPECT_EQ(nullptr, ptr2.Get()); + EXPECT_FALSE(ptr1 == ptr2); + EXPECT_TRUE(ptr1 != ptr2); + EXPECT_EQ(1, thing.delete_count()); + } + EXPECT_EQ(1, thing.delete_count()); +} + +TEST(fxcrt, WeakPtrCyclic) { + PseudoDeletable thing1; + PseudoDeletable thing2; + { + UniquePtr unique1(&thing1); + UniquePtr unique2(&thing2); + WeakPtr ptr1(std::move(unique1)); + WeakPtr ptr2(std::move(unique2)); + ptr1->SetNext(ptr2); + ptr2->SetNext(ptr1); + } + // Leaks without explicit clear. + EXPECT_EQ(0, thing1.delete_count()); + EXPECT_EQ(0, thing2.delete_count()); +} + +TEST(fxcrt, WeakPtrCyclicClear) { + PseudoDeletable thing1; + PseudoDeletable thing2; + { + UniquePtr unique1(&thing1); + UniquePtr unique2(&thing2); + WeakPtr ptr1(std::move(unique1)); + WeakPtr ptr2(std::move(unique2)); + ptr1->SetNext(ptr2); + ptr2->SetNext(ptr1); + ptr1.Clear(); + EXPECT_EQ(1, thing1.delete_count()); + EXPECT_EQ(0, thing2.delete_count()); + } + EXPECT_EQ(1, thing1.delete_count()); + EXPECT_EQ(1, thing2.delete_count()); +} diff --git a/core/fxcrt/include/cfx_weak_ptr.h b/core/fxcrt/include/cfx_weak_ptr.h new file mode 100644 index 0000000000..64218cd679 --- /dev/null +++ b/core/fxcrt/include/cfx_weak_ptr.h @@ -0,0 +1,80 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com + +#ifndef CORE_FXCRT_INCLUDE_CFX_WEAK_PTR_H_ +#define CORE_FXCRT_INCLUDE_CFX_WEAK_PTR_H_ + +#include + +#include "core/fxcrt/include/cfx_retain_ptr.h" +#include "core/fxcrt/include/fx_system.h" + +template > +class CFX_WeakPtr { + public: + CFX_WeakPtr() {} + CFX_WeakPtr(const CFX_WeakPtr& that) : m_pHandle(that.m_pHandle) {} + CFX_WeakPtr(CFX_WeakPtr&& that) { Swap(that); } + CFX_WeakPtr(std::unique_ptr pObj) + : m_pHandle(new Handle(std::move(pObj))) {} + + explicit operator bool() const { return m_pHandle && !!m_pHandle->Get(); } + bool HasOneRef() const { return m_pHandle && m_pHandle->HasOneRef(); } + T* operator->() { return m_pHandle->Get(); } + const T* operator->() const { return m_pHandle->Get(); } + CFX_WeakPtr& operator=(const CFX_WeakPtr& that) { + m_pHandle = that.m_pHandle; + return *this; + } + bool operator==(const CFX_WeakPtr& that) const { + return m_pHandle == that.m_pHandle; + } + bool operator!=(const CFX_WeakPtr& that) const { return !(*this == that); } + + T* Get() const { return m_pHandle ? m_pHandle->Get() : nullptr; } + void Clear() { + if (m_pHandle) { + m_pHandle->Clear(); + m_pHandle.Reset(); + } + } + void Reset() { m_pHandle.Reset(); } + void Reset(std::unique_ptr pObj) { + m_pHandle.Reset(new Handle(std::move(pObj))); + } + void Swap(CFX_WeakPtr& that) { m_pHandle.Swap(that.m_pHandle); } + + private: + class Handle { + public: + explicit Handle(std::unique_ptr ptr) + : m_nCount(0), m_pObj(std::move(ptr)) {} + void Reset(std::unique_ptr ptr) { m_pObj = std::move(ptr); } + void Clear() { // Now you're all weak ptrs ... + m_pObj.reset(); // unique_ptr nulls first before invoking delete. + } + T* Get() const { return m_pObj.get(); } + T* Retain() { + ++m_nCount; + return m_pObj.get(); + } + void Release() { + if (--m_nCount == 0) + delete this; + } + bool HasOneRef() const { return m_nCount == 1; } + + private: + ~Handle() {} + + intptr_t m_nCount; + std::unique_ptr m_pObj; + }; + + CFX_RetainPtr m_pHandle; +}; + +#endif // CORE_FXCRT_INCLUDE_CFX_WEAK_PTR_H_ -- cgit v1.2.3