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

2 files changed, 46 insertions, 80 deletions

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_