summaryrefslogtreecommitdiff
path: root/core/fpdfapi/parser
diff options
context:
space:
mode:
authorArtem Strygin <art-snake@yandex-team.ru>2017-09-01 16:03:58 +0300
committerChromium commit bot <commit-bot@chromium.org>2017-09-05 15:55:27 +0000
commit16b77c7176313531609a3a062d286f555c4710a1 (patch)
tree9f2bb253cbb4f33edff7139e435abb62486b9f00 /core/fpdfapi/parser
parentd2608a87173f00cb80830d56b52eeb588055ba3a (diff)
downloadpdfium-16b77c7176313531609a3a062d286f555c4710a1.tar.xz
Refactoring of CPDF_IndirectObjectHolder.
CPDF_IndirectObjectHolder is optimized. Recursively parsing of same object has been fixed. Change-Id: I22e5cfd6b03eee0677f2b1f8ba32bf29cad441fc Reviewed-on: https://pdfium-review.googlesource.com/12590 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Art Snake <art-snake@yandex-team.ru>
Diffstat (limited to 'core/fpdfapi/parser')
-rw-r--r--core/fpdfapi/parser/cpdf_indirect_object_holder.cpp61
-rw-r--r--core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp81
2 files changed, 120 insertions, 22 deletions
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
index aee527618f..434470abeb 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -13,6 +13,14 @@
#include "core/fpdfapi/parser/cpdf_parser.h"
#include "third_party/base/logging.h"
+namespace {
+
+CPDF_Object* FilterInvalidObjNum(CPDF_Object* obj) {
+ return obj && obj->GetObjNum() != CPDF_Object::kInvalidObjNum ? obj : nullptr;
+}
+
+} // namespace
+
CPDF_IndirectObjectHolder::CPDF_IndirectObjectHolder()
: m_LastObjNum(0),
m_pByteStringPool(pdfium::MakeUnique<CFX_ByteStringPool>()) {}
@@ -24,26 +32,30 @@ CPDF_IndirectObjectHolder::~CPDF_IndirectObjectHolder() {
CPDF_Object* CPDF_IndirectObjectHolder::GetIndirectObject(
uint32_t objnum) const {
auto it = m_IndirectObjs.find(objnum);
- return it != m_IndirectObjs.end() ? it->second.get() : nullptr;
+ return (it != m_IndirectObjs.end()) ? FilterInvalidObjNum(it->second.get())
+ : nullptr;
}
CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
uint32_t objnum) {
- if (objnum == 0)
+ if (objnum == 0 || objnum == CPDF_Object::kInvalidObjNum)
return nullptr;
- CPDF_Object* pObj = GetIndirectObject(objnum);
- if (pObj)
- return pObj->GetObjNum() != CPDF_Object::kInvalidObjNum ? pObj : nullptr;
+ // 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());
std::unique_ptr<CPDF_Object> pNewObj = ParseIndirectObject(objnum);
- if (!pNewObj)
+ if (!pNewObj) {
+ m_IndirectObjs.erase(insert_result.first);
return nullptr;
+ }
pNewObj->m_ObjNum = objnum;
m_LastObjNum = std::max(m_LastObjNum, objnum);
- m_IndirectObjs[objnum] = std::move(pNewObj);
- return m_IndirectObjs[objnum].get();
+ insert_result.first->second = std::move(pNewObj);
+ return insert_result.first->second.get();
}
std::unique_ptr<CPDF_Object> CPDF_IndirectObjectHolder::ParseIndirectObject(
@@ -54,38 +66,43 @@ std::unique_ptr<CPDF_Object> CPDF_IndirectObjectHolder::ParseIndirectObject(
CPDF_Object* CPDF_IndirectObjectHolder::AddIndirectObject(
std::unique_ptr<CPDF_Object> pObj) {
CHECK(!pObj->m_ObjNum);
- CPDF_Object* pUnowned = pObj.get();
pObj->m_ObjNum = ++m_LastObjNum;
- if (m_IndirectObjs[m_LastObjNum])
- m_OrphanObjs.push_back(std::move(m_IndirectObjs[m_LastObjNum]));
- m_IndirectObjs[m_LastObjNum] = std::move(pObj);
- return pUnowned;
+ auto& obj_holder = m_IndirectObjs[m_LastObjNum];
+ if (obj_holder)
+ m_OrphanObjs.push_back(std::move(obj_holder));
+
+ obj_holder = std::move(pObj);
+ return obj_holder.get();
}
bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration(
uint32_t objnum,
std::unique_ptr<CPDF_Object> pObj) {
ASSERT(objnum);
- if (!pObj)
+ if (!pObj || objnum == CPDF_Object::kInvalidObjNum)
return false;
- CPDF_Object* pOldObj = GetIndirectObject(objnum);
- if (pOldObj && pObj->GetGenNum() <= pOldObj->GetGenNum())
+ auto& obj_holder = m_IndirectObjs[objnum];
+
+ const CPDF_Object* old_object = FilterInvalidObjNum(obj_holder.get());
+
+ if (old_object && pObj->GetGenNum() <= old_object->GetGenNum())
return false;
pObj->m_ObjNum = objnum;
- if (m_IndirectObjs[objnum])
- m_OrphanObjs.push_back(std::move(m_IndirectObjs[objnum]));
- m_IndirectObjs[objnum] = std::move(pObj);
+ if (obj_holder)
+ m_OrphanObjs.push_back(std::move(obj_holder));
+
+ obj_holder = std::move(pObj);
m_LastObjNum = std::max(m_LastObjNum, objnum);
return true;
}
void CPDF_IndirectObjectHolder::DeleteIndirectObject(uint32_t objnum) {
- CPDF_Object* pObj = GetIndirectObject(objnum);
- if (!pObj || pObj->GetObjNum() == CPDF_Object::kInvalidObjNum)
+ auto it = m_IndirectObjs.find(objnum);
+ if (it == m_IndirectObjs.end() || !FilterInvalidObjNum(it->second.get()))
return;
- m_IndirectObjs.erase(objnum);
+ m_IndirectObjs.erase(it);
}
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp
new file mode 100644
index 0000000000..666264f50b
--- /dev/null
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp
@@ -0,0 +1,81 @@
+// Copyright 2017 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fpdfapi/parser/cpdf_indirect_object_holder.h"
+
+#include <memory>
+#include <utility>
+
+#include "core/fpdfapi/parser/cpdf_null.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
+
+namespace {
+
+class MockIndirectObjectHolder : public CPDF_IndirectObjectHolder {
+ public:
+ MockIndirectObjectHolder() {}
+ ~MockIndirectObjectHolder() override {}
+
+ MOCK_METHOD1(ParseIndirectObject,
+ std::unique_ptr<CPDF_Object>(uint32_t objnum));
+};
+
+} // namespace
+
+TEST(CPDF_IndirectObjectHolderTest, RecursiveParseOfSameObject) {
+ MockIndirectObjectHolder mock_holder;
+ // ParseIndirectObject should not be called again on recursively same object
+ // parse request.
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_))
+ .WillOnce(::testing::WithArg<0>(::testing::Invoke(
+ [&mock_holder](uint32_t objnum) -> std::unique_ptr<CPDF_Object> {
+ const CPDF_Object* same_parse =
+ mock_holder.GetOrParseIndirectObject(objnum);
+ CHECK(!same_parse);
+ return pdfium::MakeUnique<CPDF_Null>();
+ })));
+
+ EXPECT_TRUE(mock_holder.GetOrParseIndirectObject(1000));
+}
+
+TEST(CPDF_IndirectObjectHolderTest, GetObjectMethods) {
+ static constexpr uint32_t kObjNum = 1000;
+ MockIndirectObjectHolder mock_holder;
+
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0);
+ EXPECT_FALSE(mock_holder.GetIndirectObject(kObjNum));
+ ::testing::Mock::VerifyAndClearExpectations(&mock_holder);
+
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_))
+ .WillOnce(::testing::WithArg<0>(::testing::Invoke(
+ [](uint32_t objnum) -> std::unique_ptr<CPDF_Object> {
+ return pdfium::MakeUnique<CPDF_Null>();
+ })));
+ EXPECT_TRUE(mock_holder.GetOrParseIndirectObject(kObjNum));
+ ::testing::Mock::VerifyAndClearExpectations(&mock_holder);
+
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0);
+ ASSERT_TRUE(mock_holder.GetIndirectObject(kObjNum));
+ ::testing::Mock::VerifyAndClearExpectations(&mock_holder);
+
+ EXPECT_EQ(kObjNum, mock_holder.GetIndirectObject(kObjNum)->GetObjNum());
+}
+
+TEST(CPDF_IndirectObjectHolderTest, ParseInvalidObjNum) {
+ MockIndirectObjectHolder mock_holder;
+
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0);
+ EXPECT_FALSE(
+ mock_holder.GetOrParseIndirectObject(CPDF_Object::kInvalidObjNum));
+}
+
+TEST(CPDF_IndirectObjectHolderTest, ReplaceObjectWithInvalidObjNum) {
+ MockIndirectObjectHolder mock_holder;
+
+ EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0);
+ EXPECT_FALSE(mock_holder.ReplaceIndirectObjectIfHigherGeneration(
+ CPDF_Object::kInvalidObjNum, pdfium::MakeUnique<CPDF_Null>()));
+}