Make SharedCopyOnWrite use Retainables

This CL removes the CountedObject from SharedCopyOnWrite and instead
converts the items being shared into Retainable subclasses.

BUG: chromium:792372
Change-Id: I9570a521f2fc2434013c3ccb103273bdd2440bb8
Reviewed-on: https://pdfium-review.googlesource.com/22010
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

9 files changed, 21 insertions, 45 deletions

diff --git a/core/fpdfapi/page/cpdf_clippath.h b/core/fpdfapi/page/cpdf_clippath.h
index 89365a2d5a..91a25cda1a 100644
--- a/core/fpdfapi/page/cpdf_clippath.h
+++ b/core/fpdfapi/page/cpdf_clippath.h
@@ -44,13 +44,13 @@ class CPDF_ClipPath {
   void Transform(const CFX_Matrix& matrix);
 
  private:
-  class PathData {
+  class PathData : public Retainable {
    public:
     using PathAndTypeData = std::pair<CPDF_Path, uint8_t>;
 
     PathData();
     PathData(const PathData& that);
-    ~PathData();
+    ~PathData() override;
 
     std::vector<PathAndTypeData> m_PathAndTypeList;
     std::vector<std::unique_ptr<CPDF_TextObject>> m_TextList;
diff --git a/core/fpdfapi/page/cpdf_colorstate.h b/core/fpdfapi/page/cpdf_colorstate.h
index 36a2c2d260..9619051e2c 100644
--- a/core/fpdfapi/page/cpdf_colorstate.h
+++ b/core/fpdfapi/page/cpdf_colorstate.h
@@ -46,11 +46,11 @@ class CPDF_ColorState {
   bool HasRef() const { return !!m_Ref; }
 
  private:
-  class ColorData {
+  class ColorData : public Retainable {
    public:
     ColorData();
     ColorData(const ColorData& src);
-    ~ColorData();
+    ~ColorData() override;
 
     void SetDefault();
 
diff --git a/core/fpdfapi/page/cpdf_contentmark.h b/core/fpdfapi/page/cpdf_contentmark.h
index cb22737ca9..d9bc35a7fb 100644
--- a/core/fpdfapi/page/cpdf_contentmark.h
+++ b/core/fpdfapi/page/cpdf_contentmark.h
@@ -30,11 +30,11 @@ class CPDF_ContentMark {
   bool HasRef() const { return !!m_Ref; }
 
  private:
-  class MarkData {
+  class MarkData : public Retainable {
    public:
     MarkData();
     MarkData(const MarkData& src);
-    ~MarkData();
+    ~MarkData() override;
 
     size_t CountItems() const;
     const CPDF_ContentMarkItem& GetItem(size_t index) const;
diff --git a/core/fpdfapi/page/cpdf_generalstate.h b/core/fpdfapi/page/cpdf_generalstate.h
index 5abb87ea26..5d6d5dc552 100644
--- a/core/fpdfapi/page/cpdf_generalstate.h
+++ b/core/fpdfapi/page/cpdf_generalstate.h
@@ -77,11 +77,11 @@ class CPDF_GeneralState {
   CFX_Matrix* GetMutableMatrix();
 
  private:
-  class StateData {
+  class StateData : public Retainable {
    public:
     StateData();
     StateData(const StateData& that);
-    ~StateData();
+    ~StateData() override;
 
     ByteString m_BlendMode;
     int m_BlendType;
diff --git a/core/fpdfapi/page/cpdf_textstate.h b/core/fpdfapi/page/cpdf_textstate.h
index 2a48702318..aa128072b6 100644
--- a/core/fpdfapi/page/cpdf_textstate.h
+++ b/core/fpdfapi/page/cpdf_textstate.h
@@ -59,11 +59,11 @@ class CPDF_TextState {
   float* GetMutableCTM();
 
  private:
-  class TextData {
+  class TextData : public Retainable {
    public:
     TextData();
     TextData(const TextData& src);
-    ~TextData();
+    ~TextData() override;
 
     void SetFont(CPDF_Font* pFont);
     float GetFontSizeV() const;
diff --git a/core/fxcrt/shared_copy_on_write.h b/core/fxcrt/shared_copy_on_write.h
index c04730d5e0..f7d7a2afdb 100644
--- a/core/fxcrt/shared_copy_on_write.h
+++ b/core/fxcrt/shared_copy_on_write.h
@@ -24,7 +24,7 @@ class SharedCopyOnWrite {
 
   template <typename... Args>
   ObjClass* Emplace(Args... params) {
-    m_pObject.Reset(new CountedObj(params...));
+    m_pObject.Reset(new ObjClass(params...));
     return m_pObject.Get();
   }
 
@@ -42,7 +42,7 @@ class SharedCopyOnWrite {
     if (!m_pObject)
       return Emplace(params...);
     if (!m_pObject->HasOneRef())
-      m_pObject.Reset(new CountedObj(*m_pObject));
+      m_pObject.Reset(new ObjClass(*m_pObject));
     return m_pObject.Get();
   }
 
@@ -55,33 +55,7 @@ class SharedCopyOnWrite {
   explicit operator bool() const { return !!m_pObject; }
 
  private:
-  class CountedObj : public ObjClass {
-   public:
-    template <typename... Args>
-    // NOLINTNEXTLINE(runtime/explicit)
-    CountedObj(Args... params) : ObjClass(params...), m_RefCount(0) {}
-
-    CountedObj(const CountedObj& src) : ObjClass(src), m_RefCount(0) {}
-    ~CountedObj() { m_RefCount = 0; }
-
-    bool HasOneRef() const { return m_RefCount == 1; }
-    void Retain() { m_RefCount++; }
-    void Release() {
-      ASSERT(m_RefCount);
-      if (--m_RefCount == 0)
-        delete this;
-    }
-
-   private:
-    // To ensure ref counts do not overflow, consider the worst possible case:
-    // the entire address space contains nothing but pointers to this object.
-    // Since the count increments with each new pointer, the largest value is
-    // the number of pointers that can fit into the address space. The size of
-    // the address space itself is a good upper bound on it.
-    intptr_t m_RefCount;
-  };
-
-  RetainPtr<CountedObj> m_pObject;
+  RetainPtr<ObjClass> m_pObject;
 };
 
 }  // namespace fxcrt
diff --git a/core/fxcrt/shared_copy_on_write_unittest.cpp b/core/fxcrt/shared_copy_on_write_unittest.cpp
index 01cc09468c..57e33d1019 100644
--- a/core/fxcrt/shared_copy_on_write_unittest.cpp
+++ b/core/fxcrt/shared_copy_on_write_unittest.cpp
@@ -7,6 +7,7 @@
 #include <map>
 #include <string>
 
+#include "core/fxcrt/retain_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace fxcrt {
@@ -28,7 +29,7 @@ class Observer {
   std::map<std::string, int> destruction_counts_;
 };
 
-class Object {
+class Object : public Retainable {
  public:
   Object(Observer* observer, const std::string& name)
       : name_(name), observer_(observer) {
@@ -37,7 +38,7 @@ class Object {
   Object(const Object& that) : name_(that.name_), observer_(that.observer_) {
     observer_->OnConstruct(name_);
   }
-  ~Object() { observer_->OnDestruct(name_); }
+  ~Object() override { observer_->OnDestruct(name_); }
 
  private:
   std::string name_;
diff --git a/core/fxge/cfx_graphstatedata.h b/core/fxge/cfx_graphstatedata.h
index dc0098be3e..1afff83c1c 100644
--- a/core/fxge/cfx_graphstatedata.h
+++ b/core/fxge/cfx_graphstatedata.h
@@ -8,14 +8,15 @@
 #define CORE_FXGE_CFX_GRAPHSTATEDATA_H_
 
 #include "core/fxcrt/fx_system.h"
+#include "core/fxcrt/retain_ptr.h"
 
-class CFX_GraphStateData {
+class CFX_GraphStateData : public Retainable {
  public:
   enum LineCap { LineCapButt = 0, LineCapRound = 1, LineCapSquare = 2 };
 
   CFX_GraphStateData();
   CFX_GraphStateData(const CFX_GraphStateData& src);
-  ~CFX_GraphStateData();
+  ~CFX_GraphStateData() override;
 
   void Copy(const CFX_GraphStateData& src);
   void SetDashCount(int count);
diff --git a/core/fxge/cfx_pathdata.h b/core/fxge/cfx_pathdata.h
index 0e2bb89f9f..d346ba0666 100644
--- a/core/fxge/cfx_pathdata.h
+++ b/core/fxge/cfx_pathdata.h
@@ -29,11 +29,11 @@ class FX_PATHPOINT {
   bool m_CloseFigure;
 };
 
-class CFX_PathData {
+class CFX_PathData : public Retainable {
  public:
   CFX_PathData();
   CFX_PathData(const CFX_PathData& src);
-  ~CFX_PathData();
+  ~CFX_PathData() override;
 
   void Clear();