Return page indices from ParsePageRangeString()
Almost everywhere else in PDFium, pages are enumerated using 0-based
indices instead of 1-based numbers. Consequently, callers of
ParsePageRangeString() have to re-convert the return value of the
function.
Meanwhile:
- Update the callers to use indices directly.
- Use spans in more places.
- Update unit tests to use matchers.
Change-Id: I140ac484a775662472a8c0edcb85a8d645868624
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/81977
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index 30db260..11b9872 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -471,7 +471,7 @@
pdfium::base::checked_cast<uint32_t>(atoi(args[0].c_str()));
if (page_num == 0 || page_num > nCount)
return std::vector<uint32_t>();
- results.push_back(page_num);
+ results.push_back(page_num - 1);
} else if (args.size() == 2) {
uint32_t first_num =
pdfium::base::checked_cast<uint32_t>(atoi(args[0].c_str()));
@@ -482,7 +482,7 @@
if (last_num == 0 || first_num > last_num || last_num > nCount)
return std::vector<uint32_t>();
for (uint32_t i = first_num; i <= last_num; ++i)
- results.push_back(i);
+ results.push_back(i - 1);
} else {
return std::vector<uint32_t>();
}
diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h
index 9330d9a..126fa7f 100644
--- a/fpdfsdk/cpdfsdk_helpers.h
+++ b/fpdfsdk/cpdfsdk_helpers.h
@@ -281,6 +281,7 @@
void SetColorFromScheme(const FPDF_COLORSCHEME* pColorScheme,
CPDF_RenderOptions* pRenderOptions);
+// Returns a vector of page indices given a page range string.
std::vector<uint32_t> ParsePageRangeString(const ByteString& bsPageRange,
uint32_t nCount);
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
index 21b0061..0536c6a 100644
--- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -4,8 +4,12 @@
#include "fpdfsdk/cpdfsdk_helpers.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
TEST(CPDFSDK_HelpersTest, NulTerminateMaybeCopyAndReturnLength) {
{
const ByteString to_be_copied("toBeCopied");
@@ -40,50 +44,45 @@
}
TEST(CPDFSDK_HelpersTest, ParsePageRangeString) {
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("", 1));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(" ", 1));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams", 1));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("0", 0));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1", 0));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(",1", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,clams", 1));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams,1", 1));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("0-1", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-0", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-5", 4));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-11,", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(",1-1", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-,", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("-2,", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-clams", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams-1,", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2clams", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("0,1", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,0", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2,,,,3-4", 10));
- EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2-", 10));
- EXPECT_EQ(std::vector<uint32_t>({1}), ParsePageRangeString("1-1", 10));
- EXPECT_EQ(std::vector<uint32_t>{1}, ParsePageRangeString("1", 1));
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4}),
- ParsePageRangeString("1-4", 4));
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4}),
- ParsePageRangeString("1- 4", 4));
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4}),
- ParsePageRangeString("1 -4", 4));
- EXPECT_EQ(std::vector<uint32_t>({1, 2}), ParsePageRangeString("1,2", 10));
- EXPECT_EQ(std::vector<uint32_t>({2, 1}), ParsePageRangeString("2,1", 10));
- EXPECT_EQ(std::vector<uint32_t>({1, 50, 2}),
- ParsePageRangeString("1,50,2", 100));
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 50}),
- ParsePageRangeString("1-4,50", 100));
- EXPECT_EQ(std::vector<uint32_t>({50, 1, 2}),
- ParsePageRangeString("50,1-2", 100));
- EXPECT_EQ(std::vector<uint32_t>({50, 1, 2}),
- ParsePageRangeString("5 0, 1-2 ", 100)); // ???
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 5, 6}),
- ParsePageRangeString("1-3,4-6", 10));
- EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 3, 4, 5, 6}),
- ParsePageRangeString("1-4,3-6", 10));
+ EXPECT_THAT(ParsePageRangeString("", 1), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString(" ", 1), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("clams", 1), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("0", 0), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1", 0), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString(",1", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1,", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1,clams", 1), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("clams,1", 1), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("0-1", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-0", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-5", 4), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-11,", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString(",1-1", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-,", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("-2,", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-clams", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("clams-1,", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-2clams", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("0,1", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1,0", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-2,,,,3-4", 10), IsEmpty());
+ EXPECT_THAT(ParsePageRangeString("1-2-", 10), IsEmpty());
+
+ EXPECT_THAT(ParsePageRangeString("1-1", 10), ElementsAre(0));
+ EXPECT_THAT(ParsePageRangeString("1", 1), ElementsAre(0));
+ EXPECT_THAT(ParsePageRangeString("1-4", 4), ElementsAre(0, 1, 2, 3));
+ EXPECT_THAT(ParsePageRangeString("1- 4", 4), ElementsAre(0, 1, 2, 3));
+ EXPECT_THAT(ParsePageRangeString("1 -4", 4), ElementsAre(0, 1, 2, 3));
+ EXPECT_THAT(ParsePageRangeString("1,2", 10), ElementsAre(0, 1));
+ EXPECT_THAT(ParsePageRangeString("2,1", 10), ElementsAre(1, 0));
+ EXPECT_THAT(ParsePageRangeString("1,50,2", 100), ElementsAre(0, 49, 1));
+ EXPECT_THAT(ParsePageRangeString("1-4,50", 100), ElementsAre(0, 1, 2, 3, 49));
+ EXPECT_THAT(ParsePageRangeString("50,1-2", 100), ElementsAre(49, 0, 1));
+ EXPECT_THAT(ParsePageRangeString("5 0, 1-2 ", 100),
+ ElementsAre(49, 0, 1)); // ???
+ EXPECT_THAT(ParsePageRangeString("1-3,4-6", 10),
+ ElementsAre(0, 1, 2, 3, 4, 5));
+ EXPECT_THAT(ParsePageRangeString("1-4,3-6", 10),
+ ElementsAre(0, 1, 2, 3, 2, 3, 4, 5));
}
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index abd3340..3e46ed8 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -9,6 +9,7 @@
#include <algorithm>
#include <map>
#include <memory>
+#include <numeric>
#include <sstream>
#include <utility>
#include <vector>
@@ -33,6 +34,7 @@
#include "fpdfsdk/cpdfsdk_helpers.h"
#include "public/cpp/fpdf_scopers.h"
#include "third_party/base/check.h"
+#include "third_party/base/span.h"
namespace {
@@ -201,17 +203,15 @@
return true;
}
-std::vector<uint32_t> GetPageNumbers(const CPDF_Document& doc,
+std::vector<uint32_t> GetPageIndices(const CPDF_Document& doc,
const ByteString& bsPageRange) {
uint32_t nCount = doc.GetPageCount();
if (!bsPageRange.IsEmpty())
return ParsePageRangeString(bsPageRange, nCount);
- std::vector<uint32_t> page_numbers;
- for (uint32_t i = 1; i <= nCount; ++i)
- page_numbers.push_back(i);
-
- return page_numbers;
+ std::vector<uint32_t> page_indices(nCount);
+ std::iota(page_indices.begin(), page_indices.end(), 0);
+ return page_indices;
}
class CPDF_PageOrganizer {
@@ -382,11 +382,10 @@
CPDF_PageExporter(CPDF_Document* pDestDoc, CPDF_Document* pSrcDoc);
~CPDF_PageExporter();
- // For the pages from the source document with |pageNums| as their page
- // numbers, insert them into the destination document at page |nIndex|.
- // |pageNums| is 1-based.
- // |nIndex| is 0-based.
- bool ExportPage(const std::vector<uint32_t>& pageNums, int nIndex);
+ // For the pages from the source document with |pageIndices| as their page
+ // indices, insert them into the destination document at page |nIndex|.
+ // |pageIndices| and |nIndex| are 0-based.
+ bool ExportPage(pdfium::span<const uint32_t> pageIndices, int nIndex);
};
CPDF_PageExporter::CPDF_PageExporter(CPDF_Document* pDestDoc,
@@ -395,15 +394,15 @@
CPDF_PageExporter::~CPDF_PageExporter() = default;
-bool CPDF_PageExporter::ExportPage(const std::vector<uint32_t>& pageNums,
+bool CPDF_PageExporter::ExportPage(pdfium::span<const uint32_t> pageIndices,
int nIndex) {
if (!Init())
return false;
int curpage = nIndex;
- for (size_t i = 0; i < pageNums.size(); ++i) {
+ for (uint32_t pageIndex : pageIndices) {
CPDF_Dictionary* pDestPageDict = dest()->CreateNewPage(curpage);
- auto* pSrcPageDict = src()->GetPageDictionary(pageNums[i] - 1);
+ auto* pSrcPageDict = src()->GetPageDictionary(pageIndex);
if (!pSrcPageDict || !pDestPageDict)
return false;
@@ -473,14 +472,15 @@
CPDF_NPageToOneExporter(CPDF_Document* pDestDoc, CPDF_Document* pSrcDoc);
~CPDF_NPageToOneExporter();
- // For the pages from the source document with |pageNums| as their page
- // numbers, insert them into the destination document, starting at page 0.
- // |pageNums| is 1-based.
+ // For the pages from the source document with |pageIndices| as their page
+ // indices, insert them into the destination document, starting at page index
+ // 0.
+ // |pageIndices| is 0-based.
// |destPageSize| is the destination document page dimensions, measured in
// PDF "user space" units.
// |nPagesOnXAxis| and |nPagesOnXAxis| together defines how many source
// pages fit on one destination page.
- bool ExportNPagesToOne(const std::vector<uint32_t>& pageNums,
+ bool ExportNPagesToOne(pdfium::span<const uint32_t> pageIndices,
const CFX_SizeF& destPageSize,
size_t nPagesOnXAxis,
size_t nPagesOnYAxis);
@@ -525,7 +525,7 @@
CPDF_NPageToOneExporter::~CPDF_NPageToOneExporter() = default;
bool CPDF_NPageToOneExporter::ExportNPagesToOne(
- const std::vector<uint32_t>& pageNums,
+ pdfium::span<const uint32_t> pageIndices,
const CFX_SizeF& destPageSize,
size_t nPagesOnXAxis,
size_t nPagesOnYAxis) {
@@ -545,7 +545,7 @@
size_t curpage = 0;
const CFX_FloatRect destPageRect(0, 0, destPageSize.width,
destPageSize.height);
- for (size_t iOuterPage = 0; iOuterPage < pageNums.size();
+ for (size_t iOuterPage = 0; iOuterPage < pageIndices.size();
iOuterPage += nPagesPerSheet) {
m_XObjectNameToNumberMap.clear();
@@ -557,9 +557,9 @@
pDestPageDict->SetRectFor(pdfium::page_object::kMediaBox, destPageRect);
ByteString bsContent;
size_t iInnerPageMax =
- std::min(iOuterPage + nPagesPerSheet, pageNums.size());
+ std::min(iOuterPage + nPagesPerSheet, pageIndices.size());
for (size_t i = iOuterPage; i < iInnerPageMax; ++i) {
- auto* pSrcPageDict = src()->GetPageDictionary(pageNums[i] - 1);
+ auto* pSrcPageDict = src()->GetPageDictionary(pageIndices[i]);
if (!pSrcPageDict)
return false;
@@ -692,12 +692,12 @@
if (!pSrcDoc)
return false;
- std::vector<uint32_t> page_numbers = GetPageNumbers(*pSrcDoc, pagerange);
- if (page_numbers.empty())
+ std::vector<uint32_t> page_indices = GetPageIndices(*pSrcDoc, pagerange);
+ if (page_indices.empty())
return false;
CPDF_PageExporter exporter(pDestDoc, pSrcDoc);
- return exporter.ExportPage(page_numbers, index);
+ return exporter.ExportPage(page_indices, index);
}
FPDF_EXPORT FPDF_DOCUMENT FPDF_CALLCONV
@@ -722,19 +722,19 @@
CPDF_Document* pDestDoc = CPDFDocumentFromFPDFDocument(output_doc.get());
DCHECK(pDestDoc);
- std::vector<uint32_t> page_numbers = GetPageNumbers(*pSrcDoc, ByteString());
- if (page_numbers.empty())
+ std::vector<uint32_t> page_indices = GetPageIndices(*pSrcDoc, ByteString());
+ if (page_indices.empty())
return nullptr;
if (num_pages_on_x_axis == 1 && num_pages_on_y_axis == 1) {
CPDF_PageExporter exporter(pDestDoc, pSrcDoc);
- if (!exporter.ExportPage(page_numbers, 0))
+ if (!exporter.ExportPage(page_indices, 0))
return nullptr;
return output_doc.release();
}
CPDF_NPageToOneExporter exporter(pDestDoc, pSrcDoc);
- if (!exporter.ExportNPagesToOne(page_numbers,
+ if (!exporter.ExportNPagesToOne(page_indices,
CFX_SizeF(output_width, output_height),
num_pages_on_x_axis, num_pages_on_y_axis)) {
return nullptr;
