commit 95b363233c27dd4ca10013173249238e0e26aa51
author Tom Sepez <tsepez@chromium.org> Mon Oct 10 18:08:12 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Oct 10 18:08:12 2022 +0000
tree 2025f32a377af533f2a99bc96ad7eb4f05eae9de
parent 67a546fe614fddc7ba722871e54e97721e1f536c

Fix CPDF_DocPageData::MaybePurgeFontFileStreamAcc()

This is never actually purging any stream accessors since the caller
is holding a retained reference while the code is looking to see if
its map has the only retained reference. Callers must give up their
reference first, and this is accomplished via enforced move semantics.

Code coverage confirms this path was never being hit.

Change-Id: I4a4d801aef94a6611f6d1cfdce4e52a8160454b5
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/99092
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fpdfapi/font/cpdf_font.cpp

diff --git a/core/fpdfapi/font/cpdf_font.cpp b/core/fpdfapi/font/cpdf_font.cpp
index ad348b2..47ff701 100644
--- a/core/fpdfapi/font/cpdf_font.cpp
+++ b/core/fpdfapi/font/cpdf_font.cpp
@@ -58,7 +58,7 @@
   if (m_pFontFile) {
     auto* pPageData = m_pDocument->GetPageData();
     if (pPageData)
-      pPageData->MaybePurgeFontFileStreamAcc(m_pFontFile->GetStream());
+      pPageData->MaybePurgeFontFileStreamAcc(std::move(m_pFontFile));
   }
 }
 
@@ -214,10 +214,8 @@
   if (!m_pFontFile)
     return;
 
-  if (!m_Font.LoadEmbedded(m_pFontFile->GetSpan(), IsVertWriting(), key)) {
-    pData->MaybePurgeFontFileStreamAcc(m_pFontFile->GetStream());
-    m_pFontFile = nullptr;
-  }
+  if (!m_Font.LoadEmbedded(m_pFontFile->GetSpan(), IsVertWriting(), key))
+    pData->MaybePurgeFontFileStreamAcc(std::move(m_pFontFile));
 }
 
 void CPDF_Font::CheckFontMetrics() {

core/fpdfapi/page/cpdf_docpagedata.cpp

diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index ce3c084..6334b04 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -444,10 +444,15 @@
 }
 
 void CPDF_DocPageData::MaybePurgeFontFileStreamAcc(
-    RetainPtr<const CPDF_Stream> pFontStream) {
+    RetainPtr<CPDF_StreamAcc>&& pStreamAcc) {
+  if (!pStreamAcc)
+    return;
+
+  RetainPtr<const CPDF_Stream> pFontStream = pStreamAcc->GetStream();
   if (!pFontStream)
     return;
 
+  pStreamAcc.Reset();  // Drop moved caller's reference.
   auto it = m_FontFileMap.find(std::move(pFontStream));
   if (it != m_FontFileMap.end() && it->second->HasOneRef())
     m_FontFileMap.erase(it);

core/fpdfapi/page/cpdf_docpagedata.h

diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index d38eb42..9b34608 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -43,7 +43,7 @@
   RetainPtr<CPDF_StreamAcc> GetFontFileStreamAcc(
       RetainPtr<const CPDF_Stream> pFontStream) override;
   void MaybePurgeFontFileStreamAcc(
-      RetainPtr<const CPDF_Stream> pFontStream) override;
+      RetainPtr<CPDF_StreamAcc>&& pStreamAcc) override;
 
   // CPDF_Font::FormFactoryIFace:
   std::unique_ptr<CPDF_Font::FormIface> CreateForm(

core/fpdfapi/parser/cpdf_document.h

diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index c1a82e9..44b542c 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -55,7 +55,7 @@
     virtual RetainPtr<CPDF_StreamAcc> GetFontFileStreamAcc(
         RetainPtr<const CPDF_Stream> pFontStream) = 0;
     virtual void MaybePurgeFontFileStreamAcc(
-        RetainPtr<const CPDF_Stream> pFontStream) = 0;
+        RetainPtr<CPDF_StreamAcc>&& pStreamAcc) = 0;
 
     void SetDocument(CPDF_Document* pDoc) { m_pDoc = pDoc; }
     CPDF_Document* GetDocument() const { return m_pDoc.Get(); }