From 71a7d377ff36a0be1af1848d5cac0ccb83ae725d Mon Sep 17 00:00:00 2001 From: Henrique Nakashima Date: Thu, 1 Feb 2018 17:07:13 +0000 Subject: Deprecate FPDFDest_GetPageIndex and create a fixed version. FPDFDest_GetDestPageIndex() has a well defined return value for errors (-1). Keeping FPDFDest_GetPageIndex() to avoid changing behavior of the old API for whoever relies on it. Bug: pdfium:938 Change-Id: Iad528923cb156e957a419540c262a65f45cb777d Reviewed-on: https://pdfium-review.googlesource.com/24811 Commit-Queue: Henrique Nakashima Reviewed-by: Lei Zhang --- core/fpdfdoc/cpdf_dest.cpp | 23 ++++++++++++++++++++++- core/fpdfdoc/cpdf_dest.h | 8 +++++++- fpdfsdk/fpdfdoc.cpp | 23 +++++++++++++++++++---- fpdfsdk/fpdfdoc_embeddertest.cpp | 7 +++++++ fpdfsdk/fpdfview_c_api_test.c | 1 + fpdfsdk/fsdk_actionhandler.cpp | 2 +- fxjs/cjs_document.cpp | 4 ++-- public/fpdf_doc.h | 13 +++++++++++++ 8 files changed, 72 insertions(+), 9 deletions(-) diff --git a/core/fpdfdoc/cpdf_dest.cpp b/core/fpdfdoc/cpdf_dest.cpp index 0098f73f66..015cdcbc09 100644 --- a/core/fpdfdoc/cpdf_dest.cpp +++ b/core/fpdfdoc/cpdf_dest.cpp @@ -38,7 +38,7 @@ CPDF_Dest::CPDF_Dest(CPDF_Object* pObj) : m_pObj(pObj) {} CPDF_Dest::~CPDF_Dest() {} -int CPDF_Dest::GetPageIndex(CPDF_Document* pDoc) const { +int CPDF_Dest::GetPageIndexDeprecated(CPDF_Document* pDoc) const { CPDF_Array* pArray = ToArray(m_pObj.Get()); if (!pArray) return 0; @@ -46,10 +46,31 @@ int CPDF_Dest::GetPageIndex(CPDF_Document* pDoc) const { CPDF_Object* pPage = pArray->GetDirectObjectAt(0); if (!pPage) return 0; + if (pPage->IsNumber()) return pPage->GetInteger(); + if (!pPage->IsDictionary()) return 0; + + return pDoc->GetPageIndex(pPage->GetObjNum()); +} + +int CPDF_Dest::GetDestPageIndex(CPDF_Document* pDoc) const { + CPDF_Array* pArray = ToArray(m_pObj.Get()); + if (!pArray) + return -1; + + CPDF_Object* pPage = pArray->GetDirectObjectAt(0); + if (!pPage) + return -1; + + if (pPage->IsNumber()) + return pPage->GetInteger(); + + if (!pPage->IsDictionary()) + return -1; + return pDoc->GetPageIndex(pPage->GetObjNum()); } diff --git a/core/fpdfdoc/cpdf_dest.h b/core/fpdfdoc/cpdf_dest.h index 584669a82d..709a2470c5 100644 --- a/core/fpdfdoc/cpdf_dest.h +++ b/core/fpdfdoc/cpdf_dest.h @@ -23,7 +23,13 @@ class CPDF_Dest { CPDF_Object* GetObject() const { return m_pObj.Get(); } ByteString GetRemoteName() const; - int GetPageIndex(CPDF_Document* pDoc) const; + + // Deprecated. Use GetDestPageIndex instead. + // This method is wrong. It returns 0 for errors, when it could mean the first + // page as well. Keeping it avoids changing the behavior of + // FPDFDest_GetPageIndex(). + int GetPageIndexDeprecated(CPDF_Document* pDoc) const; + int GetDestPageIndex(CPDF_Document* pDoc) const; uint32_t GetPageObjNum() const; // Returns the zoom mode, as one of the PDFDEST_VIEW_* values in fpdf_doc.h. diff --git a/fpdfsdk/fpdfdoc.cpp b/fpdfsdk/fpdfdoc.cpp index 47ecf4223b..4b51d6391d 100644 --- a/fpdfsdk/fpdfdoc.cpp +++ b/fpdfsdk/fpdfdoc.cpp @@ -200,14 +200,29 @@ FPDFAction_GetURIPath(FPDF_DOCUMENT document, } FPDF_EXPORT unsigned long FPDF_CALLCONV -FPDFDest_GetPageIndex(FPDF_DOCUMENT document, FPDF_DEST pDict) { - if (!pDict) +FPDFDest_GetPageIndex(FPDF_DOCUMENT document, FPDF_DEST pDest) { + if (!pDest) return 0; + CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document); if (!pDoc) return 0; - CPDF_Dest dest(static_cast(pDict)); - return dest.GetPageIndex(pDoc); + + CPDF_Dest dest(static_cast(pDest)); + return dest.GetPageIndexDeprecated(pDoc); +} + +FPDF_EXPORT long FPDF_CALLCONV FPDFDest_GetDestPageIndex(FPDF_DOCUMENT document, + FPDF_DEST pDest) { + if (!pDest) + return -1; + + CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document); + if (!pDoc) + return -1; + + CPDF_Dest dest(static_cast(pDest)); + return dest.GetDestPageIndex(pDoc); } FPDF_EXPORT unsigned long FPDF_CALLCONV diff --git a/fpdfsdk/fpdfdoc_embeddertest.cpp b/fpdfsdk/fpdfdoc_embeddertest.cpp index d346330486..abe3d8c646 100644 --- a/fpdfsdk/fpdfdoc_embeddertest.cpp +++ b/fpdfsdk/fpdfdoc_embeddertest.cpp @@ -20,26 +20,31 @@ TEST_F(FPDFDocEmbeddertest, DestGetPageIndex) { // NULL FPDF_DEST case. EXPECT_EQ(0U, FPDFDest_GetPageIndex(document(), nullptr)); + EXPECT_EQ(-1, FPDFDest_GetDestPageIndex(document(), nullptr)); // Page number directly in item from Dests NameTree. FPDF_DEST dest = FPDF_GetNamedDestByName(document(), "First"); EXPECT_TRUE(dest); EXPECT_EQ(1U, FPDFDest_GetPageIndex(document(), dest)); + EXPECT_EQ(1, FPDFDest_GetDestPageIndex(document(), dest)); // Page number via object reference in item from Dests NameTree. dest = FPDF_GetNamedDestByName(document(), "Next"); EXPECT_TRUE(dest); EXPECT_EQ(1U, FPDFDest_GetPageIndex(document(), dest)); + EXPECT_EQ(1, FPDFDest_GetDestPageIndex(document(), dest)); // Page number directly in item from Dests dictionary. dest = FPDF_GetNamedDestByName(document(), "FirstAlternate"); EXPECT_TRUE(dest); EXPECT_EQ(11U, FPDFDest_GetPageIndex(document(), dest)); + EXPECT_EQ(11, FPDFDest_GetDestPageIndex(document(), dest)); // Invalid object reference in item from Dests NameTree. dest = FPDF_GetNamedDestByName(document(), "LastAlternate"); EXPECT_TRUE(dest); EXPECT_EQ(0U, FPDFDest_GetPageIndex(document(), dest)); + EXPECT_EQ(-1, FPDFDest_GetDestPageIndex(document(), dest)); } TEST_F(FPDFDocEmbeddertest, DestGetView) { @@ -106,6 +111,7 @@ TEST_F(FPDFDocEmbeddertest, DestGetLocationInPage) { // NULL FPDF_DEST case. EXPECT_EQ(0U, FPDFDest_GetPageIndex(document(), nullptr)); + EXPECT_EQ(-1, FPDFDest_GetDestPageIndex(document(), nullptr)); FPDF_DEST dest = FPDF_GetNamedDestByName(document(), "First"); EXPECT_TRUE(dest); @@ -134,6 +140,7 @@ TEST_F(FPDFDocEmbeddertest, BUG_680376) { EXPECT_TRUE(dest); EXPECT_EQ(static_cast(-1), FPDFDest_GetPageIndex(document(), dest)); + EXPECT_EQ(-1, FPDFDest_GetDestPageIndex(document(), dest)); } TEST_F(FPDFDocEmbeddertest, ActionGetFilePath) { diff --git a/fpdfsdk/fpdfview_c_api_test.c b/fpdfsdk/fpdfview_c_api_test.c index abffb7676f..193050e84a 100644 --- a/fpdfsdk/fpdfview_c_api_test.c +++ b/fpdfsdk/fpdfview_c_api_test.c @@ -107,6 +107,7 @@ int CheckPDFiumCApi() { CHK(FPDFAction_GetDest); CHK(FPDFAction_GetFilePath); CHK(FPDFAction_GetURIPath); + CHK(FPDFDest_GetDestPageIndex); CHK(FPDFDest_GetPageIndex); CHK(FPDFDest_GetLocationInPage); CHK(FPDFDest_GetView); diff --git a/fpdfsdk/fsdk_actionhandler.cpp b/fpdfsdk/fsdk_actionhandler.cpp index 0059bb919c..49e00411b8 100644 --- a/fpdfsdk/fsdk_actionhandler.cpp +++ b/fpdfsdk/fsdk_actionhandler.cpp @@ -362,7 +362,7 @@ void CPDFSDK_ActionHandler::DoAction_GoTo( ASSERT(pPDFDocument); CPDF_Dest MyDest = action.GetDest(pPDFDocument); - int nPageIndex = MyDest.GetPageIndex(pPDFDocument); + int nPageIndex = MyDest.GetPageIndexDeprecated(pPDFDocument); int nFitType = MyDest.GetZoomMode(); const CPDF_Array* pMyArray = ToArray(MyDest.GetObject()); std::vector posArray; diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp index 76d3f18b40..9af0f07f75 100644 --- a/fxjs/cjs_document.cpp +++ b/fxjs/cjs_document.cpp @@ -1467,8 +1467,8 @@ CJS_Return Document::gotoNamedDest( scrollPositionArray.push_back(arrayObject->GetFloatAt(i)); } pRuntime->BeginBlock(); - m_pFormFillEnv->DoGoToAction(dest.GetPageIndex(pDocument), dest.GetZoomMode(), - scrollPositionArray.data(), + m_pFormFillEnv->DoGoToAction(dest.GetPageIndexDeprecated(pDocument), + dest.GetZoomMode(), scrollPositionArray.data(), scrollPositionArray.size()); pRuntime->EndBlock(); return CJS_Return(true); diff --git a/public/fpdf_doc.h b/public/fpdf_doc.h index b523575967..7ec232c45e 100644 --- a/public/fpdf_doc.h +++ b/public/fpdf_doc.h @@ -177,15 +177,28 @@ FPDFAction_GetURIPath(FPDF_DOCUMENT document, void* buffer, unsigned long buflen); +// Deprecated. Use FPDFDest_GetDestPageIndex() instead. +// // Get the page index of |dest|. // // document - handle to the document. // dest - handle to the destination. // // Returns the page index containing |dest|. Page indices start from 0. +// On an error, returns 0 or -1. Note that 0 can mean the first page, hence +// do not use this API. FPDF_EXPORT unsigned long FPDF_CALLCONV FPDFDest_GetPageIndex(FPDF_DOCUMENT document, FPDF_DEST dest); +// Get the page index of |dest|. +// +// document - handle to the document. +// dest - handle to the destination. +// +// Returns the -based page index containing |dest|. Returns -1 on error. +FPDF_EXPORT long FPDF_CALLCONV FPDFDest_GetDestPageIndex(FPDF_DOCUMENT document, + FPDF_DEST dest); + // Get the view (fit type) specified by |dest|. // Experimental API. Subject to change. // -- cgit v1.2.3