Replace non-const CPDF_Array::Get*At() with GetMutable*At().
Force mutators to hold a retained reference to the object under
mutation, or use const pointers when not mutating. Then return
retained references as needed to satisfy compilation constraint.
First in a series to replace the non-const getter overloads.
Change-Id: I2c7c84f61f36532d13fd31a64033b4a0dacaf77c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/94490
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp
index 6aa6f30..bda93d6 100644
--- a/core/fpdfapi/parser/cpdf_array.cpp
+++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -155,18 +155,7 @@
}
RetainPtr<CPDF_Dictionary> CPDF_Array::GetMutableDictAt(size_t index) {
- return pdfium::WrapRetain(GetDictAt(index));
-}
-
-CPDF_Dictionary* CPDF_Array::GetDictAt(size_t index) {
- CPDF_Object* p = GetDirectObjectAt(index);
- if (!p)
- return nullptr;
- if (CPDF_Dictionary* pDict = p->AsDictionary())
- return pDict;
- if (CPDF_Stream* pStream = p->AsStream())
- return pStream->GetDict();
- return nullptr;
+ return pdfium::WrapRetain(const_cast<CPDF_Dictionary*>(GetDictAt(index)));
}
const CPDF_Dictionary* CPDF_Array::GetDictAt(size_t index) const {
@@ -180,16 +169,16 @@
return nullptr;
}
-CPDF_Stream* CPDF_Array::GetStreamAt(size_t index) {
- return ToStream(GetDirectObjectAt(index));
+RetainPtr<CPDF_Stream> CPDF_Array::GetMutableStreamAt(size_t index) {
+ return pdfium::WrapRetain(const_cast<CPDF_Stream*>(GetStreamAt(index)));
}
const CPDF_Stream* CPDF_Array::GetStreamAt(size_t index) const {
return ToStream(GetDirectObjectAt(index));
}
-CPDF_Array* CPDF_Array::GetArrayAt(size_t index) {
- return ToArray(GetDirectObjectAt(index));
+RetainPtr<CPDF_Array> CPDF_Array::GetMutableArrayAt(size_t index) {
+ return pdfium::WrapRetain(const_cast<CPDF_Array*>(GetArrayAt(index)));
}
const CPDF_Array* CPDF_Array::GetArrayAt(size_t index) const {
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h
index 223cd59..e039cd7 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -63,11 +63,10 @@
int GetIntegerAt(size_t index) const;
float GetNumberAt(size_t index) const;
RetainPtr<CPDF_Dictionary> GetMutableDictAt(size_t index);
- CPDF_Dictionary* GetDictAt(size_t index); // prefer previous form.
const CPDF_Dictionary* GetDictAt(size_t index) const;
- CPDF_Stream* GetStreamAt(size_t index);
+ RetainPtr<CPDF_Stream> GetMutableStreamAt(size_t index);
const CPDF_Stream* GetStreamAt(size_t index) const;
- CPDF_Array* GetArrayAt(size_t index);
+ RetainPtr<CPDF_Array> GetMutableArrayAt(size_t index);
const CPDF_Array* GetArrayAt(size_t index) const;
CFX_FloatRect GetRect() const;
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index 44fa2a1..e493fdc 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -37,13 +37,13 @@
return 0;
count = 0;
for (size_t i = 0; i < pKidList->size(); i++) {
- CPDF_Dictionary* pKid = pKidList->GetDictAt(i);
- if (!pKid || pdfium::Contains(*visited_pages, pKid))
+ RetainPtr<CPDF_Dictionary> pKid = pKidList->GetMutableDictAt(i);
+ if (!pKid || pdfium::Contains(*visited_pages, pKid.Get()))
continue;
if (pKid->KeyExist("Kids")) {
// Use |visited_pages| to help detect circular references of pages.
- ScopedSetInsertion<CPDF_Dictionary*> local_add(visited_pages, pKid);
- count += CountPages(pKid, visited_pages);
+ ScopedSetInsertion<CPDF_Dictionary*> local_add(visited_pages, pKid.Get());
+ count += CountPages(pKid.Get(), visited_pages);
} else {
// This page is a leaf node.
count++;
@@ -202,12 +202,12 @@
m_bReachedMaxPageLevel = true;
return nullptr;
}
- CPDF_Dictionary* page = nullptr;
+ RetainPtr<CPDF_Dictionary> page;
for (size_t i = m_pTreeTraversal[level].second; i < pKidList->size(); i++) {
if (*nPagesToGo == 0)
break;
pKidList->ConvertToIndirectObjectAt(i, this);
- CPDF_Dictionary* pKid = pKidList->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pKid = pKidList->GetMutableDictAt(i);
if (!pKid) {
(*nPagesToGo)--;
m_pTreeTraversal[level].second++;
@@ -228,7 +228,7 @@
} else {
// If the vector has size level+1, the child is not in yet
if (m_pTreeTraversal.size() == level + 1)
- m_pTreeTraversal.push_back(std::make_pair(pKid, 0));
+ m_pTreeTraversal.push_back(std::make_pair(pKid.Get(), 0));
// Now m_pTreeTraversal[level+1] should exist and be equal to pKid.
CPDF_Dictionary* pageKid = TraversePDFPages(iPage, nPagesToGo, level + 1);
// Check if child was completely processed, i.e. it popped itself out
@@ -244,7 +244,7 @@
}
if (m_pTreeTraversal[level].second == pKidList->size())
m_pTreeTraversal.pop_back();
- return page;
+ return page.Get();
}
void CPDF_Document::ResetTraversal() {
@@ -411,7 +411,7 @@
return false;
for (size_t i = 0; i < pKidList->size(); i++) {
- CPDF_Dictionary* pKid = pKidList->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pKid = pKidList->GetMutableDictAt(i);
if (pKid->GetNameFor("Type") == "Page") {
if (nPagesToGo != 0) {
nPagesToGo--;
@@ -437,10 +437,11 @@
if (pdfium::Contains(*pVisited, pKid))
return false;
- ScopedSetInsertion<CPDF_Dictionary*> insertion(pVisited, pKid);
- if (!InsertDeletePDFPage(pKid, nPagesToGo, pPageDict, bInsert, pVisited))
+ ScopedSetInsertion<CPDF_Dictionary*> insertion(pVisited, pKid.Get());
+ if (!InsertDeletePDFPage(pKid.Get(), nPagesToGo, pPageDict, bInsert,
+ pVisited)) {
return false;
-
+ }
pPages->SetNewFor<CPDF_Number>(
"Count", pPages->GetIntegerFor("Count") + (bInsert ? 1 : -1));
break;
diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp
index 306e043..cc6e100 100644
--- a/core/fpdfapi/parser/cpdf_document_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp
@@ -34,8 +34,8 @@
pageNode->SetNewFor<CPDF_Reference>("Kids", pDoc, pUnowned->GetObjNum());
pageNode->SetNewFor<CPDF_Number>("Count", count);
for (size_t i = 0; i < pUnowned->size(); i++) {
- pUnowned->GetDictAt(i)->SetNewFor<CPDF_Reference>("Parent", pDoc,
- pageNode->GetObjNum());
+ pUnowned->GetMutableDictAt(i)->SetNewFor<CPDF_Reference>(
+ "Parent", pDoc, pageNode->GetObjNum());
}
return pageNode;
}
diff --git a/core/fpdfdoc/cpdf_interactiveform.cpp b/core/fpdfdoc/cpdf_interactiveform.cpp
index b7bb38e..98971e3 100644
--- a/core/fpdfdoc/cpdf_interactiveform.cpp
+++ b/core/fpdfdoc/cpdf_interactiveform.cpp
@@ -540,7 +540,7 @@
return;
for (size_t i = 0; i < pFields->size(); ++i)
- LoadField(pFields->GetDictAt(i), 0);
+ LoadField(pFields->GetMutableDictAt(i).Get(), 0);
}
CPDF_InteractiveForm::~CPDF_InteractiveForm() = default;
@@ -766,10 +766,10 @@
if (pFirstKid->KeyExist(pdfium::form_fields::kT) ||
pFirstKid->KeyExist(pdfium::form_fields::kKids)) {
for (size_t i = 0; i < pKids->size(); i++) {
- CPDF_Dictionary* pChildDict = pKids->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pChildDict = pKids->GetMutableDictAt(i);
if (pChildDict) {
if (pChildDict->GetObjNum() != dwParentObjNum)
- LoadField(pChildDict, nLevel + 1);
+ LoadField(pChildDict.Get(), nLevel + 1);
}
}
} else {
@@ -783,9 +783,9 @@
return;
for (size_t i = 0; i < pAnnots->size(); i++) {
- CPDF_Dictionary* pAnnot = pAnnots->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pAnnot = pAnnots->GetMutableDictAt(i);
if (pAnnot && pAnnot->GetNameFor("Subtype") == "Widget")
- LoadField(pAnnot, 0);
+ LoadField(pAnnot.Get(), 0);
}
}
@@ -848,9 +848,9 @@
CPDF_Array* pKids = pFieldDict->GetArrayFor(pdfium::form_fields::kKids);
if (pKids) {
for (size_t i = 0; i < pKids->size(); i++) {
- CPDF_Dictionary* pKid = pKids->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pKid = pKids->GetMutableDictAt(i);
if (pKid && pKid->GetNameFor("Subtype") == "Widget")
- AddControl(pField, pKid);
+ AddControl(pField, pKid.Get());
}
} else {
if (pFieldDict->GetNameFor("Subtype") == "Widget")
diff --git a/core/fpdfdoc/cpdf_linklist.cpp b/core/fpdfdoc/cpdf_linklist.cpp
index 23b324e..568e363 100644
--- a/core/fpdfdoc/cpdf_linklist.cpp
+++ b/core/fpdfdoc/cpdf_linklist.cpp
@@ -57,7 +57,7 @@
return page_link_list;
for (size_t i = 0; i < pAnnotList->size(); ++i) {
- CPDF_Dictionary* pAnnot = pAnnotList->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pAnnot = pAnnotList->GetMutableDictAt(i);
bool add_link = (pAnnot && pAnnot->GetStringFor("Subtype") == "Link");
// Add non-links as nullptrs to preserve z-order.
page_link_list->emplace_back(add_link ? pAnnot : nullptr);
diff --git a/core/fpdfdoc/cpdf_nametree.cpp b/core/fpdfdoc/cpdf_nametree.cpp
index e818e1c..539b0fd 100644
--- a/core/fpdfdoc/cpdf_nametree.cpp
+++ b/core/fpdfdoc/cpdf_nametree.cpp
@@ -130,12 +130,13 @@
// Loop through the kids to find the leaf array |pFind|.
for (size_t i = 0; i < pKids->size(); ++i) {
- CPDF_Dictionary* pKid = pKids->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pKid = pKids->GetMutableDictAt(i);
if (!pKid)
continue;
- if (!UpdateNodesAndLimitsUponDeletion(pKid, pFind, csName, nLevel + 1))
+ if (!UpdateNodesAndLimitsUponDeletion(pKid.Get(), pFind, csName,
+ nLevel + 1)) {
continue;
-
+ }
// Remove this child node if it's empty.
if ((pKid->KeyExist("Names") && pKid->GetArrayFor("Names")->IsEmpty()) ||
(pKid->KeyExist("Kids") && pKid->GetArrayFor("Kids")->IsEmpty())) {
@@ -300,11 +301,11 @@
return absl::nullopt;
for (size_t i = 0; i < pKids->size(); i++) {
- CPDF_Dictionary* pKid = pKids->GetDictAt(i);
+ RetainPtr<CPDF_Dictionary> pKid = pKids->GetMutableDictAt(i);
if (!pKid)
continue;
absl::optional<IndexSearchResult> result = SearchNameNodeByIndexInternal(
- pKid, nTargetPairIndex, nLevel + 1, nCurPairIndex);
+ pKid.Get(), nTargetPairIndex, nLevel + 1, nCurPairIndex);
if (result.has_value())
return result;
}
diff --git a/core/fpdfdoc/cpdf_nametree_unittest.cpp b/core/fpdfdoc/cpdf_nametree_unittest.cpp
index 36617e7..3b9fa3b 100644
--- a/core/fpdfdoc/cpdf_nametree_unittest.cpp
+++ b/core/fpdfdoc/cpdf_nametree_unittest.cpp
@@ -130,9 +130,11 @@
// elements.
auto pRootDict = pdfium::MakeRetain<CPDF_Dictionary>();
FillNameTreeDict(pRootDict.Get());
- CPDF_Dictionary* pKid1 = pRootDict->GetArrayFor("Kids")->GetDictAt(0);
- CPDF_Dictionary* pGrandKid3 = pKid1->GetArrayFor("Kids")->GetDictAt(1);
- CPDF_Array* pLimits = pGrandKid3->GetArrayFor("Limits");
+ RetainPtr<CPDF_Dictionary> pKid1 =
+ pRootDict->GetArrayFor("Kids")->GetMutableDictAt(0);
+ RetainPtr<CPDF_Dictionary> pGrandKid3 =
+ pKid1->GetArrayFor("Kids")->GetMutableDictAt(1);
+ RetainPtr<CPDF_Array> pLimits = pGrandKid3->GetMutableArrayFor("Limits");
ASSERT_EQ(2u, pLimits->size());
pLimits->AppendNew<CPDF_Number>(5);
pLimits->AppendNew<CPDF_Number>(6);
@@ -143,8 +145,8 @@
const CPDF_Number* pNumber = ToNumber(name_tree->LookupValue(L"9.txt"));
ASSERT_TRUE(pNumber);
EXPECT_EQ(999, pNumber->GetInteger());
- CheckLimitsArray(pKid1, "1.txt", "9.txt");
- CheckLimitsArray(pGrandKid3, "9.txt", "9.txt");
+ CheckLimitsArray(pKid1.Get(), "1.txt", "9.txt");
+ CheckLimitsArray(pGrandKid3.Get(), "9.txt", "9.txt");
}
TEST(cpdf_nametree, AddIntoNames) {
