From 602aebc11a615971800d38f8d427a4e4f95c1134 Mon Sep 17 00:00:00 2001 From: tsepez Date: Tue, 29 Mar 2016 15:04:21 -0700 Subject: Add a Retained Pointer smart class. It's going to be hard to fix some XFA object leaks without this. Put this in fxcrt/ as we should be able to make the FX strings take advantage of this. BUG=pdfium:55 Review URL: https://codereview.chromium.org/1805683002 --- BUILD.gn | 2 + core/fxcrt/cfx_retain_ptr_unittest.cpp | 215 +++++++++++++++++++++++++++++++++ core/fxcrt/include/cfx_retain_ptr.h | 57 +++++++++ core/fxcrt/include/fx_basic.h | 26 ---- core/fxcrt/include/fx_memory.h | 26 ++++ pdfium.gyp | 37 +++--- 6 files changed, 320 insertions(+), 43 deletions(-) create mode 100644 core/fxcrt/cfx_retain_ptr_unittest.cpp create mode 100644 core/fxcrt/include/cfx_retain_ptr.h diff --git a/BUILD.gn b/BUILD.gn index bc680b703c..f22a418b99 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -615,6 +615,7 @@ static_library("fxcrt") { "core/fxcrt/fxcrt_stream.cpp", "core/fxcrt/fxcrt_windows.cpp", "core/fxcrt/fxcrt_windows.h", + "core/fxcrt/include/cfx_retain_ptr.h", "core/fxcrt/include/fx_basic.h", "core/fxcrt/include/fx_coordinates.h", "core/fxcrt/include/fx_ext.h", @@ -1594,6 +1595,7 @@ test("pdfium_unittests") { "core/fpdfdoc/doc_basic_unittest.cpp", "core/fpdftext/fpdf_text_int_unittest.cpp", "core/fxcodec/codec/fx_codec_jpx_unittest.cpp", + "core/fxcrt/cfx_retain_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_retain_ptr_unittest.cpp b/core/fxcrt/cfx_retain_ptr_unittest.cpp new file mode 100644 index 0000000000..4e74e52768 --- /dev/null +++ b/core/fxcrt/cfx_retain_ptr_unittest.cpp @@ -0,0 +1,215 @@ +// 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_retain_ptr.h" + +#include + +#include "testing/fx_string_testhelpers.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class PseudoRetainable { + public: + PseudoRetainable() : retain_count_(0), release_count_(0) {} + void Retain() { ++retain_count_; } + void Release() { ++release_count_; } + int retain_count() const { return retain_count_; } + int release_count() const { return release_count_; } + + private: + int retain_count_; + int release_count_; +}; + +} // namespace + +TEST(fxcrt, RetainPtrNull) { + CFX_RetainPtr ptr; + EXPECT_EQ(nullptr, ptr.Get()); +} + +TEST(fxcrt, RetainPtrNormal) { + PseudoRetainable obj; + { + CFX_RetainPtr ptr(&obj); + EXPECT_EQ(&obj, ptr.Get()); + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(0, obj.release_count()); + } + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); +} + +TEST(fxcrt, RetainPtrCopyCtor) { + PseudoRetainable obj; + { + CFX_RetainPtr ptr1(&obj); + { + CFX_RetainPtr ptr2(ptr1); + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(0, obj.release_count()); + } + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); + } + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(2, obj.release_count()); +} + +TEST(fxcrt, RetainPtrMoveCtor) { + PseudoRetainable obj; + { + CFX_RetainPtr ptr1(&obj); + { + CFX_RetainPtr ptr2(std::move(ptr1)); + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(0, obj.release_count()); + } + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); + } + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); +} + +TEST(fxcrt, RetainPtrResetNull) { + PseudoRetainable obj; + { + CFX_RetainPtr ptr(&obj); + ptr.Reset(); + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); + } + EXPECT_EQ(1, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); +} + +TEST(fxcrt, RetainPtrReset) { + PseudoRetainable obj1; + PseudoRetainable obj2; + { + CFX_RetainPtr ptr(&obj1); + ptr.Reset(&obj2); + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); + EXPECT_EQ(1, obj2.retain_count()); + EXPECT_EQ(0, obj2.release_count()); + } + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); + EXPECT_EQ(1, obj2.retain_count()); + EXPECT_EQ(1, obj2.release_count()); +} + +TEST(fxcrt, RetainPtrSwap) { + PseudoRetainable obj1; + PseudoRetainable obj2; + { + CFX_RetainPtr ptr1(&obj1); + { + CFX_RetainPtr ptr2(&obj2); + ptr1.Swap(ptr2); + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(0, obj1.release_count()); + EXPECT_EQ(1, obj2.retain_count()); + EXPECT_EQ(0, obj2.release_count()); + } + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); + EXPECT_EQ(1, obj2.retain_count()); + EXPECT_EQ(0, obj2.release_count()); + } + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); + EXPECT_EQ(1, obj2.retain_count()); + EXPECT_EQ(1, obj2.release_count()); +} + +TEST(fxcrt, RetainPtrSwapNull) { + PseudoRetainable obj1; + { + CFX_RetainPtr ptr1(&obj1); + { + CFX_RetainPtr ptr2; + ptr1.Swap(ptr2); + EXPECT_FALSE(ptr1); + EXPECT_TRUE(ptr2); + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(0, obj1.release_count()); + } + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); + } + EXPECT_EQ(1, obj1.retain_count()); + EXPECT_EQ(1, obj1.release_count()); +} + +TEST(fxcrt, RetainPtrAssign) { + PseudoRetainable obj; + { + CFX_RetainPtr ptr(&obj); + { + CFX_RetainPtr ptr2; + ptr2 = ptr; + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(0, obj.release_count()); + } + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(1, obj.release_count()); + } + EXPECT_EQ(2, obj.retain_count()); + EXPECT_EQ(2, obj.release_count()); +} + +TEST(fxcrt, RetainPtrEquals) { + PseudoRetainable obj1; + PseudoRetainable obj2; + CFX_RetainPtr null_ptr1; + CFX_RetainPtr obj1_ptr1(&obj1); + CFX_RetainPtr obj2_ptr1(&obj2); + { + CFX_RetainPtr null_ptr2; + EXPECT_TRUE(null_ptr1 == null_ptr2); + + CFX_RetainPtr obj1_ptr2(&obj1); + EXPECT_TRUE(obj1_ptr1 == obj1_ptr2); + + CFX_RetainPtr obj2_ptr2(&obj2); + EXPECT_TRUE(obj2_ptr1 == obj2_ptr2); + } + EXPECT_FALSE(null_ptr1 == obj1_ptr1); + EXPECT_FALSE(null_ptr1 == obj2_ptr1); + EXPECT_FALSE(obj1_ptr1 == obj2_ptr1); +} + +TEST(fxcrt, RetainPtrNotEquals) { + PseudoRetainable obj1; + PseudoRetainable obj2; + CFX_RetainPtr null_ptr1; + CFX_RetainPtr obj1_ptr1(&obj1); + CFX_RetainPtr obj2_ptr1(&obj2); + { + CFX_RetainPtr null_ptr2; + CFX_RetainPtr obj1_ptr2(&obj1); + CFX_RetainPtr obj2_ptr2(&obj2); + EXPECT_FALSE(null_ptr1 != null_ptr2); + EXPECT_FALSE(obj1_ptr1 != obj1_ptr2); + EXPECT_FALSE(obj2_ptr1 != obj2_ptr2); + } + EXPECT_TRUE(null_ptr1 != obj1_ptr1); + EXPECT_TRUE(null_ptr1 != obj2_ptr1); + EXPECT_TRUE(obj1_ptr1 != obj2_ptr1); +} + +TEST(fxcrt, RetainPtrBool) { + PseudoRetainable obj1; + CFX_RetainPtr null_ptr; + CFX_RetainPtr obj1_ptr(&obj1); + bool null_bool = null_ptr; + bool obj1_bool = obj1_ptr; + EXPECT_FALSE(null_bool); + EXPECT_TRUE(obj1_bool); +} diff --git a/core/fxcrt/include/cfx_retain_ptr.h b/core/fxcrt/include/cfx_retain_ptr.h new file mode 100644 index 0000000000..25edd585cb --- /dev/null +++ b/core/fxcrt/include/cfx_retain_ptr.h @@ -0,0 +1,57 @@ +// 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. + +#ifndef CORE_FXCRT_INCLUDE_CFX_RETAIN_PTR_H_ +#define CORE_FXCRT_INCLUDE_CFX_RETAIN_PTR_H_ + +#include + +#include "core/fxcrt/include/fx_memory.h" + +template +class CFX_RetainPtr { + public: + explicit CFX_RetainPtr(T* pObj) : m_pObj(pObj) { + if (m_pObj) + m_pObj->Retain(); + } + + CFX_RetainPtr() : CFX_RetainPtr(nullptr) {} + CFX_RetainPtr(const CFX_RetainPtr& that) : CFX_RetainPtr(that.Get()) {} + CFX_RetainPtr(CFX_RetainPtr&& that) { Swap(that); } + + template + CFX_RetainPtr(const CFX_RetainPtr& that) + : CFX_RetainPtr(that.Get()) {} + + void Reset(T* obj = nullptr) { + if (obj) + obj->Retain(); + m_pObj.reset(obj); + } + + T* Get() const { return m_pObj.get(); } + void Swap(CFX_RetainPtr& that) { m_pObj.swap(that.m_pObj); } + + CFX_RetainPtr& operator=(const CFX_RetainPtr& that) { + if (*this != that) + Reset(that.Get()); + return *this; + } + + bool operator==(const CFX_RetainPtr& that) const { + return Get() == that.Get(); + } + + bool operator!=(const CFX_RetainPtr& that) const { return !(*this == that); } + + operator bool() const { return !!m_pObj; } + T& operator*() const { return *m_pObj.get(); } + T* operator->() const { return m_pObj.get(); } + + private: + std::unique_ptr> m_pObj; +}; + +#endif // CORE_FXCRT_INCLUDE_CFX_RETAIN_PTR_H_ diff --git a/core/fxcrt/include/fx_basic.h b/core/fxcrt/include/fx_basic.h index 8a7afda137..feeb6e756d 100644 --- a/core/fxcrt/include/fx_basic.h +++ b/core/fxcrt/include/fx_basic.h @@ -15,32 +15,6 @@ #include "core/fxcrt/include/fx_string.h" #include "core/fxcrt/include/fx_system.h" -// The FX_ArraySize(arr) macro returns the # of elements in an array arr. -// The expression is a compile-time constant, and therefore can be -// used in defining new arrays, for example. If you use FX_ArraySize on -// a pointer by mistake, you will get a compile-time error. -// -// One caveat is that FX_ArraySize() doesn't accept any array of an -// anonymous type or a type defined inside a function. -#define FX_ArraySize(array) (sizeof(ArraySizeHelper(array))) - -// This template function declaration is used in defining FX_ArraySize. -// Note that the function doesn't need an implementation, as we only -// use its type. -template -char(&ArraySizeHelper(T(&array)[N]))[N]; - -// Used with std::unique_ptr to FX_Free raw memory. -struct FxFreeDeleter { - inline void operator()(void* ptr) const { FX_Free(ptr); } -}; - -// Used with std::unique_ptr to Release() objects that can't be deleted. -template -struct ReleaseDeleter { - inline void operator()(T* ptr) const { ptr->Release(); } -}; - class CFX_BinaryBuf { public: CFX_BinaryBuf(); diff --git a/core/fxcrt/include/fx_memory.h b/core/fxcrt/include/fx_memory.h index c3dafc8d5e..2614016550 100644 --- a/core/fxcrt/include/fx_memory.h +++ b/core/fxcrt/include/fx_memory.h @@ -74,6 +74,32 @@ inline void* FX_ReallocOrDie(void* ptr, #define FX_Free(ptr) free(ptr) +// The FX_ArraySize(arr) macro returns the # of elements in an array arr. +// The expression is a compile-time constant, and therefore can be +// used in defining new arrays, for example. If you use FX_ArraySize on +// a pointer by mistake, you will get a compile-time error. +// +// One caveat is that FX_ArraySize() doesn't accept any array of an +// anonymous type or a type defined inside a function. +#define FX_ArraySize(array) (sizeof(ArraySizeHelper(array))) + +// This template function declaration is used in defining FX_ArraySize. +// Note that the function doesn't need an implementation, as we only +// use its type. +template +char(&ArraySizeHelper(T(&array)[N]))[N]; + +// Used with std::unique_ptr to FX_Free raw memory. +struct FxFreeDeleter { + inline void operator()(void* ptr) const { FX_Free(ptr); } +}; + +// Used with std::unique_ptr to Release() objects that can't be deleted. +template +struct ReleaseDeleter { + inline void operator()(T* ptr) const { ptr->Release(); } +}; + class CFX_DestructObject { public: virtual ~CFX_DestructObject() {} diff --git a/pdfium.gyp b/pdfium.gyp index 0fb4c3eae5..e61237fafa 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -593,13 +593,6 @@ 'type': 'static_library', 'sources': [ 'core/fxcrt/extension.h', - 'core/fxcrt/fxcrt_platforms.cpp', - 'core/fxcrt/fxcrt_platforms.h', - 'core/fxcrt/fxcrt_posix.cpp', - 'core/fxcrt/fxcrt_posix.h', - 'core/fxcrt/fxcrt_stream.cpp', - 'core/fxcrt/fxcrt_windows.cpp', - 'core/fxcrt/fxcrt_windows.h', 'core/fxcrt/fx_basic_array.cpp', 'core/fxcrt/fx_basic_bstring.cpp', 'core/fxcrt/fx_basic_buffer.cpp', @@ -618,16 +611,25 @@ 'core/fxcrt/fx_unicode.cpp', 'core/fxcrt/fx_xml_composer.cpp', 'core/fxcrt/fx_xml_parser.cpp', - 'core/fxcrt/include/fx_basic.h', - 'core/fxcrt/include/fx_coordinates.h', - 'core/fxcrt/include/fx_ext.h', - 'core/fxcrt/include/fx_memory.h', - 'core/fxcrt/include/fx_safe_types.h', - 'core/fxcrt/include/fx_stream.h', - 'core/fxcrt/include/fx_string.h', - 'core/fxcrt/include/fx_system.h', - 'core/fxcrt/include/fx_ucd.h', - 'core/fxcrt/include/fx_xml.h', + 'core/fxcrt/fxcrt_platforms.cpp', + 'core/fxcrt/fxcrt_platforms.h', + 'core/fxcrt/fxcrt_posix.cpp', + 'core/fxcrt/fxcrt_posix.h', + 'core/fxcrt/fxcrt_stream.cpp', + 'core/fxcrt/fxcrt_windows.cpp', + 'core/fxcrt/fxcrt_windows.h', + 'core/fxcrt/include/cfx_retain_ptr.h', + 'core/include/fxcrt/fx_basic.h', + 'core/include/fxcrt/fx_bidi.h', + 'core/include/fxcrt/fx_coordinates.h', + 'core/include/fxcrt/fx_ext.h', + 'core/include/fxcrt/fx_memory.h', + 'core/include/fxcrt/fx_safe_types.h', + 'core/include/fxcrt/fx_stream.h', + 'core/include/fxcrt/fx_string.h', + 'core/include/fxcrt/fx_system.h', + 'core/include/fxcrt/fx_ucd.h', + 'core/include/fxcrt/fx_xml.h', 'core/fxcrt/plex.h', 'core/fxcrt/xml_int.h', ], @@ -895,6 +897,7 @@ 'test_support', ], 'sources': [ + 'core/fxcrt/cfx_retain_ptr_unittest.cpp', 'core/fpdfapi/fpdf_font/fpdf_font_cid_unittest.cpp', 'core/fpdfapi/fpdf_font/fpdf_font_unittest.cpp', 'core/fpdfapi/fpdf_page/fpdf_page_parser_old_unittest.cpp', -- cgit v1.2.3