Revert "Track indirect object holder in more CPDF_ objects." This reverts commit 69f086d657df631598bf111543c10f226be3d689. Reason for revert: presumably breaking roll into chromium, pdf_unittests. Original change's description: > Track indirect object holder in more CPDF_ objects. > > Change-Id: I4aa177089fe289b34b6ad3b7edcf9a8ca8c53a38 > Reviewed-on: https://pdfium-review.googlesource.com/c/45430 > Reviewed-by: Lei Zhang <thestig@chromium.org> > Commit-Queue: Tom Sepez <tsepez@chromium.org> TBR=thestig@chromium.org,tsepez@chromium.org Change-Id: Ib1c282566a6ec7b9cc3a84ea710e7204ab2542bf No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://pdfium-review.googlesource.com/c/45571 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/font/cpdf_fontencoding.cpp b/core/fpdfapi/font/cpdf_fontencoding.cpp index b5e254a..22950f6 100644 --- a/core/fpdfapi/font/cpdf_fontencoding.cpp +++ b/core/fpdfapi/font/cpdf_fontencoding.cpp
@@ -10,7 +10,6 @@ #include "core/fpdfapi/parser/cpdf_array.h" #include "core/fpdfapi/parser/cpdf_dictionary.h" -#include "core/fpdfapi/parser/cpdf_indirect_object_holder.h" #include "core/fpdfapi/parser/cpdf_name.h" #include "core/fpdfapi/parser/cpdf_number.h" #include "core/fpdfapi/parser/fpdf_parser_decode.h" @@ -1682,7 +1681,7 @@ } std::unique_ptr<CPDF_Object> CPDF_FontEncoding::Realize( - CPDF_IndirectObjectHolder* pHolder) { + WeakPtr<ByteStringPool> pPool) { int predefined = 0; for (int cs = PDFFONT_ENCODING_WINANSI; cs < PDFFONT_ENCODING_ZAPFDINGBATS; cs++) { @@ -1700,18 +1699,13 @@ } } if (predefined) { - if (predefined == PDFFONT_ENCODING_WINANSI) { - return pdfium::MakeUnique<CPDF_Name>(pHolder->GetByteStringPool(), - "WinAnsiEncoding"); - } - if (predefined == PDFFONT_ENCODING_MACROMAN) { - return pdfium::MakeUnique<CPDF_Name>(pHolder->GetByteStringPool(), - "MacRomanEncoding"); - } - if (predefined == PDFFONT_ENCODING_MACEXPERT) { - return pdfium::MakeUnique<CPDF_Name>(pHolder->GetByteStringPool(), - "MacExpertEncoding"); - } + if (predefined == PDFFONT_ENCODING_WINANSI) + return pdfium::MakeUnique<CPDF_Name>(pPool, "WinAnsiEncoding"); + if (predefined == PDFFONT_ENCODING_MACROMAN) + return pdfium::MakeUnique<CPDF_Name>(pPool, "MacRomanEncoding"); + if (predefined == PDFFONT_ENCODING_MACEXPERT) + return pdfium::MakeUnique<CPDF_Name>(pPool, "MacExpertEncoding"); + return nullptr; } const uint16_t* pStandard = @@ -1725,8 +1719,7 @@ pDiff->AddNew<CPDF_Name>(PDF_AdobeNameFromUnicode(m_Unicodes[i])); } - auto pDict = pdfium::MakeUnique<CPDF_Dictionary>(pHolder->GetByteStringPool(), - pHolder); + auto pDict = pdfium::MakeUnique<CPDF_Dictionary>(pPool); pDict->SetNewFor<CPDF_Name>("BaseEncoding", "WinAnsiEncoding"); pDict->SetFor("Differences", std::move(pDiff)); return std::move(pDict);
diff --git a/core/fpdfapi/font/cpdf_fontencoding.h b/core/fpdfapi/font/cpdf_fontencoding.h index 76cb7db..e9a2ee2 100644 --- a/core/fpdfapi/font/cpdf_fontencoding.h +++ b/core/fpdfapi/font/cpdf_fontencoding.h
@@ -32,7 +32,6 @@ const uint16_t* PDF_UnicodesForPredefinedCharSet(int encoding); const char* PDF_CharNameFromPredefinedCharSet(int encoding, uint8_t charcode); -class CPDF_IndirectObjectHolder; class CPDF_Object; class CPDF_FontEncoding { @@ -55,7 +54,7 @@ m_Unicodes[charcode] = unicode; } - std::unique_ptr<CPDF_Object> Realize(CPDF_IndirectObjectHolder* pHolder); + std::unique_ptr<CPDF_Object> Realize(WeakPtr<ByteStringPool> pPool); public: wchar_t m_Unicodes[kEncodingTableSize];
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp index 569dd46..7b62467 100644 --- a/core/fpdfapi/page/cpdf_docpagedata.cpp +++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -175,8 +175,10 @@ pDict->SetNewFor<CPDF_Name>("Type", "Font"); pDict->SetNewFor<CPDF_Name>("Subtype", "Type1"); pDict->SetNewFor<CPDF_Name>("BaseFont", fontName); - if (pEncoding) - pDict->SetFor("Encoding", pEncoding->Realize(m_pPDFDoc.Get())); + if (pEncoding) { + pDict->SetFor("Encoding", + pEncoding->Realize(m_pPDFDoc->GetByteStringPool())); + } std::unique_ptr<CPDF_Font> pFont = CPDF_Font::Create(m_pPDFDoc.Get(), pDict); if (!pFont)
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 9e5512e..2799684 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -1519,7 +1519,7 @@ uint32_t init_obj_count = m_pObjectHolder->GetPageObjectList()->size(); CPDF_StreamParser syntax(pdfium::make_span(pDataStart, size_left), - m_pDocument->GetByteStringPool(), m_pDocument.Get()); + m_pDocument->GetByteStringPool()); CPDF_StreamParserAutoClearer auto_clearer(&m_pSyntax, &syntax); while (1) { uint32_t cost =
diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp index ed9c200..037e72c5 100644 --- a/core/fpdfapi/page/cpdf_streamparser.cpp +++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -106,11 +106,10 @@ : m_pBuf(span) {} CPDF_StreamParser::CPDF_StreamParser(pdfium::span<const uint8_t> span, - const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder) - : m_pPool(pPool), m_pHolder(pHolder), m_pBuf(span) {} + const WeakPtr<ByteStringPool>& pPool) + : m_pPool(pPool), m_pBuf(span) {} -CPDF_StreamParser::~CPDF_StreamParser() = default; +CPDF_StreamParser::~CPDF_StreamParser() {} std::unique_ptr<CPDF_Stream> CPDF_StreamParser::ReadInlineStream( CPDF_Document* pDoc, @@ -329,7 +328,7 @@ if (m_WordSize == 1) return pdfium::MakeUnique<CPDF_String>(m_pPool, ReadHexString(), true); - auto pDict = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool, m_pHolder.Get()); + auto pDict = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool); while (1) { GetNextWord(bIsNumber); if (m_WordSize == 2 && m_WordBuffer[0] == '>')
diff --git a/core/fpdfapi/page/cpdf_streamparser.h b/core/fpdfapi/page/cpdf_streamparser.h index afbb0a5..af181ff 100644 --- a/core/fpdfapi/page/cpdf_streamparser.h +++ b/core/fpdfapi/page/cpdf_streamparser.h
@@ -18,7 +18,6 @@ #include "third_party/base/span.h" class CPDF_Dictionary; -class CPDF_IndirectObjectHolder; class CPDF_StreamParser { public: @@ -26,9 +25,7 @@ explicit CPDF_StreamParser(pdfium::span<const uint8_t> span); CPDF_StreamParser(pdfium::span<const uint8_t> span, - const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder); - + const WeakPtr<ByteStringPool>& pPool); ~CPDF_StreamParser(); SyntaxType ParseNextElement(); @@ -58,7 +55,6 @@ uint32_t m_Pos = 0; // Current byte position within m_pBuf. uint32_t m_WordSize = 0; // Current byte position within m_WordBuffer. WeakPtr<ByteStringPool> m_pPool; - UnownedPtr<CPDF_IndirectObjectHolder> m_pHolder; std::unique_ptr<CPDF_Object> m_pLastObj; pdfium::span<const uint8_t> m_pBuf; uint8_t m_WordBuffer[kMaxWordLength + 1]; // Include space for NUL.
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index 357c88c..78a52f9 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -21,9 +21,7 @@ CPDF_Array::CPDF_Array() {} -CPDF_Array::CPDF_Array(const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder) - : m_pPool(pPool), m_pHolder(pHolder) {} +CPDF_Array::CPDF_Array(const WeakPtr<ByteStringPool>& pPool) : m_pPool(pPool) {} CPDF_Array::~CPDF_Array() { // Break cycles for cyclic references.
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 6e86968..f7b4a1f 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h
@@ -18,17 +18,13 @@ #include "core/fxcrt/fx_coordinates.h" #include "third_party/base/ptr_util.h" -class CPDF_IndirectObjectHolder; - class CPDF_Array final : public CPDF_Object { public: using const_iterator = std::vector<std::unique_ptr<CPDF_Object>>::const_iterator; CPDF_Array(); - CPDF_Array(const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder); - + explicit CPDF_Array(const WeakPtr<ByteStringPool>& pPool); ~CPDF_Array() override; // CPDF_Object: @@ -67,8 +63,7 @@ // Creates object owned by the array, returns unowned pointer to it. // We have special cases for objects that can intern strings from - // a ByteStringPool, and for those that can return children back to - // an IndirectObjectHolder. + // a ByteStringPool. template <typename T, typename... Args> typename std::enable_if<!CanInternStrings<T>::value, T*>::type AddNew( Args&&... args) { @@ -76,23 +71,12 @@ Add(pdfium::MakeUnique<T>(std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - T*>::type - AddNew(Args&&... args) { + typename std::enable_if<CanInternStrings<T>::value, T*>::type AddNew( + Args&&... args) { return static_cast<T*>( Add(pdfium::MakeUnique<T>(m_pPool, std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - T*>::type - AddNew(Args&&... args) { - return static_cast<T*>(Add(pdfium::MakeUnique<T>( - m_pPool, m_pHolder.Get(), std::forward<Args>(args)...))); - } - - template <typename T, typename... Args> typename std::enable_if<!CanInternStrings<T>::value, T*>::type SetNewAt( size_t index, Args&&... args) { @@ -100,24 +84,13 @@ SetAt(index, pdfium::MakeUnique<T>(std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - T*>::type - SetNewAt(size_t index, Args&&... args) { + typename std::enable_if<CanInternStrings<T>::value, T*>::type SetNewAt( + size_t index, + Args&&... args) { return static_cast<T*>(SetAt( index, pdfium::MakeUnique<T>(m_pPool, std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - T*>::type - SetNewAt(size_t index, Args&&... args) { - return static_cast<T*>( - SetAt(index, pdfium::MakeUnique<T>(m_pPool, m_pHolder.Get(), - std::forward<Args>(args)...))); - } - - template <typename T, typename... Args> typename std::enable_if<!CanInternStrings<T>::value, T*>::type InsertNewAt( size_t index, Args&&... args) { @@ -125,22 +98,12 @@ InsertAt(index, pdfium::MakeUnique<T>(std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - T*>::type - InsertNewAt(size_t index, Args&&... args) { + typename std::enable_if<CanInternStrings<T>::value, T*>::type InsertNewAt( + size_t index, + Args&&... args) { return static_cast<T*>(InsertAt( index, pdfium::MakeUnique<T>(m_pPool, std::forward<Args>(args)...))); } - template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - T*>::type - InsertNewAt(size_t index, Args&&... args) { - return static_cast<T*>( - InsertAt(index, pdfium::MakeUnique<T>(m_pPool, m_pHolder.Get(), - std::forward<Args>(args)...))); - } void Clear(); void RemoveAt(size_t index); @@ -156,7 +119,6 @@ std::vector<std::unique_ptr<CPDF_Object>> m_Objects; WeakPtr<ByteStringPool> m_pPool; - UnownedPtr<CPDF_IndirectObjectHolder> m_pHolder; mutable uint32_t m_LockCount = 0; };
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp index d1d2410..c514e79 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -23,11 +23,11 @@ #include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" -CPDF_Dictionary::CPDF_Dictionary() = default; +CPDF_Dictionary::CPDF_Dictionary() + : CPDF_Dictionary(WeakPtr<ByteStringPool>()) {} -CPDF_Dictionary::CPDF_Dictionary(const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder) - : m_pPool(pPool), m_pHolder(pHolder) {} +CPDF_Dictionary::CPDF_Dictionary(const WeakPtr<ByteStringPool>& pPool) + : m_pPool(pPool) {} CPDF_Dictionary::~CPDF_Dictionary() { // Mark the object as deleted so that it will not be deleted again, @@ -71,7 +71,7 @@ bool bDirect, std::set<const CPDF_Object*>* pVisited) const { pVisited->insert(this); - auto pCopy = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool, m_pHolder.Get()); + auto pCopy = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool); CPDF_DictionaryLocker locker(this); for (const auto& it : locker) { if (!pdfium::ContainsKey(*pVisited, it.second.get())) {
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index d3ca8a6..e6abd26 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -29,9 +29,7 @@ std::map<ByteString, std::unique_ptr<CPDF_Object>>::const_iterator; CPDF_Dictionary(); - CPDF_Dictionary(const WeakPtr<ByteStringPool>& pPool, - CPDF_IndirectObjectHolder* pHolder); - + explicit CPDF_Dictionary(const WeakPtr<ByteStringPool>& pPool); ~CPDF_Dictionary() override; // CPDF_Object: @@ -88,24 +86,13 @@ SetFor(key, pdfium::MakeUnique<T>(std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - T*>::type - SetNewFor(const ByteString& key, Args&&... args) { + typename std::enable_if<CanInternStrings<T>::value, T*>::type SetNewFor( + const ByteString& key, + Args&&... args) { CHECK(!IsLocked()); return static_cast<T*>(SetFor( key, pdfium::MakeUnique<T>(m_pPool, std::forward<Args>(args)...))); } - template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - T*>::type - SetNewFor(const ByteString& key, Args&&... args) { - CHECK(!IsLocked()); - return static_cast<T*>( - SetFor(key, pdfium::MakeUnique<T>(m_pPool, m_pHolder.Get(), - std::forward<Args>(args)...))); - } // Convenience functions to convert native objects to array form. void SetRectFor(const ByteString& key, const CFX_FloatRect& rect); @@ -121,9 +108,6 @@ void ReplaceKey(const ByteString& oldkey, const ByteString& newkey); WeakPtr<ByteStringPool> GetByteStringPool() const { return m_pPool; } - CPDF_IndirectObjectHolder* GetIndirectObjectHolder() const { - return m_pHolder.Get(); - } private: friend class CPDF_DictionaryLocker; @@ -135,7 +119,6 @@ mutable uint32_t m_LockCount = 0; WeakPtr<ByteStringPool> m_pPool; - UnownedPtr<CPDF_IndirectObjectHolder> m_pHolder; std::map<ByteString, std::unique_ptr<CPDF_Object>> m_Map; };
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h index 96140e0..e812044 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -33,8 +33,7 @@ // Creates and adds a new object owned by the indirect object holder, // and returns an unowned pointer to it. We have a special case to - // handle objects that can intern strings from our ByteStringPool, and - // for those that can return objects back to the indirect object holder. + // handle objects that can intern strings from our ByteStringPool. template <typename T, typename... Args> typename std::enable_if<!CanInternStrings<T>::value, T*>::type NewIndirect( Args&&... args) { @@ -42,42 +41,21 @@ AddIndirectObject(pdfium::MakeUnique<T>(std::forward<Args>(args)...))); } template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - T*>::type - NewIndirect(Args&&... args) { + typename std::enable_if<CanInternStrings<T>::value, T*>::type NewIndirect( + Args&&... args) { return static_cast<T*>(AddIndirectObject( pdfium::MakeUnique<T>(m_pByteStringPool, std::forward<Args>(args)...))); } - template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - T*>::type - NewIndirect(Args&&... args) { - return static_cast<T*>(AddIndirectObject(pdfium::MakeUnique<T>( - m_pByteStringPool, this, std::forward<Args>(args)...))); - } // Creates and adds a new object not owned by the indirect object holder, // but which can intern strings from it. template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - !CanOrphanChildren<T>::value, - std::unique_ptr<T>>::type + typename std::enable_if<CanInternStrings<T>::value, std::unique_ptr<T>>::type New(Args&&... args) { return pdfium::MakeUnique<T>(m_pByteStringPool, std::forward<Args>(args)...); } - template <typename T, typename... Args> - typename std::enable_if<CanInternStrings<T>::value && - CanOrphanChildren<T>::value, - std::unique_ptr<T>>::type - New(Args&&... args) { - return pdfium::MakeUnique<T>(m_pByteStringPool, this, - std::forward<Args>(args)...); - } - // Takes ownership of |pObj|, returns unowned pointer to it. CPDF_Object* AddIndirectObject(std::unique_ptr<CPDF_Object> pObj);
diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h index 88cf0f9..9404ad2 100644 --- a/core/fpdfapi/parser/cpdf_object.h +++ b/core/fpdfapi/parser/cpdf_object.h
@@ -132,10 +132,4 @@ std::is_same<T, CPDF_String>::value; }; -template <typename T> -struct CanOrphanChildren { - static const bool value = std::is_same<T, CPDF_Array>::value || - std::is_same<T, CPDF_Dictionary>::value; -}; - #endif // CORE_FPDFAPI_PARSER_CPDF_OBJECT_H_
diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index d4f65a3..45e0f17 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -478,7 +478,7 @@ } if (word == "<<") { std::unique_ptr<CPDF_Dictionary> pDict = - pdfium::MakeUnique<CPDF_Dictionary>(m_pPool, pObjList); + pdfium::MakeUnique<CPDF_Dictionary>(m_pPool); while (1) { ByteString inner_word = GetNextWord(nullptr); if (inner_word.IsEmpty())
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp index 70a2072..2dc491b 100644 --- a/core/fpdfdoc/cpvt_generateap.cpp +++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -467,8 +467,8 @@ const CPDF_Dictionary& pAnnotDict, const ByteString& sExtGSDictName, const ByteString& sBlendMode) { - auto pGSDict = pdfium::MakeUnique<CPDF_Dictionary>( - pAnnotDict.GetByteStringPool(), pAnnotDict.GetIndirectObjectHolder()); + auto pGSDict = + pdfium::MakeUnique<CPDF_Dictionary>(pAnnotDict.GetByteStringPool()); pGSDict->SetNewFor<CPDF_Name>("Type", "ExtGState"); float fOpacity = @@ -478,8 +478,8 @@ pGSDict->SetNewFor<CPDF_Boolean>("AIS", false); pGSDict->SetNewFor<CPDF_Name>("BM", sBlendMode); - auto pExtGStateDict = pdfium::MakeUnique<CPDF_Dictionary>( - pAnnotDict.GetByteStringPool(), pAnnotDict.GetIndirectObjectHolder()); + auto pExtGStateDict = + pdfium::MakeUnique<CPDF_Dictionary>(pAnnotDict.GetByteStringPool()); pExtGStateDict->SetFor(sExtGSDictName, std::move(pGSDict)); return pExtGStateDict; }