Fix retained argument TODOs, part 8.
-- remove comments about passing arguments downwards into functions
that have limited scope of use of the argument and do not take
ownership of it.
Change-Id: I8ca4c5a2734e01072cb84723c8ffc2de0f41e0cb
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98620
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/page/cpdf_allstates.cpp b/core/fpdfapi/page/cpdf_allstates.cpp
index a9d8820..04a43a2 100644
--- a/core/fpdfapi/page/cpdf_allstates.cpp
+++ b/core/fpdfapi/page/cpdf_allstates.cpp
@@ -75,7 +75,6 @@
if (!pArray)
break;
- // TODO(tsepez): pass moved retained object.
SetLineDash(pArray.Get(), pDash->GetFloatAt(1), 1.0f);
break;
}
diff --git a/core/fpdfapi/page/cpdf_occontext.cpp b/core/fpdfapi/page/cpdf_occontext.cpp
index 69e31f6..b7a18c5 100644
--- a/core/fpdfapi/page/cpdf_occontext.cpp
+++ b/core/fpdfapi/page/cpdf_occontext.cpp
@@ -232,7 +232,6 @@
bool CPDF_OCContext::LoadOCMDState(const CPDF_Dictionary* pOCMDDict) const {
RetainPtr<const CPDF_Array> pVE = pOCMDDict->GetArrayFor("VE");
if (pVE) {
- // TODO(tsepez): pass retained argument.
return GetOCGVE(pVE.Get(), 0);
}
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index c55f124..1c532f7 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -1175,7 +1175,6 @@
return CPDF_ColorSpace::GetStockCS(CPDF_ColorSpace::Family::kDeviceCMYK);
}
- // TODO(tsepez); pass retained argument.
return CPDF_DocPageData::FromDocument(m_pDocument.Get())
->GetColorSpace(pDefObj.Get(), nullptr);
}
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index e7d49ec..195a0fb 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -6,6 +6,8 @@
#include "core/fpdfapi/parser/cpdf_document.h"
+#include <utility>
+
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_linearized_header.h"
@@ -525,8 +527,8 @@
m_PageList.erase(m_PageList.begin() + iPage);
}
-void CPDF_Document::SetRootForTesting(CPDF_Dictionary* root) {
- m_pRootDict.Reset(root);
+void CPDF_Document::SetRootForTesting(RetainPtr<CPDF_Dictionary> root) {
+ m_pRootDict = std::move(root);
}
void CPDF_Document::ResizePageListForTesting(size_t size) {
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index 6fe8d53..c1a82e9 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -144,7 +144,7 @@
protected:
void SetParser(std::unique_ptr<CPDF_Parser> pParser);
- void SetRootForTesting(CPDF_Dictionary* root);
+ void SetRootForTesting(RetainPtr<CPDF_Dictionary> root);
void ResizePageListForTesting(size_t size);
private:
diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp
index 34b939a..9c978f9 100644
--- a/core/fpdfapi/parser/cpdf_document_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp
@@ -25,10 +25,9 @@
const int kNumTestPages = 7;
-// TODO(tsepez): return retained reference.
-CPDF_Dictionary* CreatePageTreeNode(RetainPtr<CPDF_Array> kids,
- CPDF_Document* pDoc,
- int count) {
+RetainPtr<CPDF_Dictionary> CreatePageTreeNode(RetainPtr<CPDF_Array> kids,
+ CPDF_Document* pDoc,
+ int count) {
CPDF_Array* pUnowned =
pDoc->AddIndirectObject(std::move(kids))->AsMutableArray();
auto pageNode = pDoc->NewIndirect<CPDF_Dictionary>();
@@ -39,7 +38,7 @@
pUnowned->GetMutableDictAt(i)->SetNewFor<CPDF_Reference>(
"Parent", pDoc, pageNode->GetObjNum());
}
- return pageNode.Get();
+ return pageNode;
}
RetainPtr<CPDF_Dictionary> CreateNumberedPage(size_t number) {
@@ -60,14 +59,14 @@
this, AddIndirectObject(CreateNumberedPage(1))->GetObjNum());
zeroToTwo->AppendNew<CPDF_Reference>(
this, AddIndirectObject(CreateNumberedPage(2))->GetObjNum());
- CPDF_Dictionary* branch1 =
+ RetainPtr<CPDF_Dictionary> branch1 =
CreatePageTreeNode(std::move(zeroToTwo), this, 3);
auto zeroToThree = pdfium::MakeRetain<CPDF_Array>();
zeroToThree->AppendNew<CPDF_Reference>(this, branch1->GetObjNum());
zeroToThree->AppendNew<CPDF_Reference>(
this, AddIndirectObject(CreateNumberedPage(3))->GetObjNum());
- CPDF_Dictionary* branch2 =
+ RetainPtr<CPDF_Dictionary> branch2 =
CreatePageTreeNode(std::move(zeroToThree), this, 4);
auto fourFive = pdfium::MakeRetain<CPDF_Array>();
@@ -75,22 +74,23 @@
this, AddIndirectObject(CreateNumberedPage(4))->GetObjNum());
fourFive->AppendNew<CPDF_Reference>(
this, AddIndirectObject(CreateNumberedPage(5))->GetObjNum());
- CPDF_Dictionary* branch3 = CreatePageTreeNode(std::move(fourFive), this, 2);
+ RetainPtr<CPDF_Dictionary> branch3 =
+ CreatePageTreeNode(std::move(fourFive), this, 2);
auto justSix = pdfium::MakeRetain<CPDF_Array>();
justSix->AppendNew<CPDF_Reference>(
this, AddIndirectObject(CreateNumberedPage(6))->GetObjNum());
- CPDF_Dictionary* branch4 = CreatePageTreeNode(std::move(justSix), this, 1);
+ RetainPtr<CPDF_Dictionary> branch4 =
+ CreatePageTreeNode(std::move(justSix), this, 1);
auto allPages = pdfium::MakeRetain<CPDF_Array>();
allPages->AppendNew<CPDF_Reference>(this, branch2->GetObjNum());
allPages->AppendNew<CPDF_Reference>(this, branch3->GetObjNum());
allPages->AppendNew<CPDF_Reference>(this, branch4->GetObjNum());
- CPDF_Dictionary* pagesDict =
+ RetainPtr<CPDF_Dictionary> pagesDict =
CreatePageTreeNode(std::move(allPages), this, kNumTestPages);
- // TODO(tsepez): pass retained argument.
- SetRootForTesting(NewIndirect<CPDF_Dictionary>().Get());
+ SetRootForTesting(NewIndirect<CPDF_Dictionary>());
GetMutableRoot()->SetNewFor<CPDF_Reference>("Pages", this,
pagesDict->GetObjNum());
ResizePageListForTesting(kNumTestPages);
@@ -114,9 +114,9 @@
// Page without pageNum.
inlined_page_ = CreateNumberedPage(2);
allPages->Append(inlined_page_);
- CPDF_Dictionary* pagesDict =
+ RetainPtr<CPDF_Dictionary> pagesDict =
CreatePageTreeNode(std::move(allPages), this, 3);
- SetRootForTesting(NewIndirect<CPDF_Dictionary>().Get());
+ SetRootForTesting(NewIndirect<CPDF_Dictionary>());
GetMutableRoot()->SetNewFor<CPDF_Reference>("Pages", this,
pagesDict->GetObjNum());
ResizePageListForTesting(3);
@@ -141,7 +141,7 @@
pagesDict->SetNewFor<CPDF_Name>("Type", "Pages");
pagesDict->SetNewFor<CPDF_Number>("Count", 3);
ResizePageListForTesting(10);
- SetRootForTesting(NewIndirect<CPDF_Dictionary>().Get());
+ SetRootForTesting(NewIndirect<CPDF_Dictionary>());
GetMutableRoot()->SetNewFor<CPDF_Reference>("Pages", this,
pagesDict->GetObjNum());
}
diff --git a/core/fpdfapi/parser/cpdf_object_walker.cpp b/core/fpdfapi/parser/cpdf_object_walker.cpp
index 211e6cd..5e85181 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker.cpp
@@ -15,18 +15,18 @@
class StreamIterator final : public CPDF_ObjectWalker::SubobjectIterator {
public:
- explicit StreamIterator(const CPDF_Stream* stream)
- : SubobjectIterator(stream) {}
+ explicit StreamIterator(RetainPtr<const CPDF_Stream> stream)
+ : SubobjectIterator(std::move(stream)) {}
+
~StreamIterator() override = default;
bool IsFinished() const override { return IsStarted() && is_finished_; }
- // TODO(tsepez): return retained reference.
- const CPDF_Object* IncrementImpl() override {
+ RetainPtr<const CPDF_Object> IncrementImpl() override {
DCHECK(IsStarted());
DCHECK(!IsFinished());
is_finished_ = true;
- return object()->GetDict().Get();
+ return object()->GetDict();
}
void Start() override {}
@@ -37,18 +37,19 @@
class DictionaryIterator final : public CPDF_ObjectWalker::SubobjectIterator {
public:
- explicit DictionaryIterator(const CPDF_Dictionary* dictionary)
+ explicit DictionaryIterator(RetainPtr<const CPDF_Dictionary> dictionary)
: SubobjectIterator(dictionary), locker_(dictionary) {}
+
~DictionaryIterator() override = default;
bool IsFinished() const override {
return IsStarted() && dict_iterator_ == locker_.end();
}
- const CPDF_Object* IncrementImpl() override {
+ RetainPtr<const CPDF_Object> IncrementImpl() override {
DCHECK(IsStarted());
DCHECK(!IsFinished());
- const CPDF_Object* result = dict_iterator_->second.Get();
+ RetainPtr<const CPDF_Object> result = dict_iterator_->second;
dict_key_ = dict_iterator_->first;
++dict_iterator_;
return result;
@@ -69,7 +70,7 @@
class ArrayIterator final : public CPDF_ObjectWalker::SubobjectIterator {
public:
- explicit ArrayIterator(const CPDF_Array* array)
+ explicit ArrayIterator(RetainPtr<const CPDF_Array> array)
: SubobjectIterator(array), locker_(array) {}
~ArrayIterator() override = default;
@@ -78,10 +79,10 @@
return IsStarted() && arr_iterator_ == locker_.end();
}
- const CPDF_Object* IncrementImpl() override {
+ RetainPtr<const CPDF_Object> IncrementImpl() override {
DCHECK(IsStarted());
DCHECK(!IsFinished());
- const CPDF_Object* result = arr_iterator_->Get();
+ RetainPtr<const CPDF_Object> result = *arr_iterator_;
++arr_iterator_;
return result;
}
@@ -111,20 +112,20 @@
}
CPDF_ObjectWalker::SubobjectIterator::SubobjectIterator(
- const CPDF_Object* object)
- : object_(object) {
+ RetainPtr<const CPDF_Object> object)
+ : object_(std::move(object)) {
DCHECK(object_);
}
// static
std::unique_ptr<CPDF_ObjectWalker::SubobjectIterator>
-CPDF_ObjectWalker::MakeIterator(const CPDF_Object* object) {
+CPDF_ObjectWalker::MakeIterator(RetainPtr<const CPDF_Object> object) {
if (object->IsStream())
- return std::make_unique<StreamIterator>(object->AsStream());
+ return std::make_unique<StreamIterator>(ToStream(object));
if (object->IsDictionary())
- return std::make_unique<DictionaryIterator>(object->AsDictionary());
+ return std::make_unique<DictionaryIterator>(ToDictionary(object));
if (object->IsArray())
- return std::make_unique<ArrayIterator>(object->AsArray());
+ return std::make_unique<ArrayIterator>(ToArray(object));
return nullptr;
}
@@ -136,7 +137,7 @@
RetainPtr<const CPDF_Object> CPDF_ObjectWalker::GetNext() {
while (!stack_.empty() || next_object_) {
if (next_object_) {
- auto new_iterator = MakeIterator(next_object_.Get());
+ auto new_iterator = MakeIterator(next_object_);
if (new_iterator) {
// Schedule walk within composite objects.
stack_.push(std::move(new_iterator));
diff --git a/core/fpdfapi/parser/cpdf_object_walker.h b/core/fpdfapi/parser/cpdf_object_walker.h
index fe633cd..f4ffc06 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.h
+++ b/core/fpdfapi/parser/cpdf_object_walker.h
@@ -26,9 +26,9 @@
const CPDF_Object* object() const { return object_.Get(); }
protected:
- explicit SubobjectIterator(const CPDF_Object* object);
+ explicit SubobjectIterator(RetainPtr<const CPDF_Object> object);
- virtual const CPDF_Object* IncrementImpl() = 0;
+ virtual RetainPtr<const CPDF_Object> IncrementImpl() = 0;
virtual void Start() = 0;
private:
@@ -48,7 +48,7 @@
private:
static std::unique_ptr<SubobjectIterator> MakeIterator(
- const CPDF_Object* object);
+ RetainPtr<const CPDF_Object> object);
RetainPtr<const CPDF_Object> next_object_;
RetainPtr<const CPDF_Object> parent_object_;
diff --git a/core/fpdfapi/parser/cpdf_test_document.cpp b/core/fpdfapi/parser/cpdf_test_document.cpp
index 54e6098..f1d9179 100644
--- a/core/fpdfapi/parser/cpdf_test_document.cpp
+++ b/core/fpdfapi/parser/cpdf_test_document.cpp
@@ -5,6 +5,7 @@
#include "core/fpdfapi/parser/cpdf_test_document.h"
#include <memory>
+#include <utility>
#include "core/fpdfapi/page/cpdf_docpagedata.h"
#include "core/fpdfapi/render/cpdf_docrenderdata.h"
@@ -13,6 +14,6 @@
: CPDF_Document(std::make_unique<CPDF_DocRenderData>(),
std::make_unique<CPDF_DocPageData>()) {}
-void CPDF_TestDocument::SetRoot(CPDF_Dictionary* root) {
- SetRootForTesting(root);
+void CPDF_TestDocument::SetRoot(RetainPtr<CPDF_Dictionary> root) {
+ SetRootForTesting(std::move(root));
}
diff --git a/core/fpdfapi/parser/cpdf_test_document.h b/core/fpdfapi/parser/cpdf_test_document.h
index 2c148df..79848d0 100644
--- a/core/fpdfapi/parser/cpdf_test_document.h
+++ b/core/fpdfapi/parser/cpdf_test_document.h
@@ -6,12 +6,15 @@
#define CORE_FPDFAPI_PARSER_CPDF_TEST_DOCUMENT_H_
#include "core/fpdfapi/parser/cpdf_document.h"
+#include "core/fxcrt/retain_ptr.h"
+
+class CPDF_Dictionary;
class CPDF_TestDocument : public CPDF_Document {
public:
CPDF_TestDocument();
- void SetRoot(CPDF_Dictionary* root);
+ void SetRoot(RetainPtr<CPDF_Dictionary> root);
};
#endif // CORE_FPDFAPI_PARSER_CPDF_TEST_DOCUMENT_H_
diff --git a/core/fpdfdoc/cpdf_bafontmap_unittest.cpp b/core/fpdfdoc/cpdf_bafontmap_unittest.cpp
index e702169..c5e6075 100644
--- a/core/fpdfdoc/cpdf_bafontmap_unittest.cpp
+++ b/core/fpdfdoc/cpdf_bafontmap_unittest.cpp
@@ -47,7 +47,7 @@
annot_font_f1_dict->SetNewFor<CPDF_Name>("Type", "Font");
annot_font_f1_dict->SetNewFor<CPDF_Name>("Subtype", "Type1");
annot_font_f1_dict->SetNewFor<CPDF_Name>("BaseFont", "Times-Roman");
- doc.SetRoot(root_dict.Get());
+ doc.SetRoot(root_dict);
auto annot_dict = pdfium::MakeRetain<CPDF_Dictionary>();
annot_dict->SetNewFor<CPDF_Name>(pdfium::annotation::kSubtype, "Widget");
diff --git a/core/fpdfdoc/cpdf_interactiveform.cpp b/core/fpdfdoc/cpdf_interactiveform.cpp
index 0d2faab..b8dcba0 100644
--- a/core/fpdfdoc/cpdf_interactiveform.cpp
+++ b/core/fpdfdoc/cpdf_interactiveform.cpp
@@ -276,7 +276,6 @@
FX_Charset charSet = GetNativeCharSet();
RetainPtr<CPDF_Font> pFont = AddStandardFont(pDocument);
if (pFont) {
- // TODO(tsepez): pass retained reference.
AddFont(pFormDict.Get(), pDocument, pFont, &csBaseName);
}
if (charSet != FX_Charset::kANSI) {
diff --git a/fpdfsdk/fpdf_catalog_unittest.cpp b/fpdfsdk/fpdf_catalog_unittest.cpp
index dd765bf..f3cbe1e 100644
--- a/fpdfsdk/fpdf_catalog_unittest.cpp
+++ b/fpdfsdk/fpdf_catalog_unittest.cpp
@@ -48,7 +48,7 @@
EXPECT_FALSE(FPDFCatalog_IsTagged(m_pDoc.get()));
// Empty root
- pTestDoc->SetRoot(m_pRootObj.Get());
+ pTestDoc->SetRoot(m_pRootObj);
EXPECT_FALSE(FPDFCatalog_IsTagged(m_pDoc.get()));
// Root with other key
diff --git a/fpdfsdk/fpdf_doc_unittest.cpp b/fpdfsdk/fpdf_doc_unittest.cpp
index 6b786d3..1b3f145 100644
--- a/fpdfsdk/fpdf_doc_unittest.cpp
+++ b/fpdfsdk/fpdf_doc_unittest.cpp
@@ -36,7 +36,7 @@
auto pTestDoc = std::make_unique<CPDF_TestDocument>();
m_pIndirectObjs = pTestDoc.get();
m_pRootObj = m_pIndirectObjs->NewIndirect<CPDF_Dictionary>();
- pTestDoc->SetRoot(m_pRootObj.Get());
+ pTestDoc->SetRoot(m_pRootObj);
m_pDoc.reset(FPDFDocumentFromCPDFDocument(pTestDoc.release()));
}
diff --git a/fpdfsdk/fpdf_edittext.cpp b/fpdfsdk/fpdf_edittext.cpp
index 8ccd845..90333b7 100644
--- a/fpdfsdk/fpdf_edittext.cpp
+++ b/fpdfsdk/fpdf_edittext.cpp
@@ -85,12 +85,11 @@
return CFX_Font::kUntitledFontName;
}
-// TODO(tsepez): return retained references.
-CPDF_Dictionary* LoadFontDesc(CPDF_Document* pDoc,
- const ByteString& font_name,
- CFX_Font* pFont,
- pdfium::span<const uint8_t> span,
- int font_type) {
+RetainPtr<CPDF_Dictionary> LoadFontDesc(CPDF_Document* pDoc,
+ const ByteString& font_name,
+ CFX_Font* pFont,
+ pdfium::span<const uint8_t> span,
+ int font_type) {
auto pFontDesc = pDoc->NewIndirect<CPDF_Dictionary>();
pFontDesc->SetNewFor<CPDF_Name>("Type", "FontDescriptor");
pFontDesc->SetNewFor<CPDF_Name>("FontName", font_name);
@@ -130,7 +129,7 @@
}
ByteString fontFile = font_type == FPDF_FONT_TYPE1 ? "FontFile" : "FontFile2";
pFontDesc->SetNewFor<CPDF_Reference>(fontFile, pDoc, pStream->GetObjNum());
- return pFontDesc.Get();
+ return pFontDesc;
}
const char ToUnicodeStart[] =
@@ -324,7 +323,7 @@
static_cast<int>(dwCurrentChar));
pFontDict->SetNewFor<CPDF_Reference>("Widths", pDoc,
widthsArray->GetObjNum());
- CPDF_Dictionary* pFontDesc =
+ RetainPtr<CPDF_Dictionary> pFontDesc =
LoadFontDesc(pDoc, name, pFont.get(), span, font_type);
pFontDict->SetNewFor<CPDF_Reference>("FontDescriptor", pDoc,
@@ -362,7 +361,7 @@
pCIDFont->SetNewFor<CPDF_Reference>("CIDSystemInfo", pDoc,
pCIDSystemInfo->GetObjNum());
- CPDF_Dictionary* pFontDesc =
+ RetainPtr<CPDF_Dictionary> pFontDesc =
LoadFontDesc(pDoc, name, pFont.get(), span, font_type);
pCIDFont->SetNewFor<CPDF_Reference>("FontDescriptor", pDoc,
pFontDesc->GetObjNum());
