commit ca4a5ff497ba39b1dfb48bd4643f293eebe1cf94
author Tom Sepez <tsepez@chromium.org> Tue Jan 29 00:06:20 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Jan 29 00:06:20 2019 +0000
tree a4669c668c630d9f9a06cf8b9d4c8ccccbe6ea46
parent 408e5d3565804d9752bf47ac64fb6d4d636d8aec

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>

core/fpdfapi/page/cpdf_path.h

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_

core/fxcrt/shared_copy_on_write.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();
   }
 

core/fxge/cfx_pathdata.cpp

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;

core/fxge/cfx_pathdata.h

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_