Split off CFX_RetainablePathData from CFX_PathData. Because there are places where we use CFX_PathData in a non-refcounted manner. In turn, update SharedCopyOnWrite so that it can cope with classes that can only be instantiated via MakeRetain. This allows us to add a move ctor for CFX_PathData to allow using it in, say, std::vector<CFX_PathData> without incurring copying costs. Document the subtlety going on in CFX_RetainablePathData's copy ctor. Change-Id: Id5a7095558775abf11e4ae901a47f41a6fa5614f Reviewed-on: https://pdfium-review.googlesource.com/c/49170 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_path.h b/core/fpdfapi/page/cpdf_path.h index 3c68856..fc9ccb1 100644 --- a/core/fpdfapi/page/cpdf_path.h +++ b/core/fpdfapi/page/cpdf_path.h
@@ -42,7 +42,7 @@ const CFX_PathData* GetObject() const { return m_Ref.GetObject(); } private: - SharedCopyOnWrite<CFX_PathData> m_Ref; + SharedCopyOnWrite<CFX_RetainablePathData> m_Ref; }; #endif // CORE_FPDFAPI_PAGE_CPDF_PATH_H_
diff --git a/core/fxcrt/shared_copy_on_write.h b/core/fxcrt/shared_copy_on_write.h index f7d7a2a..b457746 100644 --- a/core/fxcrt/shared_copy_on_write.h +++ b/core/fxcrt/shared_copy_on_write.h
@@ -24,7 +24,7 @@ template <typename... Args> ObjClass* Emplace(Args... params) { - m_pObject.Reset(new ObjClass(params...)); + m_pObject = pdfium::MakeRetain<ObjClass>(params...); return m_pObject.Get(); } @@ -42,7 +42,7 @@ if (!m_pObject) return Emplace(params...); if (!m_pObject->HasOneRef()) - m_pObject.Reset(new ObjClass(*m_pObject)); + m_pObject = pdfium::MakeRetain<ObjClass>(*m_pObject); return m_pObject.Get(); }
diff --git a/core/fxge/cfx_pathdata.cpp b/core/fxge/cfx_pathdata.cpp index ae7d2d4..1735dc7 100644 --- a/core/fxge/cfx_pathdata.cpp +++ b/core/fxge/cfx_pathdata.cpp
@@ -166,11 +166,13 @@ FX_PATHPOINT::~FX_PATHPOINT() = default; -CFX_PathData::CFX_PathData() {} +CFX_PathData::CFX_PathData() = default; -CFX_PathData::~CFX_PathData() {} +CFX_PathData::CFX_PathData(const CFX_PathData& src) = default; -CFX_PathData::CFX_PathData(const CFX_PathData& src) : m_Points(src.m_Points) {} +CFX_PathData::CFX_PathData(CFX_PathData&& src) = default; + +CFX_PathData::~CFX_PathData() = default; void CFX_PathData::Clear() { m_Points.clear(); @@ -494,3 +496,16 @@ } return true; } + +CFX_RetainablePathData::CFX_RetainablePathData() = default; + +// Note: can't default the copy constructor since Retainable<> has a deleted +// copy constructor (as it should). Instead, we want the default Retainable<> +// constructor to be invoked so as to create a copy with a ref-count of 1 as +// of the time it is created, then populate the remainder of the members from +// the |src| object. +CFX_RetainablePathData::CFX_RetainablePathData( + const CFX_RetainablePathData& src) + : CFX_PathData(src) {} + +CFX_RetainablePathData::~CFX_RetainablePathData() = default;
diff --git a/core/fxge/cfx_pathdata.h b/core/fxge/cfx_pathdata.h index 705447c..1a1d915 100644 --- a/core/fxge/cfx_pathdata.h +++ b/core/fxge/cfx_pathdata.h
@@ -29,11 +29,12 @@ bool m_CloseFigure; }; -class CFX_PathData final : public Retainable { +class CFX_PathData { public: CFX_PathData(); CFX_PathData(const CFX_PathData& src); - ~CFX_PathData() override; + CFX_PathData(CFX_PathData&& src); + ~CFX_PathData(); void Clear(); @@ -69,4 +70,15 @@ std::vector<FX_PATHPOINT> m_Points; }; +class CFX_RetainablePathData final : public Retainable, public CFX_PathData { + public: + template <typename T, typename... Args> + friend RetainPtr<T> pdfium::MakeRetain(Args&&... args); + + private: + CFX_RetainablePathData(); + CFX_RetainablePathData(const CFX_RetainablePathData& src); + ~CFX_RetainablePathData() override; +}; + #endif // CORE_FXGE_CFX_PATHDATA_H_