Convert to CFX_UnownedPtr, part 4.

Fix strange ownership issue in cpdf_type3char.cpp, and
describe the absolutely insane stuff happening there.

Change-Id: Iae70f9eca8f125ed3ef677729f1776ba9f10183c
Reviewed-on: https://pdfium-review.googlesource.com/5830
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

11 files changed, 61 insertions, 44 deletions

diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index b318c7a7df..1c0cd69a4b 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -513,7 +513,7 @@ int32_t CPDF_Creator::WriteDoc_Stage2() {
     if (m_pEncryptDict && m_pEncryptDict->IsInline()) {
       m_dwLastObjNum += 1;
       FX_FILESIZE saveOffset = m_Archive->CurrentOffset();
-      if (!WriteIndirectObj(m_dwLastObjNum, m_pEncryptDict))
+      if (!WriteIndirectObj(m_dwLastObjNum, m_pEncryptDict.Get()))
         return -1;
 
       m_ObjectOffsets[m_dwLastObjNum] = saveOffset;
@@ -819,10 +819,10 @@ void CPDF_Creator::InitID() {
       CFX_ByteString user_pass = m_pParser->GetPassword();
       uint32_t flag = PDF_ENCRYPT_CONTENT;
       CPDF_SecurityHandler handler;
-      handler.OnCreate(m_pEncryptDict, m_pIDArray.get(), user_pass.raw_str(),
-                       user_pass.GetLength(), flag);
+      handler.OnCreate(m_pEncryptDict.Get(), m_pIDArray.get(),
+                       user_pass.raw_str(), user_pass.GetLength(), flag);
       m_pCryptoHandler = pdfium::MakeRetain<CPDF_CryptoHandler>();
-      m_pCryptoHandler->Init(m_pEncryptDict, &handler);
+      m_pCryptoHandler->Init(m_pEncryptDict.Get(), &handler);
       m_bSecurityChanged = true;
     }
   }
diff --git a/core/fpdfapi/edit/cpdf_creator.h b/core/fpdfapi/edit/cpdf_creator.h
index 618fffde79..f141c3880a 100644
--- a/core/fpdfapi/edit/cpdf_creator.h
+++ b/core/fpdfapi/edit/cpdf_creator.h
@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "core/fxcrt/cfx_retain_ptr.h"
+#include "core/fxcrt/cfx_unowned_ptr.h"
 #include "core/fxcrt/fx_basic.h"
 
 class CPDF_Array;
@@ -40,9 +41,9 @@ class CPDF_Creator {
   uint32_t GetNextObjectNumber() { return ++m_dwLastObjNum; }
   uint32_t GetLastObjectNumber() const { return m_dwLastObjNum; }
   CPDF_CryptoHandler* GetCryptoHandler() { return m_pCryptoHandler.Get(); }
-  CPDF_Document* GetDocument() const { return m_pDocument; }
+  CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
   CPDF_Array* GetIDArray() const { return m_pIDArray.get(); }
-  CPDF_Dictionary* GetEncryptDict() const { return m_pEncryptDict; }
+  CPDF_Dictionary* GetEncryptDict() const { return m_pEncryptDict.Get(); }
   uint32_t GetEncryptObjectNumber() const { return m_dwEncryptObjNum; }
 
   uint32_t GetObjectOffset(uint32_t objnum) { return m_ObjectOffsets[objnum]; }
@@ -79,13 +80,13 @@ class CPDF_Creator {
 
   bool IsXRefNeedEnd();
 
-  CPDF_Document* const m_pDocument;
-  CPDF_Parser* const m_pParser;
+  CFX_UnownedPtr<CPDF_Document> const m_pDocument;
+  CFX_UnownedPtr<CPDF_Parser> const m_pParser;
   bool m_bSecurityChanged;
-  CPDF_Dictionary* m_pEncryptDict;
+  CFX_UnownedPtr<CPDF_Dictionary> m_pEncryptDict;
   uint32_t m_dwEncryptObjNum;
   CFX_RetainPtr<CPDF_CryptoHandler> m_pCryptoHandler;
-  CPDF_Object* m_pMetadata;
+  CFX_UnownedPtr<CPDF_Object> m_pMetadata;
   uint32_t m_dwLastObjNum;
   std::unique_ptr<IFX_ArchiveStream> m_Archive;
   FX_FILESIZE m_SavedOffset;
diff --git a/core/fpdfapi/font/cpdf_cidfont.h b/core/fpdfapi/font/cpdf_cidfont.h
index 0fd5e63f1c..07982a418f 100644
--- a/core/fpdfapi/font/cpdf_cidfont.h
+++ b/core/fpdfapi/font/cpdf_cidfont.h
@@ -12,6 +12,7 @@
 
 #include "core/fpdfapi/font/cpdf_font.h"
 #include "core/fxcrt/cfx_retain_ptr.h"
+#include "core/fxcrt/cfx_unowned_ptr.h"
 #include "core/fxcrt/fx_string.h"
 #include "core/fxcrt/fx_system.h"
 
@@ -73,7 +74,7 @@ class CPDF_CIDFont : public CPDF_Font {
   wchar_t GetUnicodeFromCharCode(uint32_t charcode) const;
 
   CFX_RetainPtr<CPDF_CMap> m_pCMap;
-  CPDF_CID2UnicodeMap* m_pCID2UnicodeMap;
+  CFX_UnownedPtr<CPDF_CID2UnicodeMap> m_pCID2UnicodeMap;
   CIDSet m_Charset;
   bool m_bType1;
   bool m_bCIDIsGID;
diff --git a/core/fpdfapi/font/cpdf_type3char.cpp b/core/fpdfapi/font/cpdf_type3char.cpp
index d9794c27d3..e11193fdaa 100644
--- a/core/fpdfapi/font/cpdf_type3char.cpp
+++ b/core/fpdfapi/font/cpdf_type3char.cpp
@@ -31,10 +31,21 @@ bool CPDF_Type3Char::LoadBitmap(CPDF_RenderContext* pContext) {
     return false;
 
   m_ImageMatrix = pPageObj->AsImage()->matrix();
-  CFX_RetainPtr<CFX_DIBSource> pSource =
-      pPageObj->AsImage()->GetImage()->LoadDIBSource();
-  if (pSource)
-    m_pBitmap = pSource->Clone(nullptr);
+  {
+    // |pSource| actually gets assigned a CPDF_DIBSource, which has pointers
+    // into objects owned by |m_pForm|. Make sure it is out of scope before
+    // clearing the form.
+    CFX_RetainPtr<CFX_DIBSource> pSource =
+        pPageObj->AsImage()->GetImage()->LoadDIBSource();
+
+    // Clone() is non-virtual, and can't be overloaded by CPDF_DIBSource to
+    // return a clone of the subclass as one would typically expect from a
+    // such a method. Instead, it only clones the CFX_DIBSource, none of whose
+    // members point to objects owned by the form. As a result, |m_pBitmap|
+    // may outlive |m_pForm|.
+    if (pSource)
+      m_pBitmap = pSource->Clone(nullptr);
+  }
   m_pForm.reset();
   return true;
 }
diff --git a/core/fpdfapi/render/cpdf_devicebuffer.cpp b/core/fpdfapi/render/cpdf_devicebuffer.cpp
index be87128a49..8125ea5df1 100644
--- a/core/fpdfapi/render/cpdf_devicebuffer.cpp
+++ b/core/fpdfapi/render/cpdf_devicebuffer.cpp
@@ -14,8 +14,7 @@
 #include "core/fxge/fx_dib.h"
 #include "third_party/base/ptr_util.h"
 
-CPDF_DeviceBuffer::CPDF_DeviceBuffer()
-    : m_pDevice(nullptr), m_pContext(nullptr), m_pObject(nullptr) {}
+CPDF_DeviceBuffer::CPDF_DeviceBuffer() {}
 
 CPDF_DeviceBuffer::~CPDF_DeviceBuffer() {}
 
@@ -68,7 +67,7 @@ void CPDF_DeviceBuffer::OutputToDevice() {
   auto pBuffer = pdfium::MakeRetain<CFX_DIBitmap>();
   m_pDevice->CreateCompatibleBitmap(pBuffer, m_pBitmap->GetWidth(),
                                     m_pBitmap->GetHeight());
-  m_pContext->GetBackground(pBuffer, m_pObject, nullptr, &m_Matrix);
+  m_pContext->GetBackground(pBuffer, m_pObject.Get(), nullptr, &m_Matrix);
   pBuffer->CompositeBitmap(0, 0, pBuffer->GetWidth(), pBuffer->GetHeight(),
                            m_pBitmap, 0, 0);
   m_pDevice->StretchDIBits(pBuffer, m_Rect.left, m_Rect.top, m_Rect.Width(),
diff --git a/core/fpdfapi/render/cpdf_devicebuffer.h b/core/fpdfapi/render/cpdf_devicebuffer.h
index 61cdc9776b..bfab27503b 100644
--- a/core/fpdfapi/render/cpdf_devicebuffer.h
+++ b/core/fpdfapi/render/cpdf_devicebuffer.h
@@ -9,6 +9,8 @@
 
 #include <memory>
 
+#include "core/fxcrt/cfx_retain_ptr.h"
+#include "core/fxcrt/cfx_unowned_ptr.h"
 #include "core/fxcrt/fx_coordinates.h"
 
 class CFX_DIBitmap;
@@ -20,6 +22,7 @@ class CPDF_DeviceBuffer {
  public:
   CPDF_DeviceBuffer();
   ~CPDF_DeviceBuffer();
+
   bool Initialize(CPDF_RenderContext* pContext,
                   CFX_RenderDevice* pDevice,
                   FX_RECT* pRect,
@@ -30,11 +33,11 @@ class CPDF_DeviceBuffer {
   const CFX_Matrix* GetMatrix() const { return &m_Matrix; }
 
  private:
-  CFX_RenderDevice* m_pDevice;
-  CPDF_RenderContext* m_pContext;
-  FX_RECT m_Rect;
-  const CPDF_PageObject* m_pObject;
+  CFX_UnownedPtr<CFX_RenderDevice> m_pDevice;
+  CFX_UnownedPtr<CPDF_RenderContext> m_pContext;
+  CFX_UnownedPtr<const CPDF_PageObject> m_pObject;
   CFX_RetainPtr<CFX_DIBitmap> m_pBitmap;
+  FX_RECT m_Rect;
   CFX_Matrix m_Matrix;
 };
 
diff --git a/core/fpdfapi/render/cpdf_dibsource.cpp b/core/fpdfapi/render/cpdf_dibsource.cpp
index 3bb91ecf1b..de4ee7401f 100644
--- a/core/fpdfapi/render/cpdf_dibsource.cpp
+++ b/core/fpdfapi/render/cpdf_dibsource.cpp
@@ -752,8 +752,8 @@ CFX_RetainPtr<CPDF_DIBSource> CPDF_DIBSource::DetachMask() {
 
 int CPDF_DIBSource::StartLoadMaskDIB() {
   m_pMask = pdfium::MakeRetain<CPDF_DIBSource>();
-  int ret = m_pMask->StartLoadDIBSource(m_pDocument, m_pMaskStream, false,
-                                        nullptr, nullptr, true);
+  int ret = m_pMask->StartLoadDIBSource(m_pDocument.Get(), m_pMaskStream.Get(),
+                                        false, nullptr, nullptr, true);
   if (ret == 2) {
     if (m_Status == 0)
       m_Status = 2;
diff --git a/core/fpdfapi/render/cpdf_dibsource.h b/core/fpdfapi/render/cpdf_dibsource.h
index d0730cc4fd..50e9a038e8 100644
--- a/core/fpdfapi/render/cpdf_dibsource.h
+++ b/core/fpdfapi/render/cpdf_dibsource.h
@@ -18,6 +18,8 @@
 #include "core/fpdfapi/render/cpdf_imageloader.h"
 #include "core/fpdfapi/render/cpdf_rendercontext.h"
 #include "core/fpdfapi/render/cpdf_renderoptions.h"
+#include "core/fxcrt/cfx_retain_ptr.h"
+#include "core/fxcrt/cfx_unowned_ptr.h"
 #include "core/fxge/cfx_defaultrenderdevice.h"
 #include "core/fxge/cfx_renderdevice.h"
 
@@ -119,10 +121,10 @@ class CPDF_DIBSource : public CFX_DIBSource {
                                int clip_width) const;
   bool TransMask() const;
 
-  CPDF_Document* m_pDocument;
-  const CPDF_Stream* m_pStream;
+  CFX_UnownedPtr<CPDF_Document> m_pDocument;
+  CFX_UnownedPtr<const CPDF_Stream> m_pStream;
+  CFX_UnownedPtr<const CPDF_Dictionary> m_pDict;
   CFX_RetainPtr<CPDF_StreamAcc> m_pStreamAcc;
-  const CPDF_Dictionary* m_pDict;
   CPDF_ColorSpace* m_pColorSpace;
   uint32_t m_Family;
   uint32_t m_bpc;
@@ -145,7 +147,7 @@ class CPDF_DIBSource : public CFX_DIBSource {
   CFX_RetainPtr<CPDF_StreamAcc> m_pGlobalStream;
   std::unique_ptr<CCodec_ScanlineDecoder> m_pDecoder;
   std::unique_ptr<CCodec_Jbig2Context> m_pJbig2Context;
-  CPDF_Stream* m_pMaskStream;
+  CFX_UnownedPtr<CPDF_Stream> m_pMaskStream;
   int m_Status;
 };
 
diff --git a/core/fpdfapi/render/cpdf_imagerenderer.cpp b/core/fpdfapi/render/cpdf_imagerenderer.cpp
index 70371d847b..ff4ed9c7d9 100644
--- a/core/fpdfapi/render/cpdf_imagerenderer.cpp
+++ b/core/fpdfapi/render/cpdf_imagerenderer.cpp
@@ -59,10 +59,10 @@ bool CPDF_ImageRenderer::StartLoadDIBSource() {
   if (!image_rect.Valid())
     return false;
 
-  if (m_Loader.Start(m_pImageObject,
+  if (m_Loader.Start(m_pImageObject.Get(),
                      m_pRenderStatus->m_pContext->GetPageCache(), m_bStdCS,
                      m_pRenderStatus->m_GroupFamily,
-                     m_pRenderStatus->m_bLoadMask, m_pRenderStatus)) {
+                     m_pRenderStatus->m_bLoadMask, m_pRenderStatus.Get())) {
     m_Status = 4;
     return true;
   }
@@ -102,7 +102,7 @@ bool CPDF_ImageRenderer::StartRenderDIBSource() {
       if (m_pPattern)
         m_bPatternColor = true;
     }
-    m_FillArgb = m_pRenderStatus->GetFillArgb(m_pImageObject);
+    m_FillArgb = m_pRenderStatus->GetFillArgb(m_pImageObject.Get());
   } else if (m_pRenderStatus->m_Options.m_ColorMode == RENDER_COLOR_GRAY) {
     m_pClone = m_pDIBSource->Clone(nullptr);
     m_pClone->ConvertColorScale(m_pRenderStatus->m_Options.m_BackColor,
@@ -127,7 +127,7 @@ bool CPDF_ImageRenderer::StartRenderDIBSource() {
     return DrawMaskedImage();
 
   if (m_bPatternColor)
-    return DrawPatternImage(m_pObj2Device);
+    return DrawPatternImage(m_pObj2Device.Get());
 
   if (m_BitmapAlpha != 255 || !state.HasRef() || !state.GetFillOP() ||
       state.GetOPMode() != 0 || state.GetBlendType() != FXDIB_BLEND_NORMAL ||
@@ -284,11 +284,11 @@ bool CPDF_ImageRenderer::DrawPatternImage(const CFX_Matrix* pObj2Device) {
   patternDevice.Translate(static_cast<float>(-rect.left),
                           static_cast<float>(-rect.top));
   if (CPDF_TilingPattern* pTilingPattern = m_pPattern->AsTilingPattern()) {
-    bitmap_render.DrawTilingPattern(pTilingPattern, m_pImageObject,
+    bitmap_render.DrawTilingPattern(pTilingPattern, m_pImageObject.Get(),
                                     &patternDevice, false);
   } else if (CPDF_ShadingPattern* pShadingPattern =
                  m_pPattern->AsShadingPattern()) {
-    bitmap_render.DrawShadingPattern(pShadingPattern, m_pImageObject,
+    bitmap_render.DrawShadingPattern(pShadingPattern, m_pImageObject.Get(),
                                      &patternDevice, false);
   }
 
@@ -539,7 +539,7 @@ bool CPDF_ImageRenderer::Continue(IFX_Pause* pPause) {
                                                       pPause);
 
   if (m_Status == 4) {
-    if (m_Loader.Continue(pPause, m_pRenderStatus))
+    if (m_Loader.Continue(pPause, m_pRenderStatus.Get()))
       return true;
 
     if (StartRenderDIBSource())
diff --git a/core/fpdfapi/render/cpdf_imagerenderer.h b/core/fpdfapi/render/cpdf_imagerenderer.h
index 4468b470d9..bf73098cde 100644
--- a/core/fpdfapi/render/cpdf_imagerenderer.h
+++ b/core/fpdfapi/render/cpdf_imagerenderer.h
@@ -62,10 +62,10 @@ class CPDF_ImageRenderer {
                           const FX_RECT& rect) const;
   void HandleFilters();
 
-  CPDF_RenderStatus* m_pRenderStatus;
-  CPDF_ImageObject* m_pImageObject;
+  CFX_UnownedPtr<CPDF_RenderStatus> m_pRenderStatus;
+  CFX_UnownedPtr<CPDF_ImageObject> m_pImageObject;
   int m_Status;
-  const CFX_Matrix* m_pObj2Device;
+  CFX_UnownedPtr<const CFX_Matrix> m_pObj2Device;
   CFX_Matrix m_ImageMatrix;
   CPDF_ImageLoader m_Loader;
   CFX_RetainPtr<CFX_DIBSource> m_pDIBSource;
diff --git a/core/fpdfapi/render/cpdf_rendercontext.h b/core/fpdfapi/render/cpdf_rendercontext.h
index 0cce1ae77e..4980d37ed6 100644
--- a/core/fpdfapi/render/cpdf_rendercontext.h
+++ b/core/fpdfapi/render/cpdf_rendercontext.h
@@ -55,14 +55,14 @@ class CPDF_RenderContext {
   size_t CountLayers() const { return m_Layers.size(); }
   Layer* GetLayer(uint32_t index) { return &m_Layers[index]; }
 
-  CPDF_Document* GetDocument() const { return m_pDocument; }
-  CPDF_Dictionary* GetPageResources() const { return m_pPageResources; }
-  CPDF_PageRenderCache* GetPageCache() const { return m_pPageCache; }
+  CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
+  CPDF_Dictionary* GetPageResources() const { return m_pPageResources.Get(); }
+  CPDF_PageRenderCache* GetPageCache() const { return m_pPageCache.Get(); }
 
  protected:
-  CPDF_Document* const m_pDocument;
-  CPDF_Dictionary* m_pPageResources;
-  CPDF_PageRenderCache* m_pPageCache;
+  CFX_UnownedPtr<CPDF_Document> const m_pDocument;
+  CFX_UnownedPtr<CPDF_Dictionary> m_pPageResources;
+  CFX_UnownedPtr<CPDF_PageRenderCache> m_pPageCache;
   std::vector<Layer> m_Layers;
 };