Make CPDF_Color saner.

Stop casting a vector of floats to a PatternValue object type. Stop
performing cleanup for said PatternValue outside of the class itself.
Use std::vector in place of mallocs.

In turn, this requires making CPDF_Pattern and its subclasses
retainable, so that the cleanups happen automatically, which is
more straightforward than trying to move the old cleanup logic
into the PatternValue dtor.

In turn, make the doc page data weakly cache the CPDF_Patterns by
using the Observable pattern.

Change-Id: Idd1095cee3bd497392ea1f120bd452f3f34ce104
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56750
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_color.cpp b/core/fpdfapi/page/cpdf_color.cpp
index 96804dc..cfa6422 100644
--- a/core/fpdfapi/page/cpdf_color.cpp
+++ b/core/fpdfapi/page/cpdf_color.cpp
@@ -18,122 +18,53 @@
   *this = that;
 }
 
-CPDF_Color::~CPDF_Color() {
-  ReleaseBuffer();
-}
+CPDF_Color::~CPDF_Color() = default;
 
 bool CPDF_Color::IsPattern() const {
   return m_pCS && IsPatternInternal();
 }
 
-void CPDF_Color::ReleaseBuffer() {
-  if (!m_pBuffer)
-    return;
-
-  if (IsPatternInternal()) {
-    PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
-    CPDF_Pattern* pPattern =
-        pvalue->m_pCountedPattern ? pvalue->m_pCountedPattern->get() : nullptr;
-    if (pPattern) {
-      auto* pPageData = CPDF_DocPageData::FromDocument(pPattern->document());
-      if (pPageData)
-        pPageData->ReleasePattern(pPattern->pattern_obj());
-    }
-  }
-  FX_Free(m_pBuffer);
-  m_pBuffer = nullptr;
-}
-
 bool CPDF_Color::IsPatternInternal() const {
   return m_pCS->GetFamily() == PDFCS_PATTERN;
 }
 
 void CPDF_Color::SetColorSpace(const RetainPtr<CPDF_ColorSpace>& pCS) {
-  ASSERT(pCS);
-  if (m_pCS == pCS) {
-    if (!m_pBuffer)
-      m_pBuffer = pCS->CreateBuf();
-    m_pCS = pCS;
-    return;
-  }
-  ReleaseBuffer();
   m_pCS = pCS;
-  if (IsPatternInternal())
-    m_pBuffer = pCS->CreateBuf();
-  else
-    m_pBuffer = pCS->CreateBufAndSetDefaultColor();
+  if (IsPatternInternal()) {
+    m_Buffer.clear();
+    m_pValue = pdfium::MakeUnique<PatternValue>();
+  } else {
+    m_Buffer = pCS->CreateBufAndSetDefaultColor();
+    m_pValue.reset();
+  }
 }
 
 void CPDF_Color::SetValueForNonPattern(const std::vector<float>& values) {
-  ASSERT(m_pBuffer);
   ASSERT(!IsPatternInternal());
   ASSERT(m_pCS->CountComponents() <= values.size());
-  memcpy(m_pBuffer, values.data(), m_pCS->CountComponents() * sizeof(float));
+  m_Buffer = values;
 }
 
-void CPDF_Color::SetValueForPattern(CPDF_Pattern* pPattern,
+void CPDF_Color::SetValueForPattern(const RetainPtr<CPDF_Pattern>& pPattern,
                                     const std::vector<float>& values) {
   if (values.size() > kMaxPatternColorComps)
     return;
 
-  if (!IsPattern()) {
-    FX_Free(m_pBuffer);
-    m_pCS = CPDF_ColorSpace::GetStockCS(PDFCS_PATTERN);
-    m_pBuffer = m_pCS->CreateBuf();
-  }
+  if (!IsPattern())
+    SetColorSpace(CPDF_ColorSpace::GetStockCS(PDFCS_PATTERN));
 
-  CPDF_DocPageData* pDocPageData = nullptr;
-  PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
-  if (pvalue->m_pPattern) {
-    pDocPageData =
-        CPDF_DocPageData::FromDocument(pvalue->m_pPattern->document());
-    pDocPageData->ReleasePattern(pvalue->m_pPattern->pattern_obj());
-  }
-  pvalue->m_nComps = values.size();
-  pvalue->m_pPattern = pPattern;
-  if (!values.empty())
-    memcpy(pvalue->m_Comps, values.data(), values.size() * sizeof(float));
-
-  pvalue->m_pCountedPattern = nullptr;
-  if (pPattern) {
-    if (!pDocPageData)
-      pDocPageData = CPDF_DocPageData::FromDocument(pPattern->document());
-
-    pvalue->m_pCountedPattern =
-        pDocPageData->FindPatternPtr(pPattern->pattern_obj());
-  }
+  m_pValue->SetPattern(pPattern);
+  m_pValue->SetComps(values);
 }
 
 CPDF_Color& CPDF_Color::operator=(const CPDF_Color& that) {
   if (this == &that)
     return *this;
 
-  ReleaseBuffer();
+  m_Buffer = that.m_Buffer;
+  m_pValue = that.m_pValue ? pdfium::MakeUnique<PatternValue>(*that.m_pValue)
+                           : nullptr;
   m_pCS = that.m_pCS;
-  if (!m_pCS)
-    return *this;
-
-  CPDF_Document* pDoc = m_pCS->GetDocument();
-  const CPDF_Array* pArray = m_pCS->GetArray();
-  if (pDoc && pArray) {
-    m_pCS = CPDF_DocPageData::FromDocument(pDoc)->GetCopiedColorSpace(pArray);
-    if (!m_pCS)
-      return *this;
-  }
-  m_pBuffer = m_pCS->CreateBuf();
-  memcpy(m_pBuffer, that.m_pBuffer, m_pCS->GetBufSize());
-  if (!IsPatternInternal())
-    return *this;
-
-  PatternValue* pValue = reinterpret_cast<PatternValue*>(m_pBuffer);
-  CPDF_Pattern* pPattern = pValue->m_pPattern;
-  if (!pPattern)
-    return *this;
-
-  pValue->m_pPattern = CPDF_DocPageData::FromDocument(pPattern->document())
-                           ->GetPattern(pPattern->pattern_obj(), false,
-                                        pPattern->parent_matrix());
-
   return *this;
 }
 
@@ -146,19 +77,18 @@
 }
 
 bool CPDF_Color::GetRGB(int* R, int* G, int* B) const {
-  if (!m_pBuffer)
-    return false;
-
   float r = 0.0f;
   float g = 0.0f;
   float b = 0.0f;
-  bool result;
+  bool result = false;
   if (IsPatternInternal()) {
-    const CPDF_PatternCS* pPatternCS = m_pCS->AsPatternCS();
-    const auto* pValue = reinterpret_cast<const PatternValue*>(m_pBuffer);
-    result = pPatternCS->GetPatternRGB(*pValue, &r, &g, &b);
+    if (m_pValue) {
+      const CPDF_PatternCS* pPatternCS = m_pCS->AsPatternCS();
+      result = pPatternCS->GetPatternRGB(*m_pValue, &r, &g, &b);
+    }
   } else {
-    result = m_pCS->GetRGB(m_pBuffer, &r, &g, &b);
+    if (!m_Buffer.empty())
+      result = m_pCS->GetRGB(m_Buffer.data(), &r, &g, &b);
   }
   if (!result)
     return false;
@@ -171,10 +101,5 @@
 
 CPDF_Pattern* CPDF_Color::GetPattern() const {
   ASSERT(IsPattern());
-
-  if (!m_pBuffer)
-    return nullptr;
-
-  PatternValue* pvalue = reinterpret_cast<PatternValue*>(m_pBuffer);
-  return pvalue->m_pPattern;
+  return m_pValue ? m_pValue->GetPattern() : nullptr;
 }
diff --git a/core/fpdfapi/page/cpdf_color.h b/core/fpdfapi/page/cpdf_color.h
index d3cfc94..b533d08 100644
--- a/core/fpdfapi/page/cpdf_color.h
+++ b/core/fpdfapi/page/cpdf_color.h
@@ -7,6 +7,7 @@
 #ifndef CORE_FPDFAPI_PAGE_CPDF_COLOR_H_
 #define CORE_FPDFAPI_PAGE_CPDF_COLOR_H_
 
+#include <memory>
 #include <vector>
 
 #include "core/fxcrt/fx_system.h"
@@ -14,6 +15,7 @@
 
 class CPDF_ColorSpace;
 class CPDF_Pattern;
+class PatternValue;
 
 class CPDF_Color {
  public:
@@ -24,11 +26,11 @@
 
   CPDF_Color& operator=(const CPDF_Color& that);
 
-  bool IsNull() const { return !m_pBuffer; }
+  bool IsNull() const { return m_Buffer.empty() && !m_pValue; }
   bool IsPattern() const;
   void SetColorSpace(const RetainPtr<CPDF_ColorSpace>& pCS);
   void SetValueForNonPattern(const std::vector<float>& values);
-  void SetValueForPattern(CPDF_Pattern* pPattern,
+  void SetValueForPattern(const RetainPtr<CPDF_Pattern>& pPattern,
                           const std::vector<float>& values);
   uint32_t CountComponents() const;
   bool IsColorSpaceRGB() const;
@@ -38,13 +40,10 @@
   CPDF_Pattern* GetPattern() const;
 
  protected:
-  void ReleaseBuffer();
   bool IsPatternInternal() const;
 
-  // TODO(thestig): Convert this to a smart pointer or vector.
-  // |m_pBuffer| is created by |m_pCS|, so if it is non-null, then so is
-  // |m_pCS|, since SetColorSpace() prohibits setting to null.
-  float* m_pBuffer = nullptr;
+  std::vector<float> m_Buffer;             // Used for non-pattern colorspaces.
+  std::unique_ptr<PatternValue> m_pValue;  // Used for pattern colorspaces.
   RetainPtr<CPDF_ColorSpace> m_pCS;
 };
 
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 338025f..95af192 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -442,6 +442,12 @@
 
 }  // namespace
 
+PatternValue::PatternValue() = default;
+
+PatternValue::PatternValue(const PatternValue& that) = default;
+
+PatternValue::~PatternValue() = default;
+
 // static
 RetainPtr<CPDF_ColorSpace> CPDF_ColorSpace::ColorspaceFromName(
     const ByteString& name) {
@@ -567,24 +573,15 @@
   }
 }
 
-size_t CPDF_ColorSpace::GetBufSize() const {
-  if (m_Family == PDFCS_PATTERN)
-    return sizeof(PatternValue);
-  return m_nComponents * sizeof(float);
-}
-
-float* CPDF_ColorSpace::CreateBuf() const {
-  return reinterpret_cast<float*>(FX_Alloc(uint8_t, GetBufSize()));
-}
-
-float* CPDF_ColorSpace::CreateBufAndSetDefaultColor() const {
+std::vector<float> CPDF_ColorSpace::CreateBufAndSetDefaultColor() const {
   ASSERT(m_Family != PDFCS_PATTERN);
 
-  float* buf = CreateBuf();
   float min;
   float max;
+  std::vector<float> buf(m_nComponents);
   for (uint32_t i = 0; i < m_nComponents; i++)
     GetDefaultValue(i, &buf[i], &min, &max);
+
   return buf;
 }
 
diff --git a/core/fpdfapi/page/cpdf_colorspace.h b/core/fpdfapi/page/cpdf_colorspace.h
index d3057fc..d86747b 100644
--- a/core/fpdfapi/page/cpdf_colorspace.h
+++ b/core/fpdfapi/page/cpdf_colorspace.h
@@ -9,6 +9,7 @@
 
 #include <memory>
 #include <set>
+#include <vector>
 
 #include "core/fpdfapi/page/cpdf_pattern.h"
 #include "core/fxcrt/fx_string.h"
@@ -16,6 +17,7 @@
 #include "core/fxcrt/observed_ptr.h"
 #include "core/fxcrt/retain_ptr.h"
 #include "core/fxcrt/unowned_ptr.h"
+#include "third_party/base/span.h"
 
 #define PDFCS_DEVICEGRAY 1
 #define PDFCS_DEVICERGB 2
@@ -36,11 +38,23 @@
 
 constexpr size_t kMaxPatternColorComps = 16;
 
-struct PatternValue {
-  CPDF_Pattern* m_pPattern;
-  CPDF_CountedPattern* m_pCountedPattern;
-  int m_nComps;
-  float m_Comps[kMaxPatternColorComps];
+class PatternValue {
+ public:
+  PatternValue();
+  PatternValue(const PatternValue& that);
+  ~PatternValue();
+
+  pdfium::span<const float> GetComps() const { return m_Comps; }
+  void SetComps(const std::vector<float>& comps) { m_Comps = comps; }
+
+  CPDF_Pattern* GetPattern() const { return m_pRetainedPattern.Get(); }
+  void SetPattern(const RetainPtr<CPDF_Pattern>& pPattern) {
+    m_pRetainedPattern = pPattern;
+  }
+
+ private:
+  std::vector<float> m_Comps;
+  RetainPtr<CPDF_Pattern> m_pRetainedPattern;
 };
 
 class CPDF_ColorSpace : public Retainable, public Observable {
@@ -57,11 +71,9 @@
 
   const CPDF_Array* GetArray() const { return m_pArray.Get(); }
   CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
-  size_t GetBufSize() const;
-  float* CreateBuf() const;
 
   // Should only be called if this colorspace is not a pattern.
-  float* CreateBufAndSetDefaultColor() const;
+  std::vector<float> CreateBufAndSetDefaultColor() const;
 
   uint32_t CountComponents() const;
   int GetFamily() const { return m_Family; }
diff --git a/core/fpdfapi/page/cpdf_colorstate.cpp b/core/fpdfapi/page/cpdf_colorstate.cpp
index 706e5c9..50dbb71 100644
--- a/core/fpdfapi/page/cpdf_colorstate.cpp
+++ b/core/fpdfapi/page/cpdf_colorstate.cpp
@@ -82,13 +82,13 @@
   SetColor(pCS, values, &pData->m_StrokeColor, &pData->m_StrokeColorRef);
 }
 
-void CPDF_ColorState::SetFillPattern(CPDF_Pattern* pPattern,
+void CPDF_ColorState::SetFillPattern(const RetainPtr<CPDF_Pattern>& pPattern,
                                      const std::vector<float>& values) {
   ColorData* pData = m_Ref.GetPrivateCopy();
   SetPattern(pPattern, values, &pData->m_FillColor, &pData->m_FillColorRef);
 }
 
-void CPDF_ColorState::SetStrokePattern(CPDF_Pattern* pPattern,
+void CPDF_ColorState::SetStrokePattern(const RetainPtr<CPDF_Pattern>& pPattern,
                                        const std::vector<float>& values) {
   ColorData* pData = m_Ref.GetPrivateCopy();
   SetPattern(pPattern, values, &pData->m_StrokeColor, &pData->m_StrokeColorRef);
@@ -117,7 +117,7 @@
   *colorref = color->GetRGB(&R, &G, &B) ? FXSYS_BGR(B, G, R) : 0xFFFFFFFF;
 }
 
-void CPDF_ColorState::SetPattern(CPDF_Pattern* pPattern,
+void CPDF_ColorState::SetPattern(const RetainPtr<CPDF_Pattern>& pPattern,
                                  const std::vector<float>& values,
                                  CPDF_Color* color,
                                  FX_COLORREF* colorref) {
diff --git a/core/fpdfapi/page/cpdf_colorstate.h b/core/fpdfapi/page/cpdf_colorstate.h
index 43244cc..f0f6ebd 100644
--- a/core/fpdfapi/page/cpdf_colorstate.h
+++ b/core/fpdfapi/page/cpdf_colorstate.h
@@ -46,8 +46,9 @@
                     const std::vector<float>& values);
   void SetStrokeColor(const RetainPtr<CPDF_ColorSpace>& pCS,
                       const std::vector<float>& values);
-  void SetFillPattern(CPDF_Pattern* pattern, const std::vector<float>& values);
-  void SetStrokePattern(CPDF_Pattern* pattern,
+  void SetFillPattern(const RetainPtr<CPDF_Pattern>& pattern,
+                      const std::vector<float>& values);
+  void SetStrokePattern(const RetainPtr<CPDF_Pattern>& pattern,
                         const std::vector<float>& values);
 
   bool HasRef() const { return !!m_Ref; }
@@ -77,7 +78,7 @@
                 const std::vector<float>& values,
                 CPDF_Color* color,
                 FX_COLORREF* colorref);
-  void SetPattern(CPDF_Pattern* pPattern,
+  void SetPattern(const RetainPtr<CPDF_Pattern>& pPattern,
                   const std::vector<float>& values,
                   CPDF_Color* color,
                   FX_COLORREF* colorref);
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index a2ed1b9..bbd9ab3 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -167,12 +167,11 @@
   Clear(false);
   Clear(true);
 
-  for (auto& it : m_PatternMap)
-    delete it.second;
   m_PatternMap.clear();
 
   for (auto& it : m_FontMap)
     delete it.second;
+
   m_FontMap.clear();
 }
 
@@ -183,29 +182,7 @@
 void CPDF_DocPageData::Clear(bool bForceRelease) {
   m_bForceClear = bForceRelease;
 
-  // This is needed because if |bForceRelease| is true we will destroy any
-  // pattern we see regardless of the ref-count. The tiling pattern owns a
-  // Form object which owns a ShadingObject. The ShadingObject has an unowned
-  // pointer to a ShadingPattern. The ShadingPattern is owned by the
-  // DocPageData. So, we loop through and clear any tiling patterns before we
-  // do the same for any shading patterns, otherwise we may free the
-  // ShadingPattern before the ShadingObject and trigger an unowned pointer
-  // probe warning.
-  for (auto& it : m_PatternMap) {
-    CPDF_CountedPattern* ptData = it.second;
-    if (!ptData->get() || !ptData->get()->AsTilingPattern())
-      continue;
-    if (bForceRelease || ptData->use_count() < 2)
-      ptData->clear();
-  }
-
-  for (auto& it : m_PatternMap) {
-    CPDF_CountedPattern* ptData = it.second;
-    if (!ptData->get())
-      continue;
-    if (bForceRelease || ptData->use_count() < 2)
-      ptData->clear();
-  }
+  m_PatternMap.clear();
 
   for (auto& it : m_FontMap) {
     CPDF_CountedFont* fontData = it.second;
@@ -431,23 +408,19 @@
   return pdfium::WrapRetain(it->second.Get());
 }
 
-CPDF_Pattern* CPDF_DocPageData::GetPattern(CPDF_Object* pPatternObj,
-                                           bool bShading,
-                                           const CFX_Matrix& matrix) {
+RetainPtr<CPDF_Pattern> CPDF_DocPageData::GetPattern(CPDF_Object* pPatternObj,
+                                                     bool bShading,
+                                                     const CFX_Matrix& matrix) {
   if (!pPatternObj)
     return nullptr;
 
-  CPDF_CountedPattern* ptData = nullptr;
   auto it = m_PatternMap.find(pPatternObj);
-  if (it != m_PatternMap.end()) {
-    ptData = it->second;
-    if (ptData->get()) {
-      return ptData->AddRef();
-    }
-  }
-  std::unique_ptr<CPDF_Pattern> pPattern;
+  if (it != m_PatternMap.end() && it->second)
+    return pdfium::WrapRetain(it->second.Get());
+
+  RetainPtr<CPDF_Pattern> pPattern;
   if (bShading) {
-    pPattern = pdfium::MakeUnique<CPDF_ShadingPattern>(
+    pPattern = pdfium::MakeRetain<CPDF_ShadingPattern>(
         GetDocument(), pPatternObj, true, matrix);
   } else {
     CPDF_Dictionary* pDict = pPatternObj->GetDict();
@@ -456,43 +429,18 @@
 
     int type = pDict->GetIntegerFor("PatternType");
     if (type == CPDF_Pattern::kTiling) {
-      pPattern = pdfium::MakeUnique<CPDF_TilingPattern>(GetDocument(),
+      pPattern = pdfium::MakeRetain<CPDF_TilingPattern>(GetDocument(),
                                                         pPatternObj, matrix);
     } else if (type == CPDF_Pattern::kShading) {
-      pPattern = pdfium::MakeUnique<CPDF_ShadingPattern>(
+      pPattern = pdfium::MakeRetain<CPDF_ShadingPattern>(
           GetDocument(), pPatternObj, false, matrix);
     } else {
       return nullptr;
     }
   }
 
-  if (ptData) {
-    ptData->reset(std::move(pPattern));
-  } else {
-    ptData = new CPDF_CountedPattern(std::move(pPattern));
-    m_PatternMap[pPatternObj] = ptData;
-  }
-  return ptData->AddRef();
-}
-
-void CPDF_DocPageData::ReleasePattern(const CPDF_Object* pPatternObj) {
-  if (!pPatternObj)
-    return;
-
-  auto it = m_PatternMap.find(pPatternObj);
-  if (it == m_PatternMap.end())
-    return;
-
-  CPDF_CountedPattern* pPattern = it->second;
-  if (!pPattern->get())
-    return;
-
-  pPattern->RemoveRef();
-  if (pPattern->use_count() > 1)
-    return;
-
-  // We have item only in m_PatternMap cache. Clean it.
-  pPattern->clear();
+  m_PatternMap[pPatternObj].Reset(pPattern.Get());
+  return pPattern;
 }
 
 RetainPtr<CPDF_Image> CPDF_DocPageData::GetImage(uint32_t dwStreamObjNum) {
@@ -596,15 +544,6 @@
   return pdfium::WrapRetain(it->second.Get());
 }
 
-CPDF_CountedPattern* CPDF_DocPageData::FindPatternPtr(
-    const CPDF_Object* pPatternObj) const {
-  if (!pPatternObj)
-    return nullptr;
-
-  auto it = m_PatternMap.find(pPatternObj);
-  return it != m_PatternMap.end() ? it->second : nullptr;
-}
-
 CPDF_Font* CPDF_DocPageData::AddStandardFont(
     const char* font,
     const CPDF_FontEncoding* pEncoding) {
diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index 9140372..d25fcdc 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -11,6 +11,7 @@
 #include <set>
 
 #include "core/fpdfapi/page/cpdf_colorspace.h"
+#include "core/fpdfapi/page/cpdf_countedobject.h"
 #include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fxcrt/fx_coordinates.h"
 #include "core/fxcrt/fx_string.h"
@@ -70,10 +71,9 @@
 
   RetainPtr<CPDF_ColorSpace> GetCopiedColorSpace(const CPDF_Object* pCSObj);
 
-  CPDF_Pattern* GetPattern(CPDF_Object* pPatternObj,
-                           bool bShading,
-                           const CFX_Matrix& matrix);
-  void ReleasePattern(const CPDF_Object* pPatternObj);
+  RetainPtr<CPDF_Pattern> GetPattern(CPDF_Object* pPatternObj,
+                                     bool bShading,
+                                     const CFX_Matrix& matrix);
 
   RetainPtr<CPDF_Image> GetImage(uint32_t dwStreamObjNum);
   void MaybePurgeImage(uint32_t dwStreamObjNum);
@@ -82,7 +82,6 @@
   void MaybePurgeIccProfile(const CPDF_Stream* pProfileStream);
 
   RetainPtr<CPDF_ColorSpace> FindColorSpacePtr(const CPDF_Object* pCSObj) const;
-  CPDF_CountedPattern* FindPatternPtr(const CPDF_Object* pPatternObj) const;
 
  private:
   using CPDF_CountedFont = CPDF_CountedObject<CPDF_Font>;
@@ -112,7 +111,7 @@
   std::map<const CPDF_Dictionary*, CPDF_CountedFont*> m_FontMap;
   std::map<const CPDF_Stream*, RetainPtr<CPDF_IccProfile>> m_IccProfileMap;
   std::map<uint32_t, RetainPtr<CPDF_Image>> m_ImageMap;
-  std::map<const CPDF_Object*, CPDF_CountedPattern*> m_PatternMap;
+  std::map<const CPDF_Object*, ObservedPtr<CPDF_Pattern>> m_PatternMap;
 };
 
 #endif  // CORE_FPDFAPI_PAGE_CPDF_DOCPAGEDATA_H_
diff --git a/core/fpdfapi/page/cpdf_pattern.cpp b/core/fpdfapi/page/cpdf_pattern.cpp
index 84e641a..36ecc98 100644
--- a/core/fpdfapi/page/cpdf_pattern.cpp
+++ b/core/fpdfapi/page/cpdf_pattern.cpp
@@ -18,6 +18,14 @@
 
 CPDF_Pattern::~CPDF_Pattern() = default;
 
+CPDF_TilingPattern* CPDF_Pattern::AsTilingPattern() {
+  return nullptr;
+}
+
+CPDF_ShadingPattern* CPDF_Pattern::AsShadingPattern() {
+  return nullptr;
+}
+
 void CPDF_Pattern::SetPatternToFormMatrix() {
   const CPDF_Dictionary* pDict = pattern_obj()->GetDict();
   m_Pattern2Form = pDict->GetMatrixFor("Matrix") * m_ParentMatrix;
diff --git a/core/fpdfapi/page/cpdf_pattern.h b/core/fpdfapi/page/cpdf_pattern.h
index 3882b81..7e3eb7b 100644
--- a/core/fpdfapi/page/cpdf_pattern.h
+++ b/core/fpdfapi/page/cpdf_pattern.h
@@ -7,9 +7,9 @@
 #ifndef CORE_FPDFAPI_PAGE_CPDF_PATTERN_H_
 #define CORE_FPDFAPI_PAGE_CPDF_PATTERN_H_
 
-#include "core/fpdfapi/page/cpdf_countedobject.h"
 #include "core/fxcrt/fx_coordinates.h"
 #include "core/fxcrt/fx_system.h"
+#include "core/fxcrt/observed_ptr.h"
 #include "core/fxcrt/retain_ptr.h"
 #include "core/fxcrt/unowned_ptr.h"
 
@@ -18,15 +18,15 @@
 class CPDF_ShadingPattern;
 class CPDF_TilingPattern;
 
-class CPDF_Pattern {
+class CPDF_Pattern : public Retainable, public Observable {
  public:
   // Values used in PDFs. Do not change.
   enum PatternType { kTiling = 1, kShading = 2 };
 
-  virtual ~CPDF_Pattern();
+  ~CPDF_Pattern() override;
 
-  virtual CPDF_TilingPattern* AsTilingPattern() = 0;
-  virtual CPDF_ShadingPattern* AsShadingPattern() = 0;
+  virtual CPDF_TilingPattern* AsTilingPattern();
+  virtual CPDF_ShadingPattern* AsShadingPattern();
 
   // All the getters that return pointers return non-NULL pointers.
   CPDF_Document* document() const { return m_pDocument.Get(); }
@@ -47,6 +47,5 @@
   CFX_Matrix m_Pattern2Form;
   const CFX_Matrix m_ParentMatrix;
 };
-using CPDF_CountedPattern = CPDF_CountedObject<CPDF_Pattern>;
 
 #endif  // CORE_FPDFAPI_PAGE_CPDF_PATTERN_H_
diff --git a/core/fpdfapi/page/cpdf_patterncs.cpp b/core/fpdfapi/page/cpdf_patterncs.cpp
index cd8a921..1c5dc6c 100644
--- a/core/fpdfapi/page/cpdf_patterncs.cpp
+++ b/core/fpdfapi/page/cpdf_patterncs.cpp
@@ -60,7 +60,7 @@
                                    float* R,
                                    float* G,
                                    float* B) const {
-  if (m_pBaseCS && m_pBaseCS->GetRGB(value.m_Comps, R, G, B))
+  if (m_pBaseCS && m_pBaseCS->GetRGB(value.GetComps().data(), R, G, B))
     return true;
 
   *R = 0.75f;
diff --git a/core/fpdfapi/page/cpdf_shadingobject.h b/core/fpdfapi/page/cpdf_shadingobject.h
index c246870..072a025 100644
--- a/core/fpdfapi/page/cpdf_shadingobject.h
+++ b/core/fpdfapi/page/cpdf_shadingobject.h
@@ -9,7 +9,7 @@
 
 #include "core/fpdfapi/page/cpdf_pageobject.h"
 #include "core/fxcrt/fx_coordinates.h"
-#include "core/fxcrt/unowned_ptr.h"
+#include "core/fxcrt/retain_ptr.h"
 
 class CPDF_ShadingPattern;
 
@@ -33,7 +33,7 @@
   const CFX_Matrix& matrix() const { return m_Matrix; }
 
  private:
-  UnownedPtr<const CPDF_ShadingPattern> m_pShading;
+  RetainPtr<CPDF_ShadingPattern> m_pShading;
   CFX_Matrix m_Matrix;
 };
 
diff --git a/core/fpdfapi/page/cpdf_shadingpattern.cpp b/core/fpdfapi/page/cpdf_shadingpattern.cpp
index f3da1ac..86f5106 100644
--- a/core/fpdfapi/page/cpdf_shadingpattern.cpp
+++ b/core/fpdfapi/page/cpdf_shadingpattern.cpp
@@ -38,10 +38,6 @@
 
 CPDF_ShadingPattern::~CPDF_ShadingPattern() = default;
 
-CPDF_TilingPattern* CPDF_ShadingPattern::AsTilingPattern() {
-  return nullptr;
-}
-
 CPDF_ShadingPattern* CPDF_ShadingPattern::AsShadingPattern() {
   return this;
 }
diff --git a/core/fpdfapi/page/cpdf_shadingpattern.h b/core/fpdfapi/page/cpdf_shadingpattern.h
index 2d8fc8e..392aa27 100644
--- a/core/fpdfapi/page/cpdf_shadingpattern.h
+++ b/core/fpdfapi/page/cpdf_shadingpattern.h
@@ -38,13 +38,12 @@
 
 class CPDF_ShadingPattern final : public CPDF_Pattern {
  public:
-  CPDF_ShadingPattern(CPDF_Document* pDoc,
-                      CPDF_Object* pPatternObj,
-                      bool bShading,
-                      const CFX_Matrix& parentMatrix);
+  template <typename T, typename... Args>
+  friend RetainPtr<T> pdfium::MakeRetain(Args&&... args);
+
   ~CPDF_ShadingPattern() override;
 
-  CPDF_TilingPattern* AsTilingPattern() override;
+  // CPDF_Pattern:
   CPDF_ShadingPattern* AsShadingPattern() override;
 
   bool IsMeshShading() const {
@@ -64,6 +63,13 @@
   }
 
  private:
+  CPDF_ShadingPattern(CPDF_Document* pDoc,
+                      CPDF_Object* pPatternObj,
+                      bool bShading,
+                      const CFX_Matrix& parentMatrix);
+  CPDF_ShadingPattern(const CPDF_ShadingPattern&) = delete;
+  CPDF_ShadingPattern& operator=(const CPDF_ShadingPattern&) = delete;
+
   // Constraints in PDF 1.7 spec, 4.6.3 Shading Patterns, pages 308-331.
   bool Validate() const;
   bool ValidateFunctions(uint32_t nExpectedNumFunctions,
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index b72b2bc..6eba6dd 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -1063,7 +1063,7 @@
 
   // A valid |pLastParam| implies |m_ParamCount| > 0, so GetNamedColors() call
   // below is safe.
-  CPDF_Pattern* pPattern = FindPattern(GetString(0), false);
+  RetainPtr<CPDF_Pattern> pPattern = FindPattern(GetString(0), false);
   if (pPattern)
     m_pCurStates->m_ColorState.SetFillPattern(pPattern, GetNamedColors());
 }
@@ -1080,13 +1080,13 @@
 
   // A valid |pLastParam| implies |m_ParamCount| > 0, so GetNamedColors() call
   // below is safe.
-  CPDF_Pattern* pPattern = FindPattern(GetString(0), false);
+  RetainPtr<CPDF_Pattern> pPattern = FindPattern(GetString(0), false);
   if (pPattern)
     m_pCurStates->m_ColorState.SetStrokePattern(pPattern, GetNamedColors());
 }
 
 void CPDF_StreamContentParser::Handle_ShadeFill() {
-  CPDF_Pattern* pPattern = FindPattern(GetString(0), true);
+  RetainPtr<CPDF_Pattern> pPattern = FindPattern(GetString(0), true);
   if (!pPattern)
     return;
 
@@ -1202,8 +1202,9 @@
       ->GetColorSpace(pCSObj, nullptr);
 }
 
-CPDF_Pattern* CPDF_StreamContentParser::FindPattern(const ByteString& name,
-                                                    bool bShading) {
+RetainPtr<CPDF_Pattern> CPDF_StreamContentParser::FindPattern(
+    const ByteString& name,
+    bool bShading) {
   CPDF_Object* pPattern =
       FindResourceObj(bShading ? "Shading" : "Pattern", name);
   if (!pPattern || (!pPattern->IsDictionary() && !pPattern->IsStream())) {
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h
index 45cf0ba..6e4b46f 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.h
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.h
@@ -118,7 +118,7 @@
                         bool bText,
                         bool bGraph);
   RetainPtr<CPDF_ColorSpace> FindColorSpace(const ByteString& name);
-  CPDF_Pattern* FindPattern(const ByteString& name, bool bShading);
+  RetainPtr<CPDF_Pattern> FindPattern(const ByteString& name, bool bShading);
   CPDF_Dictionary* FindResourceHolder(const ByteString& type);
   CPDF_Object* FindResourceObj(const ByteString& type, const ByteString& name);
 
diff --git a/core/fpdfapi/page/cpdf_tilingpattern.cpp b/core/fpdfapi/page/cpdf_tilingpattern.cpp
index 9bf9a86..eca6478d 100644
--- a/core/fpdfapi/page/cpdf_tilingpattern.cpp
+++ b/core/fpdfapi/page/cpdf_tilingpattern.cpp
@@ -29,10 +29,6 @@
   return this;
 }
 
-CPDF_ShadingPattern* CPDF_TilingPattern::AsShadingPattern() {
-  return nullptr;
-}
-
 std::unique_ptr<CPDF_Form> CPDF_TilingPattern::Load(CPDF_PageObject* pPageObj) {
   const CPDF_Dictionary* pDict = pattern_obj()->GetDict();
   m_bColored = pDict->GetIntegerFor("PaintType") == 1;
diff --git a/core/fpdfapi/page/cpdf_tilingpattern.h b/core/fpdfapi/page/cpdf_tilingpattern.h
index 35dd9e0..134da8e 100644
--- a/core/fpdfapi/page/cpdf_tilingpattern.h
+++ b/core/fpdfapi/page/cpdf_tilingpattern.h
@@ -20,13 +20,13 @@
 
 class CPDF_TilingPattern final : public CPDF_Pattern {
  public:
-  CPDF_TilingPattern(CPDF_Document* pDoc,
-                     CPDF_Object* pPatternObj,
-                     const CFX_Matrix& parentMatrix);
+  template <typename T, typename... Args>
+  friend RetainPtr<T> pdfium::MakeRetain(Args&&... args);
+
   ~CPDF_TilingPattern() override;
 
+  // CPDF_Pattern:
   CPDF_TilingPattern* AsTilingPattern() override;
-  CPDF_ShadingPattern* AsShadingPattern() override;
 
   std::unique_ptr<CPDF_Form> Load(CPDF_PageObject* pPageObj);
 
@@ -36,6 +36,12 @@
   float y_step() const { return m_YStep; }
 
  private:
+  CPDF_TilingPattern(CPDF_Document* pDoc,
+                     CPDF_Object* pPatternObj,
+                     const CFX_Matrix& parentMatrix);
+  CPDF_TilingPattern(const CPDF_TilingPattern&) = delete;
+  CPDF_TilingPattern& operator=(const CPDF_TilingPattern&) = delete;
+
   bool m_bColored;
   CFX_FloatRect m_BBox;
   float m_XStep;
diff --git a/core/fpdfapi/render/cpdf_imagerenderer.cpp b/core/fpdfapi/render/cpdf_imagerenderer.cpp
index 865f72a..ee88794 100644
--- a/core/fpdfapi/render/cpdf_imagerenderer.cpp
+++ b/core/fpdfapi/render/cpdf_imagerenderer.cpp
@@ -96,7 +96,7 @@
   if (m_pDIBBase->IsAlphaMask()) {
     const CPDF_Color* pColor = m_pImageObject->m_ColorState.GetFillColor();
     if (pColor && pColor->IsPattern()) {
-      m_pPattern = pColor->GetPattern();
+      m_pPattern.Reset(pColor->GetPattern());
       if (m_pPattern)
         m_bPatternColor = true;
     }
diff --git a/core/fpdfapi/render/cpdf_imagerenderer.h b/core/fpdfapi/render/cpdf_imagerenderer.h
index bfedfc1..d7c7bf0 100644
--- a/core/fpdfapi/render/cpdf_imagerenderer.h
+++ b/core/fpdfapi/render/cpdf_imagerenderer.h
@@ -85,7 +85,7 @@
 
   UnownedPtr<CPDF_RenderStatus> m_pRenderStatus;
   UnownedPtr<CPDF_ImageObject> m_pImageObject;
-  UnownedPtr<CPDF_Pattern> m_pPattern;
+  RetainPtr<CPDF_Pattern> m_pPattern;
   RetainPtr<CFX_DIBBase> m_pDIBBase;
   CFX_Matrix m_mtObj2Device;
   CFX_Matrix m_ImageMatrix;