Use FixedUninitDataVector in CFX_FontMapper and CFX_FontMgr.
This is a reland of https://pdfium-review.googlesource.com/97691.
By introducing a simple FixedUninitDataVector that does not have slow
downs from initializing its data, this avoids the performance issues
that affected the first iteration of this CL.
Also removed the now unused CFX_MemoryStream ctor and add some extra
checks to verify font data retrieval worked as expected.
Bug: pdfium:1872
Change-Id: If1e97be6991f033ad6395ad79596cbd3f64767bd
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97976
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcrt/cfx_memorystream.cpp b/core/fxcrt/cfx_memorystream.cpp
index b1f131e..9c89bc7 100644
--- a/core/fxcrt/cfx_memorystream.cpp
+++ b/core/fxcrt/cfx_memorystream.cpp
@@ -11,12 +11,7 @@
#include "core/fxcrt/fx_safe_types.h"
-CFX_MemoryStream::CFX_MemoryStream() : m_nTotalSize(0), m_nCurSize(0) {}
-
-CFX_MemoryStream::CFX_MemoryStream(
- std::unique_ptr<uint8_t, FxFreeDeleter> pBuffer,
- size_t nSize)
- : m_data(std::move(pBuffer)), m_nTotalSize(nSize), m_nCurSize(nSize) {}
+CFX_MemoryStream::CFX_MemoryStream() = default;
CFX_MemoryStream::~CFX_MemoryStream() = default;
diff --git a/core/fxcrt/cfx_memorystream.h b/core/fxcrt/cfx_memorystream.h
index ff5ee1d..f72a211 100644
--- a/core/fxcrt/cfx_memorystream.h
+++ b/core/fxcrt/cfx_memorystream.h
@@ -34,13 +34,11 @@
private:
CFX_MemoryStream();
- CFX_MemoryStream(std::unique_ptr<uint8_t, FxFreeDeleter> pBuffer,
- size_t nSize);
~CFX_MemoryStream() override;
std::unique_ptr<uint8_t, FxFreeDeleter> m_data;
- size_t m_nTotalSize;
- size_t m_nCurSize;
+ size_t m_nTotalSize = 0;
+ size_t m_nCurSize = 0;
size_t m_nCurPos = 0;
};
diff --git a/core/fxcrt/cfx_read_only_vector_stream.cpp b/core/fxcrt/cfx_read_only_vector_stream.cpp
index 0e6488b..2d370ce 100644
--- a/core/fxcrt/cfx_read_only_vector_stream.cpp
+++ b/core/fxcrt/cfx_read_only_vector_stream.cpp
@@ -13,6 +13,11 @@
: data_(std::move(data)),
stream_(pdfium::MakeRetain<CFX_ReadOnlySpanStream>(data_)) {}
+CFX_ReadOnlyVectorStream::CFX_ReadOnlyVectorStream(
+ FixedUninitDataVector<uint8_t> data)
+ : fixed_data_(std::move(data)),
+ stream_(pdfium::MakeRetain<CFX_ReadOnlySpanStream>(fixed_data_.span())) {}
+
CFX_ReadOnlyVectorStream::~CFX_ReadOnlyVectorStream() = default;
FX_FILESIZE CFX_ReadOnlyVectorStream::GetSize() {
diff --git a/core/fxcrt/cfx_read_only_vector_stream.h b/core/fxcrt/cfx_read_only_vector_stream.h
index ac93dca..f9aa05c 100644
--- a/core/fxcrt/cfx_read_only_vector_stream.h
+++ b/core/fxcrt/cfx_read_only_vector_stream.h
@@ -8,6 +8,7 @@
#include <stdint.h>
#include "core/fxcrt/data_vector.h"
+#include "core/fxcrt/fixed_uninit_data_vector.h"
#include "core/fxcrt/fx_stream.h"
#include "core/fxcrt/retain_ptr.h"
@@ -25,9 +26,12 @@
private:
explicit CFX_ReadOnlyVectorStream(DataVector<uint8_t> data);
+ explicit CFX_ReadOnlyVectorStream(FixedUninitDataVector<uint8_t> data);
~CFX_ReadOnlyVectorStream() override;
const DataVector<uint8_t> data_;
+ const FixedUninitDataVector<uint8_t> fixed_data_;
+ // Spans over either `data_` or `fixed_data_`.
const RetainPtr<CFX_ReadOnlySpanStream> stream_;
};
diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp
index dfd367e..6370949 100644
--- a/core/fxge/cfx_fontmapper.cpp
+++ b/core/fxge/cfx_fontmapper.cpp
@@ -793,44 +793,42 @@
#endif // BUILDFLAG(IS_WIN)
#ifdef PDF_ENABLE_XFA
-std::unique_ptr<uint8_t, FxFreeDeleter> CFX_FontMapper::RawBytesForIndex(
- size_t index,
- size_t* returned_length) {
+FixedUninitDataVector<uint8_t> CFX_FontMapper::RawBytesForIndex(size_t index) {
CHECK_LT(index, m_FaceArray.size());
void* font_handle = m_pFontInfo->MapFont(0, false, FX_Charset::kDefault, 0,
GetFaceName(index));
if (!font_handle)
- return nullptr;
+ return FixedUninitDataVector<uint8_t>();
ScopedFontDeleter scoped_font(m_pFontInfo.get(), font_handle);
size_t required_size = m_pFontInfo->GetFontData(font_handle, 0, {});
if (required_size == 0)
- return nullptr;
+ return FixedUninitDataVector<uint8_t>();
- std::unique_ptr<uint8_t, FxFreeDeleter> pBuffer(
- FX_Alloc(uint8_t, required_size + 1));
- *returned_length =
- m_pFontInfo->GetFontData(font_handle, 0, {pBuffer.get(), required_size});
- return pBuffer;
+ FixedUninitDataVector<uint8_t> result(required_size);
+ size_t actual_size =
+ m_pFontInfo->GetFontData(font_handle, 0, result.writable_span());
+ CHECK_EQ(required_size, actual_size);
+ return result;
}
#endif // PDF_ENABLE_XFA
RetainPtr<CFX_Face> CFX_FontMapper::GetCachedTTCFace(void* font_handle,
size_t ttc_size,
size_t data_size) {
+ CHECK_GE(ttc_size, data_size);
uint32_t checksum = GetChecksumFromTT(font_handle);
RetainPtr<CFX_FontMgr::FontDesc> pFontDesc =
m_pFontMgr->GetCachedTTCFontDesc(ttc_size, checksum);
if (!pFontDesc) {
- std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
- FX_Alloc(uint8_t, ttc_size));
- m_pFontInfo->GetFontData(font_handle, kTableTTCF,
- {pFontData.get(), ttc_size});
- pFontDesc = m_pFontMgr->AddCachedTTCFontDesc(
- ttc_size, checksum, std::move(pFontData), ttc_size);
+ FixedUninitDataVector<uint8_t> font_data(ttc_size);
+ size_t size = m_pFontInfo->GetFontData(font_handle, kTableTTCF,
+ font_data.writable_span());
+ CHECK_EQ(ttc_size, size);
+ pFontDesc = m_pFontMgr->AddCachedTTCFontDesc(ttc_size, checksum,
+ std::move(font_data));
}
- CHECK(ttc_size >= data_size);
size_t font_offset = ttc_size - data_size;
size_t face_index =
GetTTCIndex(pFontDesc->FontData().first(ttc_size), font_offset);
@@ -855,11 +853,12 @@
RetainPtr<CFX_FontMgr::FontDesc> pFontDesc =
m_pFontMgr->GetCachedFontDesc(subst_name, weight, is_italic);
if (!pFontDesc) {
- std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
- FX_Alloc(uint8_t, data_size));
- m_pFontInfo->GetFontData(font_handle, 0, {pFontData.get(), data_size});
+ FixedUninitDataVector<uint8_t> font_data(data_size);
+ size_t size =
+ m_pFontInfo->GetFontData(font_handle, 0, font_data.writable_span());
+ CHECK_EQ(data_size, size);
pFontDesc = m_pFontMgr->AddCachedFontDesc(subst_name, weight, is_italic,
- std::move(pFontData), data_size);
+ std::move(font_data));
}
RetainPtr<CFX_Face> pFace(pFontDesc->GetFace(0));
if (pFace)
diff --git a/core/fxge/cfx_fontmapper.h b/core/fxge/cfx_fontmapper.h
index e8deaf5..5a3dc23 100644
--- a/core/fxge/cfx_fontmapper.h
+++ b/core/fxge/cfx_fontmapper.h
@@ -14,12 +14,15 @@
#include "build/build_config.h"
#include "core/fxcrt/bytestring.h"
#include "core/fxcrt/fx_codepage_forward.h"
-#include "core/fxcrt/fx_memory_wrappers.h"
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/unowned_ptr.h"
#include "core/fxge/cfx_face.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
+#ifdef PDF_ENABLE_XFA
+#include "core/fxcrt/fixed_uninit_data_vector.h"
+#endif
+
class CFX_FontMgr;
class CFX_SubstFont;
class SystemFontInfoIface;
@@ -85,9 +88,7 @@
#ifdef PDF_ENABLE_XFA
// `index` must be less than GetFaceSize().
- std::unique_ptr<uint8_t, FxFreeDeleter> RawBytesForIndex(
- size_t index,
- size_t* returned_length);
+ FixedUninitDataVector<uint8_t> RawBytesForIndex(size_t index);
#endif // PDF_ENABLE_XFA
private:
diff --git a/core/fxge/cfx_fontmgr.cpp b/core/fxge/cfx_fontmgr.cpp
index eacffc2..4418e9e 100644
--- a/core/fxge/cfx_fontmgr.cpp
+++ b/core/fxge/cfx_fontmgr.cpp
@@ -10,6 +10,7 @@
#include <memory>
#include <utility>
+#include "core/fxcrt/fixed_uninit_data_vector.h"
#include "core/fxge/cfx_face.h"
#include "core/fxge/cfx_fontmapper.h"
#include "core/fxge/cfx_substfont.h"
@@ -55,9 +56,8 @@
} // namespace
-CFX_FontMgr::FontDesc::FontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- size_t size)
- : m_Size(size), m_pFontData(std::move(pData)) {}
+CFX_FontMgr::FontDesc::FontDesc(FixedUninitDataVector<uint8_t> data)
+ : m_pFontData(std::move(data)) {}
CFX_FontMgr::FontDesc::~FontDesc() = default;
@@ -91,9 +91,8 @@
const ByteString& face_name,
int weight,
bool bItalic,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- size_t size) {
- auto pFontDesc = pdfium::MakeRetain<FontDesc>(std::move(pData), size);
+ FixedUninitDataVector<uint8_t> data) {
+ auto pFontDesc = pdfium::MakeRetain<FontDesc>(std::move(data));
m_FaceMap[{face_name, weight, bItalic}].Reset(pFontDesc.Get());
return pFontDesc;
}
@@ -109,9 +108,8 @@
RetainPtr<CFX_FontMgr::FontDesc> CFX_FontMgr::AddCachedTTCFontDesc(
size_t ttc_size,
uint32_t checksum,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- size_t size) {
- auto pNewDesc = pdfium::MakeRetain<FontDesc>(std::move(pData), size);
+ FixedUninitDataVector<uint8_t> data) {
+ auto pNewDesc = pdfium::MakeRetain<FontDesc>(std::move(data));
m_TTCFaceMap[{ttc_size, checksum}].Reset(pNewDesc.Get());
return pNewDesc;
}
diff --git a/core/fxge/cfx_fontmgr.h b/core/fxge/cfx_fontmgr.h
index 5bb9620..d936f16 100644
--- a/core/fxge/cfx_fontmgr.h
+++ b/core/fxge/cfx_fontmgr.h
@@ -15,7 +15,7 @@
#include <tuple>
#include "core/fxcrt/bytestring.h"
-#include "core/fxcrt/fx_memory_wrappers.h"
+#include "core/fxcrt/fixed_uninit_data_vector.h"
#include "core/fxcrt/observed_ptr.h"
#include "core/fxcrt/retain_ptr.h"
#include "core/fxge/freetype/fx_freetype.h"
@@ -31,17 +31,14 @@
CONSTRUCT_VIA_MAKE_RETAIN;
~FontDesc() override;
- pdfium::span<uint8_t> FontData() const {
- return {m_pFontData.get(), m_Size};
- }
+ pdfium::span<const uint8_t> FontData() const { return m_pFontData.span(); }
void SetFace(size_t index, CFX_Face* face);
CFX_Face* GetFace(size_t index) const;
private:
- FontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData, size_t size);
+ explicit FontDesc(FixedUninitDataVector<uint8_t> data);
- const size_t m_Size;
- std::unique_ptr<uint8_t, FxFreeDeleter> const m_pFontData;
+ const FixedUninitDataVector<uint8_t> m_pFontData;
ObservedPtr<CFX_Face> m_TTCFaces[16];
};
@@ -56,19 +53,15 @@
RetainPtr<FontDesc> GetCachedFontDesc(const ByteString& face_name,
int weight,
bool bItalic);
- RetainPtr<FontDesc> AddCachedFontDesc(
- const ByteString& face_name,
- int weight,
- bool bItalic,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- size_t size);
+ RetainPtr<FontDesc> AddCachedFontDesc(const ByteString& face_name,
+ int weight,
+ bool bItalic,
+ FixedUninitDataVector<uint8_t> data);
RetainPtr<FontDesc> GetCachedTTCFontDesc(size_t ttc_size, uint32_t checksum);
- RetainPtr<FontDesc> AddCachedTTCFontDesc(
- size_t ttc_size,
- uint32_t checksum,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- size_t size);
+ RetainPtr<FontDesc> AddCachedTTCFontDesc(size_t ttc_size,
+ uint32_t checksum,
+ FixedUninitDataVector<uint8_t> data);
RetainPtr<CFX_Face> NewFixedFace(RetainPtr<FontDesc> pDesc,
pdfium::span<const uint8_t> span,
diff --git a/xfa/fgas/font/cfgas_fontmgr.cpp b/xfa/fgas/font/cfgas_fontmgr.cpp
index d9f6833..abd7c3f 100644
--- a/xfa/fgas/font/cfgas_fontmgr.cpp
+++ b/xfa/fgas/font/cfgas_fontmgr.cpp
@@ -14,8 +14,9 @@
#include <utility>
#include "build/build_config.h"
-#include "core/fxcrt/cfx_memorystream.h"
+#include "core/fxcrt/cfx_read_only_vector_stream.h"
#include "core/fxcrt/data_vector.h"
+#include "core/fxcrt/fixed_uninit_data_vector.h"
#include "core/fxcrt/fx_codepage.h"
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_memory_wrappers.h"
@@ -473,13 +474,11 @@
RetainPtr<IFX_SeekableReadStream> CreateFontStream(CFX_FontMapper* pFontMapper,
size_t index) {
- size_t dwFileSize = 0;
- std::unique_ptr<uint8_t, FxFreeDeleter> pBuffer =
- pFontMapper->RawBytesForIndex(index, &dwFileSize);
- if (!pBuffer)
+ FixedUninitDataVector<uint8_t> buffer = pFontMapper->RawBytesForIndex(index);
+ if (buffer.empty())
return nullptr;
- return pdfium::MakeRetain<CFX_MemoryStream>(std::move(pBuffer), dwFileSize);
+ return pdfium::MakeRetain<CFX_ReadOnlyVectorStream>(std::move(buffer));
}
RetainPtr<IFX_SeekableReadStream> CreateFontStream(
