Garbage collect CXFA_LocaleMgr. Although not strictly required, this replaces a raw pointer to a node with a traceable member. In turn, pass LocaleMgrIface to the string formatter only on calls that are affected by it. This prevents a lower layer that knows nothing about garbage collection holding an unowned pointer to a GC'd object. Bug: pdfium:1563 Change-Id: Id6a372136f171a307b764a95d478c108cd3be5e4 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73990 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/testing/fuzzers/BUILD.gn b/testing/fuzzers/BUILD.gn index 81f5100..421fa52 100644 --- a/testing/fuzzers/BUILD.gn +++ b/testing/fuzzers/BUILD.gn
@@ -256,10 +256,13 @@ pdfium_fuzzer("pdf_cfgas_stringformatter_fuzzer") { sources = [ "pdf_cfgas_stringformatter_fuzzer.cc" ] deps = [ + ":fuzzer_utils", "../../core/fxcrt", + "../../fxjs:gc", "../../xfa/fgas", "../../xfa/fxfa/parser", ] + public_fuzzer = true } pdfium_fuzzer("pdf_cfx_barcode_fuzzer") {
diff --git a/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc b/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc index 663958b..71b4793 100644 --- a/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc +++ b/testing/fuzzers/pdf_cfgas_stringformatter_fuzzer.cc
@@ -6,10 +6,15 @@ #include <stdint.h> -#include <memory> +#include <vector> #include "core/fxcrt/fx_string.h" +#include "public/fpdfview.h" +#include "testing/fuzzers/pdfium_fuzzer_util.h" +#include "testing/fuzzers/xfa_process_state.h" #include "third_party/base/stl_util.h" +#include "v8/include/cppgc/heap.h" +#include "v8/include/cppgc/persistent.h" #include "xfa/fxfa/parser/cxfa_localemgr.h" namespace { @@ -27,11 +32,15 @@ if (size < 5 || size > 128) // Big strings are unlikely to help. return 0; + auto* state = static_cast<XFAProcessState*>(FPDF_GetFuzzerPerProcessState()); + cppgc::Heap* heap = state->GetHeap(); + // Static for speed. - static std::vector<std::unique_ptr<CXFA_LocaleMgr>> mgrs; + static std::vector<cppgc::Persistent<CXFA_LocaleMgr>> mgrs; if (mgrs.empty()) { for (const auto* locale : kLocales) - mgrs.push_back(std::make_unique<CXFA_LocaleMgr>(nullptr, locale)); + mgrs.push_back(cppgc::MakeGarbageCollected<CXFA_LocaleMgr>( + heap->GetAllocationHandle(), heap, nullptr, locale)); } uint8_t test_selector = data[0] % 10; @@ -47,8 +56,7 @@ WideString value = WideString::FromLatin1(ByteStringView(data + pattern_len, value_len)); - auto fmt = std::make_unique<CFGAS_StringFormatter>( - mgrs[locale_selector].get(), pattern); + auto fmt = std::make_unique<CFGAS_StringFormatter>(pattern); WideString result; CFX_DateTime dt; @@ -57,10 +65,11 @@ fmt->FormatText(value, &result); break; case 1: - fmt->FormatNum(value, &result); + fmt->FormatNum(mgrs[locale_selector], value, &result); break; case 2: - fmt->FormatDateTime(value, kTypes[type_selector], &result); + fmt->FormatDateTime(mgrs[locale_selector], value, kTypes[type_selector], + &result); break; case 3: fmt->FormatNull(&result); @@ -72,10 +81,11 @@ fmt->ParseText(value, &result); break; case 6: - fmt->ParseNum(value, &result); + fmt->ParseNum(mgrs[locale_selector], value, &result); break; case 7: - fmt->ParseDateTime(value, kTypes[type_selector], &dt); + fmt->ParseDateTime(mgrs[locale_selector], value, kTypes[type_selector], + &dt); break; case 8: fmt->ParseNull(value); @@ -84,5 +94,7 @@ fmt->ParseZero(value); break; } + + state->MaybeForceGCAndPump(); return 0; }
diff --git a/xfa/fgas/BUILD.gn b/xfa/fgas/BUILD.gn index fe2057d..f5aee6f 100644 --- a/xfa/fgas/BUILD.gn +++ b/xfa/fgas/BUILD.gn
@@ -50,6 +50,7 @@ deps = [ ":fgas", "../../core/fpdfapi/page", + "../../fxjs:gc", "../fxfa/parser", ] pdfium_root_dir = "../../"
diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp index 69fb1a8..2cac0de 100644 --- a/xfa/fgas/crt/cfgas_stringformatter.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter.cpp
@@ -15,6 +15,7 @@ #include "core/fxcrt/fx_safe_types.h" #include "third_party/base/stl_util.h" #include "xfa/fgas/crt/cfgas_decimal.h" +#include "xfa/fgas/crt/locale_mgr_iface.h" // NOTE: Code uses the convention for backwards-looping with unsigned types // that exploits the well-defined behaviour for unsigned underflow (and hence @@ -862,11 +863,8 @@ return true; } -CFGAS_StringFormatter::CFGAS_StringFormatter(LocaleMgrIface* pLocaleMgr, - const WideString& wsPattern) - : m_pLocaleMgr(pLocaleMgr), - m_wsPattern(wsPattern), - m_spPattern(m_wsPattern.span()) {} +CFGAS_StringFormatter::CFGAS_StringFormatter(const WideString& wsPattern) + : m_wsPattern(wsPattern), m_spPattern(m_wsPattern.span()) {} CFGAS_StringFormatter::~CFGAS_StringFormatter() = default; @@ -988,6 +986,7 @@ } LocaleIface* CFGAS_StringFormatter::GetNumericFormat( + LocaleMgrIface* pLocaleMgr, size_t* iDotIndex, uint32_t* dwStyle, WideString* wsPurgePattern) const { @@ -1027,7 +1026,7 @@ while (ccf < m_spPattern.size() && m_spPattern[ccf] != ')') wsLCID += m_spPattern[ccf++]; - pLocale = m_pLocaleMgr->GetLocaleByName(wsLCID); + pLocale = pLocaleMgr->GetLocaleByName(wsLCID); } else if (m_spPattern[ccf] == '.') { WideString wsSubCategory; ccf++; @@ -1046,7 +1045,7 @@ } } if (!pLocale) - pLocale = m_pLocaleMgr->GetDefLocale(); + pLocale = pLocaleMgr->GetDefLocale(); ASSERT(pLocale); @@ -1086,7 +1085,7 @@ if (!bFindDot) *iDotIndex = wsPurgePattern->GetLength(); if (!pLocale) - pLocale = m_pLocaleMgr->GetDefLocale(); + pLocale = pLocaleMgr->GetDefLocale(); return pLocale; } @@ -1161,7 +1160,8 @@ return iPattern == spTextFormat.size() && iText == spSrcText.size(); } -bool CFGAS_StringFormatter::ParseNum(const WideString& wsSrcNum, +bool CFGAS_StringFormatter::ParseNum(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcNum, WideString* wsValue) const { wsValue->clear(); if (wsSrcNum.IsEmpty() || m_spPattern.empty()) @@ -1171,7 +1171,7 @@ uint32_t dwFormatStyle = 0; WideString wsNumFormat; LocaleIface* pLocale = - GetNumericFormat(&dot_index_f, &dwFormatStyle, &wsNumFormat); + GetNumericFormat(pLocaleMgr, &dot_index_f, &dwFormatStyle, &wsNumFormat); if (!pLocale || wsNumFormat.IsEmpty()) return false; @@ -1582,6 +1582,7 @@ } CFGAS_StringFormatter::DateTimeType CFGAS_StringFormatter::GetDateTimeFormat( + LocaleMgrIface* pLocaleMgr, LocaleIface** pLocale, WideString* wsDatePattern, WideString* wsTimePattern) const { @@ -1608,8 +1609,7 @@ *wsTimePattern = m_wsPattern.Last(m_wsPattern.GetLength() - ccf); wsTimePattern->SetAt(0, ' '); if (!*pLocale) - *pLocale = m_pLocaleMgr->GetDefLocale(); - + *pLocale = pLocaleMgr->GetDefLocale(); return DateTimeType::kDateTime; } wsCategory += m_spPattern[ccf]; @@ -1638,7 +1638,7 @@ while (ccf < m_spPattern.size() && m_spPattern[ccf] != ')') wsLCID += m_spPattern[ccf++]; - *pLocale = m_pLocaleMgr->GetLocaleByName(wsLCID); + *pLocale = pLocaleMgr->GetLocaleByName(wsLCID); } else if (m_spPattern[ccf] == '.') { WideString wsSubCategory; ccf++; @@ -1657,7 +1657,7 @@ } } if (!*pLocale) - *pLocale = m_pLocaleMgr->GetDefLocale(); + *pLocale = pLocaleMgr->GetDefLocale(); ASSERT(*pLocale); switch (eCategory) { @@ -1705,7 +1705,7 @@ *wsTimePattern += wsTempPattern; } if (!*pLocale) - *pLocale = m_pLocaleMgr->GetDefLocale(); + *pLocale = pLocaleMgr->GetDefLocale(); if (eDateTimeType == DateTimeType::kUnknown) { wsTimePattern->clear(); *wsDatePattern = m_wsPattern; @@ -1713,7 +1713,8 @@ return eDateTimeType; } -bool CFGAS_StringFormatter::ParseDateTime(const WideString& wsSrcDateTime, +bool CFGAS_StringFormatter::ParseDateTime(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcDateTime, DateTimeType eDateTimeType, CFX_DateTime* dtValue) const { dtValue->Reset(); @@ -1724,7 +1725,7 @@ WideString wsDatePattern; WideString wsTimePattern; DateTimeType eCategory = - GetDateTimeFormat(&pLocale, &wsDatePattern, &wsTimePattern); + GetDateTimeFormat(pLocaleMgr, &pLocale, &wsDatePattern, &wsTimePattern); if (!pLocale) return false; @@ -1869,7 +1870,8 @@ return iText == spSrcText.size(); } -bool CFGAS_StringFormatter::FormatNum(const WideString& wsInputNum, +bool CFGAS_StringFormatter::FormatNum(LocaleMgrIface* pLocaleMgr, + const WideString& wsInputNum, WideString* wsOutput) const { if (wsInputNum.IsEmpty() || m_spPattern.empty()) return false; @@ -1878,7 +1880,7 @@ uint32_t dwNumStyle = 0; WideString wsNumFormat; LocaleIface* pLocale = - GetNumericFormat(&dot_index_f, &dwNumStyle, &wsNumFormat); + GetNumericFormat(pLocaleMgr, &dot_index_f, &dwNumStyle, &wsNumFormat); if (!pLocale || wsNumFormat.IsEmpty()) return false; @@ -2214,7 +2216,8 @@ return true; } -bool CFGAS_StringFormatter::FormatDateTime(const WideString& wsSrcDateTime, +bool CFGAS_StringFormatter::FormatDateTime(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcDateTime, DateTimeType eDateTimeType, WideString* wsOutput) const { if (wsSrcDateTime.IsEmpty() || m_spPattern.empty()) @@ -2224,7 +2227,7 @@ WideString wsTimePattern; LocaleIface* pLocale = nullptr; DateTimeType eCategory = - GetDateTimeFormat(&pLocale, &wsDatePattern, &wsTimePattern); + GetDateTimeFormat(pLocaleMgr, &pLocale, &wsDatePattern, &wsTimePattern); if (!pLocale) return false;
diff --git a/xfa/fgas/crt/cfgas_stringformatter.h b/xfa/fgas/crt/cfgas_stringformatter.h index 98dfc6f..4e67e52 100644 --- a/xfa/fgas/crt/cfgas_stringformatter.h +++ b/xfa/fgas/crt/cfgas_stringformatter.h
@@ -9,10 +9,11 @@ #include <vector> -#include "core/fxcrt/unowned_ptr.h" +#include "core/fxcrt/fx_string.h" #include "third_party/base/span.h" #include "xfa/fgas/crt/locale_iface.h" -#include "xfa/fgas/crt/locale_mgr_iface.h" + +class LocaleMgrIface; bool FX_DateFromCanonical(pdfium::span<const wchar_t> wsTime, CFX_DateTime* datetime); @@ -41,8 +42,7 @@ kTimeDate, }; - CFGAS_StringFormatter(LocaleMgrIface* pLocaleMgr, - const WideString& wsPattern); + explicit CFGAS_StringFormatter(const WideString& wsPattern); ~CFGAS_StringFormatter(); static std::vector<WideString> SplitOnBars(const WideString& wsFormatString); @@ -50,16 +50,22 @@ Category GetCategory() const; bool ParseText(const WideString& wsSrcText, WideString* wsValue) const; - bool ParseNum(const WideString& wsSrcNum, WideString* wsValue) const; - bool ParseDateTime(const WideString& wsSrcDateTime, + bool ParseNum(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcNum, + WideString* wsValue) const; + bool ParseDateTime(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcDateTime, DateTimeType eDateTimeType, CFX_DateTime* dtValue) const; bool ParseZero(const WideString& wsSrcText) const; bool ParseNull(const WideString& wsSrcText) const; bool FormatText(const WideString& wsSrcText, WideString* wsOutput) const; - bool FormatNum(const WideString& wsSrcNum, WideString* wsOutput) const; - bool FormatDateTime(const WideString& wsSrcDateTime, + bool FormatNum(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcNum, + WideString* wsOutput) const; + bool FormatDateTime(LocaleMgrIface* pLocaleMgr, + const WideString& wsSrcDateTime, DateTimeType eDateTimeType, WideString* wsOutput) const; bool FormatZero(WideString* wsOutput) const; @@ -67,14 +73,15 @@ private: WideString GetTextFormat(WideStringView wsCategory) const; - LocaleIface* GetNumericFormat(size_t* iDotIndex, + LocaleIface* GetNumericFormat(LocaleMgrIface* pLocaleMgr, + size_t* iDotIndex, uint32_t* dwStyle, WideString* wsPurgePattern) const; - DateTimeType GetDateTimeFormat(LocaleIface** pLocale, + DateTimeType GetDateTimeFormat(LocaleMgrIface* pLocaleMgr, + LocaleIface** pLocale, WideString* wsDatePattern, WideString* wsTimePattern) const; - UnownedPtr<LocaleMgrIface> const m_pLocaleMgr; const WideString m_wsPattern; // keep pattern string alive. const pdfium::span<const wchar_t> m_spPattern; // span into |m_wsPattern|. };
diff --git a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp index d582bb3..4b45d2a 100644 --- a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp
@@ -13,25 +13,15 @@ #include "build/build_config.h" #include "core/fpdfapi/page/cpdf_pagemodule.h" #include "testing/fx_string_testhelpers.h" +#include "testing/fxgc_unittest.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/base/stl_util.h" +#include "v8/include/cppgc/persistent.h" #include "xfa/fxfa/parser/cxfa_localemgr.h" -class CFGAS_StringFormatterTest : public testing::Test { - public: - CFGAS_StringFormatterTest() { - SetTZ("UTC"); - CPDF_PageModule::Create(); - } +namespace { - ~CFGAS_StringFormatterTest() override { CPDF_PageModule::Destroy(); } - - void TearDown() override { - fmt_.reset(); - mgr_.reset(); - } - - void SetTZ(const char* tz) { +void SetTZ(const char* tz) { #if defined(OS_WIN) _putenv_s("TZ", tz); _tzset(); @@ -39,21 +29,26 @@ setenv("TZ", tz, 1); tzset(); #endif +} + +} // namespace + +class CFGAS_StringFormatterTest : public FXGCUnitTest { + public: + CFGAS_StringFormatterTest() { + SetTZ("UTC"); + CPDF_PageModule::Create(); } - // Note, this re-creates the fmt on each call. If you need to multiple - // times store it locally. - CFGAS_StringFormatter* fmt(const WideString& locale, - const WideString& pattern) { - fmt_.reset(); // Can't outlive |mgr_|. - mgr_ = std::make_unique<CXFA_LocaleMgr>(nullptr, locale); - fmt_ = std::make_unique<CFGAS_StringFormatter>(mgr_.get(), pattern); - return fmt_.get(); + ~CFGAS_StringFormatterTest() override { + CPDF_PageModule::Destroy(); + SetTZ("UTC"); } - protected: - std::unique_ptr<CXFA_LocaleMgr> mgr_; - std::unique_ptr<CFGAS_StringFormatter> fmt_; + CXFA_LocaleMgr* Mgr(const WideString& locale) { + return cppgc::MakeGarbageCollected<CXFA_LocaleMgr>( + heap()->GetAllocationHandle(), heap(), nullptr, locale); + } }; // TODO(dsinclair): Looks like the formatter/parser does not handle the various @@ -115,10 +110,10 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern) - ->FormatDateTime(tests[i].input, - CFGAS_StringFormatter::DateTimeType::kDate, - &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatDateTime(Mgr(tests[i].locale), tests[i].input, + CFGAS_StringFormatter::DateTimeType::kDate, + &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } @@ -165,10 +160,10 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern) - ->FormatDateTime(tests[i].input, - CFGAS_StringFormatter::DateTimeType::kTime, - &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatDateTime(Mgr(tests[i].locale), tests[i].input, + CFGAS_StringFormatter::DateTimeType::kTime, + &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } @@ -198,11 +193,10 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE( - fmt(tests[i].locale, tests[i].pattern) - ->FormatDateTime(tests[i].input, - CFGAS_StringFormatter::DateTimeType::kDateTime, - &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatDateTime( + Mgr(tests[i].locale), tests[i].input, + CFGAS_StringFormatter::DateTimeType::kDateTime, &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } @@ -228,11 +222,10 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE( - fmt(tests[i].locale, tests[i].pattern) - ->FormatDateTime(tests[i].input, - CFGAS_StringFormatter::DateTimeType::kTimeDate, - &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatDateTime( + Mgr(tests[i].locale), tests[i].input, + CFGAS_StringFormatter::DateTimeType::kTimeDate, &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } @@ -291,10 +284,10 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { CFX_DateTime result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern) - ->ParseDateTime(tests[i].input, - CFGAS_StringFormatter::DateTimeType::kDate, - &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.ParseDateTime(Mgr(tests[i].locale), tests[i].input, + CFGAS_StringFormatter::DateTimeType::kDate, + &result)); EXPECT_EQ(tests[i].output, result) << " TEST: " << i; } } @@ -481,7 +474,8 @@ for (const auto& test : tests) { WideString result; - EXPECT_TRUE(fmt(test.locale, test.pattern)->ParseNum(test.input, &result)) + CFGAS_StringFormatter fmt(test.pattern); + EXPECT_TRUE(fmt.ParseNum(Mgr(test.locale), test.input, &result)) << " TEST: " << test.input << ", " << test.pattern; EXPECT_STREQ(test.output, result.c_str()) << " TEST: " << test.input << ", " << test.pattern; @@ -489,7 +483,8 @@ for (const auto& test : failures) { WideString result; - EXPECT_FALSE(fmt(test.locale, test.pattern)->ParseNum(test.input, &result)) + CFGAS_StringFormatter fmt(test.pattern); + EXPECT_FALSE(fmt.ParseNum(Mgr(test.locale), test.input, &result)) << " TEST: " << test.input << ", " << test.pattern; } } @@ -610,7 +605,8 @@ for (const auto& test : tests) { WideString result; - EXPECT_TRUE(fmt(test.locale, test.pattern)->FormatNum(test.input, &result)) + CFGAS_StringFormatter fmt(test.pattern); + EXPECT_TRUE(fmt.FormatNum(Mgr(test.locale), test.input, &result)) << " TEST: " << test.input << ", " << test.pattern; EXPECT_STREQ(test.output, result.c_str()) << " TEST: " << test.input << ", " << test.pattern; @@ -618,14 +614,14 @@ for (const auto& test : failures) { WideString result; - EXPECT_FALSE(fmt(test.locale, test.pattern)->FormatNum(test.input, &result)) + CFGAS_StringFormatter fmt(test.pattern); + EXPECT_FALSE(fmt.FormatNum(Mgr(test.locale), test.input, &result)) << " TEST: " << test.input << ", " << test.pattern; } } TEST_F(CFGAS_StringFormatterTest, TextParse) { struct { - const wchar_t* locale; const wchar_t* input; const wchar_t* pattern; const wchar_t* output; @@ -634,16 +630,16 @@ // * - zero or more whitespace // + - one or more whitespace // {L"en", L"555-1212", L"text(th_TH){999*9999}", L"5551212"}, - {L"en", L"ABC-1234-5", L"AAA-9999-X", L"ABC12345"}, - {L"en", L"ABC-1234-D", L"AAA-9999-X", L"ABC1234D"}, - {L"en", L"A1C-1234-D", L"OOO-9999-X", L"A1C1234D"}, - {L"en", L"A1C-1234-D", L"000-9999-X", L"A1C1234D"}, - {L"en", L"A1C-1234-D text", L"000-9999-X 'text'", L"A1C1234D"}}; + {L"ABC-1234-5", L"AAA-9999-X", L"ABC12345"}, + {L"ABC-1234-D", L"AAA-9999-X", L"ABC1234D"}, + {L"A1C-1234-D", L"OOO-9999-X", L"A1C1234D"}, + {L"A1C-1234-D", L"000-9999-X", L"A1C1234D"}, + {L"A1C-1234-D text", L"000-9999-X 'text'", L"A1C1234D"}}; for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern) - ->ParseText(tests[i].input, &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.ParseText(tests[i].input, &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } @@ -651,7 +647,8 @@ TEST_F(CFGAS_StringFormatterTest, InvalidTextParse) { // Input does not match mask. WideString result; - EXPECT_FALSE(fmt(L"en", L"AAA-9999-X")->ParseText(L"123-4567-8", &result)); + CFGAS_StringFormatter fmt(L"AAA-9999-X"); + EXPECT_FALSE(fmt.ParseText(L"123-4567-8", &result)); } TEST_F(CFGAS_StringFormatterTest, TextFormat) { @@ -672,102 +669,96 @@ for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern) - ->FormatText(tests[i].input, &result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatText(tests[i].input, &result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } TEST_F(CFGAS_StringFormatterTest, NullParse) { struct { - const wchar_t* locale; const wchar_t* input; const wchar_t* pattern; } tests[] = { - {L"en", L"", L"null{}"}, - {L"en", L"No data", L"null{'No data'}"}, + {L"", L"null{}"}, + {L"No data", L"null{'No data'}"}, }; for (size_t i = 0; i < pdfium::size(tests); ++i) { - EXPECT_TRUE( - fmt(tests[i].locale, tests[i].pattern)->ParseNull(tests[i].input)) - << " TEST: " << i; + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.ParseNull(tests[i].input)) << " TEST: " << i; } } TEST_F(CFGAS_StringFormatterTest, NullFormat) { struct { - const wchar_t* locale; const wchar_t* pattern; const wchar_t* output; - } tests[] = {{L"en", L"null{'n/a'}", L"n/a"}, {L"en", L"null{}", L""}}; + } tests[] = {{L"null{'n/a'}", L"n/a"}, {L"null{}", L""}}; for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern)->FormatNull(&result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatNull(&result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } TEST_F(CFGAS_StringFormatterTest, ZeroParse) { struct { - const wchar_t* locale; const wchar_t* input; const wchar_t* pattern; - } tests[] = {{L"en", L"", L"zero{}"}, - {L"en", L"9", L"zero{9}"}, - {L"en", L"a", L"zero{'a'}"}}; + } tests[] = {{L"", L"zero{}"}, {L"9", L"zero{9}"}, {L"a", L"zero{'a'}"}}; for (size_t i = 0; i < pdfium::size(tests); ++i) { - EXPECT_TRUE( - fmt(tests[i].locale, tests[i].pattern)->ParseZero(tests[i].input)) - << " TEST: " << i; + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.ParseZero(tests[i].input)) << " TEST: " << i; } } TEST_F(CFGAS_StringFormatterTest, ZeroFormat) { struct { - const wchar_t* locale; const wchar_t* input; const wchar_t* pattern; const wchar_t* output; } tests[] = {// TODO(dsinclair): The zero format can take a number specifier // which we don't take into account. - // {L"en", L"", L"zero {9}", L""}, - // {L"en", L"0", L"zero {9}", L"0"}, - // {L"en", L"0.0", L"zero{9}", L"0"}, - {L"en", L"0", L"zero{}", L""}}; + // {L"", L"zero {9}", L""}, + // {L"0", L"zero {9}", L"0"}, + // {L"0.0", L"zero{9}", L"0"}, + {L"0", L"zero{}", L""}}; for (size_t i = 0; i < pdfium::size(tests); ++i) { WideString result; - EXPECT_TRUE(fmt(tests[i].locale, tests[i].pattern)->FormatZero(&result)); + CFGAS_StringFormatter fmt(tests[i].pattern); + EXPECT_TRUE(fmt.FormatZero(&result)); EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i; } } TEST_F(CFGAS_StringFormatterTest, GetCategory) { EXPECT_EQ(CFGAS_StringFormatter::Category::kUnknown, - fmt(L"en", L"'just text'")->GetCategory()); + CFGAS_StringFormatter(L"'just text'").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kNull, - fmt(L"en", L"null{}")->GetCategory()); + CFGAS_StringFormatter(L"null{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kZero, - fmt(L"en", L"zero{}")->GetCategory()); + CFGAS_StringFormatter(L"zero{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kNum, - fmt(L"en", L"num{}")->GetCategory()); + CFGAS_StringFormatter(L"num{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kText, - fmt(L"en", L"text{}")->GetCategory()); + CFGAS_StringFormatter(L"text{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kDateTime, - fmt(L"en", L"datetime{}")->GetCategory()); + CFGAS_StringFormatter(L"datetime{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kTime, - fmt(L"en", L"time{}")->GetCategory()); + CFGAS_StringFormatter(L"time{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kDate, - fmt(L"en", L"date{}")->GetCategory()); + CFGAS_StringFormatter(L"date{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kDateTime, - fmt(L"en", L"time{} date{}")->GetCategory()); + CFGAS_StringFormatter(L"time{} date{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kDateTime, - fmt(L"en", L"date{} time{}")->GetCategory()); + CFGAS_StringFormatter(L"date{} time{}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kNum, - fmt(L"en", L"num(en_GB){}")->GetCategory()); + CFGAS_StringFormatter(L"num(en_GB){}").GetCategory()); EXPECT_EQ(CFGAS_StringFormatter::Category::kDate, - fmt(L"en", L"date.long{}")->GetCategory()); + CFGAS_StringFormatter(L"date.long{}").GetCategory()); }
diff --git a/xfa/fxfa/parser/cxfa_document.cpp b/xfa/fxfa/parser/cxfa_document.cpp index a01d8b0..2b08e2a 100644 --- a/xfa/fxfa/parser/cxfa_document.cpp +++ b/xfa/fxfa/parser/cxfa_document.cpp
@@ -1285,6 +1285,7 @@ visitor->Trace(notify_); visitor->Trace(node_owner_); visitor->Trace(m_pRootNode); + visitor->Trace(m_pLocaleMgr); visitor->Trace(m_pLayoutProcessor); visitor->Trace(m_pScriptDataWindow); visitor->Trace(m_pScriptEvent); @@ -1299,7 +1300,7 @@ void CXFA_Document::ClearLayoutData() { m_pLayoutProcessor = nullptr; m_pScriptContext.reset(); - m_pLocaleMgr.reset(); + m_pLocaleMgr.Clear(); m_pScriptDataWindow = nullptr; m_pScriptEvent = nullptr; m_pScriptHost = nullptr; @@ -1426,11 +1427,12 @@ CXFA_LocaleMgr* CXFA_Document::GetLocaleMgr() { if (!m_pLocaleMgr) { - m_pLocaleMgr = std::make_unique<CXFA_LocaleMgr>( + m_pLocaleMgr = cppgc::MakeGarbageCollected<CXFA_LocaleMgr>( + heap_->GetAllocationHandle(), heap_, ToNode(GetXFAObject(XFA_HASHCODE_LocaleSet)), GetNotify()->GetAppProvider()->GetLanguage()); } - return m_pLocaleMgr.get(); + return m_pLocaleMgr; } cppgc::Heap* CXFA_Document::GetHeap() const {
diff --git a/xfa/fxfa/parser/cxfa_document.h b/xfa/fxfa/parser/cxfa_document.h index 94449b0..c6e6b2d 100644 --- a/xfa/fxfa/parser/cxfa_document.h +++ b/xfa/fxfa/parser/cxfa_document.h
@@ -150,7 +150,7 @@ cppgc::Member<CXFA_Node> m_pRootNode; std::unique_ptr<CFXJSE_Engine> m_pScriptContext; cppgc::Member<LayoutProcessorIface> m_pLayoutProcessor; - std::unique_ptr<CXFA_LocaleMgr> m_pLocaleMgr; + cppgc::Member<CXFA_LocaleMgr> m_pLocaleMgr; cppgc::Member<CScript_DataWindow> m_pScriptDataWindow; cppgc::Member<CScript_EventPseudoModel> m_pScriptEvent; cppgc::Member<CScript_HostPseudoModel> m_pScriptHost;
diff --git a/xfa/fxfa/parser/cxfa_localemgr.cpp b/xfa/fxfa/parser/cxfa_localemgr.cpp index d866f55..6975bc6 100644 --- a/xfa/fxfa/parser/cxfa_localemgr.cpp +++ b/xfa/fxfa/parser/cxfa_localemgr.cpp
@@ -13,6 +13,7 @@ #include "core/fxcodec/flate/flatemodule.h" #include "core/fxcrt/fx_memory_wrappers.h" +#include "fxjs/gc/container_trace.h" #include "fxjs/xfa/cjx_object.h" #include "xfa/fxfa/parser/cxfa_acrobat.h" #include "xfa/fxfa/parser/cxfa_common.h" @@ -1064,8 +1065,8 @@ 0xB3, 0x85, 0xFA, 0x59, 0x2A, 0x7A, 0xFF, 0x3D, 0xC4, 0x3F, 0xDE, 0xCB, 0x8B, 0xC4}; -std::unique_ptr<LocaleIface> GetLocaleFromBuffer( - pdfium::span<const uint8_t> src_span) { +CXFA_XMLLocale* GetLocaleFromBuffer(cppgc::Heap* heap, + pdfium::span<const uint8_t> src_span) { if (src_span.empty()) return nullptr; @@ -1076,7 +1077,7 @@ if (!output) return nullptr; - return CXFA_XMLLocale::Create(pdfium::make_span(output.get(), dwSize)); + return CXFA_XMLLocale::Create(heap, pdfium::make_span(output.get(), dwSize)); } uint16_t GetLanguage(WideString wsLanguage) { @@ -1124,93 +1125,104 @@ } // namespace -CXFA_LocaleMgr::CXFA_LocaleMgr(CXFA_Node* pLocaleSet, WideString wsDeflcid) - : m_pDefLocale(GetLocaleByName(wsDeflcid)), +CXFA_LocaleMgr::CXFA_LocaleMgr(cppgc::Heap* pHeap, + CXFA_Node* pLocaleSet, + WideString wsDeflcid) + : m_pHeap(pHeap), + m_pDefLocale(GetLocaleByName(wsDeflcid)), m_dwDeflcid(GetLanguage(wsDeflcid)) { if (!pLocaleSet) return; for (CXFA_Node* pNodeLocale = pLocaleSet->GetFirstChild(); pNodeLocale; pNodeLocale = pNodeLocale->GetNextSibling()) { - m_LocaleArray.push_back(std::make_unique<CXFA_NodeLocale>(pNodeLocale)); + m_LocaleArray.push_back(cppgc::MakeGarbageCollected<CXFA_NodeLocale>( + pHeap->GetAllocationHandle(), pNodeLocale)); } } CXFA_LocaleMgr::~CXFA_LocaleMgr() = default; -LocaleIface* CXFA_LocaleMgr::GetDefLocale() { - if (m_pDefLocale) - return m_pDefLocale.Get(); - - if (!m_LocaleArray.empty()) - return m_LocaleArray[0].get(); - - if (!m_XMLLocaleArray.empty()) - return m_XMLLocaleArray[0].get(); - - std::unique_ptr<LocaleIface> locale(GetLocale(m_dwDeflcid)); - m_pDefLocale = locale.get(); - if (locale) - m_XMLLocaleArray.push_back(std::move(locale)); - - return m_pDefLocale.Get(); +void CXFA_LocaleMgr::Trace(cppgc::Visitor* visitor) const { + ContainerTrace(visitor, m_LocaleArray); + ContainerTrace(visitor, m_XMLLocaleArray); } -std::unique_ptr<LocaleIface> CXFA_LocaleMgr::GetLocale(uint16_t lcid) { +LocaleIface* CXFA_LocaleMgr::GetDefLocale() { + if (m_pDefLocale) + return m_pDefLocale; + + if (!m_LocaleArray.empty()) + return m_LocaleArray[0]; + + if (!m_XMLLocaleArray.empty()) + return m_XMLLocaleArray[0]; + + CXFA_XMLLocale* pLocale = GetLocale(m_dwDeflcid); + if (pLocale) + m_XMLLocaleArray.push_back(pLocale); + + m_pDefLocale = pLocale; + return m_pDefLocale; +} + +CXFA_XMLLocale* CXFA_LocaleMgr::GetLocale(uint16_t lcid) { switch (lcid) { case FX_LANG_zh_CN: - return GetLocaleFromBuffer(g_zhCN_Locale); + return GetLocaleFromBuffer(m_pHeap, g_zhCN_Locale); case FX_LANG_zh_TW: - return GetLocaleFromBuffer(g_zhTW_Locale); + return GetLocaleFromBuffer(m_pHeap, g_zhTW_Locale); case FX_LANG_zh_HK: - return GetLocaleFromBuffer(g_zhHK_Locale); + return GetLocaleFromBuffer(m_pHeap, g_zhHK_Locale); case FX_LANG_ja_JP: - return GetLocaleFromBuffer(g_jaJP_Locale); + return GetLocaleFromBuffer(m_pHeap, g_jaJP_Locale); case FX_LANG_ko_KR: - return GetLocaleFromBuffer(g_koKR_Locale); + return GetLocaleFromBuffer(m_pHeap, g_koKR_Locale); case FX_LANG_en_GB: - return GetLocaleFromBuffer(g_enGB_Locale); + return GetLocaleFromBuffer(m_pHeap, g_enGB_Locale); case FX_LANG_es_LA: - return GetLocaleFromBuffer(g_esLA_Locale); + return GetLocaleFromBuffer(m_pHeap, g_esLA_Locale); case FX_LANG_es_ES: - return GetLocaleFromBuffer(g_esES_Locale); + return GetLocaleFromBuffer(m_pHeap, g_esES_Locale); case FX_LANG_de_DE: - return GetLocaleFromBuffer(g_deDE_Loacale); + return GetLocaleFromBuffer(m_pHeap, g_deDE_Loacale); case FX_LANG_fr_FR: - return GetLocaleFromBuffer(g_frFR_Locale); + return GetLocaleFromBuffer(m_pHeap, g_frFR_Locale); case FX_LANG_it_IT: - return GetLocaleFromBuffer(g_itIT_Locale); + return GetLocaleFromBuffer(m_pHeap, g_itIT_Locale); case FX_LANG_pt_BR: - return GetLocaleFromBuffer(g_ptBR_Locale); + return GetLocaleFromBuffer(m_pHeap, g_ptBR_Locale); case FX_LANG_nl_NL: - return GetLocaleFromBuffer(g_nlNL_Locale); + return GetLocaleFromBuffer(m_pHeap, g_nlNL_Locale); case FX_LANG_ru_RU: - return GetLocaleFromBuffer(g_ruRU_Locale); + return GetLocaleFromBuffer(m_pHeap, g_ruRU_Locale); case FX_LANG_en_US: default: - return GetLocaleFromBuffer(g_enUS_Locale); + return GetLocaleFromBuffer(m_pHeap, g_enUS_Locale); } } LocaleIface* CXFA_LocaleMgr::GetLocaleByName(const WideString& wsLocaleName) { for (size_t i = 0; i < m_LocaleArray.size(); i++) { - LocaleIface* pLocale = m_LocaleArray[i].get(); + LocaleIface* pLocale = m_LocaleArray[i]; if (pLocale->GetName() == wsLocaleName) return pLocale; } if (wsLocaleName.GetLength() < 2) return nullptr; + for (size_t i = 0; i < m_XMLLocaleArray.size(); i++) { - LocaleIface* pLocale = m_XMLLocaleArray[i].get(); + LocaleIface* pLocale = m_XMLLocaleArray[i]; if (pLocale->GetName() == wsLocaleName) return pLocale; } - std::unique_ptr<LocaleIface> pLocale(GetLocale(GetLanguage(wsLocaleName))); - LocaleIface* pRetLocale = pLocale.get(); - if (pLocale) - m_XMLLocaleArray.push_back(std::move(pLocale)); - return pRetLocale; + CXFA_XMLLocale* pLocale = GetLocale(GetLanguage(wsLocaleName)); + if (!pLocale) + return nullptr; + + m_XMLLocaleArray.push_back(pLocale); + return pLocale; } void CXFA_LocaleMgr::SetDefLocale(LocaleIface* pLocale) {
diff --git a/xfa/fxfa/parser/cxfa_localemgr.h b/xfa/fxfa/parser/cxfa_localemgr.h index 9370988..12455b1 100644 --- a/xfa/fxfa/parser/cxfa_localemgr.h +++ b/xfa/fxfa/parser/cxfa_localemgr.h
@@ -12,16 +12,24 @@ #include "core/fxcrt/unowned_ptr.h" #include "core/fxcrt/widestring.h" +#include "fxjs/gc/heap.h" +#include "v8/include/cppgc/garbage-collected.h" +#include "v8/include/cppgc/member.h" #include "xfa/fgas/crt/locale_mgr_iface.h" class CXFA_Node; +class CXFA_NodeLocale; +class CXFA_XMLLocale; class LocaleIface; -class CXFA_LocaleMgr : public LocaleMgrIface { +class CXFA_LocaleMgr : public cppgc::GarbageCollected<CXFA_LocaleMgr>, + public LocaleMgrIface { public: - CXFA_LocaleMgr(CXFA_Node* pLocaleSet, WideString wsDeflcid); + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; ~CXFA_LocaleMgr() override; + void Trace(cppgc::Visitor* visitor) const; + LocaleIface* GetDefLocale() override; LocaleIface* GetLocaleByName(const WideString& wsLocaleName) override; @@ -29,13 +37,19 @@ WideString GetConfigLocaleName(CXFA_Node* pConfig); private: - std::unique_ptr<LocaleIface> GetLocale(uint16_t lcid); + CXFA_LocaleMgr(cppgc::Heap* pHeap, + CXFA_Node* pLocaleSet, + WideString wsDeflcid); - std::vector<std::unique_ptr<LocaleIface>> m_LocaleArray; - std::vector<std::unique_ptr<LocaleIface>> m_XMLLocaleArray; + // May allocate a new object on the cppgc heap. + CXFA_XMLLocale* GetLocale(uint16_t lcid); - // Owned by m_LocaleArray or m_XMLLocaleArray. - UnownedPtr<LocaleIface> m_pDefLocale; + UnownedPtr<cppgc::Heap> m_pHeap; + std::vector<cppgc::Member<CXFA_NodeLocale>> m_LocaleArray; + std::vector<cppgc::Member<CXFA_XMLLocale>> m_XMLLocaleArray; + + // Raw, owned by m_LocaleArray or m_XMLLocaleArray, may be GC'd after them. + LocaleIface* m_pDefLocale = nullptr; WideString m_wsConfigLocale; uint16_t m_dwDeflcid;
diff --git a/xfa/fxfa/parser/cxfa_localevalue.cpp b/xfa/fxfa/parser/cxfa_localevalue.cpp index 20f3730..5049b09 100644 --- a/xfa/fxfa/parser/cxfa_localevalue.cpp +++ b/xfa/fxfa/parser/cxfa_localevalue.cpp
@@ -65,6 +65,8 @@ } class ScopedLocale { + CPPGC_STACK_ALLOCATED(); // Raw/Unowned pointers allowed. + public: ScopedLocale(CXFA_LocaleMgr* pLocaleMgr, LocaleIface* pNewLocale) : m_pLocaleMgr(pLocaleMgr), @@ -137,8 +139,7 @@ size_t i = 0; for (; !bRet && i < wsPatterns.size(); i++) { const WideString& wsFormat = wsPatterns[i]; - auto pFormat = - std::make_unique<CFGAS_StringFormatter>(m_pLocaleMgr.Get(), wsFormat); + auto pFormat = std::make_unique<CFGAS_StringFormatter>(wsFormat); switch (ValueCategory(pFormat->GetCategory(), m_dwType)) { case CFGAS_StringFormatter::Category::kNull: bRet = pFormat->ParseNull(wsValue); @@ -152,9 +153,9 @@ break; case CFGAS_StringFormatter::Category::kNum: { WideString fNum; - bRet = pFormat->ParseNum(wsValue, &fNum); + bRet = pFormat->ParseNum(m_pLocaleMgr.Get(), wsValue, &fNum); if (!bRet) - bRet = pFormat->FormatNum(wsValue, &wsOutput); + bRet = pFormat->FormatNum(m_pLocaleMgr.Get(), wsValue, &wsOutput); break; } case CFGAS_StringFormatter::Category::kText: @@ -168,10 +169,12 @@ bRet = ValidateCanonicalDate(wsValue, &dt); if (!bRet) { bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDate, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDate, &dt); if (!bRet) { bRet = pFormat->FormatDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDate, &wsOutput); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDate, &wsOutput); } } break; @@ -179,21 +182,24 @@ case CFGAS_StringFormatter::Category::kTime: { CFX_DateTime dt; bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kTime, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kTime, &dt); if (!bRet) { bRet = pFormat->FormatDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kTime, &wsOutput); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kTime, &wsOutput); } break; } case CFGAS_StringFormatter::Category::kDateTime: { CFX_DateTime dt; bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDateTime, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDateTime, &dt); if (!bRet) { bRet = pFormat->FormatDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDateTime, - &wsOutput); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDateTime, &wsOutput); } break; } @@ -283,8 +289,7 @@ wsResult.clear(); bool bRet = false; - auto pFormat = - std::make_unique<CFGAS_StringFormatter>(m_pLocaleMgr.Get(), wsFormat); + auto pFormat = std::make_unique<CFGAS_StringFormatter>(wsFormat); CFGAS_StringFormatter::Category eCategory = ValueCategory(pFormat->GetCategory(), m_dwType); switch (eCategory) { @@ -297,22 +302,25 @@ bRet = pFormat->FormatZero(&wsResult); break; case CFGAS_StringFormatter::Category::kNum: - bRet = pFormat->FormatNum(m_wsValue, &wsResult); + bRet = pFormat->FormatNum(m_pLocaleMgr.Get(), m_wsValue, &wsResult); break; case CFGAS_StringFormatter::Category::kText: bRet = pFormat->FormatText(m_wsValue, &wsResult); break; case CFGAS_StringFormatter::Category::kDate: - bRet = pFormat->FormatDateTime( - m_wsValue, CFGAS_StringFormatter::DateTimeType::kDate, &wsResult); + bRet = pFormat->FormatDateTime(m_pLocaleMgr.Get(), m_wsValue, + CFGAS_StringFormatter::DateTimeType::kDate, + &wsResult); break; case CFGAS_StringFormatter::Category::kTime: - bRet = pFormat->FormatDateTime( - m_wsValue, CFGAS_StringFormatter::DateTimeType::kTime, &wsResult); + bRet = pFormat->FormatDateTime(m_pLocaleMgr.Get(), m_wsValue, + CFGAS_StringFormatter::DateTimeType::kTime, + &wsResult); break; case CFGAS_StringFormatter::Category::kDateTime: bRet = pFormat->FormatDateTime( - m_wsValue, CFGAS_StringFormatter::DateTimeType::kDateTime, &wsResult); + m_pLocaleMgr.Get(), m_wsValue, + CFGAS_StringFormatter::DateTimeType::kDateTime, &wsResult); break; default: wsResult = m_wsValue; @@ -559,8 +567,7 @@ bool bRet = false; for (size_t i = 0; !bRet && i < wsPatterns.size(); i++) { const WideString& wsFormat = wsPatterns[i]; - auto pFormat = - std::make_unique<CFGAS_StringFormatter>(m_pLocaleMgr.Get(), wsFormat); + auto pFormat = std::make_unique<CFGAS_StringFormatter>(wsFormat); switch (ValueCategory(pFormat->GetCategory(), m_dwType)) { case CFGAS_StringFormatter::Category::kNull: bRet = pFormat->ParseNull(wsValue); @@ -574,7 +581,7 @@ break; case CFGAS_StringFormatter::Category::kNum: { WideString fNum; - bRet = pFormat->ParseNum(wsValue, &fNum); + bRet = pFormat->ParseNum(m_pLocaleMgr.Get(), wsValue, &fNum); if (bRet) m_wsValue = std::move(fNum); break; @@ -587,7 +594,8 @@ bRet = ValidateCanonicalDate(wsValue, &dt); if (!bRet) { bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDate, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDate, &dt); } if (bRet) SetDate(dt); @@ -596,7 +604,8 @@ case CFGAS_StringFormatter::Category::kTime: { CFX_DateTime dt; bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kTime, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kTime, &dt); if (bRet) SetTime(dt); break; @@ -604,7 +613,8 @@ case CFGAS_StringFormatter::Category::kDateTime: { CFX_DateTime dt; bRet = pFormat->ParseDateTime( - wsValue, CFGAS_StringFormatter::DateTimeType::kDateTime, &dt); + m_pLocaleMgr.Get(), wsValue, + CFGAS_StringFormatter::DateTimeType::kDateTime, &dt); if (bRet) SetDateTime(dt); break;
diff --git a/xfa/fxfa/parser/cxfa_localevalue.h b/xfa/fxfa/parser/cxfa_localevalue.h index 2005905..cb154ef 100644 --- a/xfa/fxfa/parser/cxfa_localevalue.h +++ b/xfa/fxfa/parser/cxfa_localevalue.h
@@ -10,6 +10,7 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" #include "core/fxcrt/unowned_ptr.h" +#include "v8/include/cppgc/macros.h" #include "xfa/fxfa/parser/cxfa_node.h" class LocaleIface; @@ -27,6 +28,8 @@ #define XFA_VT_DATETIME 128 class CXFA_LocaleValue { + CPPGC_STACK_ALLOCATED(); // Raw/Unowned pointers allowed. + public: CXFA_LocaleValue(); CXFA_LocaleValue(uint32_t dwType, CXFA_LocaleMgr* pLocaleMgr);
diff --git a/xfa/fxfa/parser/cxfa_nodelocale.cpp b/xfa/fxfa/parser/cxfa_nodelocale.cpp index 95cb4eb..1f717ed 100644 --- a/xfa/fxfa/parser/cxfa_nodelocale.cpp +++ b/xfa/fxfa/parser/cxfa_nodelocale.cpp
@@ -38,14 +38,17 @@ return WideString(); } -CXFA_NodeLocale::CXFA_NodeLocale(CXFA_Node* pLocale) : m_pLocale(pLocale) {} +CXFA_NodeLocale::CXFA_NodeLocale(CXFA_Node* pNode) : m_pNode(pNode) {} CXFA_NodeLocale::~CXFA_NodeLocale() = default; +void CXFA_NodeLocale::Trace(cppgc::Visitor* visitor) const { + visitor->Trace(m_pNode); +} + WideString CXFA_NodeLocale::GetName() const { - return WideString(m_pLocale - ? m_pLocale->JSObject()->GetCData(XFA_Attribute::Name) - : nullptr); + return WideString(m_pNode ? m_pNode->JSObject()->GetCData(XFA_Attribute::Name) + : nullptr); } WideString CXFA_NodeLocale::GetDecimalSymbol() const { @@ -70,9 +73,9 @@ WideString CXFA_NodeLocale::GetDateTimeSymbols() const { CXFA_DateTimeSymbols* pSymbols = - m_pLocale ? m_pLocale->GetChild<CXFA_DateTimeSymbols>( - 0, XFA_Element::DateTimeSymbols, false) - : nullptr; + m_pNode ? m_pNode->GetChild<CXFA_DateTimeSymbols>( + 0, XFA_Element::DateTimeSymbols, false) + : nullptr; return pSymbols ? pSymbols->JSObject()->GetContent(false) : WideString(); } @@ -145,7 +148,7 @@ WideString CXFA_NodeLocale::GetSymbol(XFA_Element eElement, WideStringView symbol_type) const { CXFA_Node* pSymbols = - m_pLocale ? m_pLocale->GetChild<CXFA_Node>(0, eElement, false) : nullptr; + m_pNode ? m_pNode->GetChild<CXFA_Node>(0, eElement, false) : nullptr; CXFA_Node* pSymbol = GetNodeByName(pSymbols, symbol_type); return pSymbol ? pSymbol->JSObject()->GetContent(false) : WideString(); } @@ -154,9 +157,9 @@ int index, bool bAbbr) const { CXFA_CalendarSymbols* pCalendar = - m_pLocale ? m_pLocale->GetChild<CXFA_CalendarSymbols>( - 0, XFA_Element::CalendarSymbols, false) - : nullptr; + m_pNode ? m_pNode->GetChild<CXFA_CalendarSymbols>( + 0, XFA_Element::CalendarSymbols, false) + : nullptr; if (!pCalendar) return WideString();
diff --git a/xfa/fxfa/parser/cxfa_nodelocale.h b/xfa/fxfa/parser/cxfa_nodelocale.h index 4b3c205..8ad9222 100644 --- a/xfa/fxfa/parser/cxfa_nodelocale.h +++ b/xfa/fxfa/parser/cxfa_nodelocale.h
@@ -7,7 +7,9 @@ #ifndef XFA_FXFA_PARSER_CXFA_NODELOCALE_H_ #define XFA_FXFA_PARSER_CXFA_NODELOCALE_H_ -#include "core/fxcrt/unowned_ptr.h" +#include "fxjs/gc/heap.h" +#include "v8/include/cppgc/garbage-collected.h" +#include "v8/include/cppgc/member.h" #include "xfa/fgas/crt/locale_iface.h" #include "xfa/fxfa/fxfa_basic.h" @@ -15,11 +17,14 @@ WideString XFA_PatternToString(LocaleIface::NumSubcategory category); -class CXFA_NodeLocale final : public LocaleIface { +class CXFA_NodeLocale final : public cppgc::GarbageCollected<CXFA_NodeLocale>, + public LocaleIface { public: - explicit CXFA_NodeLocale(CXFA_Node* pLocale); + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; ~CXFA_NodeLocale() override; + void Trace(cppgc::Visitor* visitor) const; + // LocaleIface WideString GetName() const override; WideString GetDecimalSymbol() const override; @@ -39,13 +44,15 @@ WideString GetNumPattern(NumSubcategory eType) const override; private: + explicit CXFA_NodeLocale(CXFA_Node* pNode); + CXFA_Node* GetNodeByName(CXFA_Node* pParent, WideStringView wsName) const; WideString GetSymbol(XFA_Element eElement, WideStringView symbol_type) const; WideString GetCalendarSymbol(XFA_Element eElement, int index, bool bAbbr) const; - UnownedPtr<CXFA_Node> const m_pLocale; + cppgc::Member<CXFA_Node> const m_pNode; }; #endif // XFA_FXFA_PARSER_CXFA_NODELOCALE_H_
diff --git a/xfa/fxfa/parser/cxfa_xmllocale.cpp b/xfa/fxfa/parser/cxfa_xmllocale.cpp index f2b881f..aaad186 100644 --- a/xfa/fxfa/parser/cxfa_xmllocale.cpp +++ b/xfa/fxfa/parser/cxfa_xmllocale.cpp
@@ -29,27 +29,23 @@ } // namespace // static -std::unique_ptr<CXFA_XMLLocale> CXFA_XMLLocale::Create( - pdfium::span<uint8_t> data) { +CXFA_XMLLocale* CXFA_XMLLocale::Create(cppgc::Heap* heap, + pdfium::span<uint8_t> data) { auto stream = pdfium::MakeRetain<CFX_ReadOnlyMemoryStream>(data); CFX_XMLParser parser(stream); auto doc = parser.Parse(); if (!doc) return nullptr; - CFX_XMLElement* locale = nullptr; for (auto* child = doc->GetRoot()->GetFirstChild(); child; child = child->GetNextSibling()) { CFX_XMLElement* elem = ToXMLElement(child); if (elem && elem->GetName().EqualsASCII("locale")) { - locale = elem; - break; + return cppgc::MakeGarbageCollected<CXFA_XMLLocale>( + heap->GetAllocationHandle(), std::move(doc), elem); } } - if (!locale) - return nullptr; - - return std::make_unique<CXFA_XMLLocale>(std::move(doc), locale); + return nullptr; } CXFA_XMLLocale::CXFA_XMLLocale(std::unique_ptr<CFX_XMLDocument> doc, @@ -61,6 +57,8 @@ CXFA_XMLLocale::~CXFA_XMLLocale() = default; +void CXFA_XMLLocale::Trace(cppgc::Visitor* visitor) const {} + WideString CXFA_XMLLocale::GetName() const { return locale_->GetAttribute(L"name"); }
diff --git a/xfa/fxfa/parser/cxfa_xmllocale.h b/xfa/fxfa/parser/cxfa_xmllocale.h index 8819eb1..1fc2a98 100644 --- a/xfa/fxfa/parser/cxfa_xmllocale.h +++ b/xfa/fxfa/parser/cxfa_xmllocale.h
@@ -10,20 +10,25 @@ #include <memory> #include "core/fxcrt/unowned_ptr.h" +#include "fxjs/gc/heap.h" #include "third_party/base/span.h" +#include "v8/include/cppgc/garbage-collected.h" #include "xfa/fgas/crt/locale_iface.h" class CFX_XMLDocument; class CFX_XMLElement; -class CXFA_XMLLocale final : public LocaleIface { +class CXFA_XMLLocale final : public cppgc::GarbageCollected<CXFA_XMLLocale>, + public LocaleIface { public: - static std::unique_ptr<CXFA_XMLLocale> Create(pdfium::span<uint8_t> data); + // Object is created on cppgc heap. + static CXFA_XMLLocale* Create(cppgc::Heap* heap, pdfium::span<uint8_t> data); - explicit CXFA_XMLLocale(std::unique_ptr<CFX_XMLDocument> root, - CFX_XMLElement* locale); + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; ~CXFA_XMLLocale() override; + void Trace(cppgc::Visitor* visitor) const; + // LocaleIface WideString GetName() const override; WideString GetDecimalSymbol() const override; @@ -43,6 +48,8 @@ WideString GetNumPattern(NumSubcategory eType) const override; private: + CXFA_XMLLocale(std::unique_ptr<CFX_XMLDocument> root, CFX_XMLElement* locale); + WideString GetPattern(CFX_XMLElement* pElement, WideStringView bsTag, WideStringView wsName) const;
diff --git a/xfa/fxfa/parser/cxfa_xmllocale_unittest.cpp b/xfa/fxfa/parser/cxfa_xmllocale_unittest.cpp index b1f6de5..d6cd3ea 100644 --- a/xfa/fxfa/parser/cxfa_xmllocale_unittest.cpp +++ b/xfa/fxfa/parser/cxfa_xmllocale_unittest.cpp
@@ -6,6 +6,7 @@ #include <memory> +#include "testing/fxgc_unittest.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -88,32 +89,35 @@ "</currencySymbols>" "</locale>"; -std::unique_ptr<CXFA_XMLLocale> CreateLocaleHelper() { - return CXFA_XMLLocale::Create(pdfium::as_writable_bytes( - pdfium::make_span(const_cast<char*>(kXMLData), strlen(kXMLData)))); +CXFA_XMLLocale* CreateLocaleHelper(cppgc::Heap* heap) { + return CXFA_XMLLocale::Create( + heap, pdfium::as_writable_bytes(pdfium::make_span( + const_cast<char*>(kXMLData), strlen(kXMLData)))); } } // namespace -TEST(CXFA_XMLLocaleTest, Create) { - auto locale = CreateLocaleHelper(); +class CXFA_XMLLocaleTest : public FXGCUnitTest {}; + +TEST_F(CXFA_XMLLocaleTest, Create) { + auto* locale = CreateLocaleHelper(heap()); EXPECT_TRUE(locale != nullptr); } -TEST(CXFA_XMLLocaleTest, CreateBadXML) { - auto locale = CXFA_XMLLocale::Create(pdfium::span<uint8_t>()); +TEST_F(CXFA_XMLLocaleTest, CreateBadXML) { + auto* locale = CXFA_XMLLocale::Create(heap(), pdfium::span<uint8_t>()); EXPECT_TRUE(locale == nullptr); } -TEST(CXFA_XMLLocaleTest, GetName) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetName) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"en_US", locale->GetName()); } -TEST(CXFA_XMLLocaleTest, GetNumericSymbols) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetNumericSymbols) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L".", locale->GetDecimalSymbol()); @@ -123,15 +127,15 @@ EXPECT_EQ(L"$", locale->GetCurrencySymbol()); } -TEST(CXFA_XMLLocaleTest, GetDateTimeSymbols) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetDateTimeSymbols) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"GyMdkHmsSEDFwWahKzZ", locale->GetDateTimeSymbols()); } -TEST(CXFA_XMLLocaleTest, GetMonthName) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetMonthName) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"", locale->GetMonthName(24, false)); @@ -140,8 +144,8 @@ EXPECT_EQ(L"February", locale->GetMonthName(1, false)); } -TEST(CXFA_XMLLocaleTest, GetDayName) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetDayName) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"", locale->GetDayName(24, false)); @@ -150,24 +154,24 @@ EXPECT_EQ(L"Monday", locale->GetDayName(1, false)); } -TEST(CXFA_XMLLocaleTest, GetMeridiemName) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetMeridiemName) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"AM", locale->GetMeridiemName(true)); EXPECT_EQ(L"PM", locale->GetMeridiemName(false)); } -TEST(CXFA_XMLLocaleTest, GetEraName) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetEraName) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"AD", locale->GetEraName(true)); EXPECT_EQ(L"BC", locale->GetEraName(false)); } -TEST(CXFA_XMLLocaleTest, GetDatePattern) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetDatePattern) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"M/D/YY", @@ -182,8 +186,8 @@ locale->GetDatePattern(LocaleIface::DateTimeSubcategory::kLong)); } -TEST(CXFA_XMLLocaleTest, GetTimePattern) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetTimePattern) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"h:MM A", @@ -198,8 +202,8 @@ locale->GetTimePattern(LocaleIface::DateTimeSubcategory::kLong)); } -TEST(CXFA_XMLLocaleTest, GetNumPattern) { - auto locale = CreateLocaleHelper(); +TEST_F(CXFA_XMLLocaleTest, GetNumPattern) { + auto* locale = CreateLocaleHelper(heap()); ASSERT_TRUE(locale != nullptr); EXPECT_EQ(L"z,zzz,zzz,zzz,zzz,zzz%",