Remove CFX_MapPtrToPtr in CFX_PageRenderCache

core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp

Index: core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
index 8d0568e7196393394f784071eb9fcd4a45944b71..2a763a855586c8e3c3d32da26787be9a0c396a45 100644
--- a/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
+++ b/core/src/fpdfapi/fpdf_render/fpdf_render_cache.cpp
@@ -15,95 +15,65 @@ struct CACHEINFO {
   FX_DWORD time;
   CPDF_Stream* pStream;
 };
+
 extern "C" {
 static int compare(const void* data1, const void* data2) {
   return ((CACHEINFO*)data1)->time - ((CACHEINFO*)data2)->time;
 }
-};
-void CPDF_Page::ClearRenderCache() {
-  if (m_pPageRender) {
-    m_pPageRender->ClearAll();
-  }
-}
-void CPDF_PageRenderCache::ClearAll() {
-  FX_POSITION pos = m_ImageCaches.GetStartPosition();
-  while (pos) {
-    void* key;
-    void* value;
-    m_ImageCaches.GetNextAssoc(pos, key, value);
-    delete (CPDF_ImageCache*)value;
-  }
-  m_ImageCaches.RemoveAll();
-  m_nCacheSize = 0;
-  m_nTimeCount = 0;
+}  // extern "C"
+
+CPDF_PageRenderCache::~CPDF_PageRenderCache() {
+  for (const auto& it : m_ImageCache)
+    delete it.second;
 }
 void CPDF_PageRenderCache::CacheOptimization(int32_t dwLimitCacheSize) {
-  if (m_nCacheSize <= (FX_DWORD)dwLimitCacheSize) {
+  if (m_nCacheSize <= (FX_DWORD)dwLimitCacheSize)
     return;
-  }
-  int nCount = m_ImageCaches.GetCount();
-  CACHEINFO* pCACHEINFO =
-      (CACHEINFO*)FX_Alloc2D(uint8_t, sizeof(CACHEINFO), nCount);
-  FX_POSITION pos = m_ImageCaches.GetStartPosition();
-  int i = 0;
-  while (pos) {
-    void* key;
-    void* value;
-    m_ImageCaches.GetNextAssoc(pos, key, value);
-    pCACHEINFO[i].time = ((CPDF_ImageCache*)value)->GetTimeCount();
-    pCACHEINFO[i++].pStream = ((CPDF_ImageCache*)value)->GetStream();
+
+  unsigned int nCount = m_ImageCache.size();

[Lei Zhang, 2015/12/18 22:33:24]

size_t? also for |i| below?

[Tom Sepez, 2015/12/18 22:52:36]

Done.

+  CACHEINFO* pCACHEINFO = FX_Alloc(CACHEINFO, nCount);

[Lei Zhang, 2015/12/18 22:33:24]

Use a std::vector?

[Tom Sepez, 2015/12/18 22:52:36]

Doing a qsort on a vector's data feels wrong, so there'd be more work to make a
new compartor, etc.

+  unsigned int i = 0;
+  for (const auto& it : m_ImageCache) {
+    pCACHEINFO[i].time = it.second->GetTimeCount();
+    pCACHEINFO[i++].pStream = it.second->GetStream();
   }
   FXSYS_qsort(pCACHEINFO, nCount, sizeof(CACHEINFO), compare);
   FX_DWORD nTimeCount = m_nTimeCount;
+
+  // Check if time value is about to roll over and reset all entries.
+  // The comparision is legal because FX_DWORD is an unsigned type.
   if (nTimeCount + 1 < nTimeCount) {
-    for (i = 0; i < nCount; i++) {
-      ((CPDF_ImageCache*)(m_ImageCaches[pCACHEINFO[i].pStream]))
-          ->m_dwTimeCount = i;
-    }
+    for (i = 0; i < nCount; i++)
+      m_ImageCache[pCACHEINFO[i].pStream]->m_dwTimeCount = i;
     m_nTimeCount = nCount;
   }
+
   i = 0;
-  while (nCount > 15) {
-    ClearImageCache(pCACHEINFO[i++].pStream);
-    nCount--;
-  }
-  while (m_nCacheSize > (FX_DWORD)dwLimitCacheSize) {
-    ClearImageCache(pCACHEINFO[i++].pStream);
-  }
+  while (i + 15 < nCount)
+    ClearImageCacheEntry(pCACHEINFO[i++].pStream);
+
+  while (i < nCount && m_nCacheSize > (FX_DWORD)dwLimitCacheSize)
+    ClearImageCacheEntry(pCACHEINFO[i++].pStream);
+
   FX_Free(pCACHEINFO);
 }
-void CPDF_PageRenderCache::ClearImageCache(CPDF_Stream* pStream) {
-  void* value = m_ImageCaches.GetValueAt(pStream);
-  if (!value) {
-    m_ImageCaches.RemoveKey(pStream);
+void CPDF_PageRenderCache::ClearImageCacheEntry(CPDF_Stream* pStream) {
+  auto it = m_ImageCache.find(pStream);
+  if (it == m_ImageCache.end())
     return;
-  }
-  m_nCacheSize -= ((CPDF_ImageCache*)value)->EstimateSize();
-  delete (CPDF_ImageCache*)value;
-  m_ImageCaches.RemoveKey(pStream);
+
+  m_nCacheSize -= it->second->EstimateSize();
+  delete it->second;
+  m_ImageCache.erase(it);
 }
 FX_DWORD CPDF_PageRenderCache::EstimateSize() {
   FX_DWORD dwSize = 0;
-  FX_POSITION pos = m_ImageCaches.GetStartPosition();
-  while (pos) {
-    void* key;
-    void* value;
-    m_ImageCaches.GetNextAssoc(pos, key, value);
-    dwSize += ((CPDF_ImageCache*)value)->EstimateSize();
-  }
+  for (const auto& it : m_ImageCache)
+    dwSize += it.second->EstimateSize();
+
   m_nCacheSize = dwSize;
   return dwSize;
 }
-FX_DWORD CPDF_PageRenderCache::GetCachedSize(CPDF_Stream* pStream) const {
-  if (!pStream) {
-    return m_nCacheSize;
-  }
-  CPDF_ImageCache* pImageCache;
-  if (!m_ImageCaches.Lookup(pStream, (void*&)pImageCache)) {
-    return 0;
-  }
-  return pImageCache->EstimateSize();
-}
 void CPDF_PageRenderCache::GetCachedBitmap(CPDF_Stream* pStream,
                                            CFX_DIBSource*& pBitmap,
                                            CFX_DIBSource*& pMask,
@@ -114,21 +84,24 @@ void CPDF_PageRenderCache::GetCachedBitmap(CPDF_Stream* pStream,
                                            CPDF_RenderStatus* pRenderStatus,
                                            int32_t downsampleWidth,
                                            int32_t downsampleHeight) {
-  CPDF_ImageCache* pImageCache;
-  FX_BOOL bFind = m_ImageCaches.Lookup(pStream, (void*&)pImageCache);
-  if (!bFind) {
-    pImageCache = new CPDF_ImageCache(m_pPage->m_pDocument, pStream);
-  }
+  CPDF_ImageCacheEntry* pEntry;
+  const auto it = m_ImageCache.find(pStream);
+  FX_BOOL bFound = it != m_ImageCache.end();
+  if (bFound)
+    pEntry = it->second;
+  else
+    pEntry = new CPDF_ImageCacheEntry(m_pPage->m_pDocument, pStream);
+
   m_nTimeCount++;
-  FX_BOOL bCached = pImageCache->GetCachedBitmap(
+  FX_BOOL bAlreadyCached = pEntry->GetCachedBitmap(
       pBitmap, pMask, MatteColor, m_pPage->m_pPageResources, bStdCS,
       GroupFamily, bLoadMask, pRenderStatus, downsampleWidth, downsampleHeight);
-  if (!bFind) {
-    m_ImageCaches.SetAt(pStream, pImageCache);
-  }
-  if (!bCached) {
-    m_nCacheSize += pImageCache->EstimateSize();
-  }
+
+  if (!bFound)
+    m_ImageCache[pStream] = pEntry;
+
+  if (!bAlreadyCached)
+    m_nCacheSize += pEntry->EstimateSize();
 }
 FX_BOOL CPDF_PageRenderCache::StartGetCachedBitmap(
     CPDF_Stream* pStream,
@@ -138,54 +111,58 @@ FX_BOOL CPDF_PageRenderCache::StartGetCachedBitmap(
     CPDF_RenderStatus* pRenderStatus,
     int32_t downsampleWidth,
     int32_t downsampleHeight) {
-  m_bCurFindCache = m_ImageCaches.Lookup(pStream, (void*&)m_pCurImageCache);
-  if (!m_bCurFindCache) {
-    m_pCurImageCache = new CPDF_ImageCache(m_pPage->m_pDocument, pStream);
+  const auto it = m_ImageCache.find(pStream);
+  m_bCurFindCache = it != m_ImageCache.end();
+  if (m_bCurFindCache) {
+    m_pCurImageCacheEntry = it->second;
+  } else {
+    m_pCurImageCacheEntry =
+        new CPDF_ImageCacheEntry(m_pPage->m_pDocument, pStream);
   }
-  int ret = m_pCurImageCache->StartGetCachedBitmap(
+  int ret = m_pCurImageCacheEntry->StartGetCachedBitmap(
       pRenderStatus->m_pFormResource, m_pPage->m_pPageResources, bStdCS,
       GroupFamily, bLoadMask, pRenderStatus, downsampleWidth, downsampleHeight);
-  if (ret == 2) {
+  if (ret == 2)

[Tom Sepez, 2015/12/18 22:52:37]

FYI, speaking of enum's, this status code really bugs me.  I think its used
inconsistently, since the three states seem to be {error, incomplete, complete}
but I can't be sure.

     return TRUE;
-  }
+
   m_nTimeCount++;
-  if (!m_bCurFindCache) {
-    m_ImageCaches.SetAt(pStream, m_pCurImageCache);
-  }
-  if (!ret) {
-    m_nCacheSize += m_pCurImageCache->EstimateSize();
-  }
+  if (!m_bCurFindCache)
+    m_ImageCache[pStream] = m_pCurImageCacheEntry;
+
+  if (!ret)
+    m_nCacheSize += m_pCurImageCacheEntry->EstimateSize();
+
   return FALSE;
 }
 FX_BOOL CPDF_PageRenderCache::Continue(IFX_Pause* pPause) {
-  int ret = m_pCurImageCache->Continue(pPause);
-  if (ret == 2) {
+  int ret = m_pCurImageCacheEntry->Continue(pPause);
+  if (ret == 2)
     return TRUE;
-  }
   m_nTimeCount++;
-  if (!m_bCurFindCache) {
-    m_ImageCaches.SetAt(m_pCurImageCache->GetStream(), m_pCurImageCache);
-  }
-  if (!ret) {
-    m_nCacheSize += m_pCurImageCache->EstimateSize();
-  }
+  if (!m_bCurFindCache)
+    m_ImageCache[m_pCurImageCacheEntry->GetStream()] = m_pCurImageCacheEntry;
+  if (!ret)
+    m_nCacheSize += m_pCurImageCacheEntry->EstimateSize();
   return FALSE;
 }
 void CPDF_PageRenderCache::ResetBitmap(CPDF_Stream* pStream,
                                        const CFX_DIBitmap* pBitmap) {
-  CPDF_ImageCache* pImageCache;
-  if (!m_ImageCaches.Lookup(pStream, (void*&)pImageCache)) {
-    if (!pBitmap) {
+  CPDF_ImageCacheEntry* pEntry;
+  const auto it = m_ImageCache.find(pStream);
+  if (it == m_ImageCache.end()) {
+    if (!pBitmap)
       return;
-    }
-    pImageCache = new CPDF_ImageCache(m_pPage->m_pDocument, pStream);
-    m_ImageCaches.SetAt(pStream, pImageCache);
+    pEntry = new CPDF_ImageCacheEntry(m_pPage->m_pDocument, pStream);
+    m_ImageCache[pStream] = pEntry;
+  } else {
+    pEntry = it->second;
   }
-  int oldsize = pImageCache->EstimateSize();
-  pImageCache->Reset(pBitmap);
-  m_nCacheSize = pImageCache->EstimateSize() - oldsize;
+  m_nCacheSize -= pEntry->EstimateSize();
+  pEntry->Reset(pBitmap);
+  m_nCacheSize += pEntry->EstimateSize();

[Lei Zhang, 2015/12/18 22:33:24]

Was the old code just wrong?

[Tom Sepez, 2015/12/18 22:52:36]

That was my conclusion; assigning a delta to an absolute can't be right ...

 }
-CPDF_ImageCache::CPDF_ImageCache(CPDF_Document* pDoc, CPDF_Stream* pStream)
+CPDF_ImageCacheEntry::CPDF_ImageCacheEntry(CPDF_Document* pDoc,
+                                           CPDF_Stream* pStream)
     : m_dwTimeCount(0),
       m_pCurBitmap(NULL),
       m_pCurMask(NULL),
@@ -196,13 +173,11 @@ CPDF_ImageCache::CPDF_ImageCache(CPDF_Document* pDoc, CPDF_Stream* pStream)
       m_pCachedBitmap(NULL),
       m_pCachedMask(NULL),
       m_dwCacheSize(0) {}
-CPDF_ImageCache::~CPDF_ImageCache() {
+CPDF_ImageCacheEntry::~CPDF_ImageCacheEntry() {
   delete m_pCachedBitmap;
-  m_pCachedBitmap = NULL;
   delete m_pCachedMask;
-  m_pCachedMask = NULL;
 }
-void CPDF_ImageCache::Reset(const CFX_DIBitmap* pBitmap) {
+void CPDF_ImageCacheEntry::Reset(const CFX_DIBitmap* pBitmap) {
   delete m_pCachedBitmap;
   m_pCachedBitmap = NULL;
   if (pBitmap) {
@@ -211,15 +186,10 @@ void CPDF_ImageCache::Reset(const CFX_DIBitmap* pBitmap) {
   CalcSize();
 }
 void CPDF_PageRenderCache::ClearImageData() {
-  FX_POSITION pos = m_ImageCaches.GetStartPosition();
-  while (pos) {
-    void* key;
-    void* value;
-    m_ImageCaches.GetNextAssoc(pos, key, value);
-    ((CPDF_ImageCache*)value)->ClearImageData();
-  }
+  for (const auto& it : m_ImageCache)
+    it.second->ClearImageData();
 }
-void CPDF_ImageCache::ClearImageData() {
+void CPDF_ImageCacheEntry::ClearImageData() {
   if (m_pCachedBitmap && !m_pCachedBitmap->GetBuffer()) {
     ((CPDF_DIBSource*)m_pCachedBitmap)->ClearImageData();
   }
@@ -230,16 +200,16 @@ static FX_DWORD FPDF_ImageCache_EstimateImageSize(const CFX_DIBSource* pDIB) {
                    (FX_DWORD)pDIB->GetPaletteSize() * 4
              : 0;
 }
-FX_BOOL CPDF_ImageCache::GetCachedBitmap(CFX_DIBSource*& pBitmap,
-                                         CFX_DIBSource*& pMask,
-                                         FX_DWORD& MatteColor,
-                                         CPDF_Dictionary* pPageResources,
-                                         FX_BOOL bStdCS,
-                                         FX_DWORD GroupFamily,
-                                         FX_BOOL bLoadMask,
-                                         CPDF_RenderStatus* pRenderStatus,
-                                         int32_t downsampleWidth,
-                                         int32_t downsampleHeight) {
+FX_BOOL CPDF_ImageCacheEntry::GetCachedBitmap(CFX_DIBSource*& pBitmap,
+                                              CFX_DIBSource*& pMask,
+                                              FX_DWORD& MatteColor,
+                                              CPDF_Dictionary* pPageResources,
+                                              FX_BOOL bStdCS,
+                                              FX_DWORD GroupFamily,
+                                              FX_BOOL bLoadMask,
+                                              CPDF_RenderStatus* pRenderStatus,
+                                              int32_t downsampleWidth,
+                                              int32_t downsampleHeight) {
   if (m_pCachedBitmap) {
     pBitmap = m_pCachedBitmap;
     pMask = m_pCachedMask;
@@ -278,24 +248,24 @@ FX_BOOL CPDF_ImageCache::GetCachedBitmap(CFX_DIBSource*& pBitmap,
   CalcSize();
   return FALSE;
 }
-CFX_DIBSource* CPDF_ImageCache::DetachBitmap() {
+CFX_DIBSource* CPDF_ImageCacheEntry::DetachBitmap() {
   CFX_DIBSource* pDIBSource = m_pCurBitmap;
   m_pCurBitmap = NULL;
   return pDIBSource;
 }
-CFX_DIBSource* CPDF_ImageCache::DetachMask() {
+CFX_DIBSource* CPDF_ImageCacheEntry::DetachMask() {
   CFX_DIBSource* pDIBSource = m_pCurMask;
   m_pCurMask = NULL;
   return pDIBSource;
 }
-int CPDF_ImageCache::StartGetCachedBitmap(CPDF_Dictionary* pFormResources,
-                                          CPDF_Dictionary* pPageResources,
-                                          FX_BOOL bStdCS,
-                                          FX_DWORD GroupFamily,
-                                          FX_BOOL bLoadMask,
-                                          CPDF_RenderStatus* pRenderStatus,
-                                          int32_t downsampleWidth,
-                                          int32_t downsampleHeight) {
+int CPDF_ImageCacheEntry::StartGetCachedBitmap(CPDF_Dictionary* pFormResources,
+                                               CPDF_Dictionary* pPageResources,
+                                               FX_BOOL bStdCS,
+                                               FX_DWORD GroupFamily,
+                                               FX_BOOL bLoadMask,
+                                               CPDF_RenderStatus* pRenderStatus,
+                                               int32_t downsampleWidth,
+                                               int32_t downsampleHeight) {
   if (m_pCachedBitmap) {
     m_pCurBitmap = m_pCachedBitmap;
     m_pCurMask = m_pCachedMask;
@@ -321,7 +291,7 @@ int CPDF_ImageCache::StartGetCachedBitmap(CPDF_Dictionary* pFormResources,
   ContinueGetCachedBitmap();
   return 0;
 }
-int CPDF_ImageCache::ContinueGetCachedBitmap() {
+void CPDF_ImageCacheEntry::ContinueGetCachedBitmap() {
   m_MatteColor = ((CPDF_DIBSource*)m_pCurBitmap)->m_MatteColor;
   m_pCurMask = ((CPDF_DIBSource*)m_pCurBitmap)->DetachMask();
   CPDF_RenderContext* pContext = m_pRenderStatus->GetContext();
@@ -343,9 +313,8 @@ int CPDF_ImageCache::ContinueGetCachedBitmap() {
   m_pCurBitmap = m_pCachedBitmap;
   m_pCurMask = m_pCachedMask;
   CalcSize();
-  return 0;
 }
-int CPDF_ImageCache::Continue(IFX_Pause* pPause) {
+int CPDF_ImageCacheEntry::Continue(IFX_Pause* pPause) {
   int ret = ((CPDF_DIBSource*)m_pCurBitmap)->ContinueLoadDIBSource(pPause);
   if (ret == 2) {
     return ret;
@@ -358,7 +327,7 @@ int CPDF_ImageCache::Continue(IFX_Pause* pPause) {
   ContinueGetCachedBitmap();
   return 0;
 }
-void CPDF_ImageCache::CalcSize() {
+void CPDF_ImageCacheEntry::CalcSize() {
   m_dwCacheSize = FPDF_ImageCache_EstimateImageSize(m_pCachedBitmap) +
                   FPDF_ImageCache_EstimateImageSize(m_pCachedMask);
 }