Pass more retained references into CPDF_ObjectWalker.
Follow-on from
https://pdfium-review.googlesource.com/c/pdfium/+/97510
Move existing objects where possible, instead of re-constructing
from raw pointers.
-- move some code out of header file.
Change-Id: Ia9667968c6d27892e116b09e9c45f60c145f1098
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97511
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.cpp b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
index 69c2ac1..ec77ba7 100644
--- a/core/fpdfapi/parser/cpdf_crypto_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
@@ -239,8 +239,7 @@
RetainPtr<CPDF_Object> object_to_decrypt = object;
while (object_to_decrypt) {
- CPDF_NonConstObjectWalker walker(object_to_decrypt.Get());
- object_to_decrypt = nullptr;
+ CPDF_NonConstObjectWalker walker(std::move(object_to_decrypt));
while (RetainPtr<CPDF_Object> child = walker.GetNext()) {
const CPDF_Dictionary* parent_dict =
walker.GetParent() ? walker.GetParent()->GetDict() : nullptr;
diff --git a/core/fpdfapi/parser/cpdf_object_avail.cpp b/core/fpdfapi/parser/cpdf_object_avail.cpp
index bad6a16..b2e4e7a 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail.cpp
@@ -69,7 +69,7 @@
root_ = std::move(direct);
}
std::stack<uint32_t> non_parsed_objects_in_root;
- if (AppendObjectSubRefs(root_.Get(), &non_parsed_objects_in_root)) {
+ if (AppendObjectSubRefs(root_, &non_parsed_objects_in_root)) {
non_parsed_objects_ = std::move(non_parsed_objects_in_root);
return true;
}
@@ -97,7 +97,7 @@
continue;
if (validator_->has_read_problems() ||
- !AppendObjectSubRefs(direct.Get(), &objects_to_check)) {
+ !AppendObjectSubRefs(std::move(direct), &objects_to_check)) {
non_parsed_objects_.push(obj_num);
continue;
}
@@ -106,13 +106,13 @@
return non_parsed_objects_.empty();
}
-bool CPDF_ObjectAvail::AppendObjectSubRefs(const CPDF_Object* object,
+bool CPDF_ObjectAvail::AppendObjectSubRefs(RetainPtr<const CPDF_Object> object,
std::stack<uint32_t>* refs) const {
DCHECK(refs);
if (!object)
return true;
- CPDF_ObjectWalker walker(object);
+ CPDF_ObjectWalker walker(std::move(object));
while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
CPDF_ReadValidator::ScopedSession parse_session(validator_);
diff --git a/core/fpdfapi/parser/cpdf_object_avail.h b/core/fpdfapi/parser/cpdf_object_avail.h
index f7ce7b3..b66d311 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.h
+++ b/core/fpdfapi/parser/cpdf_object_avail.h
@@ -35,7 +35,7 @@
private:
bool LoadRootObject();
bool CheckObjects();
- bool AppendObjectSubRefs(const CPDF_Object* object,
+ bool AppendObjectSubRefs(RetainPtr<const CPDF_Object> object,
std::stack<uint32_t>* refs) const;
void CleanMemory();
bool HasObjectParsed(uint32_t obj_num) const;
diff --git a/core/fpdfapi/parser/cpdf_object_walker.cpp b/core/fpdfapi/parser/cpdf_object_walker.cpp
index c0551dc..8c8b435 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker.cpp
@@ -127,8 +127,8 @@
return nullptr;
}
-CPDF_ObjectWalker::CPDF_ObjectWalker(const CPDF_Object* root)
- : next_object_(root) {}
+CPDF_ObjectWalker::CPDF_ObjectWalker(RetainPtr<const CPDF_Object> root)
+ : next_object_(std::move(root)) {}
CPDF_ObjectWalker::~CPDF_ObjectWalker() = default;
@@ -165,3 +165,12 @@
return;
stack_.pop();
}
+
+CPDF_NonConstObjectWalker::CPDF_NonConstObjectWalker(
+ RetainPtr<CPDF_Object> root)
+ : CPDF_ObjectWalker(std::move(root)) {}
+
+RetainPtr<CPDF_Object> CPDF_NonConstObjectWalker::GetNext() {
+ return pdfium::WrapRetain(
+ const_cast<CPDF_Object*>(CPDF_ObjectWalker::GetNext().Get()));
+}
diff --git a/core/fpdfapi/parser/cpdf_object_walker.h b/core/fpdfapi/parser/cpdf_object_walker.h
index 56be755..fe633cd 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.h
+++ b/core/fpdfapi/parser/cpdf_object_walker.h
@@ -36,7 +36,7 @@
bool is_started_ = false;
};
- explicit CPDF_ObjectWalker(const CPDF_Object* root);
+ explicit CPDF_ObjectWalker(RetainPtr<const CPDF_Object> root);
~CPDF_ObjectWalker();
RetainPtr<const CPDF_Object> GetNext();
@@ -59,13 +59,8 @@
class CPDF_NonConstObjectWalker final : public CPDF_ObjectWalker {
public:
- explicit CPDF_NonConstObjectWalker(CPDF_Object* root)
- : CPDF_ObjectWalker(root) {}
-
- RetainPtr<CPDF_Object> GetNext() {
- return pdfium::WrapRetain(
- const_cast<CPDF_Object*>(CPDF_ObjectWalker::GetNext().Get()));
- }
+ explicit CPDF_NonConstObjectWalker(RetainPtr<CPDF_Object> root);
+ RetainPtr<CPDF_Object> GetNext();
};
#endif // CORE_FPDFAPI_PARSER_CPDF_OBJECT_WALKER_H_
diff --git a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
index 4106d0d..0b8f9da 100644
--- a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
@@ -21,9 +21,9 @@
namespace {
-std::string Walk(CPDF_Object* object) {
+std::string Walk(RetainPtr<CPDF_Object> object) {
std::ostringstream result;
- CPDF_ObjectWalker walker(object);
+ CPDF_ObjectWalker walker(std::move(object));
while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (obj->IsDictionary())
result << " Dict";
@@ -54,13 +54,13 @@
} // namespace
TEST(ObjectWalkerTest, Simple) {
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Null>().Get()), "Null");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Dictionary>().Get()), "Dict");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Array>().Get()), "Arr");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_String>().Get()), "Str");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Boolean>().Get()), "Bool");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Stream>().Get()), "Stream");
- EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Reference>(nullptr, 0).Get()), "Ref");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Null>()), "Null");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Dictionary>()), "Dict");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Array>()), "Arr");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_String>()), "Str");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Boolean>()), "Bool");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Stream>()), "Stream");
+ EXPECT_EQ(Walk(pdfium::MakeRetain<CPDF_Reference>(nullptr, 0)), "Ref");
}
TEST(ObjectWalkerTest, CombinedObject) {
@@ -74,7 +74,7 @@
nullptr, 0, pdfium::MakeRetain<CPDF_Dictionary>()));
dict->SetFor("3", std::move(array));
// The last number for stream length.
- EXPECT_EQ(Walk(dict.Get()), "Dict Str Bool Arr Ref Null Stream Dict Num");
+ EXPECT_EQ(Walk(dict), "Dict Str Bool Arr Ref Null Stream Dict Num");
}
TEST(ObjectWalkerTest, GetParent) {
@@ -92,7 +92,7 @@
// In this case each step will increase depth.
// And on each step the prev object should be parent for current.
RetainPtr<const CPDF_Object> cur_parent;
- CPDF_ObjectWalker walker(level_0.Get());
+ CPDF_ObjectWalker walker(level_0);
while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
EXPECT_EQ(cur_parent, walker.GetParent());
cur_parent = obj;
@@ -109,7 +109,7 @@
root_array->Append(root_array->Clone());
int non_array_objects = 0;
- CPDF_ObjectWalker walker(root_array.Get());
+ CPDF_ObjectWalker walker(root_array);
while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (obj != root_array && obj->IsArray()) {
// skip other array except root.
@@ -130,7 +130,7 @@
dict->SetFor("4", pdfium::MakeRetain<CPDF_Null>());
dict->SetFor("5", pdfium::MakeRetain<CPDF_Null>());
- CPDF_ObjectWalker walker(dict.Get());
+ CPDF_ObjectWalker walker(dict);
while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (dict == obj) {
// Ignore root dictinary object
