Make CPDF_ClipPath have a CPDF_ClipPathData rather than inheriting.

Make Data private to the ClipPath class which manages it
transparently for its callers.  This prevents the callers
from having to remember to make a copy before dirtying the
shared data, since the operations that modify state will do
this under the covers for us.

Review-Url: https://codereview.chromium.org/2301263003

9 files changed, 68 insertions, 72 deletions

diff --git a/BUILD.gn b/BUILD.gn
index ae50f1f512..3b6fb12212 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -408,8 +408,6 @@ static_library("fpdfapi") {
     "core/fpdfapi/fpdf_page/cpdf_allstates.cpp",
     "core/fpdfapi/fpdf_page/cpdf_allstates.h",
     "core/fpdfapi/fpdf_page/cpdf_clippath.cpp",
-    "core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp",
-    "core/fpdfapi/fpdf_page/cpdf_clippathdata.h",
     "core/fpdfapi/fpdf_page/cpdf_color.cpp",
     "core/fpdfapi/fpdf_page/cpdf_colorspace.cpp",
     "core/fpdfapi/fpdf_page/cpdf_colorstate.cpp",
diff --git a/core/fpdfapi/fpdf_page/cpdf_clippath.cpp b/core/fpdfapi/fpdf_page/cpdf_clippath.cpp
index e820260131..51ff593a5a 100644
--- a/core/fpdfapi/fpdf_page/cpdf_clippath.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_clippath.cpp
@@ -8,29 +8,36 @@
 
 #include <utility>
 
+#include "core/fpdfapi/fpdf_page/include/cpdf_path.h"
 #include "core/fpdfapi/fpdf_page/include/cpdf_textobject.h"
 #include "third_party/base/stl_util.h"
 
 #define FPDF_CLIPPATH_MAX_TEXTS 1024
 
+CPDF_ClipPath::CPDF_ClipPath() {}
+
+CPDF_ClipPath::CPDF_ClipPath(const CPDF_ClipPath& that) : m_Ref(that.m_Ref) {}
+
+CPDF_ClipPath::~CPDF_ClipPath() {}
+
 uint32_t CPDF_ClipPath::GetPathCount() const {
-  return pdfium::CollectionSize<uint32_t>(GetObject()->m_PathAndTypeList);
+  return pdfium::CollectionSize<uint32_t>(m_Ref.GetObject()->m_PathAndTypeList);
 }
 
 CPDF_Path CPDF_ClipPath::GetPath(size_t i) const {
-  return GetObject()->m_PathAndTypeList[i].first;
+  return m_Ref.GetObject()->m_PathAndTypeList[i].first;
 }
 
 uint8_t CPDF_ClipPath::GetClipType(size_t i) const {
-  return GetObject()->m_PathAndTypeList[i].second;
+  return m_Ref.GetObject()->m_PathAndTypeList[i].second;
 }
 
 uint32_t CPDF_ClipPath::GetTextCount() const {
-  return pdfium::CollectionSize<uint32_t>(GetObject()->m_TextList);
+  return pdfium::CollectionSize<uint32_t>(m_Ref.GetObject()->m_TextList);
 }
 
 CPDF_TextObject* CPDF_ClipPath::GetText(size_t i) const {
-  return GetObject()->m_TextList[i].get();
+  return m_Ref.GetObject()->m_TextList[i].get();
 }
 
 CFX_FloatRect CPDF_ClipPath::GetClipBox() const {
@@ -73,7 +80,7 @@ CFX_FloatRect CPDF_ClipPath::GetClipBox() const {
 }
 
 void CPDF_ClipPath::AppendPath(CPDF_Path path, uint8_t type, bool bAutoMerge) {
-  CPDF_ClipPathData* pData = GetPrivateCopy();
+  PathData* pData = m_Ref.GetPrivateCopy();
   if (!pData->m_PathAndTypeList.empty() && bAutoMerge) {
     const CPDF_Path& old_path = pData->m_PathAndTypeList.back().first;
     if (old_path.IsRect()) {
@@ -89,7 +96,7 @@ void CPDF_ClipPath::AppendPath(CPDF_Path path, uint8_t type, bool bAutoMerge) {
 
 void CPDF_ClipPath::AppendTexts(
     std::vector<std::unique_ptr<CPDF_TextObject>>* pTexts) {
-  CPDF_ClipPathData* pData = GetPrivateCopy();
+  PathData* pData = m_Ref.GetPrivateCopy();
   if (pData->m_TextList.size() + pTexts->size() <= FPDF_CLIPPATH_MAX_TEXTS) {
     for (size_t i = 0; i < pTexts->size(); i++)
       pData->m_TextList.push_back(std::move((*pTexts)[i]));
@@ -99,7 +106,7 @@ void CPDF_ClipPath::AppendTexts(
 }
 
 void CPDF_ClipPath::Transform(const CFX_Matrix& matrix) {
-  CPDF_ClipPathData* pData = GetPrivateCopy();
+  PathData* pData = m_Ref.GetPrivateCopy();
   for (auto& obj : pData->m_PathAndTypeList)
     obj.first.Transform(&matrix);
   for (auto& text : pData->m_TextList) {
@@ -107,3 +114,17 @@ void CPDF_ClipPath::Transform(const CFX_Matrix& matrix) {
       text->Transform(matrix);
   }
 }
+
+CPDF_ClipPath::PathData::PathData() {}
+
+CPDF_ClipPath::PathData::PathData(const PathData& that) {
+  m_PathAndTypeList = that.m_PathAndTypeList;
+
+  m_TextList.resize(that.m_TextList.size());
+  for (size_t i = 0; i < that.m_TextList.size(); ++i) {
+    if (that.m_TextList[i])
+      m_TextList[i].reset(that.m_TextList[i]->Clone());
+  }
+}
+
+CPDF_ClipPath::PathData::~PathData() {}
diff --git a/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp b/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp
deleted file mode 100644
index 9c84b4ae28..0000000000
--- a/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-// 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
-
-#include "core/fpdfapi/fpdf_page/cpdf_clippathdata.h"
-
-#include "core/fpdfapi/fpdf_page/include/cpdf_path.h"
-#include "core/fpdfapi/fpdf_page/include/cpdf_textobject.h"
-
-CPDF_ClipPathData::CPDF_ClipPathData() {}
-
-CPDF_ClipPathData::~CPDF_ClipPathData() {}
-
-CPDF_ClipPathData::CPDF_ClipPathData(const CPDF_ClipPathData& src) {
-  m_PathAndTypeList = src.m_PathAndTypeList;
-
-  m_TextList.resize(src.m_TextList.size());
-  for (size_t i = 0; i < src.m_TextList.size(); ++i) {
-    if (src.m_TextList[i])
-      m_TextList[i].reset(src.m_TextList[i]->Clone());
-  }
-}
diff --git a/core/fpdfapi/fpdf_page/cpdf_clippathdata.h b/core/fpdfapi/fpdf_page/cpdf_clippathdata.h
deleted file mode 100644
index 70e2b1b096..0000000000
--- a/core/fpdfapi/fpdf_page/cpdf_clippathdata.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// 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_FPDFAPI_FPDF_PAGE_CPDF_CLIPPATHDATA_H_
-#define CORE_FPDFAPI_FPDF_PAGE_CPDF_CLIPPATHDATA_H_
-
-#include <stdint.h>
-
-#include <memory>
-#include <utility>
-#include <vector>
-
-class CPDF_Path;
-class CPDF_TextObject;
-
-class CPDF_ClipPathData {
- public:
-  using PathAndTypeData = std::pair<CPDF_Path, uint8_t>;
-  CPDF_ClipPathData();
-  CPDF_ClipPathData(const CPDF_ClipPathData&);
-  ~CPDF_ClipPathData();
-
-  std::vector<PathAndTypeData> m_PathAndTypeList;
-  std::vector<std::unique_ptr<CPDF_TextObject>> m_TextList;
-};
-
-#endif  // CORE_FPDFAPI_FPDF_PAGE_CPDF_CLIPPATHDATA_H_
diff --git a/core/fpdfapi/fpdf_page/cpdf_pageobject.cpp b/core/fpdfapi/fpdf_page/cpdf_pageobject.cpp
index 1ce83a85bc..4566d97771 100644
--- a/core/fpdfapi/fpdf_page/cpdf_pageobject.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_pageobject.cpp
@@ -81,7 +81,6 @@ void CPDF_PageObject::CopyData(const CPDF_PageObject* pSrc) {
 void CPDF_PageObject::TransformClipPath(CFX_Matrix& matrix) {
   if (!m_ClipPath)
     return;
-  m_ClipPath.GetPrivateCopy();
   m_ClipPath.Transform(matrix);
 }
 
diff --git a/core/fpdfapi/fpdf_page/cpdf_shadingobject.cpp b/core/fpdfapi/fpdf_page/cpdf_shadingobject.cpp
index d869bc20b3..03dd22c134 100644
--- a/core/fpdfapi/fpdf_page/cpdf_shadingobject.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_shadingobject.cpp
@@ -35,10 +35,9 @@ CPDF_PageObject::Type CPDF_ShadingObject::GetType() const {
 }
 
 void CPDF_ShadingObject::Transform(const CFX_Matrix& matrix) {
-  if (m_ClipPath) {
-    m_ClipPath.GetPrivateCopy();
+  if (m_ClipPath)
     m_ClipPath.Transform(matrix);
-  }
+
   m_Matrix.Concat(matrix);
   if (m_ClipPath) {
     CalcBoundingBox();
diff --git a/core/fpdfapi/fpdf_page/include/cpdf_clippath.h b/core/fpdfapi/fpdf_page/include/cpdf_clippath.h
index 1daacf55d0..6b3cca0d41 100644
--- a/core/fpdfapi/fpdf_page/include/cpdf_clippath.h
+++ b/core/fpdfapi/fpdf_page/include/cpdf_clippath.h
@@ -7,16 +7,33 @@
 #ifndef CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_CLIPPATH_H_
 #define CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_CLIPPATH_H_
 
-#include "core/fpdfapi/fpdf_page/cpdf_clippathdata.h"
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include "core/fpdfapi/fpdf_page/include/cpdf_path.h"
+#include "core/fxcrt/include/cfx_count_ref.h"
 #include "core/fxcrt/include/fx_basic.h"
 #include "core/fxcrt/include/fx_coordinates.h"
-#include "core/fxcrt/include/fx_system.h"
 
+class CPDF_Path;
 class CPDF_TextObject;
 
-class CPDF_ClipPath : public CFX_CountRef<CPDF_ClipPathData> {
+class CPDF_ClipPath {
  public:
+  CPDF_ClipPath();
+  CPDF_ClipPath(const CPDF_ClipPath& that);
+  ~CPDF_ClipPath();
+
+  void Emplace() { m_Ref.Emplace(); }
+  void SetNull() { m_Ref.SetNull(); }
+
+  explicit operator bool() const { return !!m_Ref; }
+  bool operator==(const CPDF_ClipPath& that) const {
+    return m_Ref == that.m_Ref;
+  }
+  bool operator!=(const CPDF_ClipPath& that) const { return !(*this == that); }
+
   uint32_t GetPathCount() const;
   CPDF_Path GetPath(size_t i) const;
   uint8_t GetClipType(size_t i) const;
@@ -26,6 +43,21 @@ class CPDF_ClipPath : public CFX_CountRef<CPDF_ClipPathData> {
   void AppendPath(CPDF_Path path, uint8_t type, bool bAutoMerge);
   void AppendTexts(std::vector<std::unique_ptr<CPDF_TextObject>>* pTexts);
   void Transform(const CFX_Matrix& matrix);
+
+ private:
+  class PathData {
+   public:
+    using PathAndTypeData = std::pair<CPDF_Path, uint8_t>;
+
+    PathData();
+    PathData(const PathData& that);
+    ~PathData();
+
+    std::vector<PathAndTypeData> m_PathAndTypeList;
+    std::vector<std::unique_ptr<CPDF_TextObject>> m_TextList;
+  };
+
+  CFX_CountRef<PathData> m_Ref;
 };
 
 #endif  // CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_CLIPPATH_H_
diff --git a/core/fxcrt/include/cfx_count_ref.h b/core/fxcrt/include/cfx_count_ref.h
index aac18b166a..95132a73ca 100644
--- a/core/fxcrt/include/cfx_count_ref.h
+++ b/core/fxcrt/include/cfx_count_ref.h
@@ -10,6 +10,8 @@
 #include "core/fxcrt/include/cfx_retain_ptr.h"
 #include "core/fxcrt/include/fx_system.h"
 
+// A shared object with Copy on Write semantics that makes it appear as
+// if each one were independent.
 template <class ObjClass>
 class CFX_CountRef {
  public:
diff --git a/fpdfsdk/fpdf_transformpage.cpp b/fpdfsdk/fpdf_transformpage.cpp
index f76cc2be8c..c77ece7b9d 100644
--- a/fpdfsdk/fpdf_transformpage.cpp
+++ b/fpdfsdk/fpdf_transformpage.cpp
@@ -223,7 +223,6 @@ DLLEXPORT FPDF_CLIPPATH STDCALL FPDF_CreateClipPath(float left,
   Path.AppendRect(left, bottom, right, top);
 
   CPDF_ClipPath* pNewClipPath = new CPDF_ClipPath();
-  pNewClipPath->GetPrivateCopy();
   pNewClipPath->AppendPath(Path, FXFILL_ALTERNATE, FALSE);
   return pNewClipPath;
 }