From 80f9957fe156bd8e940499491ca8167fe7dad416 Mon Sep 17 00:00:00 2001 From: tsepez Date: Fri, 26 Aug 2016 10:59:04 -0700 Subject: Rework CFX_CountRef in terms of CFX_RetainPtr. Make use of existing ref count work rather than re-inventing it. Review-Url: https://codereview.chromium.org/2281683002 --- BUILD.gn | 2 + core/fpdfapi/fpdf_page/cpdf_colorstate.h | 1 + core/fpdfapi/fpdf_page/cpdf_contentmark.h | 1 + core/fpdfapi/fpdf_page/include/cpdf_path.h | 4 +- core/fxcrt/cfx_count_ref_unittest.cpp | 129 +++++++++++++++++++++++++++++ core/fxcrt/include/cfx_count_ref.h | 74 +++++++++++++++++ core/fxcrt/include/fx_basic.h | 69 --------------- core/fxge/include/fx_dib.h | 2 + 8 files changed, 211 insertions(+), 71 deletions(-) create mode 100644 core/fxcrt/cfx_count_ref_unittest.cpp create mode 100644 core/fxcrt/include/cfx_count_ref.h diff --git a/BUILD.gn b/BUILD.gn index f603099d0f..c8da0dc35e 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -688,6 +688,7 @@ static_library("fxcrt") { "core/fxcrt/fxcrt_stream.cpp", "core/fxcrt/fxcrt_windows.cpp", "core/fxcrt/fxcrt_windows.h", + "core/fxcrt/include/cfx_count_ref.h", "core/fxcrt/include/cfx_retain_ptr.h", "core/fxcrt/include/fx_basic.h", "core/fxcrt/include/fx_coordinates.h", @@ -1601,6 +1602,7 @@ test("pdfium_unittests") { "core/fpdftext/fpdf_text_int_unittest.cpp", "core/fxcodec/codec/fx_codec_jpx_unittest.cpp", "core/fxcodec/jbig2/JBig2_Image_unittest.cpp", + "core/fxcrt/cfx_count_ref_unittest.cpp", "core/fxcrt/cfx_retain_ptr_unittest.cpp", "core/fxcrt/fx_basic_bstring_unittest.cpp", "core/fxcrt/fx_basic_gcc_unittest.cpp", diff --git a/core/fpdfapi/fpdf_page/cpdf_colorstate.h b/core/fpdfapi/fpdf_page/cpdf_colorstate.h index da4c49d3c9..c7b8854be4 100644 --- a/core/fpdfapi/fpdf_page/cpdf_colorstate.h +++ b/core/fpdfapi/fpdf_page/cpdf_colorstate.h @@ -8,6 +8,7 @@ #define CORE_FPDFAPI_FPDF_PAGE_CPDF_COLORSTATE_H_ #include "core/fpdfapi/fpdf_page/cpdf_colorstatedata.h" +#include "core/fxcrt/include/cfx_count_ref.h" #include "core/fxcrt/include/fx_basic.h" #include "core/fxcrt/include/fx_system.h" diff --git a/core/fpdfapi/fpdf_page/cpdf_contentmark.h b/core/fpdfapi/fpdf_page/cpdf_contentmark.h index 056edc23a8..680aed4b99 100644 --- a/core/fpdfapi/fpdf_page/cpdf_contentmark.h +++ b/core/fpdfapi/fpdf_page/cpdf_contentmark.h @@ -8,6 +8,7 @@ #define CORE_FPDFAPI_FPDF_PAGE_CPDF_CONTENTMARK_H_ #include "core/fpdfapi/fpdf_page/cpdf_contentmarkdata.h" +#include "core/fxcrt/include/cfx_count_ref.h" #include "core/fxcrt/include/fx_basic.h" class CPDF_ContentMark : public CFX_CountRef { diff --git a/core/fpdfapi/fpdf_page/include/cpdf_path.h b/core/fpdfapi/fpdf_page/include/cpdf_path.h index 0c077bb443..0e8c491f9e 100644 --- a/core/fpdfapi/fpdf_page/include/cpdf_path.h +++ b/core/fpdfapi/fpdf_page/include/cpdf_path.h @@ -27,8 +27,8 @@ class CPDF_Path : public CFX_CountRef { FX_BOOL IsRect() const { return m_pObject->IsRect(); } void Transform(const CFX_Matrix* pMatrix) { GetModify()->Transform(pMatrix); } - void Append(CPDF_Path src, const CFX_Matrix* pMatrix) { - m_pObject->Append(src.m_pObject, pMatrix); + void Append(const CPDF_Path& other, const CFX_Matrix* pMatrix) { + m_pObject->Append(other.GetObject(), pMatrix); } void AppendRect(FX_FLOAT left, diff --git a/core/fxcrt/cfx_count_ref_unittest.cpp b/core/fxcrt/cfx_count_ref_unittest.cpp new file mode 100644 index 0000000000..11c7c9b86d --- /dev/null +++ b/core/fxcrt/cfx_count_ref_unittest.cpp @@ -0,0 +1,129 @@ +// 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_count_ref.h" + +#include +#include + +#include "testing/fx_string_testhelpers.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class Observer { + public: + void OnConstruct(const std::string& name) { construction_counts_[name]++; } + void OnDestruct(const std::string& name) { destruction_counts_[name]++; } + int GetConstructionCount(const std::string& name) { + return construction_counts_[name]; + } + int GetDestructionCount(const std::string& name) { + return destruction_counts_[name]; + } + + private: + std::map construction_counts_; + std::map destruction_counts_; +}; + +class Object { + public: + Object(Observer* observer, const std::string& name) + : name_(name), observer_(observer) { + observer->OnConstruct(name_); + } + Object(const Object& that) : name_(that.name_), observer_(that.observer_) { + observer_->OnConstruct(name_); + } + ~Object() { observer_->OnDestruct(name_); } + + private: + std::string name_; + Observer* observer_; +}; + +} // namespace + +TEST(fxcrt, CountRefNull) { + Observer observer; + { + CFX_CountRef ptr; + EXPECT_EQ(nullptr, ptr.GetObject()); + } +} + +TEST(fxcrt, CountRefCopy) { + Observer observer; + { + CFX_CountRef ptr1; + ptr1.New(&observer, std::string("one")); + { + CFX_CountRef ptr2 = ptr1; + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + } + { + CFX_CountRef ptr3(ptr1); + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + } + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + } + EXPECT_EQ(1, observer.GetDestructionCount("one")); +} + +TEST(fxcrt, CountRefAssignOverOld) { + Observer observer; + { + CFX_CountRef ptr1; + ptr1.New(&observer, std::string("one")); + ptr1.New(&observer, std::string("two")); + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(1, observer.GetConstructionCount("two")); + EXPECT_EQ(1, observer.GetDestructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("two")); + } + EXPECT_EQ(1, observer.GetDestructionCount("two")); +} + +TEST(fxcrt, CountRefAssignOverRetained) { + Observer observer; + { + CFX_CountRef ptr1; + ptr1.New(&observer, std::string("one")); + CFX_CountRef ptr2(ptr1); + ptr1.New(&observer, std::string("two")); + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(1, observer.GetConstructionCount("two")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("two")); + } + EXPECT_EQ(1, observer.GetDestructionCount("one")); + EXPECT_EQ(1, observer.GetDestructionCount("two")); +} + +TEST(fxcrt, CountRefGetModify) { + Observer observer; + { + CFX_CountRef ptr; + EXPECT_NE(nullptr, ptr.GetModify(&observer, std::string("one"))); + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + + EXPECT_NE(nullptr, ptr.GetModify(&observer, std::string("one"))); + EXPECT_EQ(1, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + { + CFX_CountRef other(ptr); + EXPECT_NE(nullptr, ptr.GetModify(&observer, std::string("one"))); + EXPECT_EQ(2, observer.GetConstructionCount("one")); + EXPECT_EQ(0, observer.GetDestructionCount("one")); + } + EXPECT_EQ(2, observer.GetConstructionCount("one")); + EXPECT_EQ(1, observer.GetDestructionCount("one")); + } + EXPECT_EQ(2, observer.GetDestructionCount("one")); +} diff --git a/core/fxcrt/include/cfx_count_ref.h b/core/fxcrt/include/cfx_count_ref.h new file mode 100644 index 0000000000..cc7cf3d9ed --- /dev/null +++ b/core/fxcrt/include/cfx_count_ref.h @@ -0,0 +1,74 @@ +// 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_COUNT_REF_H_ +#define CORE_FXCRT_INCLUDE_CFX_COUNT_REF_H_ + +#include "core/fxcrt/include/cfx_retain_ptr.h" +#include "core/fxcrt/include/fx_system.h" + +template +class CFX_CountRef { + public: + CFX_CountRef() {} + CFX_CountRef(const CFX_CountRef& other) : m_pObject(other.m_pObject) {} + ~CFX_CountRef() {} + + template + ObjClass* New(Args... params) { + m_pObject.Reset(new CountedObj(params...)); + return m_pObject.Get(); + } + + CFX_CountRef& operator=(const CFX_CountRef& that) { + if (*this != that) + m_pObject = that.m_pObject; + return *this; + } + + void SetNull() { m_pObject.Reset(); } + bool IsNull() const { return !m_pObject; } + bool NotNull() const { return !IsNull(); } + + const ObjClass* GetObject() const { return m_pObject.Get(); } + + template + ObjClass* GetModify(Args... params) { + if (!m_pObject) + return New(params...); + if (!m_pObject->HasOneRef()) + m_pObject.Reset(new CountedObj(*m_pObject)); + return m_pObject.Get(); + } + + bool operator==(const CFX_CountRef& that) const { + return m_pObject == that.m_pObject; + } + bool operator!=(const CFX_CountRef& that) const { return !(*this == that); } + + protected: + class CountedObj : public ObjClass { + public: + template + CountedObj(Args... params) : ObjClass(params...), m_RefCount(0) {} + + CountedObj(const CountedObj& src) : ObjClass(src), m_RefCount(0) {} + + bool HasOneRef() const { return m_RefCount == 1; } + void Retain() { m_RefCount++; } + void Release() { + if (--m_RefCount <= 0) + delete this; + } + + private: + intptr_t m_RefCount; + }; + + CFX_RetainPtr m_pObject; +}; + +#endif // CORE_FXCRT_INCLUDE_CFX_COUNT_REF_H_ diff --git a/core/fxcrt/include/fx_basic.h b/core/fxcrt/include/fx_basic.h index bc05a3479d..6a8988d5dc 100644 --- a/core/fxcrt/include/fx_basic.h +++ b/core/fxcrt/include/fx_basic.h @@ -649,75 +649,6 @@ class CFX_BitStream { const uint8_t* m_pData; }; -template -class CFX_CountRef { - public: - using Ref = CFX_CountRef; - - class CountedObj : public ObjClass { - public: - CountedObj() {} - CountedObj(const CountedObj& src) : ObjClass(src) {} - - int m_RefCount; - }; - - CFX_CountRef() : m_pObject(nullptr) {} - CFX_CountRef(const Ref& ref) : m_pObject(ref.m_pObject) { - if (m_pObject) - m_pObject->m_RefCount++; - } - - ~CFX_CountRef() { SetNull(); } - - ObjClass* New() { - SetNull(); - m_pObject = new CountedObj; - m_pObject->m_RefCount = 1; - return m_pObject; - } - - void operator=(const Ref& ref) { - if (ref.m_pObject) - ref.m_pObject->m_RefCount++; - SetNull(); - m_pObject = ref.m_pObject; - } - - bool IsNull() const { return !m_pObject; } - bool NotNull() const { return !IsNull(); } - - const ObjClass* GetObject() const { return m_pObject; } - ObjClass* GetModify() { - if (!m_pObject) { - m_pObject = new CountedObj; - m_pObject->m_RefCount = 1; - } else if (m_pObject->m_RefCount > 1) { - m_pObject->m_RefCount--; - CountedObj* pOldObject = m_pObject; - m_pObject = new CountedObj(*pOldObject); - m_pObject->m_RefCount = 1; - } - return m_pObject; - } - - void SetNull() { - if (!m_pObject) { - return; - } - m_pObject->m_RefCount--; - if (m_pObject->m_RefCount <= 0) { - delete m_pObject; - } - m_pObject = nullptr; - } - - bool operator==(const Ref& ref) const { return m_pObject == ref.m_pObject; } - - protected: - CountedObj* m_pObject; -}; - class IFX_Pause { public: virtual ~IFX_Pause() {} diff --git a/core/fxge/include/fx_dib.h b/core/fxge/include/fx_dib.h index 540996cfc1..011e6f7856 100644 --- a/core/fxge/include/fx_dib.h +++ b/core/fxge/include/fx_dib.h @@ -10,6 +10,7 @@ #include #include +#include "core/fxcrt/include/cfx_count_ref.h" #include "core/fxcrt/include/fx_basic.h" #include "core/fxcrt/include/fx_coordinates.h" @@ -389,6 +390,7 @@ class CFX_DIBExtractor { }; typedef CFX_CountRef CFX_DIBitmapRef; + class CFX_FilteredDIB : public CFX_DIBSource { public: CFX_FilteredDIB(); -- cgit v1.2.3