Return retained objects from indirect object holder, part 2.
Convert GetOrParseIndirectObject() to return retained references.
Change-Id: I136ee3ed58fdce03dc11d58f0d163d02b77ed3d4
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97311
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index cbb7460..530f73a 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -154,12 +154,12 @@
m_ObjectOffsets[objnum] = m_Archive->CurrentOffset();
bool bExistInMap = !!m_pDocument->GetIndirectObject(objnum);
- CPDF_Object* pObj = m_pDocument->GetOrParseIndirectObject(objnum);
+ RetainPtr<CPDF_Object> pObj = m_pDocument->GetOrParseIndirectObject(objnum);
if (!pObj) {
m_ObjectOffsets.erase(objnum);
return true;
}
- if (!WriteIndirectObj(pObj->GetObjNum(), pObj))
+ if (!WriteIndirectObj(pObj->GetObjNum(), pObj.Get()))
return false;
if (!bExistInMap)
m_pDocument->DeleteIndirectObject(objnum);
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index b2d1f5c..c5bf71e 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -135,7 +135,8 @@
bool CPDF_Document::TryInit() {
SetLastObjNum(m_pParser->GetLastObjNum());
- CPDF_Object* pRootObj = GetOrParseIndirectObject(m_pParser->GetRootObjNum());
+ RetainPtr<CPDF_Object> pRootObj =
+ GetOrParseIndirectObject(m_pParser->GetRootObjNum());
if (pRootObj)
m_pRootDict = pRootObj->GetMutableDict();
@@ -171,7 +172,7 @@
}
uint32_t objnum = linearized_header->GetFirstPageObjNum();
- if (!IsValidPageObject(GetOrParseIndirectObject(objnum))) {
+ if (!IsValidPageObject(GetOrParseIndirectObject(objnum).Get())) {
m_PageList.resize(RetrievePageCount());
return;
}
@@ -285,9 +286,12 @@
const uint32_t objnum = m_PageList[iPage];
if (objnum) {
- CPDF_Dictionary* result = ToDictionary(GetOrParseIndirectObject(objnum));
- if (result)
- return result;
+ RetainPtr<CPDF_Dictionary> result =
+ ToDictionary(GetOrParseIndirectObject(objnum));
+ if (result) {
+ // TODO(tsepez): return retained result.
+ return result.Get();
+ }
}
CPDF_Dictionary* pPages = GetPagesDict();
@@ -353,7 +357,7 @@
return -1;
// Only update |m_PageList| when |objnum| points to a /Page object.
- if (IsValidPageObject(GetOrParseIndirectObject(objnum)))
+ if (IsValidPageObject(GetOrParseIndirectObject(objnum).Get()))
m_PageList[found_index] = objnum;
return found_index;
}
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
index ab9dbeb..539aae5 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -44,7 +44,7 @@
return FilterInvalidObjNum(it->second);
}
-CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
+RetainPtr<CPDF_Object> CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
uint32_t objnum) {
if (objnum == 0 || objnum == CPDF_Object::kInvalidObjNum)
return nullptr;
@@ -52,7 +52,7 @@
// Add item anyway to prevent recursively parsing of same object.
auto insert_result = m_IndirectObjs.insert(std::make_pair(objnum, nullptr));
if (!insert_result.second)
- return FilterInvalidObjNum(insert_result.first->second).Get();
+ return FilterInvalidObjNum(insert_result.first->second);
RetainPtr<CPDF_Object> pNewObj = ParseIndirectObject(objnum);
if (!pNewObj) {
@@ -62,8 +62,8 @@
pNewObj->SetObjNum(objnum);
m_LastObjNum = std::max(m_LastObjNum, objnum);
- insert_result.first->second = std::move(pNewObj);
- return insert_result.first->second.Get();
+ insert_result.first->second = pNewObj;
+ return pNewObj;
}
RetainPtr<CPDF_Object> CPDF_IndirectObjectHolder::ParseIndirectObject(
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
index 8154520..cabddf1 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -26,7 +26,7 @@
CPDF_IndirectObjectHolder();
virtual ~CPDF_IndirectObjectHolder();
- virtual CPDF_Object* GetOrParseIndirectObject(uint32_t objnum);
+ virtual RetainPtr<CPDF_Object> GetOrParseIndirectObject(uint32_t objnum);
RetainPtr<const CPDF_Object> GetIndirectObject(uint32_t objnum) const;
RetainPtr<CPDF_Object> GetMutableIndirectObject(uint32_t objnum);
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp
index 49e8eee..ac27f99 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp
@@ -28,7 +28,7 @@
EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_))
.WillOnce(::testing::WithArg<0>(::testing::Invoke(
[&mock_holder](uint32_t objnum) -> RetainPtr<CPDF_Object> {
- const CPDF_Object* same_parse =
+ RetainPtr<const CPDF_Object> same_parse =
mock_holder.GetOrParseIndirectObject(objnum);
CHECK(!same_parse);
return pdfium::MakeRetain<CPDF_Null>();
diff --git a/core/fpdfapi/parser/cpdf_object_avail.cpp b/core/fpdfapi/parser/cpdf_object_avail.cpp
index f9fff08..c47be27 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail.cpp
@@ -60,12 +60,13 @@
}
CPDF_ReadValidator::ScopedSession parse_session(validator_);
- CPDF_Object* direct = holder_->GetOrParseIndirectObject(ref_obj_num);
+ RetainPtr<CPDF_Object> direct =
+ holder_->GetOrParseIndirectObject(ref_obj_num);
if (validator_->has_read_problems())
return false;
parsed_objnums_.insert(ref_obj_num);
- root_.Reset(direct);
+ root_ = std::move(direct);
}
std::stack<uint32_t> non_parsed_objects_in_root;
if (AppendObjectSubRefs(root_.Get(), &non_parsed_objects_in_root)) {
@@ -90,12 +91,13 @@
continue;
CPDF_ReadValidator::ScopedSession parse_session(validator_);
- const CPDF_Object* direct = holder_->GetOrParseIndirectObject(obj_num);
+ RetainPtr<const CPDF_Object> direct =
+ holder_->GetOrParseIndirectObject(obj_num);
if (direct == root_)
continue;
if (validator_->has_read_problems() ||
- !AppendObjectSubRefs(direct, &objects_to_check)) {
+ !AppendObjectSubRefs(direct.Get(), &objects_to_check)) {
non_parsed_objects_.push(obj_num);
continue;
}
diff --git a/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp b/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
index 3136446..a183c79 100644
--- a/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail_unittest.cpp
@@ -44,7 +44,7 @@
~TestHolder() override = default;
// CPDF_IndirectObjectHolder overrides:
- CPDF_Object* GetOrParseIndirectObject(uint32_t objnum) override {
+ RetainPtr<CPDF_Object> GetOrParseIndirectObject(uint32_t objnum) override {
auto it = objects_data_.find(objnum);
if (it == objects_data_.end())
return nullptr;
@@ -54,7 +54,7 @@
validator_->SimulateReadError();
return nullptr;
}
- return obj_data.object.Get();
+ return obj_data.object;
}
RetainPtr<CPDF_ReadValidator> GetValidator() { return validator_; }
diff --git a/core/fpdfapi/parser/cpdf_page_object_avail_unittest.cpp b/core/fpdfapi/parser/cpdf_page_object_avail_unittest.cpp
index e6af52e..469014d 100644
--- a/core/fpdfapi/parser/cpdf_page_object_avail_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_page_object_avail_unittest.cpp
@@ -44,7 +44,7 @@
~TestHolder() override = default;
// CPDF_IndirectObjectHolder overrides:
- CPDF_Object* GetOrParseIndirectObject(uint32_t objnum) override {
+ RetainPtr<CPDF_Object> GetOrParseIndirectObject(uint32_t objnum) override {
auto it = objects_data_.find(objnum);
if (it == objects_data_.end())
return nullptr;
@@ -54,7 +54,7 @@
validator_->SimulateReadError();
return nullptr;
}
- return obj_data.object.Get();
+ return obj_data.object;
}
RetainPtr<CPDF_ReadValidator> GetValidator() { return validator_; }
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index b204ae0..0472b93 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -853,7 +853,7 @@
}
const CPDF_Dictionary* CPDF_Parser::GetRoot() const {
- CPDF_Object* obj =
+ RetainPtr<CPDF_Object> obj =
m_pObjectsHolder->GetOrParseIndirectObject(GetRootObjNum());
return obj ? obj->GetDict() : nullptr;
}
@@ -870,8 +870,10 @@
return ToDictionary(pEncryptObj);
if (pEncryptObj->IsReference()) {
+ // TODO(tsepez): return retained object.
return ToDictionary(m_pObjectsHolder->GetOrParseIndirectObject(
- pEncryptObj->AsReference()->GetRefObjNum()));
+ pEncryptObj->AsReference()->GetRefObjNum()))
+ .Get();
}
return nullptr;
}
diff --git a/core/fpdfapi/parser/cpdf_parser_unittest.cpp b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
index d1747c6..8f24d95 100644
--- a/core/fpdfapi/parser/cpdf_parser_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_parser_unittest.cpp
@@ -38,7 +38,8 @@
// CPDF_Parser::ParsedObjectsHolder:
bool TryInit() override { return true; }
- MOCK_METHOD1(GetOrParseIndirectObject, CPDF_Object*(uint32_t objnum));
+ MOCK_METHOD1(GetOrParseIndirectObject,
+ RetainPtr<CPDF_Object>(uint32_t objnum));
};
} // namespace
@@ -358,7 +359,7 @@
// /XRef stream and avoid also providing other valid dictionaries.
auto dummy_root = pdfium::MakeRetain<CPDF_Dictionary>();
EXPECT_CALL(parser.object_holder(), GetOrParseIndirectObject)
- .WillRepeatedly(Return(dummy_root.Get()));
+ .WillRepeatedly(Return(dummy_root));
// Since /Index starts at 4194303, the object number will go past
// `kMaxObjectNumber`.
@@ -410,7 +411,7 @@
// /XRef stream and avoid also providing other valid dictionaries.
auto dummy_root = pdfium::MakeRetain<CPDF_Dictionary>();
EXPECT_CALL(parser.object_holder(), GetOrParseIndirectObject)
- .WillRepeatedly(Return(dummy_root.Get()));
+ .WillRepeatedly(Return(dummy_root));
// 0xFF in the first object in the xref object stream is invalid.
const unsigned char kData[] =
diff --git a/core/fpdfapi/parser/cpdf_reference.cpp b/core/fpdfapi/parser/cpdf_reference.cpp
index 8196a27..e652459 100644
--- a/core/fpdfapi/parser/cpdf_reference.cpp
+++ b/core/fpdfapi/parser/cpdf_reference.cpp
@@ -80,7 +80,7 @@
}
const CPDF_Object* CPDF_Reference::GetDirect() const {
- return m_pObjList ? m_pObjList->GetOrParseIndirectObject(m_RefObjNum)
+ return m_pObjList ? m_pObjList->GetOrParseIndirectObject(m_RefObjNum).Get()
: nullptr;
}
