Revert "Make orphaning old objects automatic during dictionary set/removal." This reverts commit 1399a22912fa38b0d5af2532cd0b37b0b3951d50. Reason for revert: Speculative revert to see if this is blocking PDFium roll into chromium, e.g. https://chromium-review.googlesource.com/c/chromium/src/+/1336650 Original change's description: > 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> TBR=thestig@chromium.org,tsepez@chromium.org Change-Id: If923e7ba8c93411e9033278721e178ccc51e2cb4 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:901654 Reviewed-on: https://pdfium-review.googlesource.com/c/45570 Reviewed-by: Tom Sepez <tsepez@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 0cfcce6..7172bf2 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->RemoveAndOrphan("Filter"); + m_pClonedDict->RemoveFor("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->RemoveAndOrphan(pdfium::stream::kDecodeParms); + m_pClonedDict->RemoveFor(pdfium::stream::kDecodeParms); ASSERT(!m_pDict); }
diff --git a/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp b/core/fpdfapi/edit/cpdf_pagecontentmanager.cpp index 8f02694..f9057db 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->RemoveAndOrphan("Contents"); + page_dict->RemoveFor("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 97ec7a4..d1d2410 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()); - RemoveAndOrphan(key); - if (!pObj) + if (!pObj) { + m_Map.erase(key); return nullptr; - + } ASSERT(pObj->IsInline()); CPDF_Object* pRet = pObj.get(); m_Map[MaybeIntern(key)] = std::move(pObj); @@ -240,12 +240,6 @@ 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 d0e302b..d3ca8a6 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -114,16 +114,8 @@ 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) - 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); + std::unique_ptr<CPDF_Object> RemoveFor(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 db38c41..8009024 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->RemoveAndOrphan("Filter"); - m_pDict->RemoveAndOrphan(pdfium::stream::kDecodeParms); + m_pDict->RemoveFor("Filter"); + m_pDict->RemoveFor(pdfium::stream::kDecodeParms); } void CPDF_Stream::SetDataFromStringstreamAndRemoveFilter(
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp index 2c211ec..0872317 100644 --- a/core/fpdfdoc/cpdf_formfield.cpp +++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -235,13 +235,15 @@ 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_pDict->RemoveAndOrphan("V"); - m_pDict->RemoveAndOrphan("RV"); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V")); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV")); } if (notify == NotificationOption::kNotify) NotifyAfterValueChange(); @@ -382,11 +384,15 @@ 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) + if (m_Type == kRichText && !bDefault) { + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("RV")); m_pDict->SetNewFor<CPDF_String>("RV", bsEncodeText, false); - m_pDict->RemoveAndOrphan("I"); + } + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("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); @@ -502,8 +508,8 @@ if (!NotifyListOrComboBoxBeforeChange(csValue)) return false; } - m_pDict->RemoveAndOrphan("V"); - m_pDict->RemoveAndOrphan("I"); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V")); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I")); if (notify == NotificationOption::kNotify) NotifyListOrComboBoxAfterChange(); return true; @@ -590,7 +596,7 @@ SelectOption(index, false, NotificationOption::kDoNotNotify); if (pValue->IsString()) { if (pValue->GetUnicodeText() == opt_value) - m_pDict->RemoveAndOrphan("V"); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V")); } else if (pValue->IsArray()) { auto pArray = pdfium::MakeUnique<CPDF_Array>(); for (int i = 0; i < CountOptions(); i++) { @@ -600,12 +606,13 @@ } } if (pArray->size() > 0) { + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V")); m_pDict->SetFor("V", std::move(pArray)); } } } else { - m_pDict->RemoveAndOrphan("V"); - m_pDict->RemoveAndOrphan("I"); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V")); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I")); } } } @@ -852,7 +859,7 @@ if (bSelected) pArray->AddNew<CPDF_Number>(iOptIndex); if (pArray->IsEmpty()) - m_pDict->RemoveAndOrphan("I"); + m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I")); } if (notify == NotificationOption::kNotify) NotifyListOrComboBoxAfterChange();
diff --git a/fpdfsdk/cpdfsdk_baannot.cpp b/fpdfsdk/cpdfsdk_baannot.cpp index 7d44485..962d189 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()->RemoveAndOrphan("NM"); + GetAnnotDict()->RemoveFor("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()->RemoveAndOrphan("AS"); + GetAnnotDict()->RemoveFor("AS"); else GetAnnotDict()->SetNewFor<CPDF_String>("AS", str, false); }
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp index 10f91d2..5a96818 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->RemoveAndOrphan("AP"); + pAnnotDict->RemoveFor("AP"); else - pApDict->RemoveAndOrphan(modeKey); + pApDict->RemoveFor(modeKey); } }
diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp index 9c3effa..5ac0be8 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->RemoveAndOrphan("Annots"); + pPageDict->RemoveFor("Annots"); return FLATTEN_SUCCESS; }
diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp index 2e02742..a81a4eb 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_->RemoveAndOrphan(sAPType); + dict_->RemoveFor(sAPType); } ByteString CPWL_AppStream::GetBackgroundAppStream() const {