Catch stray Retains() and Releases() outside of RetainPtr<>.

The previous CLs made the code clean, so now we can mark more things
private, and add friends as appropriate.

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

6 files changed, 60 insertions, 27 deletions

diff --git a/core/fxcrt/cfx_retain_ptr.h b/core/fxcrt/cfx_retain_ptr.h
index 8c7bf289a7..bff1b9691c 100644
--- a/core/fxcrt/cfx_retain_ptr.h
+++ b/core/fxcrt/cfx_retain_ptr.h
@@ -60,15 +60,24 @@ class CFX_RetainPtr {
 // Trivial implementation - internal ref count with virtual destructor.
 class CFX_Retainable {
  public:
+  bool HasOneRef() const { return m_nRefCount == 1; }
+
+ protected:
+  virtual ~CFX_Retainable() {}
+
+ private:
+  template <typename U>
+  friend struct ReleaseDeleter;
+
+  template <typename U>
+  friend class CFX_RetainPtr;
+
   void Retain() { ++m_nRefCount; }
   void Release() {
     if (--m_nRefCount == 0)
       delete this;
   }
-  bool HasOneRef() const { return m_nRefCount == 1; }
 
- protected:
-  virtual ~CFX_Retainable() {}
   intptr_t m_nRefCount = 0;
 };
 
@@ -76,6 +85,8 @@ namespace pdfium {
 
 // Helper to make a CFX_RetainPtr along the lines of std::make_unique<>(),
 // or pdfium::MakeUnique<>(). Arguments are forwarded to T's constructor.
+// Classes managed by CFX_RetainPtr should have protected (or private)
+// constructors, and should friend this function.
 template <typename T, typename... Args>
 CFX_RetainPtr<T> MakeRetain(Args&&... args) {
   return CFX_RetainPtr<T>(new T(std::forward<Args>(args)...));
diff --git a/core/fxcrt/fx_extension.cpp b/core/fxcrt/fx_extension.cpp
index cbbb86f26b..630d4da2aa 100644
--- a/core/fxcrt/fx_extension.cpp
+++ b/core/fxcrt/fx_extension.cpp
@@ -74,8 +74,8 @@ bool CFX_CRTFileAccess::Init(const CFX_WideStringC& wsPath) {
 
 class CFX_CRTFileStream final : public IFX_SeekableStream {
  public:
-  explicit CFX_CRTFileStream(std::unique_ptr<IFXCRT_FileAccess> pFA);
-  ~CFX_CRTFileStream() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFX_SeekableStream:
   FX_FILESIZE GetSize() override;
@@ -87,6 +87,9 @@ class CFX_CRTFileStream final : public IFX_SeekableStream {
   bool Flush() override;
 
  private:
+  explicit CFX_CRTFileStream(std::unique_ptr<IFXCRT_FileAccess> pFA);
+  ~CFX_CRTFileStream() override;
+
   std::unique_ptr<IFXCRT_FileAccess> m_pFile;
 };
 
@@ -133,9 +136,8 @@ bool CFX_CRTFileStream::Flush() {
 
 class CFX_MemoryStream final : public IFX_MemoryStream {
  public:
-  explicit CFX_MemoryStream(bool bConsecutive);
-  CFX_MemoryStream(uint8_t* pBuffer, size_t nSize, bool bTakeOver);
-  ~CFX_MemoryStream() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFX_MemoryStream
   FX_FILESIZE GetSize() override;
@@ -154,6 +156,10 @@ class CFX_MemoryStream final : public IFX_MemoryStream {
   void DetachBuffer() override;
 
  private:
+  explicit CFX_MemoryStream(bool bConsecutive);
+  CFX_MemoryStream(uint8_t* pBuffer, size_t nSize, bool bTakeOver);
+  ~CFX_MemoryStream() override;
+
   bool ExpandBlocks(size_t size);
 
   CFX_ArrayTemplate<uint8_t*> m_Blocks;
@@ -396,8 +402,7 @@ CFX_RetainPtr<IFX_SeekableStream> IFX_SeekableStream::CreateFromFilename(
   std::unique_ptr<IFXCRT_FileAccess> pFA(IFXCRT_FileAccess::Create());
   if (!pFA->Open(filename, dwModes))
     return nullptr;
-  return CFX_RetainPtr<IFX_SeekableStream>(
-      new CFX_CRTFileStream(std::move(pFA)));
+  return pdfium::MakeRetain<CFX_CRTFileStream>(std::move(pFA));
 }
 
 // static
@@ -407,8 +412,7 @@ CFX_RetainPtr<IFX_SeekableStream> IFX_SeekableStream::CreateFromFilename(
   std::unique_ptr<IFXCRT_FileAccess> pFA(IFXCRT_FileAccess::Create());
   if (!pFA->Open(filename, dwModes))
     return nullptr;
-  return CFX_RetainPtr<IFX_SeekableStream>(
-      new CFX_CRTFileStream(std::move(pFA)));
+  return pdfium::MakeRetain<CFX_CRTFileStream>(std::move(pFA));
 }
 
 // static
@@ -421,13 +425,12 @@ IFX_SeekableReadStream::CreateFromFilename(const FX_CHAR* filename) {
 CFX_RetainPtr<IFX_MemoryStream> IFX_MemoryStream::Create(uint8_t* pBuffer,
                                                          size_t dwSize,
                                                          bool bTakeOver) {
-  return CFX_RetainPtr<IFX_MemoryStream>(
-      new CFX_MemoryStream(pBuffer, dwSize, bTakeOver));
+  return pdfium::MakeRetain<CFX_MemoryStream>(pBuffer, dwSize, bTakeOver);
 }
 
 // static
 CFX_RetainPtr<IFX_MemoryStream> IFX_MemoryStream::Create(bool bConsecutive) {
-  return CFX_RetainPtr<IFX_MemoryStream>(new CFX_MemoryStream(bConsecutive));
+  return pdfium::MakeRetain<CFX_MemoryStream>(bConsecutive);
 }
 
 FX_FLOAT FXSYS_tan(FX_FLOAT a) {
diff --git a/core/fxcrt/fx_xml_parser.cpp b/core/fxcrt/fx_xml_parser.cpp
index ab30926387..1b562c2083 100644
--- a/core/fxcrt/fx_xml_parser.cpp
+++ b/core/fxcrt/fx_xml_parser.cpp
@@ -70,8 +70,8 @@ bool g_FXCRT_XML_IsNameChar(uint8_t ch) {
 
 class CXML_DataBufAcc : public IFX_BufferedReadStream {
  public:
-  CXML_DataBufAcc(const uint8_t* pBuffer, size_t size);
-  ~CXML_DataBufAcc() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFX_BufferedReadStream
   bool IsEOF() override;
@@ -83,6 +83,9 @@ class CXML_DataBufAcc : public IFX_BufferedReadStream {
   FX_FILESIZE GetBlockOffset() override;
 
  private:
+  CXML_DataBufAcc(const uint8_t* pBuffer, size_t size);
+  ~CXML_DataBufAcc() override;
+
   const uint8_t* m_pBuffer;
   size_t m_dwSize;
   size_t m_dwCurPos;
@@ -130,9 +133,8 @@ FX_FILESIZE CXML_DataBufAcc::GetBlockOffset() {
 
 class CXML_DataStmAcc : public IFX_BufferedReadStream {
  public:
-  explicit CXML_DataStmAcc(
-      const CFX_RetainPtr<IFX_SeekableReadStream>& pFileRead);
-  ~CXML_DataStmAcc() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFX_BufferedReadStream
   bool IsEOF() override;
@@ -144,6 +146,10 @@ class CXML_DataStmAcc : public IFX_BufferedReadStream {
   FX_FILESIZE GetBlockOffset() override;
 
  private:
+  explicit CXML_DataStmAcc(
+      const CFX_RetainPtr<IFX_SeekableReadStream>& pFileRead);
+  ~CXML_DataStmAcc() override;
+
   CFX_RetainPtr<IFX_SeekableReadStream> m_pFileRead;
   uint8_t* m_pBuffer;
   FX_FILESIZE m_nStart;
@@ -215,12 +221,12 @@ CXML_Parser::CXML_Parser()
 CXML_Parser::~CXML_Parser() {}
 
 bool CXML_Parser::Init(uint8_t* pBuffer, size_t size) {
-  m_pDataAcc.Reset(new CXML_DataBufAcc(pBuffer, size));
+  m_pDataAcc = pdfium::MakeRetain<CXML_DataBufAcc>(pBuffer, size);
   return Init();
 }
 
 bool CXML_Parser::Init(const CFX_RetainPtr<IFX_SeekableReadStream>& pFileRead) {
-  m_pDataAcc.Reset(new CXML_DataStmAcc(pFileRead));
+  m_pDataAcc = pdfium::MakeRetain<CXML_DataStmAcc>(pFileRead);
   return Init();
 }
 
diff --git a/xfa/fgas/crt/fgas_stream.cpp b/xfa/fgas/crt/fgas_stream.cpp
index 966f615676..424b191cb8 100644
--- a/xfa/fgas/crt/fgas_stream.cpp
+++ b/xfa/fgas/crt/fgas_stream.cpp
@@ -194,8 +194,8 @@ enum FX_STREAMTYPE {
 
 class CFGAS_Stream : public IFGAS_Stream {
  public:
-  CFGAS_Stream();
-  ~CFGAS_Stream() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   bool LoadFile(const FX_WCHAR* pszSrcFileName, uint32_t dwAccess);
   bool LoadBuffer(uint8_t* pData, int32_t iTotalSize, uint32_t dwAccess);
@@ -227,6 +227,9 @@ class CFGAS_Stream : public IFGAS_Stream {
                                                  int32_t iLength) override;
 
  protected:
+  CFGAS_Stream();
+  ~CFGAS_Stream() override;
+
   FX_STREAMTYPE m_eStreamType;
   IFGAS_StreamImp* m_pStreamImp;
   uint32_t m_dwAccess;
@@ -239,8 +242,8 @@ class CFGAS_Stream : public IFGAS_Stream {
 
 class CFGAS_TextStream : public IFGAS_Stream {
  public:
-  explicit CFGAS_TextStream(const CFX_RetainPtr<IFGAS_Stream>& pStream);
-  ~CFGAS_TextStream() override;
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFGAS_Stream
   uint32_t GetAccessModes() const override;
@@ -262,6 +265,9 @@ class CFGAS_TextStream : public IFGAS_Stream {
                                                  int32_t iLength) override;
 
  protected:
+  explicit CFGAS_TextStream(const CFX_RetainPtr<IFGAS_Stream>& pStream);
+  ~CFGAS_TextStream() override;
+
   void InitStream();
 
   uint16_t m_wCodePage;
diff --git a/xfa/fxfa/parser/cxfa_widetextread.cpp b/xfa/fxfa/parser/cxfa_widetextread.cpp
index 54442df7c0..34fa6b9c5e 100644
--- a/xfa/fxfa/parser/cxfa_widetextread.cpp
+++ b/xfa/fxfa/parser/cxfa_widetextread.cpp
@@ -14,6 +14,8 @@
 CXFA_WideTextRead::CXFA_WideTextRead(const CFX_WideString& wsBuffer)
     : m_wsBuffer(wsBuffer), m_iPosition(0) {}
 
+CXFA_WideTextRead::~CXFA_WideTextRead() {}
+
 uint32_t CXFA_WideTextRead::GetAccessModes() const {
   return FX_STREAMACCESS_Read | FX_STREAMACCESS_Text;
 }
diff --git a/xfa/fxfa/parser/cxfa_widetextread.h b/xfa/fxfa/parser/cxfa_widetextread.h
index d3d3b3792a..2ccb042908 100644
--- a/xfa/fxfa/parser/cxfa_widetextread.h
+++ b/xfa/fxfa/parser/cxfa_widetextread.h
@@ -7,11 +7,13 @@
 #ifndef XFA_FXFA_PARSER_CXFA_WIDETEXTREAD_H_
 #define XFA_FXFA_PARSER_CXFA_WIDETEXTREAD_H_
 
+#include "core/fxcrt/cfx_retain_ptr.h"
 #include "xfa/fgas/crt/fgas_stream.h"
 
 class CXFA_WideTextRead : public IFGAS_Stream {
  public:
-  explicit CXFA_WideTextRead(const CFX_WideString& wsBuffer);
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   // IFGAS_Stream
   uint32_t GetAccessModes() const override;
@@ -35,6 +37,9 @@ class CXFA_WideTextRead : public IFGAS_Stream {
   CFX_WideString GetSrcText() const;
 
  protected:
+  explicit CXFA_WideTextRead(const CFX_WideString& wsBuffer);
+  ~CXFA_WideTextRead() override;
+
   CFX_WideString m_wsBuffer;
   int32_t m_iPosition;
 };