From 211d4edbe2f71ca62c76f36ce25090342c58e43c Mon Sep 17 00:00:00 2001 From: tsepez Date: Fri, 11 Nov 2016 17:23:48 -0800 Subject: Add fpdfppo_embeddertest.cpp. The lack of coverage of the fpdfppo APIs was noticed while trying to diagnose another issue. Adding basic calls to these APIs then kicked out an assert in XFA, where duplicate global CFXA_TimeZoneProviders were not expected. These are cheap to create except for the global C RTL tzset() call, so keep track of that and make these on demand. Review-Url: https://codereview.chromium.org/2488403004 --- BUILD.gn | 1 + fpdfsdk/fpdfppo_embeddertest.cpp | 92 +++++++++++++++++++++++++++++++++++ xfa/fgas/localization/fgas_locale.cpp | 6 +-- xfa/fgas/localization/fgas_locale.h | 2 +- xfa/fxfa/app/xfa_ffapp.cpp | 5 +- xfa/fxfa/fm2js/xfa_fm2jscontext.cpp | 28 +++++------ xfa/fxfa/parser/xfa_locale.cpp | 10 ++-- xfa/fxfa/parser/xfa_locale.h | 4 +- xfa/fxfa/parser/xfa_localemgr.cpp | 44 +++++------------ xfa/fxfa/parser/xfa_localemgr.h | 7 +-- 10 files changed, 134 insertions(+), 65 deletions(-) create mode 100644 fpdfsdk/fpdfppo_embeddertest.cpp diff --git a/BUILD.gn b/BUILD.gn index c83854e2c1..831a5a0701 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1725,6 +1725,7 @@ test("pdfium_embeddertests") { "fpdfsdk/fpdfedit_embeddertest.cpp", "fpdfsdk/fpdfext_embeddertest.cpp", "fpdfsdk/fpdfformfill_embeddertest.cpp", + "fpdfsdk/fpdfppo_embeddertest.cpp", "fpdfsdk/fpdfsave_embeddertest.cpp", "fpdfsdk/fpdftext_embeddertest.cpp", "fpdfsdk/fpdfview_c_api_test.c", diff --git a/fpdfsdk/fpdfppo_embeddertest.cpp b/fpdfsdk/fpdfppo_embeddertest.cpp new file mode 100644 index 0000000000..3149f072f5 --- /dev/null +++ b/fpdfsdk/fpdfppo_embeddertest.cpp @@ -0,0 +1,92 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "public/fpdf_ppo.h" + +#include "core/fxcrt/fx_basic.h" +#include "public/fpdf_edit.h" +#include "public/fpdfview.h" +#include "testing/embedder_test.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/test_support.h" + +namespace { + +class FPDFPPOEmbeddertest : public EmbedderTest {}; + +} // namespace + +TEST_F(FPDFPPOEmbeddertest, NoViewerPreferences) { + EXPECT_TRUE(OpenDocument("hello_world.pdf")); + + FPDF_DOCUMENT output_doc = FPDF_CreateNewDocument(); + EXPECT_TRUE(output_doc); + EXPECT_FALSE(FPDF_CopyViewerPreferences(output_doc, document())); + FPDF_CloseDocument(output_doc); +} + +TEST_F(FPDFPPOEmbeddertest, ViewerPreferences) { + EXPECT_TRUE(OpenDocument("viewer_ref.pdf")); + + FPDF_DOCUMENT output_doc = FPDF_CreateNewDocument(); + EXPECT_TRUE(output_doc); + EXPECT_TRUE(FPDF_CopyViewerPreferences(output_doc, document())); + FPDF_CloseDocument(output_doc); +} + +TEST_F(FPDFPPOEmbeddertest, ImportPages) { + EXPECT_TRUE(OpenDocument("viewer_ref.pdf")); + + FPDF_PAGE page = LoadPage(0); + EXPECT_TRUE(page); + + FPDF_DOCUMENT output_doc = FPDF_CreateNewDocument(); + EXPECT_TRUE(output_doc); + EXPECT_TRUE(FPDF_CopyViewerPreferences(output_doc, document())); + EXPECT_TRUE(FPDF_ImportPages(output_doc, document(), "1", 0)); + EXPECT_EQ(1, FPDF_GetPageCount(output_doc)); + FPDF_CloseDocument(output_doc); + + UnloadPage(page); +} + +TEST_F(FPDFPPOEmbeddertest, BadRanges) { + EXPECT_TRUE(OpenDocument("viewer_ref.pdf")); + + FPDF_PAGE page = LoadPage(0); + EXPECT_TRUE(page); + + FPDF_DOCUMENT output_doc = FPDF_CreateNewDocument(); + EXPECT_TRUE(output_doc); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "clams", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "0", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "42", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "1,2", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "1-2", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), ",1", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "1,", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "1-", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "-1", 0)); + EXPECT_FALSE(FPDF_ImportPages(output_doc, document(), "-,0,,,1-", 0)); + FPDF_CloseDocument(output_doc); + + UnloadPage(page); +} + +TEST_F(FPDFPPOEmbeddertest, GoodRanges) { + EXPECT_TRUE(OpenDocument("viewer_ref.pdf")); + + FPDF_PAGE page = LoadPage(0); + EXPECT_TRUE(page); + + FPDF_DOCUMENT output_doc = FPDF_CreateNewDocument(); + EXPECT_TRUE(output_doc); + EXPECT_TRUE(FPDF_CopyViewerPreferences(output_doc, document())); + EXPECT_TRUE(FPDF_ImportPages(output_doc, document(), "1,1,1,1", 0)); + EXPECT_TRUE(FPDF_ImportPages(output_doc, document(), "1-1", 0)); + EXPECT_EQ(5, FPDF_GetPageCount(output_doc)); + FPDF_CloseDocument(output_doc); + + UnloadPage(page); +} diff --git a/xfa/fgas/localization/fgas_locale.cpp b/xfa/fgas/localization/fgas_locale.cpp index da4acb2a81..68fd94e29f 100644 --- a/xfa/fgas/localization/fgas_locale.cpp +++ b/xfa/fgas/localization/fgas_locale.cpp @@ -2241,7 +2241,7 @@ static void FX_ResolveZone(uint8_t& wHour, IFX_Locale* pLocale) { int32_t iMinuteDiff = wHour * 60 + wMinute; FX_TIMEZONE tzLocale; - pLocale->GetTimeZone(tzLocale); + pLocale->GetTimeZone(&tzLocale); iMinuteDiff += tzLocale.tzHour * 60 + (tzLocale.tzHour < 0 ? -tzLocale.tzMinute : tzLocale.tzMinute); iMinuteDiff -= tzDiff.tzHour * 60 + @@ -3923,7 +3923,7 @@ static bool FX_TimeFormat(const CFX_WideString& wsTimePattern, } else if (dwSymbol == FXBSTR_ID(0, 0, 'Z', '1')) { wsResult += FX_WSTRC(L"GMT"); FX_TIMEZONE tz; - pLocale->GetTimeZone(tz); + pLocale->GetTimeZone(&tz); if (!bGMT && (tz.tzHour != 0 || tz.tzMinute != 0)) { if (tz.tzHour < 0) { wsResult += FX_WSTRC(L"-"); @@ -3936,7 +3936,7 @@ static bool FX_TimeFormat(const CFX_WideString& wsTimePattern, } } else if (dwSymbol == FXBSTR_ID(0, 0, 'z', '1')) { FX_TIMEZONE tz; - pLocale->GetTimeZone(tz); + pLocale->GetTimeZone(&tz); if (!bGMT && tz.tzHour != 0 && tz.tzMinute != 0) { if (tz.tzHour < 0) { wsResult += FX_WSTRC(L"-"); diff --git a/xfa/fgas/localization/fgas_locale.h b/xfa/fgas/localization/fgas_locale.h index f9b84c5487..a3272b455f 100644 --- a/xfa/fgas/localization/fgas_locale.h +++ b/xfa/fgas/localization/fgas_locale.h @@ -68,7 +68,7 @@ class IFX_Locale { bool bAbbr = true) const = 0; virtual void GetMeridiemName(CFX_WideString& wsMeridiemName, bool bAM = true) const = 0; - virtual void GetTimeZone(FX_TIMEZONE& tz) const = 0; + virtual void GetTimeZone(FX_TIMEZONE* tz) const = 0; virtual void GetEraName(CFX_WideString& wsEraName, bool bAD = true) const = 0; virtual void GetDatePattern(FX_LOCALEDATETIMESUBCATEGORY eType, CFX_WideString& wsPattern) const = 0; diff --git a/xfa/fxfa/app/xfa_ffapp.cpp b/xfa/fxfa/app/xfa_ffapp.cpp index bfb9822ecc..ccdd6427ef 100644 --- a/xfa/fxfa/app/xfa_ffapp.cpp +++ b/xfa/fxfa/app/xfa_ffapp.cpp @@ -74,12 +74,9 @@ CXFA_FFApp::CXFA_FFApp(IXFA_AppProvider* pProvider) : m_pProvider(pProvider), m_pWidgetMgrDelegate(nullptr), m_pFWLApp(new IFWL_App(this)) { - CXFA_TimeZoneProvider::Create(); } -CXFA_FFApp::~CXFA_FFApp() { - CXFA_TimeZoneProvider::Destroy(); -} +CXFA_FFApp::~CXFA_FFApp() {} CXFA_FFDocHandler* CXFA_FFApp::GetDocHandler() { if (!m_pDocHandler) diff --git a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp index 947ef7941e..5c267717a3 100644 --- a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp +++ b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp @@ -1182,7 +1182,7 @@ void CXFA_FM2JSContext::IsoTime2Num(CFXJSE_Value* pThis, int32_t milSecond = uniTime.GetMillisecond(); FX_TIMEZONE tzLocale; - pMgr->GetDefLocale()->GetTimeZone(tzLocale); + pMgr->GetDefLocale()->GetTimeZone(&tzLocale); // TODO(dsinclair): See if there is other time conversion code in pdfium and // consolidate. @@ -1605,19 +1605,19 @@ void CXFA_FM2JSContext::Time2Num(CFXJSE_Value* pThis, int32_t second = uniTime.GetSecond(); int32_t milSecond = uniTime.GetMillisecond(); int32_t mins = hour * 60 + min; - CXFA_TimeZoneProvider* pProvider = CXFA_TimeZoneProvider::Get(); - if (pProvider) { - FX_TIMEZONE tz; - pProvider->GetTimeZone(tz); - mins -= (tz.tzHour * 60); - while (mins > 1440) - mins -= 1440; - while (mins < 0) - mins += 1440; - - hour = mins / 60; - min = mins % 60; - } + + FX_TIMEZONE tz; + CXFA_TimeZoneProvider provider; + provider.GetTimeZone(&tz); + mins -= (tz.tzHour * 60); + while (mins > 1440) + mins -= 1440; + + while (mins < 0) + mins += 1440; + + hour = mins / 60; + min = mins % 60; args.GetReturnValue()->SetInteger(hour * 3600000 + min * 60000 + second * 1000 + milSecond + 1); } diff --git a/xfa/fxfa/parser/xfa_locale.cpp b/xfa/fxfa/parser/xfa_locale.cpp index 8c106209aa..5b18bcf64b 100644 --- a/xfa/fxfa/parser/xfa_locale.cpp +++ b/xfa/fxfa/parser/xfa_locale.cpp @@ -103,8 +103,9 @@ void CXFA_XMLLocale::GetMeridiemName(CFX_WideString& wsMeridiemName, wsMeridiemName = GetCalendarSymbol("meridiem", bAM ? 0 : 1, false); } -void CXFA_XMLLocale::GetTimeZone(FX_TIMEZONE& tz) const { - CXFA_TimeZoneProvider::Get()->GetTimeZone(tz); +void CXFA_XMLLocale::GetTimeZone(FX_TIMEZONE* tz) const { + CXFA_TimeZoneProvider provider; + provider.GetTimeZone(tz); } void CXFA_XMLLocale::GetEraName(CFX_WideString& wsEraName, bool bAD) const { @@ -288,8 +289,9 @@ void CXFA_NodeLocale::GetMeridiemName(CFX_WideString& wsMeridiemName, GetCalendarSymbol(XFA_Element::MeridiemNames, bAM ? 0 : 1, false); } -void CXFA_NodeLocale::GetTimeZone(FX_TIMEZONE& tz) const { - CXFA_TimeZoneProvider::Get()->GetTimeZone(tz); +void CXFA_NodeLocale::GetTimeZone(FX_TIMEZONE* tz) const { + CXFA_TimeZoneProvider provider; + provider.GetTimeZone(tz); } void CXFA_NodeLocale::GetEraName(CFX_WideString& wsEraName, bool bAD) const { diff --git a/xfa/fxfa/parser/xfa_locale.h b/xfa/fxfa/parser/xfa_locale.h index b9a3259ead..6d03843419 100644 --- a/xfa/fxfa/parser/xfa_locale.h +++ b/xfa/fxfa/parser/xfa_locale.h @@ -31,7 +31,7 @@ class CXFA_XMLLocale : public IFX_Locale { bool bAbbr = true) const override; void GetMeridiemName(CFX_WideString& wsMeridiemName, bool bAM = true) const override; - void GetTimeZone(FX_TIMEZONE& tz) const override; + void GetTimeZone(FX_TIMEZONE* tz) const override; void GetEraName(CFX_WideString& wsEraName, bool bAD = true) const override; void GetDatePattern(FX_LOCALEDATETIMESUBCATEGORY eType, @@ -73,7 +73,7 @@ class CXFA_NodeLocale : public IFX_Locale { bool bAbbr = true) const override; void GetMeridiemName(CFX_WideString& wsMeridiemName, bool bAM = true) const override; - void GetTimeZone(FX_TIMEZONE& tz) const override; + void GetTimeZone(FX_TIMEZONE* tz) const override; void GetEraName(CFX_WideString& wsEraName, bool bAD = true) const override; void GetDatePattern(FX_LOCALEDATETIMESUBCATEGORY eType, diff --git a/xfa/fxfa/parser/xfa_localemgr.cpp b/xfa/fxfa/parser/xfa_localemgr.cpp index 74565a7ea0..cfa5801e3c 100644 --- a/xfa/fxfa/parser/xfa_localemgr.cpp +++ b/xfa/fxfa/parser/xfa_localemgr.cpp @@ -6,6 +6,8 @@ #include "xfa/fxfa/parser/xfa_localemgr.h" +#include + #include #include @@ -1245,37 +1247,21 @@ CFX_WideStringC CXFA_LocaleMgr::GetConfigLocaleName(CXFA_Node* pConfig) { return m_wsConfigLocale.AsStringC(); } -static CXFA_TimeZoneProvider* g_pProvider = nullptr; - -// Static. -CXFA_TimeZoneProvider* CXFA_TimeZoneProvider::Create() { - ASSERT(!g_pProvider); - g_pProvider = new CXFA_TimeZoneProvider(); - return g_pProvider; -} - -// Static. -CXFA_TimeZoneProvider* CXFA_TimeZoneProvider::Get() { - if (!g_pProvider) { - g_pProvider = new CXFA_TimeZoneProvider(); - } - return g_pProvider; -} - -// Static. -void CXFA_TimeZoneProvider::Destroy() { - delete g_pProvider; - g_pProvider = nullptr; -} +static bool g_bProviderTimeZoneSet = false; -#include CXFA_TimeZoneProvider::CXFA_TimeZoneProvider() { #if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ - _tzset(); + if (!g_bProviderTimeZoneSet) { + g_bProviderTimeZoneSet = true; + _tzset(); + } m_tz.tzHour = (int8_t)(_timezone / 3600 * -1); m_tz.tzMinute = (int8_t)((FXSYS_abs(_timezone) % 3600) / 60); #else - tzset(); + if (!g_bProviderTimeZoneSet) { + g_bProviderTimeZoneSet = true; + tzset(); + } m_tz.tzHour = (int8_t)(timezone / 3600 * -1); m_tz.tzMinute = (int8_t)((FXSYS_abs((int)timezone) % 3600) / 60); #endif @@ -1283,10 +1269,6 @@ CXFA_TimeZoneProvider::CXFA_TimeZoneProvider() { CXFA_TimeZoneProvider::~CXFA_TimeZoneProvider() {} -void CXFA_TimeZoneProvider::SetTimeZone(FX_TIMEZONE& tz) { - m_tz = tz; -} - -void CXFA_TimeZoneProvider::GetTimeZone(FX_TIMEZONE& tz) { - tz = m_tz; +void CXFA_TimeZoneProvider::GetTimeZone(FX_TIMEZONE* tz) const { + *tz = m_tz; } diff --git a/xfa/fxfa/parser/xfa_localemgr.h b/xfa/fxfa/parser/xfa_localemgr.h index 51c6a3b5fb..eb405dcfc7 100644 --- a/xfa/fxfa/parser/xfa_localemgr.h +++ b/xfa/fxfa/parser/xfa_localemgr.h @@ -62,12 +62,7 @@ class CXFA_TimeZoneProvider { CXFA_TimeZoneProvider(); ~CXFA_TimeZoneProvider(); - static CXFA_TimeZoneProvider* Create(); - static CXFA_TimeZoneProvider* Get(); - static void Destroy(); - - void SetTimeZone(FX_TIMEZONE& tz); - void GetTimeZone(FX_TIMEZONE& tz); + void GetTimeZone(FX_TIMEZONE* tz) const; private: FX_TIMEZONE m_tz; -- cgit v1.2.3