From 393fe4943226846a9b99878406d0bf75f31bb643 Mon Sep 17 00:00:00 2001 From: tsepez Date: Tue, 20 Sep 2016 11:28:25 -0700 Subject: Assert that dictionary can own the objects it is given. Upon indirect object holder destruction, all indirect objects are destroyed -- currently by order of increasing object number -- but ideally without ordering constraints. So currently, we can get away with a dictionary pointing directly at an indirect object with a higher number. It gets destroyed first, invoking Release() on its subordinates, which skips destroying them if they are indirect objects. But we don't want to rely on this artifact of destruction order. Should it happen to be reversed, the dictionary would invoke Release() on freed memory. Interestingly, CPDF_Array skirts the issue by replacing any indirect objects it is given with references. Not clear whether we should do the same thing for dictionaries, or remove it from arrays. The technique certainly complicates understanding ownership. The one violation found is in the unittest that broke the previous CL which tried to use unique_ptrs in indirect object holder. Review-Url: https://codereview.chromium.org/2353093002 --- core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp | 1 + core/fpdfdoc/cpdf_formfield_unittest.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp index cc395a2e08..aeee3382f9 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp @@ -169,6 +169,7 @@ bool CPDF_Dictionary::IsSignatureDict() const { } void CPDF_Dictionary::SetFor(const CFX_ByteString& key, CPDF_Object* pObj) { + ASSERT(!pObj || pObj->GetObjNum() == 0); auto it = m_Map.find(key); if (it == m_Map.end()) { if (pObj) diff --git a/core/fpdfdoc/cpdf_formfield_unittest.cpp b/core/fpdfdoc/cpdf_formfield_unittest.cpp index 7677f45284..d6feff9718 100644 --- a/core/fpdfdoc/cpdf_formfield_unittest.cpp +++ b/core/fpdfdoc/cpdf_formfield_unittest.cpp @@ -26,7 +26,6 @@ TEST(cpdf_formfield, FPDF_GetFullName) { EXPECT_STREQ("bar.foo", name.UTF8Encode().c_str()); CPDF_Dictionary* dict2 = new CPDF_Dictionary; - obj_holder.AddIndirectObject(dict2); dict1->SetFor("Parent", dict2); name = FPDF_GetFullName(root); EXPECT_STREQ("bar.foo", name.UTF8Encode().c_str()); -- cgit v1.2.3