From 17103b84ebde9ab2b05dff38d473b5d44f723ff2 Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 21 Sep 2016 13:30:29 -0700 Subject: 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 --- core/fpdfapi/fpdf_page/cpdf_contentmark.cpp | 25 +++++------- core/fpdfapi/fpdf_page/cpdf_contentmarkitem.cpp | 51 ++++++++++++++++--------- core/fpdfapi/fpdf_page/cpdf_contentmarkitem.h | 23 +++++++---- core/fpdftext/cpdf_textpage.cpp | 12 ++---- 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>( + 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> 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 + +#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> 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> + 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"); } -- cgit v1.2.3