commit 050ae79e8b95529ed556b2d84c179c4024ac705b
author Lei Zhang <thestig@chromium.org> Thu Feb 22 19:13:18 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Feb 22 19:13:18 2024 +0000
tree 535350d4be37905acca8775b4cbf3e0b4744735b
parent 038d39b314a6c6327959e97b0c7d3d64c1edf183

Stop making CPDF_IccProfile an Observable

This effectively reverts https://pdfium-review.googlesource.com/58231
and makes CPDF_DocPageData hold on to CPDF_IccProfile via RetainPtr once
again. As a result, the cached CPDF_IccProfile entries remain alive for
much longer and that can greatly increase the cache hit rate. Thus PDFs
that have frequently used, but expensive to parse ICC entries render
much faster.

At the same time, the CPDF_DocPageData class members are ordered
correctly, such that there are no circular dependencies that can
complicate CPDF_DocPageData destruction.

Bug: chromium:326161385,pdfium:2131
Change-Id: I837251a366d5d17c500afa6f137d77d6d1005f61
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116891
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>

core/fpdfapi/page/cpdf_docpagedata.cpp

diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index 9fd6e36..d235419 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -408,8 +408,9 @@
   CHECK(pProfileStream);
 
   auto it = m_IccProfileMap.find(pProfileStream);
-  if (it != m_IccProfileMap.end() && it->second)
-    return pdfium::WrapRetain(it->second.Get());
+  if (it != m_IccProfileMap.end()) {
+    return it->second;
+  }
 
   auto pAccessor = pdfium::MakeRetain<CPDF_StreamAcc>(pProfileStream);
   pAccessor->LoadAllDataFiltered();
@@ -426,12 +427,13 @@
   auto hash_it = m_HashIccProfileMap.find(hash_profile_key);
   if (hash_it != m_HashIccProfileMap.end()) {
     auto it_copied_stream = m_IccProfileMap.find(hash_it->second);
-    if (it_copied_stream != m_IccProfileMap.end() && it_copied_stream->second)
-      return pdfium::WrapRetain(it_copied_stream->second.Get());
+    if (it_copied_stream != m_IccProfileMap.end()) {
+      return it_copied_stream->second;
+    }
   }
   auto pProfile = pdfium::MakeRetain<CPDF_IccProfile>(
       pProfileStream, pAccessor->GetSpan(), expected_components);
-  m_IccProfileMap[pProfileStream].Reset(pProfile.Get());
+  m_IccProfileMap[pProfileStream] = pProfile;
   m_HashIccProfileMap[hash_profile_key] = std::move(pProfileStream);
   return pProfile;
 }

core/fpdfapi/page/cpdf_docpagedata.h

diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index 85aa7f4..1c578ab 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -124,7 +124,7 @@
       m_ColorSpaceMap;
   std::map<RetainPtr<const CPDF_Stream>, RetainPtr<CPDF_StreamAcc>>
       m_FontFileMap;
-  std::map<RetainPtr<const CPDF_Stream>, ObservedPtr<CPDF_IccProfile>>
+  std::map<RetainPtr<const CPDF_Stream>, RetainPtr<CPDF_IccProfile>>
       m_IccProfileMap;
   std::map<RetainPtr<const CPDF_Object>, ObservedPtr<CPDF_Pattern>>
       m_PatternMap;

core/fpdfapi/page/cpdf_iccprofile.h

diff --git a/core/fpdfapi/page/cpdf_iccprofile.h b/core/fpdfapi/page/cpdf_iccprofile.h
index 407f578..be98792 100644
--- a/core/fpdfapi/page/cpdf_iccprofile.h
+++ b/core/fpdfapi/page/cpdf_iccprofile.h
@@ -11,7 +11,6 @@
 
 #include <memory>
 
-#include "core/fxcrt/observed_ptr.h"
 #include "core/fxcrt/retain_ptr.h"
 #include "core/fxcrt/span.h"
 
@@ -21,7 +20,7 @@
 class IccTransform;
 }  // namespace fxcodec
 
-class CPDF_IccProfile final : public Retainable, public Observable {
+class CPDF_IccProfile final : public Retainable {
  public:
   CONSTRUCT_VIA_MAKE_RETAIN;