From f7992b39cec908e549fb995acae477a9efba538b Mon Sep 17 00:00:00 2001
From: Lei Zhang <thestig@chromium.org>
Date: Mon, 26 Oct 2015 15:36:17 -0700
Subject: Remove default argument from CPDF_Dictionary::SetAt().

Also fix a potential object leak.

R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1414963005 .
---
 core/include/fpdfapi/fpdf_objects.h                | 10 +------
 .../src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp | 13 +++++----
 .../fpdfapi/fpdf_parser/fpdf_parser_objects.cpp    | 31 ++++++++++------------
 .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 10 +++++--
 4 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/core/include/fpdfapi/fpdf_objects.h b/core/include/fpdfapi/fpdf_objects.h
index 419d5fc23e..d6050aec0b 100644
--- a/core/include/fpdfapi/fpdf_objects.h
+++ b/core/include/fpdfapi/fpdf_objects.h
@@ -410,9 +410,7 @@ class CPDF_Dictionary : public CPDF_Object {
 
   CPDF_Object* GetNextElement(FX_POSITION& pos, CFX_ByteString& key) const;
 
-  void SetAt(const CFX_ByteStringC& key,
-             CPDF_Object* pObj,
-             CPDF_IndirectObjects* pObjs = NULL);
+  void SetAt(const CFX_ByteStringC& key, CPDF_Object* pObj);
 
   void SetAtName(const CFX_ByteStringC& key, const CFX_ByteString& name);
 
@@ -436,12 +434,6 @@ class CPDF_Dictionary : public CPDF_Object {
                     CPDF_IndirectObjects* pDoc,
                     FX_DWORD objnum);
 
-  void AddReference(const CFX_ByteStringC& key,
-                    CPDF_IndirectObjects* pDoc,
-                    CPDF_Object* obj) {
-    AddReference(key, pDoc, obj->GetObjNum());
-  }
-
   void SetAtRect(const CFX_ByteStringC& key, const CFX_FloatRect& rect);
 
   void SetAtMatrix(const CFX_ByteStringC& key, const CFX_AffineMatrix& matrix);
diff --git a/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp b/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
index b86d00851c..694d5234a8 100644
--- a/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
+++ b/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
@@ -77,11 +77,14 @@ void CPDF_StreamContentParser::Handle_BeginImage() {
     }
     CFX_ByteString key((const FX_CHAR*)m_pSyntax->GetWordBuf() + 1,
                        m_pSyntax->GetWordSize() - 1);
-    CPDF_Object* pObj = m_pSyntax->ReadNextObject();
+    nonstd::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pObj(
+        m_pSyntax->ReadNextObject());
     if (!key.IsEmpty()) {
-      pDict->SetAt(key, pObj, m_pDocument);
-    } else if (pObj) {
-      pObj->Release();
+      FX_DWORD dwObjNum = pObj->GetObjNum();
+      if (dwObjNum)
+        pDict->SetAtReference(key, m_pDocument, dwObjNum);
+      else
+        pDict->SetAt(key, pObj.release());
     }
   }
   _PDF_ReplaceAbbr(pDict);
@@ -95,7 +98,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() {
         pCSObj = FindResourceObj(FX_BSTRC("ColorSpace"), name);
         if (pCSObj && !pCSObj->GetObjNum()) {
           pCSObj = pCSObj->Clone();
-          pDict->SetAt(FX_BSTRC("ColorSpace"), pCSObj, m_pDocument);
+          pDict->SetAt(FX_BSTRC("ColorSpace"), pCSObj);
         }
       }
     }
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp
index 07b1a208d7..46875d88a5 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp
@@ -717,27 +717,24 @@ FX_BOOL CPDF_Dictionary::KeyExist(const CFX_ByteStringC& key) const {
   void* value;
   return m_Map.Lookup(key, value);
 }
-void CPDF_Dictionary::SetAt(const CFX_ByteStringC& key,
-                            CPDF_Object* pObj,
-                            CPDF_IndirectObjects* pObjs) {
-  ASSERT(m_Type == PDFOBJ_DICTIONARY);
-  CPDF_Object* p = NULL;
-  m_Map.Lookup(key, (void*&)p);
-  if (p == pObj) {
+
+void CPDF_Dictionary::SetAt(const CFX_ByteStringC& key, CPDF_Object* pObj) {
+  ASSERT(IsDictionary());
+  void* pValue = nullptr;
+  m_Map.Lookup(key, pValue);
+  CPDF_Object* pExisting = static_cast<CPDF_Object*>(pValue);
+  if (pExisting == pObj)
     return;
-  }
-  if (p)
-    p->Release();
-  if (pObj) {
-    if (pObj->GetObjNum()) {
-      ASSERT(pObjs != NULL);
-      pObj = new CPDF_Reference(pObjs, pObj->GetObjNum());
-    }
+
+  if (pExisting)
+    pExisting->Release();
+
+  if (pObj)
     m_Map.SetAt(key, pObj);
-  } else {
+  else
     m_Map.RemoveKey(key);
-  }
 }
+
 void CPDF_Dictionary::AddValue(const CFX_ByteStringC& key, CPDF_Object* pObj) {
   ASSERT(m_Type == PDFOBJ_DICTIONARY);
   m_Map.AddValue(key, pObj);
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index 27cc8688cc..cbbeee6024 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -845,9 +845,15 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
                         FX_POSITION pos = pTrailer->GetStartPos();
                         while (pos) {
                           CFX_ByteString key;
-                          CPDF_Object* pObj =
+                          CPDF_Object* pElement =
                               pTrailer->GetNextElement(pos, key);
-                          m_pTrailer->SetAt(key, pObj->Clone(), m_pDocument);
+                          FX_DWORD dwObjNum = pElement->GetObjNum();
+                          if (dwObjNum) {
+                            m_pTrailer->SetAtReference(key, m_pDocument,
+                                                       dwObjNum);
+                          } else {
+                            m_pTrailer->SetAt(key, pElement->Clone());
+                          }
                         }
                         pObj->Release();
                       } else {
-- 
cgit v1.2.3