From 4e4d1a662b09d9ee1dc93dd8ee37694d3c4eb519 Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 13 Oct 2016 15:56:53 -0700 Subject: Make CPDF_Image() constructors saner. Introduce the UniqueDictionary typedef and friends, to allow moving to unique_ptrs before the Release() deleter issue is fully resolved. This will go away down the road. Review-Url: https://codereview.chromium.org/2420743002 --- core/fpdfapi/page/cpdf_docpagedata.cpp | 11 ++-- core/fpdfapi/page/cpdf_image.cpp | 81 +++++++++++++------------- core/fpdfapi/page/cpdf_image.h | 32 +++++----- core/fpdfapi/page/cpdf_streamcontentparser.cpp | 3 +- core/fpdfapi/parser/cpdf_array.h | 9 +++ core/fpdfapi/parser/cpdf_dictionary.h | 12 ++++ core/fpdfapi/parser/cpdf_object.h | 2 + core/fpdfapi/parser/cpdf_stream.h | 10 +++- 8 files changed, 97 insertions(+), 63 deletions(-) diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp index 7a85e60da2..5f1f561ebb 100644 --- a/core/fpdfapi/page/cpdf_docpagedata.cpp +++ b/core/fpdfapi/page/cpdf_docpagedata.cpp @@ -397,14 +397,15 @@ CPDF_Image* CPDF_DocPageData::GetImage(CPDF_Object* pImageStream) { if (!pImageStream) return nullptr; - const uint32_t dwImageObjNum = pImageStream->GetObjNum(); - auto it = m_ImageMap.find(dwImageObjNum); + ASSERT(!pImageStream->IsInline()); + const uint32_t dwObjNum = pImageStream->GetObjNum(); + auto it = m_ImageMap.find(dwObjNum); if (it != m_ImageMap.end()) return it->second->AddRef(); - CPDF_CountedImage* pCountedImage = new CPDF_CountedImage( - new CPDF_Image(m_pPDFDoc, pImageStream->AsStream(), false)); - m_ImageMap[dwImageObjNum] = pCountedImage; + CPDF_CountedImage* pCountedImage = + new CPDF_CountedImage(new CPDF_Image(m_pPDFDoc, dwObjNum)); + m_ImageMap[dwObjNum] = pCountedImage; return pCountedImage->AddRef(); } diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp index 29d10e1d6b..50768c5058 100644 --- a/core/fpdfapi/page/cpdf_image.cpp +++ b/core/fpdfapi/page/cpdf_image.cpp @@ -15,6 +15,7 @@ #include "core/fpdfapi/page/cpdf_page.h" #include "core/fpdfapi/parser/cpdf_array.h" #include "core/fpdfapi/parser/cpdf_boolean.h" +#include "core/fpdfapi/parser/cpdf_dictionary.h" #include "core/fpdfapi/parser/cpdf_document.h" #include "core/fpdfapi/parser/cpdf_string.h" #include "core/fpdfapi/render/cpdf_pagerendercache.h" @@ -22,55 +23,55 @@ #include "core/fxcodec/fx_codec.h" #include "core/fxge/fx_dib.h" -CPDF_Image::CPDF_Image(CPDF_Document* pDoc) - : CPDF_Image(pDoc, nullptr, false) {} +CPDF_Image::CPDF_Image(CPDF_Document* pDoc) : m_pDocument(pDoc) {} -CPDF_Image::CPDF_Image(CPDF_Document* pDoc, CPDF_Stream* pStream, bool bInline) - : m_pDIBSource(nullptr), - m_pMask(nullptr), - m_MatteColor(0), - m_pStream(pStream), - m_bInline(bInline), - m_pInlineDict(nullptr), - m_Height(0), - m_Width(0), - m_bIsMask(false), - m_bInterpolate(false), - m_pDocument(pDoc), - m_pOC(nullptr) { - if (!pStream) +CPDF_Image::CPDF_Image(CPDF_Document* pDoc, UniqueStream pStream) + : m_pDocument(pDoc), + m_pStream(pStream.get()), + m_pOwnedStream(std::move(pStream)) { + if (!m_pStream) return; - CPDF_Dictionary* pDict = pStream->GetDict(); - if (m_bInline) - m_pInlineDict = ToDictionary(pDict->Clone()); - - m_pOC = pDict->GetDictFor("OC"); - m_bIsMask = - !pDict->KeyExist("ColorSpace") || pDict->GetIntegerFor("ImageMask"); - m_bInterpolate = !!pDict->GetIntegerFor("Interpolate"); - m_Height = pDict->GetIntegerFor("Height"); - m_Width = pDict->GetIntegerFor("Width"); + m_pOwnedDict = ToDictionary(UniqueObject(m_pStream->GetDict()->Clone())); + m_pDict = m_pOwnedDict.get(); + FinishInitialization(); } -CPDF_Image::~CPDF_Image() { - if (m_bInline) { - if (m_pStream) - m_pStream->Release(); - if (m_pInlineDict) - m_pInlineDict->Release(); - } +CPDF_Image::CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum) + : m_pDocument(pDoc), + m_pStream(ToStream(pDoc->GetIndirectObject(dwStreamObjNum))) { + if (!m_pStream) + return; + + m_pDict = m_pStream->GetDict(); + FinishInitialization(); } -CPDF_Image* CPDF_Image::Clone() { - if (!m_pStream->IsInline()) - return m_pDocument->GetPageData()->GetImage(m_pStream); +CPDF_Image::~CPDF_Image() {} - CPDF_Image* pImage = - new CPDF_Image(m_pDocument, ToStream(m_pStream->Clone()), m_bInline); - if (m_bInline) - pImage->SetInlineDict(ToDictionary(m_pInlineDict->CloneDirectObject())); +void CPDF_Image::FinishInitialization() { + m_pOC = m_pDict->GetDictFor("OC"); + m_bIsMask = + !m_pDict->KeyExist("ColorSpace") || m_pDict->GetIntegerFor("ImageMask"); + m_bInterpolate = !!m_pDict->GetIntegerFor("Interpolate"); + m_Height = m_pDict->GetIntegerFor("Height"); + m_Width = m_pDict->GetIntegerFor("Width"); +} +CPDF_Image* CPDF_Image::Clone() { + CPDF_Image* pImage = new CPDF_Image(m_pDocument); + if (m_pOwnedStream) { + pImage->m_pOwnedStream = ToStream(UniqueObject(m_pOwnedStream->Clone())); + pImage->m_pStream = pImage->m_pOwnedStream.get(); + } else { + pImage->m_pStream = m_pStream; + } + if (m_pOwnedDict) { + pImage->m_pOwnedDict = ToDictionary(UniqueObject(m_pOwnedDict->Clone())); + pImage->m_pDict = pImage->m_pOwnedDict.get(); + } else { + pImage->m_pDict = m_pDict; + } return pImage; } diff --git a/core/fpdfapi/page/cpdf_image.h b/core/fpdfapi/page/cpdf_image.h index 7058019ba3..72de0d8cef 100644 --- a/core/fpdfapi/page/cpdf_image.h +++ b/core/fpdfapi/page/cpdf_image.h @@ -28,12 +28,13 @@ class IFX_Pause; class CPDF_Image { public: explicit CPDF_Image(CPDF_Document* pDoc); - CPDF_Image(CPDF_Document* pDoc, CPDF_Stream* pStream, bool bInline); + CPDF_Image(CPDF_Document* pDoc, UniqueStream pStream); + CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum); ~CPDF_Image(); CPDF_Image* Clone(); - CPDF_Dictionary* GetInlineDict() const { return m_pInlineDict; } + CPDF_Dictionary* GetInlineDict() const { return m_pDict; } CPDF_Stream* GetStream() const { return m_pStream; } CPDF_Dictionary* GetDict() const { return m_pStream ? m_pStream->GetDict() : nullptr; @@ -44,7 +45,7 @@ class CPDF_Image { int32_t GetPixelHeight() const { return m_Height; } int32_t GetPixelWidth() const { return m_Width; } - bool IsInline() const { return m_bInline; } + bool IsInline() const { return !!m_pOwnedStream; } bool IsMask() const { return m_bIsMask; } bool IsInterpol() const { return m_bInterpolate; } @@ -54,7 +55,6 @@ class CPDF_Image { uint32_t GroupFamily = 0, FX_BOOL bLoadMask = FALSE) const; - void SetInlineDict(CPDF_Dictionary* pDict) { m_pInlineDict = pDict; } void SetImage(const CFX_DIBitmap* pDIBitmap, int32_t iCompress); void SetJpegImage(IFX_FileRead* pFile); @@ -69,22 +69,24 @@ class CPDF_Image { CFX_DIBSource* DetachBitmap(); CFX_DIBSource* DetachMask(); - CFX_DIBSource* m_pDIBSource; - CFX_DIBSource* m_pMask; - uint32_t m_MatteColor; + CFX_DIBSource* m_pDIBSource = nullptr; + CFX_DIBSource* m_pMask = nullptr; + uint32_t m_MatteColor = 0; private: + void FinishInitialization(); CPDF_Dictionary* InitJPEG(uint8_t* pData, uint32_t size); - CPDF_Stream* m_pStream; - const bool m_bInline; - CPDF_Dictionary* m_pInlineDict; - int32_t m_Height; - int32_t m_Width; - bool m_bIsMask; - bool m_bInterpolate; + int32_t m_Height = 0; + int32_t m_Width = 0; + bool m_bIsMask = false; + bool m_bInterpolate = false; CPDF_Document* const m_pDocument; - CPDF_Dictionary* m_pOC; + CPDF_Stream* m_pStream = nullptr; + CPDF_Dictionary* m_pDict = nullptr; + UniqueStream m_pOwnedStream; + UniqueDictionary m_pOwnedDict; + CPDF_Dictionary* m_pOC = nullptr; }; #endif // CORE_FPDFAPI_PAGE_CPDF_IMAGE_H_ diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 60c9b527a3..7f20b227b6 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -706,8 +706,7 @@ CPDF_ImageObject* CPDF_StreamContentParser::AddImage(UniqueStream pStream) { auto pImageObj = pdfium::MakeUnique(); pImageObj->SetOwnedImage( - pdfium::MakeUnique(m_pDocument, pStream.release(), true)); - + pdfium::MakeUnique(m_pDocument, std::move(pStream))); return AddImageObject(std::move(pImageObj)); } diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 46e9d47667..8cfa0333bb 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h @@ -69,6 +69,7 @@ class CPDF_Array : public CPDF_Object { std::vector m_Objects; }; +using UniqueArray = std::unique_ptr>; inline CPDF_Array* ToArray(CPDF_Object* obj) { return obj ? obj->AsArray() : nullptr; @@ -78,4 +79,12 @@ inline const CPDF_Array* ToArray(const CPDF_Object* obj) { return obj ? obj->AsArray() : nullptr; } +inline UniqueArray ToArray(UniqueObject obj) { + CPDF_Array* pArray = ToArray(obj.get()); + if (!pArray) + return nullptr; + obj.release(); + return UniqueArray(pArray); +} + #endif // CORE_FPDFAPI_PARSER_CPDF_ARRAY_H_ diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index 9cf575ddd4..fb8200f78c 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h @@ -15,6 +15,7 @@ #include "core/fxcrt/cfx_weak_ptr.h" #include "core/fxcrt/fx_coordinates.h" #include "core/fxcrt/fx_string.h" +#include "third_party/base/ptr_util.h" class CPDF_IndirectObjectHolder; @@ -98,6 +99,9 @@ class CPDF_Dictionary : public CPDF_Object { std::map m_Map; }; +using UniqueDictionary = + std::unique_ptr>; + inline CPDF_Dictionary* ToDictionary(CPDF_Object* obj) { return obj ? obj->AsDictionary() : nullptr; } @@ -106,4 +110,12 @@ inline const CPDF_Dictionary* ToDictionary(const CPDF_Object* obj) { return obj ? obj->AsDictionary() : nullptr; } +inline UniqueDictionary ToDictionary(UniqueObject obj) { + CPDF_Dictionary* pDict = ToDictionary(obj.get()); + if (!pDict) + return nullptr; + obj.release(); + return UniqueDictionary(pDict); +} + #endif // CORE_FPDFAPI_PARSER_CPDF_DICTIONARY_H_ diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h index 3cf23188a7..c888605d72 100644 --- a/core/fpdfapi/parser/cpdf_object.h +++ b/core/fpdfapi/parser/cpdf_object.h @@ -118,4 +118,6 @@ class CPDF_Object { CPDF_Object(const CPDF_Object& src) {} }; +using UniqueObject = std::unique_ptr>; + #endif // CORE_FPDFAPI_PARSER_CPDF_OBJECT_H_ diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h index 575a9ebe0b..756eccfba1 100644 --- a/core/fpdfapi/parser/cpdf_stream.h +++ b/core/fpdfapi/parser/cpdf_stream.h @@ -58,7 +58,7 @@ class CPDF_Stream : public CPDF_Object { IFX_FileRead* m_pFile = nullptr; }; -using UniqueStream = std::unique_ptr>; +using UniqueStream = std::unique_ptr>; inline CPDF_Stream* ToStream(CPDF_Object* obj) { return obj ? obj->AsStream() : nullptr; @@ -68,4 +68,12 @@ inline const CPDF_Stream* ToStream(const CPDF_Object* obj) { return obj ? obj->AsStream() : nullptr; } +inline UniqueStream ToStream(UniqueObject obj) { + CPDF_Stream* pStream = ToStream(obj.get()); + if (!pStream) + return nullptr; + obj.release(); + return UniqueStream(pStream); +} + #endif // CORE_FPDFAPI_PARSER_CPDF_STREAM_H_ -- cgit v1.2.3