Clean up CPDF_Stream.

Replace the CPDF_Stream(nullptr, 0, nullptr) pattern with
a default ctor.

Remove unused parameters from CPDF_Stream::SetData(). Both
are always passed as FALSE.

CPDF_Stream declared its own m_GenNum, which shadowed the one
in CPDF_Object. It was used only to distinguish file/memory
streams, so add a bool explicitly for this purpose.

Remove the union, it would be sad if we confused user data
with a C++ object with virtual function calls.

Use unique_ptrs with appropriate deleters to manage memory.

Review-Url: https://codereview.chromium.org/2347993002

5 files changed, 65 insertions, 99 deletions

diff --git a/core/fpdfapi/fpdf_edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/fpdf_edit/cpdf_pagecontentgenerator.cpp
index d3d55b91b5..7acd368683 100644
--- a/core/fpdfapi/fpdf_edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/fpdf_edit/cpdf_pagecontentgenerator.cpp
@@ -53,8 +53,8 @@ void CPDF_PageContentGenerator::GenerateContent() {
   if (pContent) {
     pPageDict->RemoveFor("Contents");
   }
-  CPDF_Stream* pStream = new CPDF_Stream(nullptr, 0, nullptr);
-  pStream->SetData(buf.GetBuffer(), buf.GetLength(), FALSE, FALSE);
+  CPDF_Stream* pStream = new CPDF_Stream;
+  pStream->SetData(buf.GetBuffer(), buf.GetLength());
   m_pDocument->AddIndirectObject(pStream);
   pPageDict->SetReferenceFor("Contents", m_pDocument, pStream->GetObjNum());
 }
@@ -109,21 +109,25 @@ void CPDF_PageContentGenerator::ProcessForm(CFX_ByteTextBuf& buf,
                                             const uint8_t* data,
                                             uint32_t size,
                                             CFX_Matrix& matrix) {
-  if (!data || !size) {
+  if (!data || !size)
     return;
-  }
-  CPDF_Stream* pStream = new CPDF_Stream(nullptr, 0, nullptr);
+
   CPDF_Dictionary* pFormDict = new CPDF_Dictionary;
   pFormDict->SetNameFor("Type", "XObject");
   pFormDict->SetNameFor("Subtype", "Form");
+
   CFX_FloatRect bbox = m_pPage->GetPageBBox();
   matrix.TransformRect(bbox);
   pFormDict->SetRectFor("BBox", bbox);
+
+  CPDF_Stream* pStream = new CPDF_Stream;
   pStream->InitStream(data, size, pFormDict);
   buf << "q " << matrix << " cm ";
+
   CFX_ByteString name = RealizeResource(pStream, "XObject");
   buf << "/" << PDF_NameEncode(name) << " Do Q\n";
 }
+
 void CPDF_PageContentGenerator::TransformContent(CFX_Matrix& matrix) {
   CPDF_Dictionary* pDict = m_pPage->m_pFormDict;
   CPDF_Object* pContent =
@@ -164,8 +168,8 @@ void CPDF_PageContentGenerator::TransformContent(CFX_Matrix& matrix) {
     contentStream.LoadAllData(pStream);
     ProcessForm(buf, contentStream.GetData(), contentStream.GetSize(), matrix);
   }
-  CPDF_Stream* pStream = new CPDF_Stream(nullptr, 0, nullptr);
-  pStream->SetData(buf.GetBuffer(), buf.GetLength(), FALSE, FALSE);
+  CPDF_Stream* pStream = new CPDF_Stream;
+  pStream->SetData(buf.GetBuffer(), buf.GetLength());
   m_pDocument->AddIndirectObject(pStream);
   m_pPage->m_pFormDict->SetReferenceFor("Contents", m_pDocument,
                                         pStream->GetObjNum());
diff --git a/core/fpdfapi/fpdf_page/cpdf_image.cpp b/core/fpdfapi/fpdf_page/cpdf_image.cpp
index 02e492b27d..32b69bf9cb 100644
--- a/core/fpdfapi/fpdf_page/cpdf_image.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_image.cpp
@@ -116,7 +116,7 @@ CPDF_Dictionary* CPDF_Image::InitJPEG(uint8_t* pData, uint32_t size) {
   m_Width = width;
   m_Height = height;
   if (!m_pStream)
-    m_pStream = new CPDF_Stream(nullptr, 0, nullptr);
+    m_pStream = new CPDF_Stream;
   return pDict;
 }
 
@@ -329,9 +329,9 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
       dest_offset = 0;
     }
   }
-  if (!m_pStream) {
-    m_pStream = new CPDF_Stream(nullptr, 0, nullptr);
-  }
+  if (!m_pStream)
+    m_pStream = new CPDF_Stream;
+
   m_pStream->InitStream(dest_buf, dest_size, pDict);
   m_bIsMask = pBitmap->IsAlphaMask();
   m_Width = BitmapWidth;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp
index e25180bedb..cc584b78b3 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_stream.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_stream.cpp
@@ -9,29 +9,24 @@
 #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h"
 #include "core/fpdfapi/fpdf_parser/include/fpdf_parser_decode.h"
+#include "third_party/base/numerics/safe_conversions.h"
 #include "third_party/base/stl_util.h"
 
+CPDF_Stream::CPDF_Stream() {}
+
 CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict)
     : m_pDict(pDict),
       m_dwSize(size),
-      m_GenNum(kMemoryBasedGenNum),
       m_pDataBuf(pData) {}
 
-CPDF_Stream::~CPDF_Stream() {
-  m_ObjNum = kInvalidObjNum;
-  if (IsMemoryBased())
-    FX_Free(m_pDataBuf);
-
-  if (m_pDict)
-    m_pDict->Release();
-}
+CPDF_Stream::~CPDF_Stream() {}
 
 CPDF_Object::Type CPDF_Stream::GetType() const {
   return STREAM;
 }
 
 CPDF_Dictionary* CPDF_Stream::GetDict() const {
-  return m_pDict;
+  return m_pDict.get();
 }
 
 bool CPDF_Stream::IsStream() const {
@@ -46,31 +41,29 @@ const CPDF_Stream* CPDF_Stream::AsStream() const {
   return this;
 }
 
-void CPDF_Stream::InitStreamInternal(CPDF_Dictionary* pDict) {
-  if (pDict) {
-    if (m_pDict)
-      m_pDict->Release();
-    m_pDict = pDict;
-  }
-  if (IsMemoryBased())
-    FX_Free(m_pDataBuf);
-
-  m_GenNum = 0;
-  m_pFile = nullptr;
-}
-
 void CPDF_Stream::InitStream(const uint8_t* pData,
                              uint32_t size,
                              CPDF_Dictionary* pDict) {
-  InitStreamInternal(pDict);
-  m_GenNum = kMemoryBasedGenNum;
-  m_pDataBuf = FX_Alloc(uint8_t, size);
+  m_pDict.reset(pDict);
+  m_bMemoryBased = true;
+  m_pFile = nullptr;
+  m_pDataBuf.reset(FX_Alloc(uint8_t, size));
   if (pData)
-    FXSYS_memcpy(m_pDataBuf, pData, size);
-
+    FXSYS_memcpy(m_pDataBuf.get(), pData, size);
   m_dwSize = size;
   if (m_pDict)
-    m_pDict->SetIntegerFor("Length", size);
+    m_pDict->SetIntegerFor("Length", m_dwSize);
+}
+
+void CPDF_Stream::InitStreamFromFile(IFX_FileRead* pFile,
+                                     CPDF_Dictionary* pDict) {
+  m_pDict.reset(pDict);
+  m_bMemoryBased = false;
+  m_pDataBuf.reset();
+  m_pFile = pFile;
+  m_dwSize = pdfium::base::checked_cast<uint32_t>(pFile->GetSize());
+  if (m_pDict)
+    m_pDict->SetIntegerFor("Length", m_dwSize);
 }
 
 CPDF_Object* CPDF_Stream::Clone() const {
@@ -93,53 +86,31 @@ CPDF_Object* CPDF_Stream::CloneNonCyclic(
   return new CPDF_Stream(acc.DetachData(), streamSize, pDict);
 }
 
-void CPDF_Stream::SetData(const uint8_t* pData,
-                          uint32_t size,
-                          FX_BOOL bCompressed,
-                          FX_BOOL bKeepBuf) {
-  if (IsMemoryBased())
-    FX_Free(m_pDataBuf);
-  m_GenNum = kMemoryBasedGenNum;
-
-  if (bKeepBuf) {
-    m_pDataBuf = const_cast<uint8_t*>(pData);
-  } else {
-    m_pDataBuf = FX_Alloc(uint8_t, size);
-    if (pData) {
-      FXSYS_memcpy(m_pDataBuf, pData, size);
-    }
-  }
+void CPDF_Stream::SetData(const uint8_t* pData, uint32_t size) {
+  m_bMemoryBased = true;
+  m_pDataBuf.reset(FX_Alloc(uint8_t, size));
+  if (pData)
+    FXSYS_memcpy(m_pDataBuf.get(), pData, size);
   m_dwSize = size;
   if (!m_pDict)
-    m_pDict = new CPDF_Dictionary;
+    m_pDict.reset(new CPDF_Dictionary);
   m_pDict->SetIntegerFor("Length", size);
-  if (!bCompressed) {
-    m_pDict->RemoveFor("Filter");
-    m_pDict->RemoveFor("DecodeParms");
-  }
+  m_pDict->RemoveFor("Filter");
+  m_pDict->RemoveFor("DecodeParms");
 }
 
 FX_BOOL CPDF_Stream::ReadRawData(FX_FILESIZE offset,
                                  uint8_t* buf,
                                  uint32_t size) const {
-  if (!IsMemoryBased() && m_pFile)
+  if (m_bMemoryBased && m_pFile)
     return m_pFile->ReadBlock(buf, offset, size);
 
   if (m_pDataBuf)
-    FXSYS_memcpy(buf, m_pDataBuf + offset, size);
+    FXSYS_memcpy(buf, m_pDataBuf.get() + offset, size);
 
   return TRUE;
 }
 
-void CPDF_Stream::InitStreamFromFile(IFX_FileRead* pFile,
-                                     CPDF_Dictionary* pDict) {
-  InitStreamInternal(pDict);
-  m_pFile = pFile;
-  m_dwSize = (uint32_t)pFile->GetSize();
-  if (m_pDict)
-    m_pDict->SetIntegerFor("Length", m_dwSize);
-}
-
 CFX_WideString CPDF_Stream::GetUnicodeText() const {
   CPDF_StreamAcc stream;
   stream.LoadAllData(this, FALSE);
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_stream.h b/core/fpdfapi/fpdf_parser/include/cpdf_stream.h
index 7ea761ef51..b39b20a1e9 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_stream.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_stream.h
@@ -7,14 +7,18 @@
 #ifndef CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_STREAM_H_
 #define CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_STREAM_H_
 
+#include <memory>
 #include <set>
 
 #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_object.h"
-#include "core/fxcrt/include/fx_stream.h"
+#include "core/fxcrt/include/fx_basic.h"
 
 class CPDF_Stream : public CPDF_Object {
  public:
+  CPDF_Stream();
+
+  // Takes onwership of |pData| and |pDict|.
   CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict);
 
   // CPDF_Object.
@@ -27,12 +31,10 @@ class CPDF_Stream : public CPDF_Object {
   const CPDF_Stream* AsStream() const override;
 
   uint32_t GetRawSize() const { return m_dwSize; }
-  uint8_t* GetRawData() const { return m_pDataBuf; }
+  uint8_t* GetRawData() const { return m_pDataBuf.get(); }
 
-  void SetData(const uint8_t* pData,
-               uint32_t size,
-               FX_BOOL bCompressed,
-               FX_BOOL bKeepBuf);
+  // Does not takes onwership of |pData|, copies into internally-owned buffer.
+  void SetData(const uint8_t* pData, uint32_t size);
 
   void InitStream(const uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict);
   void InitStreamFromFile(IFX_FileRead* pFile, CPDF_Dictionary* pDict);
@@ -41,26 +43,19 @@ class CPDF_Stream : public CPDF_Object {
                       uint8_t* pBuf,
                       uint32_t buf_size) const;
 
-  bool IsMemoryBased() const { return m_GenNum == kMemoryBasedGenNum; }
+  bool IsMemoryBased() const { return m_bMemoryBased; }
 
  protected:
-  static const uint32_t kMemoryBasedGenNum = (uint32_t)-1;
-
   ~CPDF_Stream() override;
   CPDF_Object* CloneNonCyclic(
       bool bDirect,
       std::set<const CPDF_Object*>* pVisited) const override;
 
-  void InitStreamInternal(CPDF_Dictionary* pDict);
-
-  CPDF_Dictionary* m_pDict;
-  uint32_t m_dwSize;
-  uint32_t m_GenNum;
-
-  union {
-    uint8_t* m_pDataBuf;
-    IFX_FileRead* m_pFile;
-  };
+  std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> m_pDict;
+  bool m_bMemoryBased = true;
+  uint32_t m_dwSize = 0;
+  std::unique_ptr<uint8_t, FxFreeDeleter> m_pDataBuf;
+  IFX_FileRead* m_pFile = nullptr;
 };
 
 #endif  // CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_STREAM_H_
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp
index d37aaf3acb..1e531799e8 100644
--- a/core/fpdfdoc/cpvt_generateap.cpp
+++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -166,7 +166,7 @@ bool GenerateWidgetAP(CPDF_Document* pDoc,
   }
   CPDF_Stream* pNormalStream = pAPDict->GetStreamFor("N");
   if (!pNormalStream) {
-    pNormalStream = new CPDF_Stream(nullptr, 0, nullptr);
+    pNormalStream = new CPDF_Stream;
     int32_t objnum = pDoc->AddIndirectObject(pNormalStream);
     pAnnotDict->GetDictFor("AP")->SetReferenceFor("N", pDoc, objnum);
   }
@@ -418,8 +418,7 @@ bool GenerateWidgetAP(CPDF_Document* pDoc,
     } break;
   }
   if (pNormalStream) {
-    pNormalStream->SetData(sAppStream.GetBuffer(), sAppStream.GetSize(), FALSE,
-                           FALSE);
+    pNormalStream->SetData(sAppStream.GetBuffer(), sAppStream.GetSize());
     pStreamDict = pNormalStream->GetDict();
     if (pStreamDict) {
       pStreamDict->SetMatrixFor("Matrix", matrix);
@@ -591,12 +590,10 @@ void GenerateAndSetAPDict(CPDF_Document* pDoc,
   CPDF_Dictionary* pAPDict = new CPDF_Dictionary;
   pAnnotDict->SetFor("AP", pAPDict);
 
-  CPDF_Stream* pNormalStream = new CPDF_Stream(nullptr, 0, nullptr);
+  CPDF_Stream* pNormalStream = new CPDF_Stream;
   int32_t objnum = pDoc->AddIndirectObject(pNormalStream);
   pAnnotDict->GetDictFor("AP")->SetReferenceFor("N", pDoc, objnum);
-
-  pNormalStream->SetData(sAppStream.GetBuffer(), sAppStream.GetSize(), FALSE,
-                         FALSE);
+  pNormalStream->SetData(sAppStream.GetBuffer(), sAppStream.GetSize());
 
   CPDF_Dictionary* pStreamDict = pNormalStream->GetDict();
   pStreamDict->SetIntegerFor("FormType", 1);
@@ -607,7 +604,6 @@ void GenerateAndSetAPDict(CPDF_Document* pDoc,
                            ? CPDF_Annot::RectFromQuadPoints(pAnnotDict)
                            : pAnnotDict->GetRectFor("Rect");
   pStreamDict->SetRectFor("BBox", rect);
-
   pStreamDict->SetFor("Resources", pResourceDict);
 }