Temporary not living long enough in cfx_fontmgr.cpp.
Since CTTFontDesc::SetFace() isn't going to prolong the life of
its CFX_Face argument, it shouldn't take a RetainPtr reference.
Making that change shows that the result of GetFixedFace() needs
to be assigned to and returned from a local.
Broken in https://pdfium-review.googlesource.com/c/pdfium/+/54790
Noticed while investigating linked bug, but probably a different problem.
Bug: chromium:974091
Change-Id: Ife76e82f5176f20586282da14ddb6ca1d66676e8
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56351
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/cfx_fontmgr.cpp b/core/fxge/cfx_fontmgr.cpp
index 2961e18..f5ae895 100644
--- a/core/fxge/cfx_fontmgr.cpp
+++ b/core/fxge/cfx_fontmgr.cpp
@@ -119,7 +119,7 @@
CTTFontDesc* pFontDesc = it->second.get();
*pFontData = pFontDesc->FontData();
- return pFontDesc->GetFace(0);
+ return pdfium::WrapRetain(pFontDesc->GetFace(0));
}
RetainPtr<CFX_Face> CFX_FontMgr::AddCachedFace(
@@ -140,11 +140,9 @@
return nullptr;
auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(std::move(pData));
- pFontDesc->SetFace(0, std::move(face));
-
- CTTFontDesc* pResult = pFontDesc.get();
+ pFontDesc->SetFace(0, face.Get());
m_FaceMap[KeyNameFromFace(face_name, weight, bItalic)] = std::move(pFontDesc);
- return pResult->GetFace(0);
+ return face;
}
RetainPtr<CFX_Face> CFX_FontMgr::GetCachedTTCFace(int ttc_size,
@@ -158,12 +156,14 @@
CTTFontDesc* pFontDesc = it->second.get();
*pFontData = pFontDesc->FontData();
int face_index = GetTTCIndex(pFontDesc->FontData(), ttc_size, font_offset);
- if (!pFontDesc->GetFace(face_index)) {
- pFontDesc->SetFace(face_index, GetFixedFace({pFontDesc->FontData(),
- static_cast<size_t>(ttc_size)},
- face_index));
- }
- return pFontDesc->GetFace(face_index);
+ RetainPtr<CFX_Face> pFace(pFontDesc->GetFace(face_index));
+ if (pFace)
+ return pFace;
+
+ pFace = GetFixedFace({pFontDesc->FontData(), static_cast<size_t>(ttc_size)},
+ face_index);
+ pFontDesc->SetFace(face_index, pFace.Get());
+ return pFace;
}
RetainPtr<CFX_Face> CFX_FontMgr::AddCachedTTCFace(
@@ -176,7 +176,7 @@
RetainPtr<CFX_Face> face =
GetFixedFace({pData.get(), static_cast<size_t>(ttc_size)}, face_index);
auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(std::move(pData));
- pFontDesc->SetFace(face_index, face);
+ pFontDesc->SetFace(face_index, face.Get());
m_FaceMap[KeyNameFromSize(ttc_size, checksum)] = std::move(pFontDesc);
return face;
}
diff --git a/core/fxge/cttfontdesc.cpp b/core/fxge/cttfontdesc.cpp
index 9d504bb..27bc770 100644
--- a/core/fxge/cttfontdesc.cpp
+++ b/core/fxge/cttfontdesc.cpp
@@ -8,18 +8,20 @@
#include <utility>
+#include "core/fxge/cfx_face.h"
+
CTTFontDesc::CTTFontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData)
: m_pFontData(std::move(pData)) {
}
CTTFontDesc::~CTTFontDesc() = default;
-void CTTFontDesc::SetFace(size_t index, const RetainPtr<CFX_Face>& face) {
+void CTTFontDesc::SetFace(size_t index, CFX_Face* face) {
ASSERT(index < FX_ArraySize(m_TTCFaces));
- m_TTCFaces[index].Reset(face.Get());
+ m_TTCFaces[index].Reset(face);
}
-RetainPtr<CFX_Face> CTTFontDesc::GetFace(size_t index) const {
+CFX_Face* CTTFontDesc::GetFace(size_t index) const {
ASSERT(index < FX_ArraySize(m_TTCFaces));
- return pdfium::WrapRetain(m_TTCFaces[index].Get());
+ return m_TTCFaces[index].Get();
}
diff --git a/core/fxge/cttfontdesc.h b/core/fxge/cttfontdesc.h
index c6b79f3..31a0520 100644
--- a/core/fxge/cttfontdesc.h
+++ b/core/fxge/cttfontdesc.h
@@ -11,7 +11,9 @@
#include "core/fxcrt/fx_memory.h"
#include "core/fxcrt/fx_system.h"
-#include "core/fxge/cfx_face.h"
+#include "core/fxcrt/observed_ptr.h"
+
+class CFX_Face;
class CTTFontDesc {
public:
@@ -19,8 +21,8 @@
~CTTFontDesc();
uint8_t* FontData() const { return m_pFontData.get(); }
- void SetFace(size_t index, const RetainPtr<CFX_Face>& face);
- RetainPtr<CFX_Face> GetFace(size_t index) const;
+ void SetFace(size_t index, CFX_Face* face);
+ CFX_Face* GetFace(size_t index) const;
private:
std::unique_ptr<uint8_t, FxFreeDeleter> const m_pFontData;