From 80f25a5a8135933a405349ffc798d13273b3d690 Mon Sep 17 00:00:00 2001 From: thestig Date: Thu, 19 May 2016 14:36:00 -0700 Subject: Fix leak in CPDF_StreamContentParser::AddTextObject(). ... by using STL containers in more places. Remove dead / duplicate code as well. BUG=603904 Review-Url: https://codereview.chromium.org/1998583002 --- core/fpdfapi/fpdf_page/cpdf_clippath.cpp | 104 ++++++++++--------------- core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp | 67 ++-------------- core/fpdfapi/fpdf_page/cpdf_clippathdata.h | 14 ++-- core/fpdfapi/fpdf_page/fpdf_page_parser.cpp | 25 +++--- core/fpdfapi/fpdf_page/include/cpdf_clippath.h | 17 ++-- core/fpdfapi/fpdf_page/pageint.h | 4 +- core/fpdfapi/fpdf_render/fpdf_render_text.cpp | 5 +- core/fxcrt/include/fx_basic.h | 39 +--------- 8 files changed, 82 insertions(+), 193 deletions(-) diff --git a/core/fpdfapi/fpdf_page/cpdf_clippath.cpp b/core/fpdfapi/fpdf_page/cpdf_clippath.cpp index 3a3300ae2f..ea854d1b29 100644 --- a/core/fpdfapi/fpdf_page/cpdf_clippath.cpp +++ b/core/fpdfapi/fpdf_page/cpdf_clippath.cpp @@ -6,10 +6,33 @@ #include "core/fpdfapi/fpdf_page/include/cpdf_clippath.h" +#include + #include "core/fpdfapi/fpdf_page/include/cpdf_textobject.h" +#include "third_party/base/stl_util.h" #define FPDF_CLIPPATH_MAX_TEXTS 1024 +uint32_t CPDF_ClipPath::GetPathCount() const { + return pdfium::CollectionSize(m_pObject->m_PathAndTypeList); +} + +CPDF_Path CPDF_ClipPath::GetPath(size_t i) const { + return m_pObject->m_PathAndTypeList[i].first; +} + +uint8_t CPDF_ClipPath::GetClipType(size_t i) const { + return m_pObject->m_PathAndTypeList[i].second; +} + +uint32_t CPDF_ClipPath::GetTextCount() const { + return pdfium::CollectionSize(m_pObject->m_TextList); +} + +CPDF_TextObject* CPDF_ClipPath::GetText(size_t i) const { + return m_pObject->m_TextList[i].get(); +} + CFX_FloatRect CPDF_ClipPath::GetClipBox() const { CFX_FloatRect rect; FX_BOOL bStarted = FALSE; @@ -49,83 +72,38 @@ CFX_FloatRect CPDF_ClipPath::GetClipBox() const { return rect; } -void CPDF_ClipPath::AppendPath(CPDF_Path path, int type, FX_BOOL bAutoMerge) { +void CPDF_ClipPath::AppendPath(CPDF_Path path, uint8_t type, bool bAutoMerge) { CPDF_ClipPathData* pData = GetModify(); - if (pData->m_PathCount && bAutoMerge) { - CPDF_Path old_path = pData->m_pPathList[pData->m_PathCount - 1]; + if (!pData->m_PathAndTypeList.empty() && bAutoMerge) { + const CPDF_Path& old_path = pData->m_PathAndTypeList.back().first; if (old_path.IsRect()) { CFX_FloatRect old_rect(old_path.GetPointX(0), old_path.GetPointY(0), old_path.GetPointX(2), old_path.GetPointY(2)); CFX_FloatRect new_rect = path.GetBoundingBox(); - if (old_rect.Contains(new_rect)) { - pData->m_PathCount--; - pData->m_pPathList[pData->m_PathCount].SetNull(); - } + if (old_rect.Contains(new_rect)) + pData->m_PathAndTypeList.pop_back(); } } - if (pData->m_PathCount % 8 == 0) { - CPDF_Path* pNewPath = new CPDF_Path[pData->m_PathCount + 8]; - for (int i = 0; i < pData->m_PathCount; i++) { - pNewPath[i] = pData->m_pPathList[i]; - } - delete[] pData->m_pPathList; - uint8_t* pNewType = FX_Alloc(uint8_t, pData->m_PathCount + 8); - FXSYS_memcpy(pNewType, pData->m_pTypeList, pData->m_PathCount); - FX_Free(pData->m_pTypeList); - pData->m_pPathList = pNewPath; - pData->m_pTypeList = pNewType; - } - pData->m_pPathList[pData->m_PathCount] = path; - pData->m_pTypeList[pData->m_PathCount] = (uint8_t)type; - pData->m_PathCount++; -} - -void CPDF_ClipPath::DeletePath(int index) { - CPDF_ClipPathData* pData = GetModify(); - if (index >= pData->m_PathCount) { - return; - } - pData->m_pPathList[index].SetNull(); - for (int i = index; i < pData->m_PathCount - 1; i++) { - pData->m_pPathList[i] = pData->m_pPathList[i + 1]; - } - pData->m_pPathList[pData->m_PathCount - 1].SetNull(); - FXSYS_memmove(pData->m_pTypeList + index, pData->m_pTypeList + index + 1, - pData->m_PathCount - index - 1); - pData->m_PathCount--; + pData->m_PathAndTypeList.push_back(std::make_pair(path, type)); } -void CPDF_ClipPath::AppendTexts(CPDF_TextObject** pTexts, int count) { +void CPDF_ClipPath::AppendTexts( + std::vector>* pTexts) { CPDF_ClipPathData* pData = GetModify(); - if (pData->m_TextCount + count > FPDF_CLIPPATH_MAX_TEXTS) { - for (int i = 0; i < count; i++) { - delete pTexts[i]; - } - return; + if (pData->m_TextList.size() + pTexts->size() <= FPDF_CLIPPATH_MAX_TEXTS) { + for (size_t i = 0; i < pTexts->size(); i++) + pData->m_TextList.push_back(std::move((*pTexts)[i])); + pData->m_TextList.push_back(std::unique_ptr()); } - CPDF_TextObject** pNewList = - FX_Alloc(CPDF_TextObject*, pData->m_TextCount + count + 1); - if (pData->m_pTextList) { - FXSYS_memcpy(pNewList, pData->m_pTextList, - pData->m_TextCount * sizeof(CPDF_TextObject*)); - FX_Free(pData->m_pTextList); - } - pData->m_pTextList = pNewList; - for (int i = 0; i < count; i++) { - pData->m_pTextList[pData->m_TextCount + i] = pTexts[i]; - } - pData->m_pTextList[pData->m_TextCount + count] = NULL; - pData->m_TextCount += count + 1; + pTexts->clear(); } void CPDF_ClipPath::Transform(const CFX_Matrix& matrix) { CPDF_ClipPathData* pData = GetModify(); - int i; - for (i = 0; i < pData->m_PathCount; i++) { - pData->m_pPathList[i].Transform(&matrix); + for (auto& obj : pData->m_PathAndTypeList) + obj.first.Transform(&matrix); + for (auto& text : pData->m_TextList) { + if (text) + text->Transform(matrix); } - for (i = 0; i < pData->m_TextCount; i++) - if (pData->m_pTextList[i]) { - pData->m_pTextList[i]->Transform(matrix); - } } diff --git a/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp b/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp index e6a5227269..9c84b4ae28 100644 --- a/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp +++ b/core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp @@ -9,69 +9,16 @@ #include "core/fpdfapi/fpdf_page/include/cpdf_path.h" #include "core/fpdfapi/fpdf_page/include/cpdf_textobject.h" -CPDF_ClipPathData::CPDF_ClipPathData() - : m_PathCount(0), - m_pPathList(nullptr), - m_pTypeList(nullptr), - m_TextCount(0), - m_pTextList(nullptr) {} +CPDF_ClipPathData::CPDF_ClipPathData() {} -CPDF_ClipPathData::~CPDF_ClipPathData() { - delete[] m_pPathList; - FX_Free(m_pTypeList); - - for (int i = m_TextCount - 1; i > -1; i--) - delete m_pTextList[i]; - FX_Free(m_pTextList); -} +CPDF_ClipPathData::~CPDF_ClipPathData() {} CPDF_ClipPathData::CPDF_ClipPathData(const CPDF_ClipPathData& src) { - m_pPathList = nullptr; - m_pPathList = nullptr; - m_pTextList = nullptr; - - m_PathCount = src.m_PathCount; - if (m_PathCount) { - int alloc_size = m_PathCount; - if (alloc_size % 8) - alloc_size += 8 - (alloc_size % 8); - - m_pPathList = new CPDF_Path[alloc_size]; - for (int i = 0; i < m_PathCount; i++) - m_pPathList[i] = src.m_pPathList[i]; - - m_pTypeList = FX_Alloc(uint8_t, alloc_size); - FXSYS_memcpy(m_pTypeList, src.m_pTypeList, m_PathCount); - } else { - m_pPathList = nullptr; - m_pTypeList = nullptr; - } - - m_TextCount = src.m_TextCount; - if (m_TextCount) { - m_pTextList = FX_Alloc(CPDF_TextObject*, m_TextCount); - for (int i = 0; i < m_TextCount; i++) { - if (src.m_pTextList[i]) - m_pTextList[i] = src.m_pTextList[i]->Clone(); - else - m_pTextList[i] = nullptr; - } - } else { - m_pTextList = nullptr; - } -} - -void CPDF_ClipPathData::SetCount(int path_count, int text_count) { - ASSERT(m_TextCount == 0 && m_PathCount == 0); - if (path_count) { - m_PathCount = path_count; - int alloc_size = (path_count + 7) / 8 * 8; - m_pPathList = new CPDF_Path[alloc_size]; - m_pTypeList = FX_Alloc(uint8_t, alloc_size); - } + m_PathAndTypeList = src.m_PathAndTypeList; - if (text_count) { - m_TextCount = text_count; - m_pTextList = FX_Alloc(CPDF_TextObject*, text_count); + m_TextList.resize(src.m_TextList.size()); + for (size_t i = 0; i < src.m_TextList.size(); ++i) { + if (src.m_TextList[i]) + m_TextList[i].reset(src.m_TextList[i]->Clone()); } } diff --git a/core/fpdfapi/fpdf_page/cpdf_clippathdata.h b/core/fpdfapi/fpdf_page/cpdf_clippathdata.h index 05a247f753..70e2b1b096 100644 --- a/core/fpdfapi/fpdf_page/cpdf_clippathdata.h +++ b/core/fpdfapi/fpdf_page/cpdf_clippathdata.h @@ -9,22 +9,22 @@ #include +#include +#include +#include + class CPDF_Path; class CPDF_TextObject; class CPDF_ClipPathData { public: + using PathAndTypeData = std::pair; CPDF_ClipPathData(); CPDF_ClipPathData(const CPDF_ClipPathData&); ~CPDF_ClipPathData(); - void SetCount(int path_count, int text_count); - - int m_PathCount; - CPDF_Path* m_pPathList; - uint8_t* m_pTypeList; - int m_TextCount; - CPDF_TextObject** m_pTextList; + std::vector m_PathAndTypeList; + std::vector> m_TextList; }; #endif // CORE_FPDFAPI_FPDF_PAGE_CPDF_CLIPPATHDATA_H_ diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp index 8c16d8c41f..0374915af4 100644 --- a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp +++ b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp @@ -813,18 +813,12 @@ void CPDF_StreamContentParser::Handle_EndMarkedContent() { } void CPDF_StreamContentParser::Handle_EndText() { - int count = m_ClipTextList.GetSize(); - if (count == 0) { + if (m_ClipTextList.empty()) return; - } - if (m_pCurStates->m_TextState.GetObject()->m_TextMode < 4) { - for (int i = 0; i < count; i++) { - delete m_ClipTextList.GetAt(i); - } - } else { - m_pCurStates->m_ClipPath.AppendTexts(m_ClipTextList.GetData(), count); - } - m_ClipTextList.RemoveAll(); + + if (m_pCurStates->m_TextState.GetObject()->m_TextMode >= 4) + m_pCurStates->m_ClipPath.AppendTexts(&m_ClipTextList); + m_ClipTextList.clear(); } void CPDF_StreamContentParser::Handle_FillPath() { @@ -1284,8 +1278,10 @@ void CPDF_StreamContentParser::AddTextObject(CFX_ByteString* pStrs, m_pCurStates->m_TextHorzScale, m_Level); m_pCurStates->m_TextX += x_advance; m_pCurStates->m_TextY += y_advance; - if (textmode > 3) - m_ClipTextList.Add(pText->Clone()); + if (textmode > 3) { + m_ClipTextList.push_back( + std::unique_ptr(pText->Clone())); + } m_pObjectHolder->GetPageObjectList()->push_back(std::move(pText)); } if (pKerning && pKerning[nsegs - 1] != 0) { @@ -1483,7 +1479,8 @@ void CPDF_StreamContentParser::AddPathPoint(FX_FLOAT x, FX_FLOAT y, int flag) { } void CPDF_StreamContentParser::AddPathObject(int FillType, FX_BOOL bStroke) { - int PathPointCount = m_PathPointCount, PathClipType = m_PathClipType; + int PathPointCount = m_PathPointCount; + uint8_t PathClipType = m_PathClipType; m_PathPointCount = 0; m_PathClipType = 0; if (PathPointCount <= 1) { diff --git a/core/fpdfapi/fpdf_page/include/cpdf_clippath.h b/core/fpdfapi/fpdf_page/include/cpdf_clippath.h index 6f239e661f..1daacf55d0 100644 --- a/core/fpdfapi/fpdf_page/include/cpdf_clippath.h +++ b/core/fpdfapi/fpdf_page/include/cpdf_clippath.h @@ -17,17 +17,14 @@ class CPDF_TextObject; class CPDF_ClipPath : public CFX_CountRef { public: - uint32_t GetPathCount() const { return m_pObject->m_PathCount; } - CPDF_Path GetPath(int i) const { return m_pObject->m_pPathList[i]; } - int GetClipType(int i) const { return m_pObject->m_pTypeList[i]; } - uint32_t GetTextCount() const { return m_pObject->m_TextCount; } - CPDF_TextObject* GetText(int i) const { return m_pObject->m_pTextList[i]; } + uint32_t GetPathCount() const; + CPDF_Path GetPath(size_t i) const; + uint8_t GetClipType(size_t i) const; + uint32_t GetTextCount() const; + CPDF_TextObject* GetText(size_t i) const; CFX_FloatRect GetClipBox() const; - - void AppendPath(CPDF_Path path, int type, FX_BOOL bAutoMerge); - void DeletePath(int layer_index); - - void AppendTexts(CPDF_TextObject** pTexts, int count); + void AppendPath(CPDF_Path path, uint8_t type, bool bAutoMerge); + void AppendTexts(std::vector>* pTexts); void Transform(const CFX_Matrix& matrix); }; diff --git a/core/fpdfapi/fpdf_page/pageint.h b/core/fpdfapi/fpdf_page/pageint.h index 6534186dc8..d1364d0dd1 100644 --- a/core/fpdfapi/fpdf_page/pageint.h +++ b/core/fpdfapi/fpdf_page/pageint.h @@ -250,7 +250,7 @@ class CPDF_StreamContentParser { CPDF_StreamParser* m_pSyntax; std::unique_ptr m_pCurStates; CPDF_ContentMark m_CurContentMark; - CFX_ArrayTemplate m_ClipTextList; + std::vector> m_ClipTextList; CPDF_TextObject* m_pLastTextObject; FX_FLOAT m_DefFontSize; FX_PATHPOINT* m_pPathPoints; @@ -260,7 +260,7 @@ class CPDF_StreamContentParser { FX_FLOAT m_PathStartY; FX_FLOAT m_PathCurrentX; FX_FLOAT m_PathCurrentY; - int m_PathClipType; + uint8_t m_PathClipType; CFX_ByteString m_LastImageName; CPDF_Image* m_pLastImage; CFX_BinaryBuf m_LastImageDict; diff --git a/core/fpdfapi/fpdf_render/fpdf_render_text.cpp b/core/fpdfapi/fpdf_render/fpdf_render_text.cpp index 221d04fd70..f2704b7d60 100644 --- a/core/fpdfapi/fpdf_render/fpdf_render_text.cpp +++ b/core/fpdfapi/fpdf_render/fpdf_render_text.cpp @@ -736,10 +736,11 @@ void CPDF_RenderStatus::DrawTextPathWithPattern(const CPDF_TextObject* textobj, FX_BOOL bStroke) { if (!bStroke) { CPDF_PathObject path; - CPDF_TextObject* pCopy = textobj->Clone(); + std::vector> pCopy; + pCopy.push_back(std::unique_ptr(textobj->Clone())); path.m_bStroke = FALSE; path.m_FillType = FXFILL_WINDING; - path.m_ClipPath.AppendTexts(&pCopy, 1); + path.m_ClipPath.AppendTexts(&pCopy); path.m_ColorState = textobj->m_ColorState; path.m_Path.New()->AppendRect(textobj->m_Left, textobj->m_Bottom, textobj->m_Right, textobj->m_Top); diff --git a/core/fxcrt/include/fx_basic.h b/core/fxcrt/include/fx_basic.h index 1835d9e0a6..7fdfbf699d 100644 --- a/core/fxcrt/include/fx_basic.h +++ b/core/fxcrt/include/fx_basic.h @@ -717,53 +717,22 @@ class CFX_CountRef { } } - ~CFX_CountRef() { - if (!m_pObject) { - return; - } - m_pObject->m_RefCount--; - if (m_pObject->m_RefCount <= 0) { - delete m_pObject; - } - } + ~CFX_CountRef() { SetNull(); } ObjClass* New() { - if (m_pObject) { - m_pObject->m_RefCount--; - if (m_pObject->m_RefCount <= 0) { - delete m_pObject; - } - } + SetNull(); m_pObject = new CountedObj; m_pObject->m_RefCount = 1; return m_pObject; } void operator=(const Ref& ref) { - if (ref.m_pObject) { + if (ref.m_pObject) ref.m_pObject->m_RefCount++; - } - if (m_pObject) { - m_pObject->m_RefCount--; - if (m_pObject->m_RefCount <= 0) { - delete m_pObject; - } - } + SetNull(); m_pObject = ref.m_pObject; } - void operator=(void* p) { - ASSERT(p == 0); - if (!m_pObject) { - return; - } - m_pObject->m_RefCount--; - if (m_pObject->m_RefCount <= 0) { - delete m_pObject; - } - m_pObject = NULL; - } - const ObjClass* GetObject() const { return m_pObject; } operator const ObjClass*() const { return m_pObject; } -- cgit v1.2.3