From 08f4b7762a4453818c76c680f5295986e21418ce Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 23 May 2017 17:21:01 -0700 Subject: 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 Commit-Queue: Tom Sepez --- core/fpdfapi/edit/cpdf_creator.cpp | 8 ++++---- core/fpdfapi/edit/cpdf_creator.h | 13 +++++++------ core/fpdfapi/font/cpdf_cidfont.h | 3 ++- core/fpdfapi/font/cpdf_type3char.cpp | 19 +++++++++++++++---- core/fpdfapi/render/cpdf_devicebuffer.cpp | 5 ++--- core/fpdfapi/render/cpdf_devicebuffer.h | 11 +++++++---- core/fpdfapi/render/cpdf_dibsource.cpp | 4 ++-- core/fpdfapi/render/cpdf_dibsource.h | 10 ++++++---- core/fpdfapi/render/cpdf_imagerenderer.cpp | 14 +++++++------- core/fpdfapi/render/cpdf_imagerenderer.h | 6 +++--- core/fpdfapi/render/cpdf_rendercontext.h | 12 ++++++------ 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(); - 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 #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 const m_pDocument; + CFX_UnownedPtr const m_pParser; bool m_bSecurityChanged; - CPDF_Dictionary* m_pEncryptDict; + CFX_UnownedPtr m_pEncryptDict; uint32_t m_dwEncryptObjNum; CFX_RetainPtr m_pCryptoHandler; - CPDF_Object* m_pMetadata; + CFX_UnownedPtr m_pMetadata; uint32_t m_dwLastObjNum; std::unique_ptr 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 m_pCMap; - CPDF_CID2UnicodeMap* m_pCID2UnicodeMap; + CFX_UnownedPtr 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 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 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(); 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 +#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 m_pDevice; + CFX_UnownedPtr m_pContext; + CFX_UnownedPtr m_pObject; CFX_RetainPtr 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::DetachMask() { int CPDF_DIBSource::StartLoadMaskDIB() { m_pMask = pdfium::MakeRetain(); - 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 m_pDocument; + CFX_UnownedPtr m_pStream; + CFX_UnownedPtr m_pDict; CFX_RetainPtr 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 m_pGlobalStream; std::unique_ptr m_pDecoder; std::unique_ptr m_pJbig2Context; - CPDF_Stream* m_pMaskStream; + CFX_UnownedPtr 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(-rect.left), static_cast(-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 m_pRenderStatus; + CFX_UnownedPtr m_pImageObject; int m_Status; - const CFX_Matrix* m_pObj2Device; + CFX_UnownedPtr m_pObj2Device; CFX_Matrix m_ImageMatrix; CPDF_ImageLoader m_Loader; CFX_RetainPtr 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 const m_pDocument; + CFX_UnownedPtr m_pPageResources; + CFX_UnownedPtr m_pPageCache; std::vector m_Layers; }; -- cgit v1.2.3