From fe06d5109cd575c1e53b9b1cc3cc4ec3c5d7364f Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 1 May 2018 17:25:25 +0000 Subject: Make FPDF_Document always be CPDF_Document. Greatly minimize the impact between going back and forth from XFA being on/off, so that XFA case is just an extension beyond the non-XFA data structures we've shipped for years, instead of being a complete replacement of them. Change-Id: I6c98206e0ec99ea443547a4931eba912b1764d54 Reviewed-on: https://pdfium-review.googlesource.com/31690 Reviewed-by: dsinclair Commit-Queue: Tom Sepez --- core/fpdfapi/parser/cpdf_document.h | 5 ++++- fpdfsdk/cpdfsdk_helpers.cpp | 23 +++------------------ fpdfsdk/cpdfsdk_helpers.h | 4 ---- fpdfsdk/fpdf_catalog_unittest.cpp | 33 ++++++++--------------------- fpdfsdk/fpdf_doc_unittest.cpp | 30 ++++++--------------------- fpdfsdk/fpdf_editpage.cpp | 15 +++++++++++--- fpdfsdk/fpdf_formfill.cpp | 9 ++++---- fpdfsdk/fpdf_save.cpp | 3 ++- fpdfsdk/fpdf_view.cpp | 41 +++++++++++++++++++++++++++---------- public/fpdfview.h | 2 +- testing/xfa_js_embedder_test.cpp | 4 +++- 11 files changed, 75 insertions(+), 94 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h index adf4eca75d..4466df640e 100644 --- a/core/fpdfapi/parser/cpdf_document.h +++ b/core/fpdfapi/parser/cpdf_document.h @@ -42,7 +42,10 @@ class JBig2_DocumentContext; class CPDF_Document : public CPDF_IndirectObjectHolder { public: // Type from which the XFA extension can subclass itself. - class Extension {}; + class Extension { + public: + virtual ~Extension() {} + }; explicit CPDF_Document(std::unique_ptr pParser); ~CPDF_Document() override; diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp index 040b0f2e5a..01d88f9f59 100644 --- a/fpdfsdk/cpdfsdk_helpers.cpp +++ b/fpdfsdk/cpdfsdk_helpers.cpp @@ -25,10 +25,6 @@ namespace { constexpr char kQuadPoints[] = "QuadPoints"; -FPDF_DOCUMENT FPDFDocumentFromUnderlying(UnderlyingDocumentType* doc) { - return static_cast(doc); -} - bool RaiseUnSupportError(int nError) { CFSDK_UnsupportInfo_Adapter* pAdapter = CPDF_ModuleMgr::Get()->GetUnsupportInfoAdapter(); @@ -148,33 +144,20 @@ bool FPDF_FileHandlerContext::Flush() { } // namespace -UnderlyingDocumentType* UnderlyingFromFPDFDocument(FPDF_DOCUMENT doc) { - return static_cast(doc); -} - UnderlyingPageType* UnderlyingFromFPDFPage(FPDF_PAGE page) { return static_cast(page); } CPDF_Document* CPDFDocumentFromFPDFDocument(FPDF_DOCUMENT doc) { -#ifdef PDF_ENABLE_XFA - return doc ? UnderlyingFromFPDFDocument(doc)->GetPDFDoc() : nullptr; -#else // PDF_ENABLE_XFA - return UnderlyingFromFPDFDocument(doc); -#endif // PDF_ENABLE_XFA + return reinterpret_cast(doc); } FPDF_DOCUMENT FPDFDocumentFromCPDFDocument(CPDF_Document* doc) { #ifdef PDF_ENABLE_XFA - if (!doc) - return nullptr; - if (!doc->GetExtension()) + if (doc && !doc->GetExtension()) doc->SetExtension(new CPDFXFA_Context(pdfium::WrapUnique(doc))); - return FPDFDocumentFromUnderlying( - static_cast(doc->GetExtension())); -#else // PDF_ENABLE_XFA - return FPDFDocumentFromUnderlying(doc); #endif // PDF_ENABLE_XFA + return reinterpret_cast(doc); } CPDF_Page* CPDFPageFromFPDFPage(FPDF_PAGE page) { diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h index a9708ccddb..afb6dde239 100644 --- a/fpdfsdk/cpdfsdk_helpers.h +++ b/fpdfsdk/cpdfsdk_helpers.h @@ -40,16 +40,12 @@ class CPDFXFA_Page; // from fpdfsdk. For master, these are CPDF_ types, but for XFA, these are // CPDFXFA_ types. #ifndef PDF_ENABLE_XFA -using UnderlyingDocumentType = CPDF_Document; using UnderlyingPageType = CPDF_Page; #else // PDF_ENABLE_XFA -using UnderlyingDocumentType = CPDFXFA_Context; using UnderlyingPageType = CPDFXFA_Page; #endif // PDF_ENABLE_XFA // Conversions to/from underlying types. -UnderlyingDocumentType* UnderlyingFromFPDFDocument(FPDF_DOCUMENT doc); - UnderlyingPageType* UnderlyingFromFPDFPage(FPDF_PAGE page); // Conversions to/from FPDF_ types. diff --git a/fpdfsdk/fpdf_catalog_unittest.cpp b/fpdfsdk/fpdf_catalog_unittest.cpp index 648cb24e7a..54eb4f77a0 100644 --- a/fpdfsdk/fpdf_catalog_unittest.cpp +++ b/fpdfsdk/fpdf_catalog_unittest.cpp @@ -12,6 +12,7 @@ #include "core/fpdfapi/parser/cpdf_parser.h" #include "core/fpdfapi/parser/cpdf_string.h" #include "fpdfsdk/cpdfsdk_helpers.h" +#include "public/cpp/fpdf_scopers.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" @@ -29,31 +30,12 @@ class CPDF_TestDocument : public CPDF_Document { } }; -#ifdef PDF_ENABLE_XFA -class CPDF_TestXFAContext : public CPDFXFA_Context { - public: - CPDF_TestXFAContext() - : CPDFXFA_Context(pdfium::MakeUnique()) {} - - void SetRoot(CPDF_Dictionary* root) { - static_cast(GetPDFDoc())->SetRoot(root); - } - - CPDF_IndirectObjectHolder* GetHolder() { return GetPDFDoc(); } -}; -using CPDF_TestPdfDocument = CPDF_TestXFAContext; -#else // PDF_ENABLE_XFA -using CPDF_TestPdfDocument = CPDF_TestDocument; -#endif // PDF_ENABLE_XFA - class PDFCatalogTest : public testing::Test { public: void SetUp() override { CPDF_ModuleMgr::Get()->Init(); - - m_pDoc = pdfium::MakeUnique(); - - // Setup the root directory. + auto pTestDoc = pdfium::MakeUnique(); + m_pDoc.reset(FPDFDocumentFromCPDFDocument(pTestDoc.release())); m_pRootObj = pdfium::MakeUnique(); } @@ -63,7 +45,7 @@ class PDFCatalogTest : public testing::Test { } protected: - std::unique_ptr m_pDoc; + ScopedFPDFDocument m_pDoc; std::unique_ptr m_pRootObj; }; @@ -71,12 +53,15 @@ TEST_F(PDFCatalogTest, IsTagged) { // Null doc EXPECT_FALSE(FPDFCatalog_IsTagged(nullptr)); + CPDF_TestDocument* pTestDoc = static_cast( + CPDFDocumentFromFPDFDocument(m_pDoc.get())); + // No root - m_pDoc->SetRoot(nullptr); + pTestDoc->SetRoot(nullptr); EXPECT_FALSE(FPDFCatalog_IsTagged(m_pDoc.get())); // Empty root - m_pDoc->SetRoot(m_pRootObj.get()); + pTestDoc->SetRoot(m_pRootObj.get()); EXPECT_FALSE(FPDFCatalog_IsTagged(m_pDoc.get())); // Root with other key diff --git a/fpdfsdk/fpdf_doc_unittest.cpp b/fpdfsdk/fpdf_doc_unittest.cpp index bfd42628d1..01c790507f 100644 --- a/fpdfsdk/fpdf_doc_unittest.cpp +++ b/fpdfsdk/fpdf_doc_unittest.cpp @@ -18,6 +18,7 @@ #include "core/fpdfapi/parser/cpdf_string.h" #include "core/fpdfdoc/cpdf_dest.h" #include "fpdfsdk/cpdfsdk_helpers.h" +#include "public/cpp/fpdf_scopers.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" #include "third_party/base/ptr_util.h" @@ -34,23 +35,6 @@ class CPDF_TestDocument : public CPDF_Document { CPDF_IndirectObjectHolder* GetHolder() { return this; } }; -#ifdef PDF_ENABLE_XFA -class CPDF_TestXFAContext : public CPDFXFA_Context { - public: - CPDF_TestXFAContext() - : CPDFXFA_Context(pdfium::MakeUnique()) {} - - void SetRoot(CPDF_Dictionary* root) { - static_cast(GetPDFDoc())->SetRoot(root); - } - - CPDF_IndirectObjectHolder* GetHolder() { return GetPDFDoc(); } -}; -using CPDF_TestPdfDocument = CPDF_TestXFAContext; -#else // PDF_ENABLE_XFA -using CPDF_TestPdfDocument = CPDF_TestDocument; -#endif // PDF_ENABLE_XFA - class PDFDocTest : public testing::Test { public: struct DictObjInfo { @@ -60,13 +44,11 @@ class PDFDocTest : public testing::Test { void SetUp() override { CPDF_ModuleMgr::Get()->Init(); - - m_pDoc = pdfium::MakeUnique(); - m_pIndirectObjs = m_pDoc->GetHolder(); - - // Setup the root directory. + auto pTestDoc = pdfium::MakeUnique(); + m_pIndirectObjs = pTestDoc->GetHolder(); m_pRootObj = pdfium::MakeUnique(); - m_pDoc->SetRoot(m_pRootObj.get()); + pTestDoc->SetRoot(m_pRootObj.get()); + m_pDoc.reset(FPDFDocumentFromCPDFDocument(pTestDoc.release())); } void TearDown() override { @@ -87,7 +69,7 @@ class PDFDocTest : public testing::Test { } protected: - std::unique_ptr m_pDoc; + ScopedFPDFDocument m_pDoc; UnownedPtr m_pIndirectObjs; std::unique_ptr m_pRootObj; }; diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index fc6b21f24c..3acbe76d10 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp @@ -153,8 +153,17 @@ FPDF_EXPORT FPDF_DOCUMENT FPDF_CALLCONV FPDF_CreateNewDocument() { FPDF_EXPORT void FPDF_CALLCONV FPDFPage_Delete(FPDF_DOCUMENT document, int page_index) { - if (UnderlyingDocumentType* pDoc = UnderlyingFromFPDFDocument(document)) - pDoc->DeletePage(page_index); + auto* pDoc = CPDFDocumentFromFPDFDocument(document); + if (!pDoc) + return; +#ifdef PDF_ENABLE_XFA + CPDFXFA_Context* pContext = + static_cast(pDoc->GetExtension()); + if (pContext) + pContext->DeletePage(page_index); +#else + pDoc->DeletePage(page_index); +#endif } FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document, @@ -176,7 +185,7 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document, #ifdef PDF_ENABLE_XFA auto pXFAPage = pdfium::MakeRetain( - static_cast(document), page_index); + static_cast(pDoc->GetExtension()), page_index); pXFAPage->LoadPDFPage(pPageDict); return pXFAPage.Leak(); // Caller takes ownership. #else // PDF_ENABLE_XFA diff --git a/fpdfsdk/fpdf_formfill.cpp b/fpdfsdk/fpdf_formfill.cpp index 5d762ee1f0..3170627cd5 100644 --- a/fpdfsdk/fpdf_formfill.cpp +++ b/fpdfsdk/fpdf_formfill.cpp @@ -287,7 +287,7 @@ FPDFDOC_InitFormFillEnvironment(FPDF_DOCUMENT document, if (!formInfo || formInfo->version != kRequiredVersion) return nullptr; - UnderlyingDocumentType* pDocument = UnderlyingFromFPDFDocument(document); + auto* pDocument = CPDFDocumentFromFPDFDocument(document); if (!pDocument) return nullptr; @@ -295,15 +295,16 @@ FPDFDOC_InitFormFillEnvironment(FPDF_DOCUMENT document, // If the CPDFXFA_Context has a FormFillEnvironment already then we've done // this and can just return the old Env. Otherwise, we'll end up setting a new // environment into the XFADocument and, that could get weird. - if (pDocument->GetFormFillEnv()) - return pDocument->GetFormFillEnv(); + auto* pContext = static_cast(pDocument->GetExtension()); + if (pContext->GetFormFillEnv()) + return pContext->GetFormFillEnv(); #endif auto pFormFillEnv = pdfium::MakeUnique( CPDFDocumentFromFPDFDocument(document), formInfo); #ifdef PDF_ENABLE_XFA - pDocument->SetFormFillEnv(pFormFillEnv.get()); + pContext->SetFormFillEnv(pFormFillEnv.get()); #endif // PDF_ENABLE_XFA return pFormFillEnv.release(); // Caller takes ownership. diff --git a/fpdfsdk/fpdf_save.cpp b/fpdfsdk/fpdf_save.cpp index 7da48f375d..2844f5a965 100644 --- a/fpdfsdk/fpdf_save.cpp +++ b/fpdfsdk/fpdf_save.cpp @@ -227,7 +227,8 @@ bool FPDF_Doc_Save(FPDF_DOCUMENT document, return 0; #ifdef PDF_ENABLE_XFA - CPDFXFA_Context* pContext = static_cast(document); + CPDFXFA_Context* pContext = + static_cast(pPDFDoc->GetExtension()); std::vector> fileList; SendPreSaveToXFADoc(pContext, &fileList); #endif // PDF_ENABLE_XFA diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index 0015716d98..00c59bbda5 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp @@ -256,7 +256,9 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetFormType(FPDF_DOCUMENT document) { #ifdef PDF_ENABLE_XFA FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDF_LoadXFA(FPDF_DOCUMENT document) { - return document && static_cast(document)->LoadXFADoc(); + auto* pDoc = CPDFDocumentFromFPDFDocument(document); + return pDoc && + static_cast(pDoc->GetExtension())->LoadXFADoc(); } #endif // PDF_ENABLE_XFA @@ -320,21 +322,30 @@ FPDF_GetSecurityHandlerRevision(FPDF_DOCUMENT document) { } FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageCount(FPDF_DOCUMENT document) { - UnderlyingDocumentType* pDoc = UnderlyingFromFPDFDocument(document); - return pDoc ? pDoc->GetPageCount() : 0; + auto* pDoc = CPDFDocumentFromFPDFDocument(document); + if (!pDoc) + return 0; +#ifdef PDF_ENABLE_XFA + auto* pContext = static_cast(pDoc->GetExtension()); + return pContext ? pContext->GetPageCount() : 0; +#else + return pDoc->GetPageCount(); +#endif } FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document, int page_index) { - UnderlyingDocumentType* pDoc = UnderlyingFromFPDFDocument(document); + auto* pDoc = CPDFDocumentFromFPDFDocument(document); if (!pDoc) return nullptr; - if (page_index < 0 || page_index >= pDoc->GetPageCount()) + if (page_index < 0 || page_index >= FPDF_GetPageCount(document)) return nullptr; #ifdef PDF_ENABLE_XFA - return pDoc->GetXFAPage(page_index).Leak(); + return static_cast(pDoc->GetExtension()) + ->GetXFAPage(page_index) + .Leak(); #else // PDF_ENABLE_XFA CPDF_Dictionary* pDict = pDoc->GetPage(page_index); if (!pDict) @@ -740,7 +751,14 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) { } FPDF_EXPORT void FPDF_CALLCONV FPDF_CloseDocument(FPDF_DOCUMENT document) { - delete UnderlyingFromFPDFDocument(document); + auto* pDoc = CPDFDocumentFromFPDFDocument(document); +#if PDF_ENABLE_XFA + // Deleting the extension will delete the document + if (pDoc) + delete pDoc->GetExtension(); +#else + delete pDoc; +#endif } FPDF_EXPORT unsigned long FPDF_CALLCONV FPDF_GetLastError() { @@ -918,15 +936,16 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document, int page_index, double* width, double* height) { - UnderlyingDocumentType* pDoc = UnderlyingFromFPDFDocument(document); + auto* pDoc = CPDFDocumentFromFPDFDocument(document); if (!pDoc) return false; #ifdef PDF_ENABLE_XFA - int count = pDoc->GetPageCount(); - if (page_index < 0 || page_index >= count) + if (page_index < 0 || page_index >= FPDF_GetPageCount(document)) return false; - RetainPtr pPage = pDoc->GetXFAPage(page_index); + RetainPtr pPage = + static_cast(pDoc->GetExtension()) + ->GetXFAPage(page_index); if (!pPage) return false; *width = pPage->GetPageWidth(); diff --git a/public/fpdfview.h b/public/fpdfview.h index 20f710f3c6..9ebfd8d99d 100644 --- a/public/fpdfview.h +++ b/public/fpdfview.h @@ -40,7 +40,7 @@ typedef void* FPDF_BITMAP; typedef void* FPDF_BOOKMARK; typedef void* FPDF_CLIPPATH; typedef void* FPDF_DEST; -typedef void* FPDF_DOCUMENT; +typedef struct fpdf_document_t__* FPDF_DOCUMENT; typedef void* FPDF_FONT; typedef void* FPDF_LINK; typedef void* FPDF_PAGE; diff --git a/testing/xfa_js_embedder_test.cpp b/testing/xfa_js_embedder_test.cpp index 995c0ed1d5..375497f9ea 100644 --- a/testing/xfa_js_embedder_test.cpp +++ b/testing/xfa_js_embedder_test.cpp @@ -44,7 +44,9 @@ void XFAJSEmbedderTest::TearDown() { } CXFA_Document* XFAJSEmbedderTest::GetXFADocument() { - return UnderlyingFromFPDFDocument(document())->GetXFADoc()->GetXFADoc(); + auto* pDoc = CPDFDocumentFromFPDFDocument(document()); + auto* pContext = static_cast(pDoc->GetExtension()); + return pContext->GetXFADoc()->GetXFADoc(); } bool XFAJSEmbedderTest::OpenDocumentWithOptions( -- cgit v1.2.3