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
diff --git a/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp b/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
index 99a1600..1c47f00 100644
--- a/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_contentmark.cpp
@@ -66,11 +66,7 @@
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 @@
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 @@
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 f449725..5e2d21f 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) {}
+ : m_ParamType(None), m_pPropertiesDict(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;
- }
+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 f46592d..d5148af 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 @@
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 1056943..d88464b 100644
--- a/core/fpdftext/cpdf_textpage.cpp
+++ b/core/fpdftext/cpdf_textpage.cpp
@@ -818,11 +818,10 @@
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 @@
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");
}
