Return retained result from CPDF_Dictionary::GetPageDictionary()
Change-Id: I9ce486422f1ec96dac07cc41e8ab94f53da0850f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98272
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp
index 900d814..1981cdb 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -822,7 +822,8 @@
const HintsScope hints_scope(GetValidator(), pHints);
if (m_pLinearized) {
if (dwPage == m_pLinearized->GetFirstPageNo()) {
- auto* pPageDict = m_pDocument->GetPageDictionary(iPage);
+ RetainPtr<const CPDF_Dictionary> pPageDict =
+ m_pDocument->GetPageDictionary(iPage);
if (!pPageDict)
return kDataError;
@@ -881,7 +882,7 @@
{
auto page_num_obj = std::make_pair(
dwPage, std::make_unique<CPDF_PageObjectAvail>(
- GetValidator(), m_pDocument.Get(), pPageDict.Get()));
+ GetValidator(), m_pDocument.Get(), pPageDict));
CPDF_PageObjectAvail* page_obj_avail =
m_PagesObjAvail.insert(std::move(page_num_obj)).first->second.get();
const DocAvailStatus status = page_obj_avail->CheckAvail();
@@ -913,9 +914,8 @@
CPDF_PageObjectAvail* resource_avail =
m_PagesResourcesAvail
.insert(std::make_pair(
- resources,
- std::make_unique<CPDF_PageObjectAvail>(
- GetValidator(), m_pDocument.Get(), resources.Get())))
+ resources, std::make_unique<CPDF_PageObjectAvail>(
+ GetValidator(), m_pDocument.Get(), resources)))
.first->second.get();
return resource_avail->CheckAvail();
}
@@ -935,10 +935,11 @@
return m_pDocument ? m_pDocument->GetPageCount() : 0;
}
-const CPDF_Dictionary* CPDF_DataAvail::GetPageDictionary(int index) const {
+RetainPtr<const CPDF_Dictionary> CPDF_DataAvail::GetPageDictionary(
+ int index) const {
if (!m_pDocument || index < 0 || index >= GetPageCount())
return nullptr;
- const CPDF_Dictionary* page = m_pDocument->GetPageDictionary(index);
+ RetainPtr<const CPDF_Dictionary> page = m_pDocument->GetPageDictionary(index);
if (page)
return page;
if (!m_pLinearized || !m_pHintTables)
@@ -993,9 +994,8 @@
if (!pAcroForm)
return kFormNotExist;
- // TODO(tsepez): pass retained argument.
m_pFormAvail = std::make_unique<CPDF_PageObjectAvail>(
- GetValidator(), m_pDocument.Get(), pAcroForm.Get());
+ GetValidator(), m_pDocument.Get(), std::move(pAcroForm));
}
switch (m_pFormAvail->CheckAvail()) {
case kDataError:
@@ -1012,10 +1012,13 @@
bool CPDF_DataAvail::ValidatePage(uint32_t dwPage) const {
int iPage = pdfium::base::checked_cast<int>(dwPage);
- auto* pPageDict = m_pDocument->GetPageDictionary(iPage);
+ RetainPtr<const CPDF_Dictionary> pPageDict =
+ m_pDocument->GetPageDictionary(iPage);
if (!pPageDict)
return false;
- CPDF_PageObjectAvail obj_avail(GetValidator(), m_pDocument.Get(), pPageDict);
+
+ CPDF_PageObjectAvail obj_avail(GetValidator(), m_pDocument.Get(),
+ std::move(pPageDict));
return obj_avail.CheckAvail() == kDataAvailable;
}
diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h
index 3b5b46c..87ab321 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.h
+++ b/core/fpdfapi/parser/cpdf_data_avail.h
@@ -81,7 +81,7 @@
DocFormStatus IsFormAvail(DownloadHints* pHints);
DocLinearizationStatus IsLinearizedPDF();
int GetPageCount() const;
- const CPDF_Dictionary* GetPageDictionary(int index) const;
+ RetainPtr<const CPDF_Dictionary> GetPageDictionary(int index) const;
RetainPtr<CPDF_ReadValidator> GetValidator() const;
std::pair<CPDF_Parser::Error, std::unique_ptr<CPDF_Document>> ParseDocument(
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index f8f10ef..1f22bd0 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -283,7 +283,7 @@
return !!m_PageList[iPage];
}
-const CPDF_Dictionary* CPDF_Document::GetPageDictionary(int iPage) {
+RetainPtr<const CPDF_Dictionary> CPDF_Document::GetPageDictionary(int iPage) {
if (!fxcrt::IndexInBounds(m_PageList, iPage))
return nullptr;
@@ -291,10 +291,8 @@
if (objnum) {
RetainPtr<CPDF_Dictionary> result =
ToDictionary(GetOrParseIndirectObject(objnum));
- if (result) {
- // TODO(tsepez): return retained result.
- return result.Get();
- }
+ if (result)
+ return result;
}
CPDF_Dictionary* pPages = GetPagesDict();
@@ -306,14 +304,15 @@
m_pTreeTraversal.push_back(std::make_pair(pPages, 0));
}
int nPagesToGo = iPage - m_iNextPageToTraverse + 1;
+ // TODO(tsepez): return retained page from TraversePDFPages().
CPDF_Dictionary* pPage = TraversePDFPages(iPage, &nPagesToGo, 0);
m_iNextPageToTraverse = iPage + 1;
- return pPage;
+ return pdfium::WrapRetain(pPage);
}
RetainPtr<CPDF_Dictionary> CPDF_Document::GetMutablePageDictionary(int iPage) {
return pdfium::WrapRetain(
- const_cast<CPDF_Dictionary*>(GetPageDictionary(iPage)));
+ const_cast<CPDF_Dictionary*>(GetPageDictionary(iPage).Get()));
}
void CPDF_Document::SetPageObjNum(int iPage, uint32_t objNum) {
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index 6ebc541..861948e 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -98,7 +98,7 @@
void DeletePage(int iPage);
int GetPageCount() const;
bool IsPageLoaded(int iPage) const;
- const CPDF_Dictionary* GetPageDictionary(int iPage);
+ RetainPtr<const CPDF_Dictionary> GetPageDictionary(int iPage);
RetainPtr<CPDF_Dictionary> GetMutablePageDictionary(int iPage);
int GetPageIndex(uint32_t objnum);
uint32_t GetUserPermissions() const;
diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp
index 27050fe..34b939a 100644
--- a/core/fpdfapi/parser/cpdf_document_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp
@@ -162,22 +162,24 @@
std::unique_ptr<CPDF_TestDocumentForPages> document =
std::make_unique<CPDF_TestDocumentForPages>();
for (int i = 0; i < kNumTestPages; i++) {
- const CPDF_Dictionary* page = document->GetPageDictionary(i);
+ RetainPtr<const CPDF_Dictionary> page = document->GetPageDictionary(i);
ASSERT_TRUE(page);
ASSERT_TRUE(page->KeyExist("PageNumbering"));
EXPECT_EQ(i, page->GetIntegerFor("PageNumbering"));
}
- const CPDF_Dictionary* page = document->GetPageDictionary(kNumTestPages);
+ RetainPtr<const CPDF_Dictionary> page =
+ document->GetPageDictionary(kNumTestPages);
EXPECT_FALSE(page);
}
TEST_F(DocumentTest, GetPageWithoutObjNumTwice) {
auto document = std::make_unique<CPDF_TestDocumentWithPageWithoutPageNum>();
- const CPDF_Dictionary* page = document->GetPageDictionary(2);
+ RetainPtr<const CPDF_Dictionary> page = document->GetPageDictionary(2);
ASSERT_TRUE(page);
ASSERT_EQ(document->inlined_page(), page);
- const CPDF_Dictionary* second_call_page = document->GetPageDictionary(2);
+ RetainPtr<const CPDF_Dictionary> second_call_page =
+ document->GetPageDictionary(2);
EXPECT_TRUE(second_call_page);
EXPECT_EQ(page, second_call_page);
}
@@ -186,12 +188,13 @@
std::unique_ptr<CPDF_TestDocumentForPages> document =
std::make_unique<CPDF_TestDocumentForPages>();
for (int i = 6; i >= 0; i--) {
- const CPDF_Dictionary* page = document->GetPageDictionary(i);
+ RetainPtr<const CPDF_Dictionary> page = document->GetPageDictionary(i);
ASSERT_TRUE(page);
ASSERT_TRUE(page->KeyExist("PageNumbering"));
EXPECT_EQ(i, page->GetIntegerFor("PageNumbering"));
}
- const CPDF_Dictionary* page = document->GetPageDictionary(kNumTestPages);
+ RetainPtr<const CPDF_Dictionary> page =
+ document->GetPageDictionary(kNumTestPages);
EXPECT_FALSE(page);
}
@@ -199,7 +202,7 @@
std::unique_ptr<CPDF_TestDocumentForPages> document =
std::make_unique<CPDF_TestDocumentForPages>();
- const CPDF_Dictionary* page = document->GetPageDictionary(1);
+ RetainPtr<const CPDF_Dictionary> page = document->GetPageDictionary(1);
ASSERT_TRUE(page);
ASSERT_TRUE(page->KeyExist("PageNumbering"));
EXPECT_EQ(1, page->GetIntegerFor("PageNumbering"));
diff --git a/core/fpdfapi/parser/cpdf_object_avail.cpp b/core/fpdfapi/parser/cpdf_object_avail.cpp
index b2e4e7a..f52504f 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail.cpp
@@ -16,8 +16,10 @@
CPDF_ObjectAvail::CPDF_ObjectAvail(RetainPtr<CPDF_ReadValidator> validator,
CPDF_IndirectObjectHolder* holder,
- const CPDF_Object* root)
- : validator_(std::move(validator)), holder_(holder), root_(root) {
+ RetainPtr<const CPDF_Object> root)
+ : validator_(std::move(validator)),
+ holder_(holder),
+ root_(std::move(root)) {
DCHECK(validator_);
DCHECK(holder);
DCHECK(root_);
diff --git a/core/fpdfapi/parser/cpdf_object_avail.h b/core/fpdfapi/parser/cpdf_object_avail.h
index b66d311..cf58c79 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.h
+++ b/core/fpdfapi/parser/cpdf_object_avail.h
@@ -21,7 +21,7 @@
public:
CPDF_ObjectAvail(RetainPtr<CPDF_ReadValidator> validator,
CPDF_IndirectObjectHolder* holder,
- const CPDF_Object* root);
+ RetainPtr<const CPDF_Object> root);
CPDF_ObjectAvail(RetainPtr<CPDF_ReadValidator> validator,
CPDF_IndirectObjectHolder* holder,
uint32_t obj_num);
diff --git a/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp b/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
index 89a089e..0dfafb6 100644
--- a/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
@@ -326,11 +326,10 @@
"Dict");
root->SetNewFor<CPDF_Reference>("Self", &holder, 1);
-
holder.AddObject(2, pdfium::MakeRetain<CPDF_String>(nullptr, "Data", false),
TestHolder::ObjectState::Unavailable);
- CPDF_ObjectAvail avail(holder.GetValidator(), &holder, root.Get());
+ CPDF_ObjectAvail avail(holder.GetValidator(), &holder, root);
EXPECT_EQ(CPDF_DataAvail::kDataNotAvailable, avail.CheckAvail());
holder.SetObjectState(2, TestHolder::ObjectState::Available);
diff --git a/core/fpdfapi/parser/cpdf_seekablemultistream.cpp b/core/fpdfapi/parser/cpdf_seekablemultistream.cpp
index 2e5f7fd..6587c75 100644
--- a/core/fpdfapi/parser/cpdf_seekablemultistream.cpp
+++ b/core/fpdfapi/parser/cpdf_seekablemultistream.cpp
@@ -7,6 +7,7 @@
#include "core/fpdfapi/parser/cpdf_seekablemultistream.h"
#include <algorithm>
+#include <utility>
#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fpdfapi/parser/cpdf_stream_acc.h"
@@ -16,10 +17,9 @@
#include "third_party/base/notreached.h"
CPDF_SeekableMultiStream::CPDF_SeekableMultiStream(
- const std::vector<const CPDF_Stream*>& streams) {
- for (const CPDF_Stream* pStream : streams) {
- m_Data.push_back(
- pdfium::MakeRetain<CPDF_StreamAcc>(pdfium::WrapRetain(pStream)));
+ std::vector<RetainPtr<const CPDF_Stream>> streams) {
+ for (auto& pStream : streams) {
+ m_Data.push_back(pdfium::MakeRetain<CPDF_StreamAcc>(std::move(pStream)));
m_Data.back()->LoadAllDataFiltered();
}
}
diff --git a/core/fpdfapi/parser/cpdf_seekablemultistream.h b/core/fpdfapi/parser/cpdf_seekablemultistream.h
index 30a8479..fa5a6d0 100644
--- a/core/fpdfapi/parser/cpdf_seekablemultistream.h
+++ b/core/fpdfapi/parser/cpdf_seekablemultistream.h
@@ -18,7 +18,7 @@
class CPDF_SeekableMultiStream final : public IFX_SeekableStream {
public:
explicit CPDF_SeekableMultiStream(
- const std::vector<const CPDF_Stream*>& streams);
+ std::vector<RetainPtr<const CPDF_Stream>> streams);
~CPDF_SeekableMultiStream() override;
// IFX_SeekableReadStream
diff --git a/core/fpdfapi/parser/cpdf_seekablemultistream_unittest.cpp b/core/fpdfapi/parser/cpdf_seekablemultistream_unittest.cpp
index 9b9be20..6eb6762 100644
--- a/core/fpdfapi/parser/cpdf_seekablemultistream_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_seekablemultistream_unittest.cpp
@@ -4,6 +4,7 @@
#include "core/fpdfapi/parser/cpdf_seekablemultistream.h"
+#include <utility>
#include <vector>
#include "core/fpdfapi/parser/cpdf_dictionary.h"
@@ -11,8 +12,9 @@
#include "testing/gtest/include/gtest/gtest.h"
TEST(CPDFSeekableMultiStreamTest, NoStreams) {
- std::vector<const CPDF_Stream*> streams;
- auto fileread = pdfium::MakeRetain<CPDF_SeekableMultiStream>(streams);
+ std::vector<RetainPtr<const CPDF_Stream>> streams;
+ auto fileread =
+ pdfium::MakeRetain<CPDF_SeekableMultiStream>(std::move(streams));
uint8_t output_buffer[16];
memset(output_buffer, 0xbd, sizeof(output_buffer));
@@ -21,10 +23,10 @@
}
TEST(CXFAFileReadTest, EmptyStreams) {
- std::vector<const CPDF_Stream*> streams;
- auto stream1 = pdfium::MakeRetain<CPDF_Stream>();
- streams.push_back(stream1.Get());
- auto fileread = pdfium::MakeRetain<CPDF_SeekableMultiStream>(streams);
+ std::vector<RetainPtr<const CPDF_Stream>> streams;
+ streams.push_back(pdfium::MakeRetain<CPDF_Stream>());
+ auto fileread =
+ pdfium::MakeRetain<CPDF_SeekableMultiStream>(std::move(streams));
uint8_t output_buffer[16];
memset(output_buffer, 0xbd, sizeof(output_buffer));
@@ -33,7 +35,6 @@
}
TEST(CXFAFileReadTest, NormalStreams) {
- std::vector<const CPDF_Stream*> streams;
// 16 chars total.
auto stream1 =
pdfium::MakeRetain<CPDF_Stream>(ByteStringView("one t").raw_span(),
@@ -44,10 +45,12 @@
pdfium::MakeRetain<CPDF_Stream>(ByteStringView("three!!!").raw_span(),
pdfium::MakeRetain<CPDF_Dictionary>());
- streams.push_back(stream1.Get());
- streams.push_back(stream2.Get());
- streams.push_back(stream3.Get());
- auto fileread = pdfium::MakeRetain<CPDF_SeekableMultiStream>(streams);
+ std::vector<RetainPtr<const CPDF_Stream>> streams;
+ streams.push_back(std::move(stream1));
+ streams.push_back(std::move(stream2));
+ streams.push_back(std::move(stream3));
+ auto fileread =
+ pdfium::MakeRetain<CPDF_SeekableMultiStream>(std::move(streams));
uint8_t output_buffer[16];
memset(output_buffer, 0xbd, sizeof(output_buffer));
diff --git a/fpdfsdk/cpdfsdk_interactiveform.cpp b/fpdfsdk/cpdfsdk_interactiveform.cpp
index e1d8c74..0568c9f 100644
--- a/fpdfsdk/cpdfsdk_interactiveform.cpp
+++ b/fpdfsdk/cpdfsdk_interactiveform.cpp
@@ -178,7 +178,8 @@
DCHECK(pAnnotDict);
for (int i = 0, sz = pDocument->GetPageCount(); i < sz; i++) {
- const CPDF_Dictionary* pPageDict = pDocument->GetPageDictionary(i);
+ RetainPtr<const CPDF_Dictionary> pPageDict =
+ pDocument->GetPageDictionary(i);
if (!pPageDict)
continue;
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index 4a50cf7..4cb2e59 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -149,8 +149,9 @@
return CalculatePageEdit(iSubX, iSubY, pagesize);
}
-const CPDF_Object* PageDictGetInheritableTag(const CPDF_Dictionary* pDict,
- const ByteString& bsSrcTag) {
+const CPDF_Object* PageDictGetInheritableTag(
+ RetainPtr<const CPDF_Dictionary> pDict,
+ const ByteString& bsSrcTag) {
if (!pDict || bsSrcTag.IsEmpty())
return nullptr;
if (!pDict->KeyExist(pdfium::page_object::kParent) ||
@@ -184,13 +185,13 @@
}
bool CopyInheritable(RetainPtr<CPDF_Dictionary> pDestPageDict,
- const CPDF_Dictionary* pSrcPageDict,
+ RetainPtr<const CPDF_Dictionary> pSrcPageDict,
const ByteString& key) {
if (pDestPageDict->KeyExist(key))
return true;
const CPDF_Object* pInheritable =
- PageDictGetInheritableTag(pSrcPageDict, key);
+ PageDictGetInheritableTag(std::move(pSrcPageDict), key);
if (!pInheritable)
return false;
@@ -395,7 +396,8 @@
int curpage = nIndex;
for (uint32_t pageIndex : pageIndices) {
RetainPtr<CPDF_Dictionary> pDestPageDict = dest()->CreateNewPage(curpage);
- auto* pSrcPageDict = src()->GetPageDictionary(pageIndex);
+ RetainPtr<const CPDF_Dictionary> pSrcPageDict =
+ src()->GetPageDictionary(pageIndex);
if (!pSrcPageDict || !pDestPageDict)
return false;
@@ -601,7 +603,8 @@
// TODO(tsepez): return retained object.
CPDF_Stream* CPDF_NPageToOneExporter::MakeXObjectFromPageRaw(
const RetainPtr<CPDF_Page>& pSrcPage) {
- const CPDF_Dictionary* pSrcPageDict = pSrcPage->GetDict();
+ // TODO(tsepez): return retained object from CPDF_Page::GetDict()'
+ RetainPtr<const CPDF_Dictionary> pSrcPageDict(pSrcPage->GetDict());
RetainPtr<const CPDF_Object> pSrcContentObj =
pSrcPageDict->GetDirectObjectFor(pdfium::page_object::kContents);
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
index cadb7aa..4927a8a 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
@@ -72,23 +72,21 @@
if (!pElementXFA)
return nullptr;
- std::vector<const CPDF_Stream*> xfaStreams;
+ std::vector<RetainPtr<const CPDF_Stream>> xfa_streams;
if (pElementXFA->IsArray()) {
const CPDF_Array* pXFAArray = pElementXFA->AsArray();
for (size_t i = 0; i < pXFAArray->size() / 2; i++) {
RetainPtr<const CPDF_Stream> pStream = pXFAArray->GetStreamAt(i * 2 + 1);
- if (pStream) {
- // TODO(tsepez): push retained objects.
- xfaStreams.push_back(pStream.Get());
- }
+ if (pStream)
+ xfa_streams.push_back(std::move(pStream));
}
} else if (pElementXFA->IsStream()) {
- xfaStreams.push_back(pElementXFA->AsStream());
+ xfa_streams.push_back(ToStream(pElementXFA));
}
- if (xfaStreams.empty())
+ if (xfa_streams.empty())
return nullptr;
- return pdfium::MakeRetain<CPDF_SeekableMultiStream>(xfaStreams);
+ return pdfium::MakeRetain<CPDF_SeekableMultiStream>(std::move(xfa_streams));
}
} // namespace
