Pass retained arguments to CPDF_Action constructor
Change-Id: Ie8c9d9e19af39975d80a5190b07c6c7e6c3897db
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98530
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_aaction.cpp b/core/fpdfdoc/cpdf_aaction.cpp
index ae0563a..3c9535a 100644
--- a/core/fpdfdoc/cpdf_aaction.cpp
+++ b/core/fpdfdoc/cpdf_aaction.cpp
@@ -54,9 +54,7 @@
}
CPDF_Action CPDF_AAction::GetAction(AActionType eType) const {
- // TOOD(tsepez): pass retained object.
- return CPDF_Action(m_pDict ? m_pDict->GetDictFor(kAATypes[eType]).Get()
- : nullptr);
+ return CPDF_Action(m_pDict ? m_pDict->GetDictFor(kAATypes[eType]) : nullptr);
}
// static
diff --git a/core/fpdfdoc/cpdf_action.cpp b/core/fpdfdoc/cpdf_action.cpp
index 010658b..42909b2 100644
--- a/core/fpdfdoc/cpdf_action.cpp
+++ b/core/fpdfdoc/cpdf_action.cpp
@@ -7,6 +7,7 @@
#include "core/fpdfdoc/cpdf_action.h"
#include <iterator>
+#include <utility>
#include "constants/stream_dict_common.h"
#include "core/fpdfapi/parser/cpdf_array.h"
@@ -26,7 +27,8 @@
} // namespace
-CPDF_Action::CPDF_Action(const CPDF_Dictionary* pDict) : m_pDict(pDict) {}
+CPDF_Action::CPDF_Action(RetainPtr<const CPDF_Dictionary> pDict)
+ : m_pDict(std::move(pDict)) {}
CPDF_Action::CPDF_Action(const CPDF_Action& that) = default;
@@ -112,8 +114,8 @@
return m_pDict->GetIntegerFor("Flags");
}
-std::vector<const CPDF_Object*> CPDF_Action::GetAllFields() const {
- std::vector<const CPDF_Object*> result;
+std::vector<RetainPtr<const CPDF_Object>> CPDF_Action::GetAllFields() const {
+ std::vector<RetainPtr<const CPDF_Object>> result;
if (!m_pDict)
return result;
@@ -125,16 +127,18 @@
return result;
if (pFields->IsDictionary() || pFields->IsString()) {
- // TODO(tsepez): push retained arguments.
- result.push_back(pFields.Get());
- } else if (const CPDF_Array* pArray = pFields->AsArray()) {
- for (size_t i = 0; i < pArray->size(); ++i) {
- RetainPtr<const CPDF_Object> pObj = pArray->GetDirectObjectAt(i);
- if (pObj) {
- // TODO(tsepez): push retained objects.
- result.push_back(pObj.Get());
- }
- }
+ result.push_back(std::move(pFields));
+ return result;
+ }
+
+ const CPDF_Array* pArray = pFields->AsArray();
+ if (!pArray)
+ return result;
+
+ for (size_t i = 0; i < pArray->size(); ++i) {
+ RetainPtr<const CPDF_Object> pObj = pArray->GetDirectObjectAt(i);
+ if (pObj)
+ result.push_back(std::move(pObj));
}
return result;
}
@@ -172,23 +176,20 @@
if (!pNext)
return CPDF_Action(nullptr);
- if (const CPDF_Array* pArray = pNext->AsArray()) {
- // TODO(tsepez): Actions should take retained arguments.
- return CPDF_Action(pArray->GetDictAt(iIndex).Get());
- }
+ if (const CPDF_Array* pArray = pNext->AsArray())
+ return CPDF_Action(pArray->GetDictAt(iIndex));
+
if (const CPDF_Dictionary* pDict = pNext->AsDictionary()) {
if (iIndex == 0)
- return CPDF_Action(pDict);
+ return CPDF_Action(pdfium::WrapRetain(pDict));
}
return CPDF_Action(nullptr);
}
-const CPDF_Object* CPDF_Action::GetJavaScriptObject() const {
+RetainPtr<const CPDF_Object> CPDF_Action::GetJavaScriptObject() const {
if (!m_pDict)
return nullptr;
RetainPtr<const CPDF_Object> pJS = m_pDict->GetDirectObjectFor("JS");
-
- // TODO(tsepez): return retained references.
- return (pJS && (pJS->IsString() || pJS->IsStream())) ? pJS.Get() : nullptr;
+ return (pJS && (pJS->IsString() || pJS->IsStream())) ? pJS : nullptr;
}
diff --git a/core/fpdfdoc/cpdf_action.h b/core/fpdfdoc/cpdf_action.h
index d4dba32..18d6e60 100644
--- a/core/fpdfdoc/cpdf_action.h
+++ b/core/fpdfdoc/cpdf_action.h
@@ -43,7 +43,7 @@
kLast = kGoTo3DView
};
- explicit CPDF_Action(const CPDF_Dictionary* pDict);
+ explicit CPDF_Action(RetainPtr<const CPDF_Dictionary> pDict);
CPDF_Action(const CPDF_Action& that);
~CPDF_Action();
@@ -57,7 +57,7 @@
ByteString GetNamedAction() const;
uint32_t GetFlags() const;
- std::vector<const CPDF_Object*> GetAllFields() const;
+ std::vector<RetainPtr<const CPDF_Object>> GetAllFields() const;
// Differentiates between empty JS entry and no JS entry.
absl::optional<WideString> MaybeGetJavaScript() const;
@@ -69,7 +69,7 @@
CPDF_Action GetSubAction(size_t iIndex) const;
private:
- const CPDF_Object* GetJavaScriptObject() const;
+ RetainPtr<const CPDF_Object> GetJavaScriptObject() const;
RetainPtr<const CPDF_Dictionary> const m_pDict;
};
diff --git a/core/fpdfdoc/cpdf_action_unittest.cpp b/core/fpdfdoc/cpdf_action_unittest.cpp
index 100a6d6..8f5e935 100644
--- a/core/fpdfdoc/cpdf_action_unittest.cpp
+++ b/core/fpdfdoc/cpdf_action_unittest.cpp
@@ -81,16 +81,12 @@
for (const auto& test_case : kValidTestCases) {
{
// Type is present.
- RetainPtr<CPDF_Dictionary> dict =
- CreateActionDictWithType(test_case.action_type);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(CreateActionDictWithType(test_case.action_type));
EXPECT_EQ(test_case.expected_type, action.GetType());
}
{
// Type is optional, so omitting it is ok.
- RetainPtr<CPDF_Dictionary> dict =
- CreateActionDictWithoutType(test_case.action_type);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(CreateActionDictWithoutType(test_case.action_type));
EXPECT_EQ(test_case.expected_type, action.GetType());
}
}
@@ -99,23 +95,20 @@
for (const auto& test_case : kValidTestCases) {
{
// Type is optional, but must be valid if present.
- RetainPtr<CPDF_Dictionary> dict =
- CreateActionDictWithInvalidType(test_case.action_type);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(
+ CreateActionDictWithInvalidType(test_case.action_type));
EXPECT_EQ(CPDF_Action::Type::kUnknown, action.GetType());
}
{
// The action type (/S) must be a name.
- RetainPtr<CPDF_Dictionary> dict =
- CreateInvalidActionDictWithType(test_case.action_type);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(
+ CreateInvalidActionDictWithType(test_case.action_type));
EXPECT_EQ(CPDF_Action::Type::kUnknown, action.GetType());
}
{
// The action type (/S) must be a name.
- RetainPtr<CPDF_Dictionary> dict =
- CreateInvalidActionDictWithoutType(test_case.action_type);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(
+ CreateInvalidActionDictWithoutType(test_case.action_type));
EXPECT_EQ(CPDF_Action::Type::kUnknown, action.GetType());
}
}
@@ -128,8 +121,7 @@
// Test invalid actions.
for (const char* test_case : kInvalidTestCases) {
- RetainPtr<CPDF_Dictionary> dict = CreateActionDictWithType(test_case);
- CPDF_Action action(dict.Get());
+ CPDF_Action action(CreateActionDictWithType(test_case));
EXPECT_EQ(CPDF_Action::Type::kUnknown, action.GetType());
}
}
diff --git a/core/fpdfdoc/cpdf_bookmark.cpp b/core/fpdfdoc/cpdf_bookmark.cpp
index cb50d25..9de19c5 100644
--- a/core/fpdfdoc/cpdf_bookmark.cpp
+++ b/core/fpdfdoc/cpdf_bookmark.cpp
@@ -49,8 +49,7 @@
}
CPDF_Action CPDF_Bookmark::GetAction() const {
- // TODO(tsepez): pass retained object.
- return CPDF_Action(m_pDict ? m_pDict->GetDictFor("A").Get() : nullptr);
+ return CPDF_Action(m_pDict ? m_pDict->GetDictFor("A") : nullptr);
}
int CPDF_Bookmark::GetCount() const {
diff --git a/core/fpdfdoc/cpdf_link.cpp b/core/fpdfdoc/cpdf_link.cpp
index ac349de..24d1a5d 100644
--- a/core/fpdfdoc/cpdf_link.cpp
+++ b/core/fpdfdoc/cpdf_link.cpp
@@ -25,5 +25,5 @@
}
CPDF_Action CPDF_Link::GetAction() {
- return CPDF_Action(m_pDict->GetDictFor("A").Get());
+ return CPDF_Action(m_pDict->GetDictFor("A"));
}
diff --git a/fpdfsdk/cpdfsdk_baannot.cpp b/fpdfsdk/cpdfsdk_baannot.cpp
index 7e912a0..a72ec83 100644
--- a/fpdfsdk/cpdfsdk_baannot.cpp
+++ b/fpdfsdk/cpdfsdk_baannot.cpp
@@ -207,7 +207,7 @@
}
CPDF_Action CPDFSDK_BAAnnot::GetAction() const {
- return CPDF_Action(GetAnnotDict()->GetDictFor("A").Get());
+ return CPDF_Action(GetAnnotDict()->GetDictFor("A"));
}
CPDF_AAction CPDFSDK_BAAnnot::GetAAction() const {
diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index 2ab2472..269fc4b 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -649,7 +649,8 @@
size_t count = name_tree->GetCount();
for (size_t i = 0; i < count; ++i) {
WideString name;
- CPDF_Action action(ToDictionary(name_tree->LookupValueAndName(i, &name)));
+ CPDF_Action action(ToDictionary(
+ pdfium::WrapRetain(name_tree->LookupValueAndName(i, &name))));
DoActionJavaScript(action, name);
}
}
@@ -668,11 +669,11 @@
if (pOpenAction->IsArray())
return true;
- const CPDF_Dictionary* pDict = pOpenAction->AsDictionary();
+ RetainPtr<const CPDF_Dictionary> pDict = ToDictionary(pOpenAction);
if (!pDict)
return false;
- DoActionDocOpen(CPDF_Action(pDict));
+ DoActionDocOpen(CPDF_Action(std::move(pDict)));
return true;
}
diff --git a/fpdfsdk/cpdfsdk_interactiveform.cpp b/fpdfsdk/cpdfsdk_interactiveform.cpp
index c4f6477..bebbb2c 100644
--- a/fpdfsdk/cpdfsdk_interactiveform.cpp
+++ b/fpdfsdk/cpdfsdk_interactiveform.cpp
@@ -505,7 +505,7 @@
}
std::vector<CPDF_FormField*> CPDFSDK_InteractiveForm::GetFieldFromObjects(
- const std::vector<const CPDF_Object*>& objects) const {
+ const std::vector<RetainPtr<const CPDF_Object>>& objects) const {
std::vector<CPDF_FormField*> fields;
for (const CPDF_Object* pObject : objects) {
if (!pObject || !pObject->IsString())
diff --git a/fpdfsdk/cpdfsdk_interactiveform.h b/fpdfsdk/cpdfsdk_interactiveform.h
index 4de69b6..4f72d08 100644
--- a/fpdfsdk/cpdfsdk_interactiveform.h
+++ b/fpdfsdk/cpdfsdk_interactiveform.h
@@ -69,7 +69,7 @@
void DoAction_ResetForm(const CPDF_Action& action);
std::vector<CPDF_FormField*> GetFieldFromObjects(
- const std::vector<const CPDF_Object*>& objects) const;
+ const std::vector<RetainPtr<const CPDF_Object>>& objects) const;
bool SubmitFields(const WideString& csDestination,
const std::vector<CPDF_FormField*>& fields,
bool bIncludeOrExclude,
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index e0271f6..5469840 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -168,7 +168,7 @@
if (!action)
return PDFACTION_UNSUPPORTED;
- CPDF_Action cAction(CPDFDictionaryFromFPDFAction(action));
+ CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
switch (cAction.GetType()) {
case CPDF_Action::Type::kGoTo:
return PDFACTION_GOTO;
@@ -196,7 +196,7 @@
type != PDFACTION_EMBEDDEDGOTO) {
return nullptr;
}
- CPDF_Action cAction(CPDFDictionaryFromFPDFAction(action));
+ CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
return FPDFDestFromCPDFArray(cAction.GetDest(pDoc).GetArray());
}
@@ -208,7 +208,7 @@
return 0;
}
- CPDF_Action cAction(CPDFDictionaryFromFPDFAction(action));
+ CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
ByteString path = cAction.GetFilePath().ToUTF8();
return NulTerminateMaybeCopyAndReturnLength(path, buffer, buflen);
}
@@ -226,7 +226,7 @@
if (type != PDFACTION_URI)
return 0;
- CPDF_Action cAction(CPDFDictionaryFromFPDFAction(action));
+ CPDF_Action cAction(pdfium::WrapRetain(CPDFDictionaryFromFPDFAction(action)));
ByteString path = cAction.GetURI(pDoc);
// Table 206 in the ISO 32000-1:2008 spec states the type for the URI field is
diff --git a/fpdfsdk/fpdf_javascript.cpp b/fpdfsdk/fpdf_javascript.cpp
index 0f6826a..9207a9e 100644
--- a/fpdfsdk/fpdf_javascript.cpp
+++ b/fpdfsdk/fpdf_javascript.cpp
@@ -45,7 +45,7 @@
return nullptr;
// Validate |obj|. Type is optional, but must be valid if present.
- CPDF_Action action(obj);
+ CPDF_Action action(pdfium::WrapRetain(obj));
if (action.GetType() != CPDF_Action::Type::kJavaScript)
return nullptr;
