Use vector of unique_ptrs for page node children.
Also be sure that a loop terminates without relying on
specific behaviour of the callers.
Review-Url: https://codereview.chromium.org/2598473002
diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp
index 76ea73d..d94ef2c 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -24,6 +24,7 @@
#include "core/fxcrt/fx_ext.h"
#include "core/fxcrt/fx_safe_types.h"
#include "third_party/base/numerics/safe_conversions.h"
+#include "third_party/base/ptr_util.h"
#include "third_party/base/stl_util.h"
CPDF_DataAvail::FileAvail::~FileAvail() {}
@@ -1147,16 +1148,16 @@
if (!pKid)
continue;
- PageNode* pNode = new PageNode();
- pPageNode->m_childNode.Add(pNode);
+ auto pNode = pdfium::MakeUnique<PageNode>();
pNode->m_dwPageNo = pKid->GetRefObjNum();
+ pPageNode->m_ChildNodes.push_back(std::move(pNode));
}
return true;
}
-bool CPDF_DataAvail::CheckUnkownPageNode(uint32_t dwPageNo,
- PageNode* pPageNode,
- DownloadHints* pHints) {
+bool CPDF_DataAvail::CheckUnknownPageNode(uint32_t dwPageNo,
+ PageNode* pPageNode,
+ DownloadHints* pHints) {
bool bExist = false;
std::unique_ptr<CPDF_Object> pPage = GetObject(dwPageNo, pHints, &bExist);
if (!bExist) {
@@ -1195,9 +1196,9 @@
switch (pKids->GetType()) {
case CPDF_Object::REFERENCE: {
CPDF_Reference* pKid = pKids->AsReference();
- PageNode* pNode = new PageNode();
- pPageNode->m_childNode.Add(pNode);
+ auto pNode = pdfium::MakeUnique<PageNode>();
pNode->m_dwPageNo = pKid->GetRefObjNum();
+ pPageNode->m_ChildNodes.push_back(std::move(pNode));
} break;
case CPDF_Object::ARRAY: {
CPDF_Array* pKidsArray = pKids->AsArray();
@@ -1206,9 +1207,9 @@
if (!pKid)
continue;
- PageNode* pNode = new PageNode();
- pPageNode->m_childNode.Add(pNode);
+ auto pNode = pdfium::MakeUnique<PageNode>();
pNode->m_dwPageNo = pKid->GetRefObjNum();
+ pPageNode->m_ChildNodes.push_back(std::move(pNode));
}
} break;
default:
@@ -1223,7 +1224,7 @@
return true;
}
-bool CPDF_DataAvail::CheckPageNode(CPDF_DataAvail::PageNode& pageNodes,
+bool CPDF_DataAvail::CheckPageNode(const CPDF_DataAvail::PageNode& pageNode,
int32_t iPage,
int32_t& iCount,
DownloadHints* pHints,
@@ -1231,24 +1232,27 @@
if (level >= kMaxPageRecursionDepth)
return false;
- int32_t iSize = pageNodes.m_childNode.GetSize();
+ int32_t iSize = pdfium::CollectionSize<int32_t>(pageNode.m_ChildNodes);
if (iSize <= 0 || iPage >= iSize) {
m_docStatus = PDF_DATAAVAIL_ERROR;
return false;
}
-
for (int32_t i = 0; i < iSize; ++i) {
- PageNode* pNode = pageNodes.m_childNode.GetAt(i);
+ PageNode* pNode = pageNode.m_ChildNodes[i].get();
if (!pNode)
continue;
+ if (pNode->m_type == PDF_PAGENODE_UNKNOWN) {
+ // Updates the type for the unknown page node.
+ if (!CheckUnknownPageNode(pNode->m_dwPageNo, pNode, pHints))
+ return false;
+ }
+ if (pNode->m_type == PDF_PAGENODE_ARRAY) {
+ // Updates a more specific type for the array page node.
+ if (!CheckArrayPageNode(pNode->m_dwPageNo, pNode, pHints))
+ return false;
+ }
switch (pNode->m_type) {
- case PDF_PAGENODE_UNKNOWN:
- if (!CheckUnkownPageNode(pNode->m_dwPageNo, pNode, pHints)) {
- return false;
- }
- --i;
- break;
case PDF_PAGENODE_PAGE:
iCount++;
if (iPage == iCount && m_pDocument)
@@ -1258,13 +1262,11 @@
if (!CheckPageNode(*pNode, iPage, iCount, pHints, level + 1))
return false;
break;
+ case PDF_PAGENODE_UNKNOWN:
case PDF_PAGENODE_ARRAY:
- if (!CheckArrayPageNode(pNode->m_dwPageNo, pNode, pHints))
- return false;
- --i;
- break;
+ // Already converted above, error if we get here.
+ return false;
}
-
if (iPage == iCount) {
m_docStatus = PDF_DATAAVAIL_DONE;
return true;
@@ -1281,17 +1283,12 @@
m_docStatus = PDF_DATAAVAIL_DONE;
return true;
}
-
- if (m_pageNodes.m_type == PDF_PAGENODE_PAGE) {
- if (iPage == 0) {
- m_docStatus = PDF_DATAAVAIL_DONE;
- return true;
- }
- m_docStatus = PDF_DATAAVAIL_ERROR;
+ if (m_PageNode.m_type == PDF_PAGENODE_PAGE) {
+ m_docStatus = iPage == 0 ? PDF_DATAAVAIL_DONE : PDF_DATAAVAIL_ERROR;
return true;
}
int32_t iCount = -1;
- return CheckPageNode(m_pageNodes, iPage, iCount, pHints, 0);
+ return CheckPageNode(m_PageNode, iPage, iCount, pHints, 0);
}
bool CPDF_DataAvail::CheckPageCount(DownloadHints* pHints) {
@@ -1319,7 +1316,7 @@
}
bool CPDF_DataAvail::LoadDocPages(DownloadHints* pHints) {
- if (!CheckUnkownPageNode(m_PagesObjNum, &m_pageNodes, pHints))
+ if (!CheckUnknownPageNode(m_PagesObjNum, &m_PageNode, pHints))
return false;
if (CheckPageCount(pHints)) {
@@ -1703,8 +1700,4 @@
CPDF_DataAvail::PageNode::PageNode() : m_type(PDF_PAGENODE_UNKNOWN) {}
-CPDF_DataAvail::PageNode::~PageNode() {
- for (int32_t i = 0; i < m_childNode.GetSize(); ++i)
- delete m_childNode[i];
- m_childNode.RemoveAll();
-}
+CPDF_DataAvail::PageNode::~PageNode() {}
diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h
index 1ced2a2..42013e9 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.h
+++ b/core/fpdfapi/parser/cpdf_data_avail.h
@@ -121,7 +121,7 @@
PDF_PAGENODE_TYPE m_type;
uint32_t m_dwPageNo;
- CFX_ArrayTemplate<PageNode*> m_childNode;
+ std::vector<std::unique_ptr<PageNode>> m_ChildNodes;
};
static const int kMaxDataAvailRecursionDepth = 64;
@@ -179,14 +179,14 @@
bool CheckPage(uint32_t dwPage, DownloadHints* pHints);
bool LoadDocPages(DownloadHints* pHints);
bool LoadDocPage(uint32_t dwPage, DownloadHints* pHints);
- bool CheckPageNode(PageNode& pageNodes,
+ bool CheckPageNode(const PageNode& pageNode,
int32_t iPage,
int32_t& iCount,
DownloadHints* pHints,
int level);
- bool CheckUnkownPageNode(uint32_t dwPageNo,
- PageNode* pPageNode,
- DownloadHints* pHints);
+ bool CheckUnknownPageNode(uint32_t dwPageNo,
+ PageNode* pPageNode,
+ DownloadHints* pHints);
bool CheckArrayPageNode(uint32_t dwPageNo,
PageNode* pPageNode,
DownloadHints* pHints);
@@ -247,7 +247,7 @@
FX_FILESIZE m_dwPrevXRefOffset;
bool m_bTotalLoadPageTree;
bool m_bCurPageDictLoadOK;
- PageNode m_pageNodes;
+ PageNode m_PageNode;
std::set<uint32_t> m_pageMapCheckState;
std::set<uint32_t> m_pagesLoadState;
std::unique_ptr<CPDF_HintTables> m_pHintTables;
