Pass retained array to CPDF_Dest constructor.
Change-Id: I620cc187773cc0ee39cc0420eb5d17e6e4f1212c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98451
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_dest.cpp b/core/fpdfdoc/cpdf_dest.cpp
index 62ca7d5..9d555e8 100644
--- a/core/fpdfdoc/cpdf_dest.cpp
+++ b/core/fpdfdoc/cpdf_dest.cpp
@@ -8,6 +8,7 @@
#include <algorithm>
#include <iterator>
+#include <utility>
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_document.h"
@@ -30,7 +31,8 @@
} // namespace
-CPDF_Dest::CPDF_Dest(const CPDF_Array* pArray) : m_pArray(pArray) {}
+CPDF_Dest::CPDF_Dest(RetainPtr<const CPDF_Array> pArray)
+ : m_pArray(std::move(pArray)) {}
CPDF_Dest::CPDF_Dest(const CPDF_Dest& that) = default;
@@ -42,12 +44,10 @@
if (!pDest)
return CPDF_Dest(nullptr);
- if (pDest->IsString() || pDest->IsName()) {
- // TODO(tsepez): make CPDF_Dest constructor take retained args.
- return CPDF_Dest(
- CPDF_NameTree::LookupNamedDest(pDoc, pDest->GetString()).Get());
- }
- return CPDF_Dest(pDest->AsArray());
+ if (pDest->IsString() || pDest->IsName())
+ return CPDF_Dest(CPDF_NameTree::LookupNamedDest(pDoc, pDest->GetString()));
+
+ return CPDF_Dest(ToArray(pDest));
}
int CPDF_Dest::GetDestPageIndex(CPDF_Document* pDoc) const {
diff --git a/core/fpdfdoc/cpdf_dest.h b/core/fpdfdoc/cpdf_dest.h
index 75d93aa..5c5e7fc 100644
--- a/core/fpdfdoc/cpdf_dest.h
+++ b/core/fpdfdoc/cpdf_dest.h
@@ -7,15 +7,15 @@
#ifndef CORE_FPDFDOC_CPDF_DEST_H_
#define CORE_FPDFDOC_CPDF_DEST_H_
+#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fxcrt/retain_ptr.h"
-class CPDF_Array;
class CPDF_Document;
class CPDF_Object;
class CPDF_Dest {
public:
- explicit CPDF_Dest(const CPDF_Array* pArray);
+ explicit CPDF_Dest(RetainPtr<const CPDF_Array> pArray);
CPDF_Dest(const CPDF_Dest& that);
~CPDF_Dest();
diff --git a/core/fpdfdoc/cpdf_dest_unittest.cpp b/core/fpdfdoc/cpdf_dest_unittest.cpp
index 0157eaa..295a9b5 100644
--- a/core/fpdfdoc/cpdf_dest_unittest.cpp
+++ b/core/fpdfdoc/cpdf_dest_unittest.cpp
@@ -26,19 +26,19 @@
array->AppendNew<CPDF_Name>("XYZ");
array->AppendNew<CPDF_Number>(4); // X
{
- auto dest = std::make_unique<CPDF_Dest>(nullptr);
- EXPECT_FALSE(dest->GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
+ CPDF_Dest dest(nullptr);
+ EXPECT_FALSE(dest.GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
}
{
// Not enough entries.
- auto dest = std::make_unique<CPDF_Dest>(array.Get());
- EXPECT_FALSE(dest->GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
+ CPDF_Dest dest(array);
+ EXPECT_FALSE(dest.GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
}
array->AppendNew<CPDF_Number>(5); // Y
array->AppendNew<CPDF_Number>(6); // Zoom.
{
- auto dest = std::make_unique<CPDF_Dest>(array.Get());
- EXPECT_TRUE(dest->GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
+ CPDF_Dest dest(array);
+ EXPECT_TRUE(dest.GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
EXPECT_TRUE(hasX);
EXPECT_TRUE(hasY);
EXPECT_TRUE(hasZoom);
@@ -49,8 +49,8 @@
// Set zoom to 0.
array->SetNewAt<CPDF_Number>(4, 0);
{
- auto dest = std::make_unique<CPDF_Dest>(array.Get());
- EXPECT_TRUE(dest->GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
+ CPDF_Dest dest(array);
+ EXPECT_TRUE(dest.GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
EXPECT_FALSE(hasZoom);
}
// Set values to null.
@@ -58,8 +58,8 @@
array->SetNewAt<CPDF_Null>(3);
array->SetNewAt<CPDF_Null>(4);
{
- auto dest = std::make_unique<CPDF_Dest>(array.Get());
- EXPECT_TRUE(dest->GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
+ CPDF_Dest dest(array);
+ EXPECT_TRUE(dest.GetXYZ(&hasX, &hasY, &hasZoom, &x, &y, &zoom));
EXPECT_FALSE(hasX);
EXPECT_FALSE(hasY);
EXPECT_FALSE(hasZoom);
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index c6c396e..e0271f6 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -250,7 +250,7 @@
if (!dest)
return -1;
- CPDF_Dest destination(CPDFArrayFromFPDFDest(dest));
+ CPDF_Dest destination(pdfium::WrapRetain(CPDFArrayFromFPDFDest(dest)));
return destination.GetDestPageIndex(pDoc);
}
@@ -261,7 +261,7 @@
return 0;
}
- CPDF_Dest destination(CPDFArrayFromFPDFDest(dest));
+ CPDF_Dest destination(pdfium::WrapRetain(CPDFArrayFromFPDFDest(dest)));
const unsigned long nParams =
pdfium::base::checked_cast<unsigned long>(destination.GetNumParams());
DCHECK(nParams <= 4);
@@ -282,13 +282,13 @@
if (!dest)
return false;
- auto destination = std::make_unique<CPDF_Dest>(CPDFArrayFromFPDFDest(dest));
+ CPDF_Dest destination(pdfium::WrapRetain(CPDFArrayFromFPDFDest(dest)));
// FPDF_BOOL is an int, GetXYZ expects bools.
bool bHasX;
bool bHasY;
bool bHasZoom;
- if (!destination->GetXYZ(&bHasX, &bHasY, &bHasZoom, x, y, zoom))
+ if (!destination.GetXYZ(&bHasX, &bHasY, &bHasZoom, x, y, zoom))
return false;
*hasXVal = bHasX;
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index 4195257..e8983e6 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -1388,8 +1388,7 @@
if (!dest_array)
return CJS_Result::Failure(JSMessage::kBadObjectError);
- // TODO(tsepez): make CPDF_Dest constructor take retained argument.
- CPDF_Dest dest(dest_array.Get());
+ CPDF_Dest dest(std::move(dest_array));
const CPDF_Array* arrayObject = dest.GetArray();
std::vector<float> scrollPositionArray;
if (arrayObject) {
