Make CPDF_Stream() take unique_ptr's to its dictionary.

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

8 files changed, 60 insertions, 39 deletions

diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index 01624cbc61..cabd2c391c 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -12,6 +12,7 @@
 #include "core/fpdfapi/page/cpdf_page.h"
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_document.h"
+#include "core/fpdfapi/parser/cpdf_name.h"
 #include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/fpdf_parser_decode.h"
diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp
index 60ac1c9815..7513403918 100644
--- a/core/fpdfapi/page/cpdf_image.cpp
+++ b/core/fpdfapi/page/cpdf_image.cpp
@@ -7,6 +7,7 @@
 #include "core/fpdfapi/page/cpdf_image.h"
 
 #include <algorithm>
+#include <memory>
 #include <utility>
 #include <vector>
 
@@ -19,11 +20,13 @@
 #include "core/fpdfapi/parser/cpdf_name.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
 #include "core/fpdfapi/parser/cpdf_reference.h"
+#include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_string.h"
 #include "core/fpdfapi/render/cpdf_pagerendercache.h"
 #include "core/fpdfapi/render/render_int.h"
 #include "core/fxcodec/fx_codec.h"
 #include "core/fxge/fx_dib.h"
+#include "third_party/base/ptr_util.h"
 
 CPDF_Image::CPDF_Image(CPDF_Document* pDoc) : m_pDocument(pDoc) {}
 
@@ -147,7 +150,7 @@ void CPDF_Image::SetJpegImage(IFX_SeekableReadStream* pFile) {
   if (!pDict)
     return;
 
-  m_pStream->InitStreamFromFile(pFile, pDict);
+  m_pStream->InitStreamFromFile(pFile, pdfium::WrapUnique(pDict));
 }
 
 void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
@@ -160,8 +163,8 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
   int32_t src_pitch = pBitmap->GetPitch();
   int32_t bpp = pBitmap->GetBPP();
 
-  CPDF_Dictionary* pDict =
-      new CPDF_Dictionary(m_pDocument->GetByteStringPool());
+  auto pDict =
+      pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
   pDict->SetNewFor<CPDF_Name>("Type", "XObject");
   pDict->SetNewFor<CPDF_Name>("Subtype", "Image");
   pDict->SetNewFor<CPDF_Number>("Width", BitmapWidth);
@@ -222,9 +225,10 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
         ptr[2] = (uint8_t)argb;
         ptr += 3;
       }
+      auto pNewDict =
+          pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
       CPDF_Stream* pCTS = m_pDocument->NewIndirect<CPDF_Stream>(
-          pColorTable, iPalette * 3,
-          new CPDF_Dictionary(m_pDocument->GetByteStringPool()));
+          pColorTable, iPalette * 3, std::move(pNewDict));
       pCS->AddNew<CPDF_Reference>(m_pDocument, pCTS->GetObjNum());
       pDict->SetNewFor<CPDF_Reference>("ColorSpace", m_pDocument,
                                        pCS->GetObjNum());
@@ -259,8 +263,8 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
     int32_t maskHeight = pMaskBitmap->GetHeight();
     uint8_t* mask_buf = nullptr;
     FX_STRSIZE mask_size = 0;
-    CPDF_Dictionary* pMaskDict =
-        new CPDF_Dictionary(m_pDocument->GetByteStringPool());
+    auto pMaskDict =
+        pdfium::MakeUnique<CPDF_Dictionary>(m_pDocument->GetByteStringPool());
     pMaskDict->SetNewFor<CPDF_Name>("Type", "XObject");
     pMaskDict->SetNewFor<CPDF_Name>("Subtype", "Image");
     pMaskDict->SetNewFor<CPDF_Number>("Width", maskWidth);
@@ -279,10 +283,10 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
       }
     }
     pMaskDict->SetNewFor<CPDF_Number>("Length", mask_size);
-    pDict->SetNewFor<CPDF_Reference>(
-        "SMask", m_pDocument,
-        m_pDocument->NewIndirect<CPDF_Stream>(mask_buf, mask_size, pMaskDict)
-            ->GetObjNum());
+    CPDF_Stream* pNewStream = m_pDocument->NewIndirect<CPDF_Stream>(
+        mask_buf, mask_size, std::move(pMaskDict));
+    pDict->SetNewFor<CPDF_Reference>("SMask", m_pDocument,
+                                     pNewStream->GetObjNum());
     if (bDeleteMask)
       delete pMaskBitmap;
   }
@@ -295,8 +299,6 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
         pNewBitmap->Copy(pBitmap);
         pNewBitmap->ConvertFormat(FXDIB_Rgb);
         SetImage(pNewBitmap, iCompress);
-        delete pDict;
-        pDict = nullptr;
         FX_Free(dest_buf);
         dest_buf = nullptr;
         dest_size = 0;
@@ -340,7 +342,7 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) {
     m_pOwnedStream = pdfium::MakeUnique<CPDF_Stream>();
     m_pStream = m_pOwnedStream.get();
   }
-  m_pStream->InitStream(dest_buf, dest_size, pDict);
+  m_pStream->InitStream(dest_buf, dest_size, std::move(pDict));
   m_bIsMask = pBitmap->IsAlphaMask();
   m_Width = BitmapWidth;
   m_Height = BitmapHeight;
diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index 6b4a362257..f271e58d78 100644
--- a/core/fpdfapi/page/cpdf_streamparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -233,7 +233,7 @@ CPDF_Stream* CPDF_StreamParser::ReadInlineStream(CPDF_Document* pDoc,
     m_Pos += dwStreamSize;
   }
   pDict->SetNewFor<CPDF_Number>("Length", (int)dwStreamSize);
-  return new CPDF_Stream(pData, dwStreamSize, pDict);
+  return new CPDF_Stream(pData, dwStreamSize, pdfium::WrapUnique(pDict));
 }
 
 CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() {
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index 86645934bf..303ad6fa7a 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -71,10 +71,12 @@ class PDFObjectsTest : public testing::Test {
     size_t buf_len = FX_ArraySize(content);
     uint8_t* buf = reinterpret_cast<uint8_t*>(malloc(buf_len));
     memcpy(buf, content, buf_len);
-    m_StreamDictObj = new CPDF_Dictionary();
+    auto pNewDict = pdfium::MakeUnique<CPDF_Dictionary>();
+    m_StreamDictObj = pNewDict.get();
     m_StreamDictObj->SetNewFor<CPDF_String>("key1", L" test dict");
     m_StreamDictObj->SetNewFor<CPDF_Number>("key2", -1);
-    CPDF_Stream* stream_obj = new CPDF_Stream(buf, buf_len, m_StreamDictObj);
+    CPDF_Stream* stream_obj =
+        new CPDF_Stream(buf, buf_len, std::move(pNewDict));
     // Null Object.
     CPDF_Null* null_obj = new CPDF_Null;
     // All direct objects.
@@ -588,7 +590,8 @@ TEST(PDFArrayTest, GetTypeAt) {
       size_t data_size = FX_ArraySize(content);
       uint8_t* data = reinterpret_cast<uint8_t*>(malloc(data_size));
       memcpy(data, content, data_size);
-      stream_vals[i] = arr->AddNew<CPDF_Stream>(data, data_size, vals[i]);
+      stream_vals[i] = arr->AddNew<CPDF_Stream>(data, data_size,
+                                                pdfium::WrapUnique(vals[i]));
     }
     for (size_t i = 0; i < 3; ++i) {
       TestArrayAccessors(arr.get(), i,     // Array and index.
@@ -633,8 +636,8 @@ TEST(PDFArrayTest, GetTypeAt) {
     size_t buf_size = sizeof(data);
     uint8_t* buf = reinterpret_cast<uint8_t*>(malloc(buf_size));
     memcpy(buf, data, buf_size);
-    CPDF_Stream* stream_val =
-        arr->InsertNewAt<CPDF_Stream>(13, buf, buf_size, stream_dict);
+    CPDF_Stream* stream_val = arr->InsertNewAt<CPDF_Stream>(
+        13, buf, buf_size, pdfium::WrapUnique(stream_dict));
     const char* const expected_str[] = {
         "true",          "false", "0",    "-1234", "2345", "0.05", "",
         "It is a test!", "NAME",  "test", "",      "",     "",     ""};
@@ -816,10 +819,12 @@ TEST(PDFObjectTest, CloneCheckLoop) {
     EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectFor("arr"));
   }
   {
-    // Create a dictionary/stream pair with a reference loop.
+    // Create a dictionary/stream pair with a reference loop. It takes
+    // some work to do this nowadays, in particular we need the
+    // anti-pattern pdfium::WrapUnique(dict.get()).
     auto dict_obj = pdfium::MakeUnique<CPDF_Dictionary>();
-    CPDF_Stream* stream_obj =
-        dict_obj->SetNewFor<CPDF_Stream>("stream", nullptr, 0, dict_obj.get());
+    CPDF_Stream* stream_obj = dict_obj->SetNewFor<CPDF_Stream>(
+        "stream", nullptr, 0, pdfium::WrapUnique(dict_obj.get()));
     // Clone this object to see whether stack overflow will be triggered.
     std::unique_ptr<CPDF_Stream> cloned_stream = ToStream(stream_obj->Clone());
     // Cloned object should be the same as the original.
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index fd7f08f688..7a54fcf8ff 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -6,6 +6,8 @@
 
 #include "core/fpdfapi/parser/cpdf_stream.h"
 
+#include <utility>
+
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
 #include "core/fpdfapi/parser/cpdf_stream_acc.h"
@@ -15,8 +17,10 @@
 
 CPDF_Stream::CPDF_Stream() {}
 
-CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict)
-    : m_dwSize(size), m_pDict(pDict), m_pDataBuf(pData) {}
+CPDF_Stream::CPDF_Stream(uint8_t* pData,
+                         uint32_t size,
+                         std::unique_ptr<CPDF_Dictionary> pDict)
+    : m_dwSize(size), m_pDict(std::move(pDict)), m_pDataBuf(pData) {}
 
 CPDF_Stream::~CPDF_Stream() {
   m_ObjNum = kInvalidObjNum;
@@ -46,8 +50,8 @@ const CPDF_Stream* CPDF_Stream::AsStream() const {
 
 void CPDF_Stream::InitStream(const uint8_t* pData,
                              uint32_t size,
-                             CPDF_Dictionary* pDict) {
-  m_pDict.reset(pDict);
+                             std::unique_ptr<CPDF_Dictionary> pDict) {
+  m_pDict = std::move(pDict);
   m_bMemoryBased = true;
   m_pFile = nullptr;
   m_pDataBuf.reset(FX_Alloc(uint8_t, size));
@@ -59,8 +63,8 @@ void CPDF_Stream::InitStream(const uint8_t* pData,
 }
 
 void CPDF_Stream::InitStreamFromFile(IFX_SeekableReadStream* pFile,
-                                     CPDF_Dictionary* pDict) {
-  m_pDict.reset(pDict);
+                                     std::unique_ptr<CPDF_Dictionary> pDict) {
+  m_pDict = std::move(pDict);
   m_bMemoryBased = false;
   m_pDataBuf.reset();
   m_pFile = pFile;
@@ -79,14 +83,16 @@ std::unique_ptr<CPDF_Object> CPDF_Stream::CloneNonCyclic(
   pVisited->insert(this);
   CPDF_StreamAcc acc;
   acc.LoadAllData(this, true);
+
   uint32_t streamSize = acc.GetSize();
   CPDF_Dictionary* pDict = GetDict();
+  std::unique_ptr<CPDF_Dictionary> pNewDict;
   if (pDict && !pdfium::ContainsKey(*pVisited, pDict)) {
-    pDict = ToDictionary(static_cast<CPDF_Object*>(pDict)
-                             ->CloneNonCyclic(bDirect, pVisited)
-                             .release());
+    pNewDict = ToDictionary(
+        static_cast<CPDF_Object*>(pDict)->CloneNonCyclic(bDirect, pVisited));
   }
-  return pdfium::MakeUnique<CPDF_Stream>(acc.DetachData(), streamSize, pDict);
+  return pdfium::MakeUnique<CPDF_Stream>(acc.DetachData(), streamSize,
+                                         std::move(pNewDict));
 }
 
 void CPDF_Stream::SetData(const uint8_t* pData, uint32_t size) {
diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h
index c4f9aacf2b..fb2b67cd82 100644
--- a/core/fpdfapi/parser/cpdf_stream.h
+++ b/core/fpdfapi/parser/cpdf_stream.h
@@ -18,8 +18,11 @@ class CPDF_Stream : public CPDF_Object {
  public:
   CPDF_Stream();
 
-  // Takes ownership of |pData| and |pDict|.
-  CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict);
+  // Takes ownership of |pData|.
+  CPDF_Stream(uint8_t* pData,
+              uint32_t size,
+              std::unique_ptr<CPDF_Dictionary> pDict);
+
   ~CPDF_Stream() override;
 
   // CPDF_Object:
@@ -37,9 +40,11 @@ class CPDF_Stream : public CPDF_Object {
   // 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 InitStream(const uint8_t* pData,
+                  uint32_t size,
+                  std::unique_ptr<CPDF_Dictionary> pDict);
   void InitStreamFromFile(IFX_SeekableReadStream* pFile,
-                          CPDF_Dictionary* pDict);
+                          std::unique_ptr<CPDF_Dictionary> pDict);
 
   bool ReadRawData(FX_FILESIZE start_pos,
                    uint8_t* pBuf,
diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
index 4a0fd3a408..17e4954f2a 100644
--- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -752,7 +752,8 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
     }
   }
 
-  auto pStream = pdfium::MakeUnique<CPDF_Stream>(pData, len, pDict);
+  auto pStream =
+      pdfium::MakeUnique<CPDF_Stream>(pData, len, pdfium::WrapUnique(pDict));
   streamStartPos = m_Pos;
   FXSYS_memset(m_WordBuffer, 0, kEndObjStr.GetLength() + 1);
   GetNextWordInternal(nullptr);
diff --git a/core/fxge/dib/fx_dib_engine_unittest.cpp b/core/fxge/dib/fx_dib_engine_unittest.cpp
index faacebd5fc..d7d66f0755 100644
--- a/core/fxge/dib/fx_dib_engine_unittest.cpp
+++ b/core/fxge/dib/fx_dib_engine_unittest.cpp
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <memory>
+#include <utility>
 
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
@@ -21,7 +22,7 @@ TEST(CStretchEngine, OverflowInCtor) {
   dict_obj->SetNewFor<CPDF_Number>("Width", 71000);
   dict_obj->SetNewFor<CPDF_Number>("Height", 12500);
   std::unique_ptr<CPDF_Stream> stream =
-      pdfium::MakeUnique<CPDF_Stream>(nullptr, 0, dict_obj.release());
+      pdfium::MakeUnique<CPDF_Stream>(nullptr, 0, std::move(dict_obj));
   CPDF_DIBSource dib_source;
   dib_source.Load(nullptr, stream.get(), nullptr, nullptr, nullptr, nullptr,
                   false, 0, false);