Store the address of the page data map's value for proper referencing.

core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp

Index: core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp
diff --git a/core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp b/core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp
index 8e578f6f8f2eb0d6bb7b49f876ee48c5c3525a20..44915cc10865d9a3bd44facd23b52b8f24db3f77 100644
--- a/core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp
+++ b/core/src/fpdfapi/fpdf_page/fpdf_page_doc.cpp
@@ -147,15 +147,53 @@ CPDF_DocPageData::~CPDF_DocPageData()
 {
     Clear(FALSE);
     Clear(TRUE);
+    FX_POSITION pos = NULL;

[Tom Sepez, 2014/10/14 23:07:44]

nit: no need to initialize since we assign in the next line?

+    pos = m_PatternMap.GetStartPosition();

[Tom Sepez, 2014/10/14 23:07:44]

nit: for future cleanup, there should be a method of the templated map class
(maybe called deleteAll or something) that does this.

+    while (pos)
+    {
+        CPDF_Object* ptObj;
+        CPDF_CountedObject<CPDF_Pattern*>* ptData;
+        m_PatternMap.GetNextAssoc(pos, ptObj, ptData);
+        delete ptData;
+    }
+    m_PatternMap.RemoveAll();
+    pos = m_FontMap.GetStartPosition();
+    while (pos)
+    {
+        CPDF_Dictionary* fontDict;
+        CPDF_CountedObject<CPDF_Font*>* fontData;
+        m_FontMap.GetNextAssoc(pos, fontDict, fontData);
+        delete fontData;
+    }
+    m_FontMap.RemoveAll();
+    pos = m_ColorSpaceMap.GetStartPosition();
+    while (pos)
+    {
+        CPDF_Object* csKey;
+        CPDF_CountedObject<CPDF_ColorSpace*>* csData;
+        m_ColorSpaceMap.GetNextAssoc(pos, csKey, csData);
+        delete csData;
+    }
+    m_ColorSpaceMap.RemoveAll();
 }
 void CPDF_DocPageData::Clear(FX_BOOL bForceRelease)
 {
     FX_POSITION pos;
-
     m_bForceClear = bForceRelease;
-
-    // Release objects saved in the resource maps like font map and color space map.
-    // The compound objects shall be released before simple ones.
+    pos = m_PatternMap.GetStartPosition();
+    while (pos) {

[Tom Sepez, 2014/10/14 23:07:44]

nit: for future cleanup, again this should be a method of the templated map
class.  There's too much boilerplate here.

+        CPDF_Object* ptObj;
+        CPDF_CountedObject<CPDF_Pattern*>* ptData;
+        m_PatternMap.GetNextAssoc(pos, ptObj, ptData);
+        if (!ptData->m_Obj) {
+            continue;
+        }
+        if (bForceRelease || ptData->m_nCount < 2) {
+            ptData->m_Obj->SetForceClear(bForceRelease);
+            delete ptData->m_Obj;
+            ptData->m_Obj = NULL;
+        }
+    }
     pos = m_FontMap.GetStartPosition();
     while (pos) {
         CPDF_Dictionary* fontDict;
@@ -203,7 +241,6 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease)
                 }
             }
             delete ipData->m_Obj;
-            ipData->m_Obj = NULL;
             delete ipData;

[Tom Sepez, 2014/10/14 23:07:44]

Not clear to me why some of these delete their countedobjects but others wait
til later?  Its probably fine, but I'd like to understand why.

[Bo Xu, 2014/10/14 23:37:49]

For colorspace, pattern and font which are managed in their corresponding map, I
deter the deletion.

This is because, if |pattern1| = |ptData->mObj| is deleted, ptData->mObj is set
to NULL and we still want to keep ptData alive. Then when deleting pattern2 or
colorspace3 (they reference pattern1), we can check:

(assuming m_pCountedBaseCS  = ptData)
CPDF_ColorSpace* pCS = m_pCountedBaseCS ? m_pCountedBaseCS->m_Obj : NULL;

             m_IccProfileMap.RemoveKey(ipKey);
         }
@@ -218,25 +255,10 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease)
         }
         if (bForceRelease || ftData->m_nCount < 2) {
             delete ftData->m_Obj;
-            ftData->m_Obj = NULL;
             delete ftData;
             m_FontFileMap.RemoveKey(ftKey);
         }
     }
-    pos = m_PatternMap.GetStartPosition();
-    while (pos) {
-        CPDF_Object* ptObj;
-        CPDF_CountedObject<CPDF_Pattern*>* ptData;
-        m_PatternMap.GetNextAssoc(pos, ptObj, ptData);
-        if (!ptData->m_Obj) {
-            continue;
-        }
-        if (bForceRelease || ptData->m_nCount < 2) {
-            ptData->m_Obj->SetForceClear(bForceRelease);
-            delete ptData->m_Obj;
-            ptData->m_Obj = NULL;
-        }
-    }
     pos = m_ImageMap.GetStartPosition();
     while (pos) {
         FX_DWORD objNum;
@@ -247,7 +269,6 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease)
         }
         if (bForceRelease || imageData->m_nCount < 2) {
             delete imageData->m_Obj;
-            imageData->m_Obj = NULL;
             delete imageData;
             m_ImageMap.RemoveKey(objNum);
         }
@@ -653,4 +674,24 @@ void CPDF_DocPageData::ReleaseFontFileStreamAcc(CPDF_Stream* pFontStream, FX_BOO
         return;
     }
     PDF_DocPageData_Release<CPDF_Stream*, CPDF_StreamAcc*>(m_FontFileMap, pFontStream, NULL, bForce);
+}
+CPDF_CountedColorSpace* CPDF_DocPageData::FindColorSpacePtr(CPDF_Object* pCSObj) const

[Tom Sepez, 2014/10/14 23:07:44]

nit:  Again, this might be a method of the templated map class to save some
boilerplate code.

+{
+    if (!pCSObj) return NULL;
+    CPDF_CountedObject<CPDF_ColorSpace*>* csData;
+    if (m_ColorSpaceMap.Lookup(pCSObj, csData))
+    {
+        return csData;
+    }
+    return NULL;
 }
+CPDF_CountedPattern* CPDF_DocPageData::FindPatternPtr(CPDF_Object* pPatternObj) const
+{
+    if (!pPatternObj) return NULL;
+    CPDF_CountedObject<CPDF_Pattern*>* ptData;
+    if (m_PatternMap.Lookup(pPatternObj, ptData))
+    {
+        return ptData;
+    }
+    return NULL;
+}