Make CPDF_Image() constructors saner.chromium/2890

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

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<CPDF_ImageObject>();
   pImageObj->SetOwnedImage(
-      pdfium::MakeUnique<CPDF_Image>(m_pDocument, pStream.release(), true));
-
+      pdfium::MakeUnique<CPDF_Image>(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<CPDF_Object*> m_Objects;
 };
+using UniqueArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Object>>;
 
 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<CFX_ByteString, CPDF_Object*> m_Map;
 };
 
+using UniqueDictionary =
+    std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Object>>;
+
 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<CPDF_Object, ReleaseDeleter<CPDF_Object>>;
+
 #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<CPDF_Stream, ReleaseDeleter<CPDF_Stream>>;
+using UniqueStream = std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Object>>;
 
 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_