Make ownership explicit in CPDF_ContentMarkItem.

The old SetParam() method had "maybe take ownership" semanitcs
based upon the type argument.

Make GetParam() handle the None case and simplify callers
based upon that behaviour.

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

4 files changed, 61 insertions, 50 deletions

diff --git a/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp b/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
index 99a16004cd..1c47f00182 100644
--- a/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
@@ -66,11 +66,7 @@ bool CPDF_ContentMark::LookupMark(const CFX_ByteStringC& mark,
   for (int i = 0; i < pData->CountItems(); i++) {
     const CPDF_ContentMarkItem& item = pData->GetItem(i);
     if (item.GetName() == mark) {
-      pDict = nullptr;
-      if (item.GetParamType() == CPDF_ContentMarkItem::PropertiesDict ||
-          item.GetParamType() == CPDF_ContentMarkItem::DirectDict) {
-        pDict = item.GetParam();
-      }
+      pDict = item.GetParam();
       return true;
     }
   }
@@ -99,13 +95,9 @@ const CPDF_ContentMarkItem& CPDF_ContentMark::MarkData::GetItem(
 
 int CPDF_ContentMark::MarkData::GetMCID() const {
   for (const auto& mark : m_Marks) {
-    CPDF_ContentMarkItem::ParamType type = mark.GetParamType();
-    if (type == CPDF_ContentMarkItem::PropertiesDict ||
-        type == CPDF_ContentMarkItem::DirectDict) {
-      CPDF_Dictionary* pDict = mark.GetParam();
-      if (pDict->KeyExist("MCID"))
-        return pDict->GetIntegerFor("MCID");
-    }
+    CPDF_Dictionary* pDict = mark.GetParam();
+    if (pDict && pDict->KeyExist("MCID"))
+      return pDict->GetIntegerFor("MCID");
   }
   return -1;
 }
@@ -117,13 +109,14 @@ void CPDF_ContentMark::MarkData::AddMark(const CFX_ByteString& name,
   item.SetName(name);
   if (pDict) {
     if (bDirect) {
-      item.SetParam(CPDF_ContentMarkItem::DirectDict,
-                    ToDictionary(pDict->Clone()));
+      item.SetDirectDict(
+          std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>(
+              ToDictionary(pDict->Clone())));
     } else {
-      item.SetParam(CPDF_ContentMarkItem::PropertiesDict, pDict);
+      item.SetPropertiesDict(pDict);
     }
   }
-  m_Marks.push_back(item);
+  m_Marks.push_back(std::move(item));
 }
 
 void CPDF_ContentMark::MarkData::DeleteLastMark() {
diff --git a/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.cpp b/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.cpp
index f44972568d..5e2d21f850 100644
--- a/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.cpp
@@ -9,27 +9,42 @@
 #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
 
 CPDF_ContentMarkItem::CPDF_ContentMarkItem()
-    : m_ParamType(None), m_pParam(nullptr) {}
-
-CPDF_ContentMarkItem::CPDF_ContentMarkItem(const CPDF_ContentMarkItem& src) {
-  m_MarkName = src.m_MarkName;
-  m_ParamType = src.m_ParamType;
-  if (m_ParamType == DirectDict) {
-    m_pParam = ToDictionary(src.m_pParam->Clone());
-  } else {
-    m_pParam = src.m_pParam;
-  }
+    : m_ParamType(None), m_pPropertiesDict(nullptr) {}
+
+CPDF_ContentMarkItem::CPDF_ContentMarkItem(const CPDF_ContentMarkItem& that)
+    : m_MarkName(that.m_MarkName),
+      m_ParamType(that.m_ParamType),
+      m_pPropertiesDict(that.m_pPropertiesDict) {
+  if (that.m_pDirectDict)
+    m_pDirectDict.reset(that.m_pDirectDict->Clone()->AsDictionary());
 }
 
-CPDF_ContentMarkItem::~CPDF_ContentMarkItem() {
-  if (m_ParamType == DirectDict && m_pParam)
-    m_pParam->Release();
+CPDF_ContentMarkItem::~CPDF_ContentMarkItem() {}
+
+CPDF_Dictionary* CPDF_ContentMarkItem::GetParam() const {
+  switch (m_ParamType) {
+    case PropertiesDict:
+      return m_pPropertiesDict;
+    case DirectDict:
+      return m_pDirectDict.get();
+    case None:
+    default:
+      return nullptr;
+  }
 }
 
 FX_BOOL CPDF_ContentMarkItem::HasMCID() const {
-  if (m_pParam &&
-      (m_ParamType == DirectDict || m_ParamType == PropertiesDict)) {
-    return m_pParam->KeyExist("MCID");
-  }
-  return FALSE;
+  CPDF_Dictionary* pDict = GetParam();
+  return pDict && pDict->KeyExist("MCID");
+}
+
+void CPDF_ContentMarkItem::SetDirectDict(
+    std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict) {
+  m_ParamType = DirectDict;
+  m_pDirectDict = std::move(pDict);
+}
+
+void CPDF_ContentMarkItem::SetPropertiesDict(CPDF_Dictionary* pDict) {
+  m_ParamType = PropertiesDict;
+  m_pPropertiesDict = pDict;
 }
diff --git a/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.h b/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.h
index f46592d0a7..d5148af6da 100644
--- a/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.h
+++ b/core/fpdfapi/fpdf_page/cpdf_contentmarkitem.h
@@ -7,6 +7,9 @@
 #ifndef CORE_FPDFAPI_FPDF_PAGE_CPDF_CONTENTMARKITEM_H_
 #define CORE_FPDFAPI_FPDF_PAGE_CPDF_CONTENTMARKITEM_H_
 
+#include <memory>
+
+#include "core/fxcrt/include/fx_memory.h"
 #include "core/fxcrt/include/fx_string.h"
 #include "core/fxcrt/include/fx_system.h"
 
@@ -17,23 +20,27 @@ class CPDF_ContentMarkItem {
   enum ParamType { None, PropertiesDict, DirectDict };
 
   CPDF_ContentMarkItem();
-  CPDF_ContentMarkItem(const CPDF_ContentMarkItem& src);
+  CPDF_ContentMarkItem(const CPDF_ContentMarkItem& that);
   ~CPDF_ContentMarkItem();
 
-  const CFX_ByteString& GetName() const { return m_MarkName; }
+  CPDF_ContentMarkItem& operator=(CPDF_ContentMarkItem&& other) = default;
+
+  CFX_ByteString GetName() const { return m_MarkName; }
   ParamType GetParamType() const { return m_ParamType; }
-  CPDF_Dictionary* GetParam() const { return m_pParam; }
+  CPDF_Dictionary* GetParam() const;
   FX_BOOL HasMCID() const;
+
   void SetName(const CFX_ByteString& name) { m_MarkName = name; }
-  void SetParam(ParamType type, CPDF_Dictionary* param) {
-    m_ParamType = type;
-    m_pParam = param;
-  }
+  void SetDirectDict(
+      std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict);
+  void SetPropertiesDict(CPDF_Dictionary* pDict);
 
  private:
   CFX_ByteString m_MarkName;
   ParamType m_ParamType;
-  CPDF_Dictionary* m_pParam;
+  CPDF_Dictionary* m_pPropertiesDict;  // not owned.
+  std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>
+      m_pDirectDict;
 };
 
 #endif  // CORE_FPDFAPI_FPDF_PAGE_CPDF_CONTENTMARKITEM_H_
diff --git a/core/fpdftext/cpdf_textpage.cpp b/core/fpdftext/cpdf_textpage.cpp
index 1056943292..d88464bfc4 100644
--- a/core/fpdftext/cpdf_textpage.cpp
+++ b/core/fpdftext/cpdf_textpage.cpp
@@ -818,11 +818,10 @@ FPDFText_MarkedContent CPDF_TextPage::PreMarkedContent(PDFTEXT_Obj Obj) {
   int n = 0;
   for (n = 0; n < nContentMark; n++) {
     const CPDF_ContentMarkItem& item = pTextObj->m_ContentMark.GetItem(n);
-    if (item.GetParamType() == CPDF_ContentMarkItem::ParamType::None)
-      continue;
     pDict = item.GetParam();
-    CPDF_String* temp =
-        ToString(pDict ? pDict->GetObjectFor("ActualText") : nullptr);
+    if (!pDict)
+      continue;
+    CPDF_String* temp = ToString(pDict->GetObjectFor("ActualText"));
     if (temp) {
       bExist = true;
       actText = temp->GetUnicodeText();
@@ -877,12 +876,9 @@ void CPDF_TextPage::ProcessMarkedContent(PDFTEXT_Obj Obj) {
     return;
 
   CFX_WideString actText;
-  CPDF_Dictionary* pDict = nullptr;
   for (int n = 0; n < nContentMark; n++) {
     const CPDF_ContentMarkItem& item = pTextObj->m_ContentMark.GetItem(n);
-    if (item.GetParamType() == CPDF_ContentMarkItem::ParamType::None)
-      continue;
-    pDict = item.GetParam();
+    CPDF_Dictionary* pDict = item.GetParam();
     if (pDict)
       actText = pDict->GetUnicodeTextFor("ActualText");
   }