Make orphaning old objects automatic during dictionary set/removal.

Remove specific fixes for bug 901654, this should happen automatically
everywhere where updating a directory key's value might be an issue.

Bug: chromium:901654

Change-Id: Id74d922306275718d909ac7a7b4f7c7e7a5d9955
Reviewed-on: https://pdfium-review.googlesource.com/c/45450
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_flateencoder.cpp b/core/fpdfapi/edit/cpdf_flateencoder.cpp
index 7172bf2..0cfcce6 100644
--- a/core/fpdfapi/edit/cpdf_flateencoder.cpp
+++ b/core/fpdfapi/edit/cpdf_flateencoder.cpp
@@ -28,7 +28,7 @@
     m_dwSize = pDestAcc->GetSize();
     m_pData = pDestAcc->DetachData();
     m_pClonedDict = ToDictionary(pStream->GetDict()->Clone());
-    m_pClonedDict->RemoveFor("Filter");
+    m_pClonedDict->RemoveAndOrphan("Filter");
     ASSERT(!m_pDict);
     return;
   }
@@ -48,7 +48,7 @@
   m_pClonedDict = ToDictionary(pStream->GetDict()->Clone());
   m_pClonedDict->SetNewFor<CPDF_Number>("Length", static_cast<int>(m_dwSize));
   m_pClonedDict->SetNewFor<CPDF_Name>("Filter", "FlateDecode");
-  m_pClonedDict->RemoveFor(pdfium::stream::kDecodeParms);
+  m_pClonedDict->RemoveAndOrphan(pdfium::stream::kDecodeParms);
   ASSERT(!m_pDict);
 }
 
diff --git a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
index f9057db..8f02694 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp
@@ -107,7 +107,7 @@
     // Only stream that can be removed is 0.
     if (streams_to_remove_.find(0) != streams_to_remove_.end()) {
       CPDF_Dictionary* page_dict = obj_holder_->GetDict();
-      page_dict->RemoveFor("Contents");
+      page_dict->RemoveAndOrphan("Contents");
       contents_stream_ = nullptr;
     }
   } else if (contents_array_) {
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index d1d2410..97ec7a4 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -207,10 +207,10 @@
 CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key,
                                      std::unique_ptr<CPDF_Object> pObj) {
   CHECK(!IsLocked());
-  if (!pObj) {
-    m_Map.erase(key);
+  RemoveAndOrphan(key);
+  if (!pObj)
     return nullptr;
-  }
+
   ASSERT(pObj->IsInline());
   CPDF_Object* pRet = pObj.get();
   m_Map[MaybeIntern(key)] = std::move(pObj);
@@ -240,6 +240,12 @@
   return result;
 }
 
+void CPDF_Dictionary::RemoveAndOrphan(const ByteString& key) {
+  auto result = RemoveFor(key);
+  if (m_pHolder)
+    m_pHolder->AddOrphan(std::move(result));
+}
+
 void CPDF_Dictionary::ReplaceKey(const ByteString& oldkey,
                                  const ByteString& newkey) {
   CHECK(!IsLocked());
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index d3ca8a6..d0e302b 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -114,8 +114,16 @@
   void ConvertToIndirectObjectFor(const ByteString& key,
                                   CPDF_IndirectObjectHolder* pHolder);
 
+  // Removes |key| and returns the object bound to it for re-use.
   // Invalidates iterators for the element with the key |key|.
-  std::unique_ptr<CPDF_Object> RemoveFor(const ByteString& key);
+  std::unique_ptr<CPDF_Object> RemoveFor(const ByteString& key)
+      WARN_UNUSED_RESULT;
+
+  // Removes |key| but keeps the object bound to it alive for the duration
+  // that the indirect object holder is in existence, in case there are
+  // additional references.
+  // Invalidates iterators for the element with the key |key|.
+  void RemoveAndOrphan(const ByteString& key);
 
   // Invalidates iterators for the element with the key |oldkey|.
   void ReplaceKey(const ByteString& oldkey, const ByteString& newkey);
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 3837462..11ea67e 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -112,8 +112,8 @@
 
 void CPDF_Stream::SetDataAndRemoveFilter(pdfium::span<const uint8_t> pData) {
   SetData(pData);
-  m_pDict->RemoveFor("Filter");
-  m_pDict->RemoveFor(pdfium::stream::kDecodeParms);
+  m_pDict->RemoveAndOrphan("Filter");
+  m_pDict->RemoveAndOrphan(pdfium::stream::kDecodeParms);
 }
 
 void CPDF_Stream::SetDataFromStringstreamAndRemoveFilter(
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index 0872317..2c211ec 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -235,15 +235,13 @@
         if (!pClone)
           return false;
 
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
         m_pDict->SetFor("V", std::move(pClone));
         if (pRV) {
-          m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
           m_pDict->SetFor("RV", pDV->Clone());
         }
       } else {
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
+        m_pDict->RemoveAndOrphan("V");
+        m_pDict->RemoveAndOrphan("RV");
       }
       if (notify == NotificationOption::kNotify)
         NotifyAfterValueChange();
@@ -384,15 +382,11 @@
       int iIndex = FindOptionValue(csValue);
       if (iIndex < 0) {
         ByteString bsEncodeText = PDF_EncodeText(csValue);
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
         m_pDict->SetNewFor<CPDF_String>(key, bsEncodeText, false);
-        if (m_Type == kRichText && !bDefault) {
-          m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV"));
+        if (m_Type == kRichText && !bDefault)
           m_pDict->SetNewFor<CPDF_String>("RV", bsEncodeText, false);
-        }
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+        m_pDict->RemoveAndOrphan("I");
       } else {
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor(key));
         m_pDict->SetNewFor<CPDF_String>(key, PDF_EncodeText(csValue), false);
         if (!bDefault) {
           ClearSelection(NotificationOption::kDoNotNotify);
@@ -508,8 +502,8 @@
     if (!NotifyListOrComboBoxBeforeChange(csValue))
       return false;
   }
-  m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-  m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+  m_pDict->RemoveAndOrphan("V");
+  m_pDict->RemoveAndOrphan("I");
   if (notify == NotificationOption::kNotify)
     NotifyListOrComboBoxAfterChange();
   return true;
@@ -596,7 +590,7 @@
         SelectOption(index, false, NotificationOption::kDoNotNotify);
         if (pValue->IsString()) {
           if (pValue->GetUnicodeText() == opt_value)
-            m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+            m_pDict->RemoveAndOrphan("V");
         } else if (pValue->IsArray()) {
           auto pArray = pdfium::MakeUnique<CPDF_Array>();
           for (int i = 0; i < CountOptions(); i++) {
@@ -606,13 +600,12 @@
             }
           }
           if (pArray->size() > 0) {
-            m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
             m_pDict->SetFor("V", std::move(pArray));
           }
         }
       } else {
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+        m_pDict->RemoveAndOrphan("V");
+        m_pDict->RemoveAndOrphan("I");
       }
     }
   }
@@ -859,7 +852,7 @@
     if (bSelected)
       pArray->AddNew<CPDF_Number>(iOptIndex);
     if (pArray->IsEmpty())
-      m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+      m_pDict->RemoveAndOrphan("I");
   }
   if (notify == NotificationOption::kNotify)
     NotifyListOrComboBoxAfterChange();
diff --git a/fpdfsdk/cpdfsdk_baannot.cpp b/fpdfsdk/cpdfsdk_baannot.cpp
index 962d189..7d44485 100644
--- a/fpdfsdk/cpdfsdk_baannot.cpp
+++ b/fpdfsdk/cpdfsdk_baannot.cpp
@@ -96,7 +96,7 @@
 
 void CPDFSDK_BAAnnot::SetAnnotName(const WideString& sName) {
   if (sName.IsEmpty())
-    GetAnnotDict()->RemoveFor("NM");
+    GetAnnotDict()->RemoveAndOrphan("NM");
   else
     GetAnnotDict()->SetNewFor<CPDF_String>("NM", PDF_EncodeText(sName), false);
 }
@@ -115,7 +115,7 @@
 
 void CPDFSDK_BAAnnot::SetAppState(const ByteString& str) {
   if (str.IsEmpty())
-    GetAnnotDict()->RemoveFor("AS");
+    GetAnnotDict()->RemoveAndOrphan("AS");
   else
     GetAnnotDict()->SetNewFor<CPDF_String>("AS", str, false);
 }
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 5a96818..10f91d2 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -793,9 +793,9 @@
   } else {
     if (pApDict) {
       if (appearanceMode == FPDF_ANNOT_APPEARANCEMODE_NORMAL)
-        pAnnotDict->RemoveFor("AP");
+        pAnnotDict->RemoveAndOrphan("AP");
       else
-        pApDict->RemoveFor(modeKey);
+        pApDict->RemoveAndOrphan(modeKey);
     }
   }
 
diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp
index 5ac0be8..9c3effa 100644
--- a/fpdfsdk/fpdf_flatten.cpp
+++ b/fpdfsdk/fpdf_flatten.cpp
@@ -404,6 +404,6 @@
                                   m.e, m.f, sFormName.c_str());
     pNewXObject->SetDataAndRemoveFilter(sStream.AsRawSpan());
   }
-  pPageDict->RemoveFor("Annots");
+  pPageDict->RemoveAndOrphan("Annots");
   return FLATTEN_SUCCESS;
 }
diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp
index a81a4eb..2e02742 100644
--- a/fpdfsdk/pwl/cpwl_appstream.cpp
+++ b/fpdfsdk/pwl/cpwl_appstream.cpp
@@ -1947,7 +1947,7 @@
 }
 
 void CPWL_AppStream::Remove(const ByteString& sAPType) {
-  dict_->RemoveFor(sAPType);
+  dict_->RemoveAndOrphan(sAPType);
 }
 
 ByteString CPWL_AppStream::GetBackgroundAppStream() const {