From f74ad998d2e8d2636fb25e94823946a3b151e34e Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 11 May 2016 10:26:05 -0700 Subject: Replace some calls to Release() with direct delete, part 1. Searching for the anti-pattern: void Release() { delete this; } We must be explicit on the ownership model. Add unique_ptrs as a result. Review-Url: https://codereview.chromium.org/1960673003 --- xfa/fxfa/app/xfa_ffwidgetacc.cpp | 75 ++++++++++++++++------------------------ xfa/fxfa/app/xfa_fwltheme.cpp | 12 ++----- xfa/fxfa/app/xfa_fwltheme.h | 4 ++- xfa/fxfa/app/xfa_textlayout.cpp | 51 +++++++++++++-------------- xfa/fxfa/app/xfa_textlayout.h | 5 +-- 5 files changed, 64 insertions(+), 83 deletions(-) (limited to 'xfa/fxfa/app') diff --git a/xfa/fxfa/app/xfa_ffwidgetacc.cpp b/xfa/fxfa/app/xfa_ffwidgetacc.cpp index ad8cabad3a..fd3757ace5 100644 --- a/xfa/fxfa/app/xfa_ffwidgetacc.cpp +++ b/xfa/fxfa/app/xfa_ffwidgetacc.cpp @@ -7,6 +7,7 @@ #include "xfa/fxfa/app/xfa_ffwidgetacc.h" #include +#include #include "xfa/fde/tto/fde_textout.h" #include "xfa/fde/xml/fde_xml_imp.h" @@ -101,50 +102,30 @@ class CXFA_ImageLayoutData : public CXFA_WidgetLayoutData { int32_t m_iImageXDpi; int32_t m_iImageYDpi; }; + class CXFA_FieldLayoutData : public CXFA_WidgetLayoutData { public: - CXFA_FieldLayoutData() - : m_pCapTextLayout(NULL), - m_pCapTextProvider(NULL), - m_pTextOut(NULL), - m_pFieldSplitArray(NULL) {} - ~CXFA_FieldLayoutData() { - if (m_pCapTextLayout) { - delete m_pCapTextLayout; - } - m_pCapTextLayout = NULL; - if (m_pCapTextProvider) { - delete m_pCapTextProvider; - } - m_pCapTextProvider = NULL; - if (m_pTextOut) { - m_pTextOut->Release(); - } - m_pTextOut = NULL; - if (m_pFieldSplitArray) { - m_pFieldSplitArray->RemoveAll(); - delete m_pFieldSplitArray; - m_pFieldSplitArray = NULL; - } - } + CXFA_FieldLayoutData() {} + ~CXFA_FieldLayoutData() {} + FX_BOOL LoadCaption(CXFA_WidgetAcc* pAcc) { - if (m_pCapTextLayout) { + if (m_pCapTextLayout) return TRUE; - } CXFA_Caption caption = pAcc->GetCaption(); - if (caption && caption.GetPresence() != XFA_ATTRIBUTEENUM_Hidden) { - m_pCapTextProvider = - new CXFA_TextProvider(pAcc, XFA_TEXTPROVIDERTYPE_Caption); - m_pCapTextLayout = new CXFA_TextLayout(m_pCapTextProvider); - return TRUE; - } - return FALSE; + if (!caption || caption.GetPresence() == XFA_ATTRIBUTEENUM_Hidden) + return FALSE; + m_pCapTextProvider.reset( + new CXFA_TextProvider(pAcc, XFA_TEXTPROVIDERTYPE_Caption)); + m_pCapTextLayout.reset(new CXFA_TextLayout(m_pCapTextProvider.get())); + return TRUE; } - CXFA_TextLayout* m_pCapTextLayout; - CXFA_TextProvider* m_pCapTextProvider; - CFDE_TextOut* m_pTextOut; - CFX_FloatArray* m_pFieldSplitArray; + + std::unique_ptr m_pCapTextLayout; + std::unique_ptr m_pCapTextProvider; + std::unique_ptr m_pTextOut; + std::unique_ptr m_pFieldSplitArray; }; + class CXFA_TextEditData : public CXFA_FieldLayoutData { public: }; @@ -763,7 +744,7 @@ void CXFA_WidgetAcc::CalcCaptionSize(CFX_SizeF& szCap) { iCapPlacement == XFA_ATTRIBUTEENUM_Bottom; const bool bReserveExit = fCapReserve > 0.01; CXFA_TextLayout* pCapTextLayout = - ((CXFA_FieldLayoutData*)m_pLayoutData)->m_pCapTextLayout; + static_cast(m_pLayoutData)->m_pCapTextLayout.get(); if (pCapTextLayout) { if (!bVert && eUIType != XFA_ELEMENT_Button) { szCap.x = fCapReserve; @@ -886,8 +867,8 @@ void CXFA_WidgetAcc::CalculateTextContentSize(CFX_SizeF& size) { CXFA_FieldLayoutData* layoutData = static_cast(m_pLayoutData); if (!layoutData->m_pTextOut) { - layoutData->m_pTextOut = new CFDE_TextOut; - CFDE_TextOut* pTextOut = layoutData->m_pTextOut; + layoutData->m_pTextOut.reset(new CFDE_TextOut); + CFDE_TextOut* pTextOut = layoutData->m_pTextOut.get(); pTextOut->SetFont(GetFDEFont()); pTextOut->SetFontSize(fFontSize); pTextOut->SetLineBreakTolerance(fFontSize * 0.2f); @@ -1264,12 +1245,13 @@ FX_BOOL CXFA_WidgetAcc::FindSplitPos(int32_t iBlockIndex, iLinesCount = ((CXFA_FieldLayoutData*)m_pLayoutData)->m_pTextOut->GetTotalLines(); } - if (!((CXFA_FieldLayoutData*)m_pLayoutData)->m_pFieldSplitArray) { - ((CXFA_FieldLayoutData*)m_pLayoutData)->m_pFieldSplitArray = - new CFX_FloatArray; + if (!static_cast(m_pLayoutData)->m_pFieldSplitArray) { + static_cast(m_pLayoutData) + ->m_pFieldSplitArray.reset(new CFX_FloatArray); } CFX_FloatArray* pFieldArray = - ((CXFA_FieldLayoutData*)m_pLayoutData)->m_pFieldSplitArray; + static_cast(m_pLayoutData) + ->m_pFieldSplitArray.get(); int32_t iFieldSplitCount = pFieldArray->GetSize(); for (int32_t i = 0; i < iBlockIndex * 3; i += 3) { iLinesCount -= (int32_t)pFieldArray->GetAt(i + 1); @@ -1486,8 +1468,9 @@ FX_BOOL CXFA_WidgetAcc::LoadCaption() { } CXFA_TextLayout* CXFA_WidgetAcc::GetCaptionTextLayout() { return m_pLayoutData - ? ((CXFA_FieldLayoutData*)m_pLayoutData)->m_pCapTextLayout - : NULL; + ? static_cast(m_pLayoutData) + ->m_pCapTextLayout.get() + : nullptr; } CXFA_TextLayout* CXFA_WidgetAcc::GetTextLayout() { return m_pLayoutData ? ((CXFA_TextLayoutData*)m_pLayoutData)->m_pTextLayout diff --git a/xfa/fxfa/app/xfa_fwltheme.cpp b/xfa/fxfa/app/xfa_fwltheme.cpp index fac873cac9..df09fb504a 100644 --- a/xfa/fxfa/app/xfa_fwltheme.cpp +++ b/xfa/fxfa/app/xfa_fwltheme.cpp @@ -44,7 +44,6 @@ CXFA_FFWidget* XFA_ThemeGetOuterWidget(IFWL_Widget* pWidget) { return NULL; } CXFA_FWLTheme::CXFA_FWLTheme(CXFA_FFApp* pApp) : m_pApp(pApp) { - m_pTextOut = NULL; m_dwCapacity = 0; m_fCapacity = 0; m_pCalendarFont = NULL; @@ -77,7 +76,7 @@ CXFA_FWLTheme::~CXFA_FWLTheme() { delete m_pBarcodeTP; } FWL_Error CXFA_FWLTheme::Initialize() { - m_pTextOut = new CFDE_TextOut; + m_pTextOut.reset(new CFDE_TextOut); for (size_t i = 0; !m_pCalendarFont && i < FX_ArraySize(g_FWLTheme_CalFonts); ++i) { m_pCalendarFont = IFX_Font::LoadFont(g_FWLTheme_CalFonts[i], 0, 0, @@ -98,13 +97,10 @@ FWL_Error CXFA_FWLTheme::Initialize() { return FWL_Error::Succeeded; } FWL_Error CXFA_FWLTheme::Finalize() { - if (m_pTextOut) { - m_pTextOut->Release(); - m_pTextOut = NULL; - } + m_pTextOut.reset(); if (m_pCalendarFont) { m_pCalendarFont->Release(); - m_pCalendarFont = NULL; + m_pCalendarFont = nullptr; } FWLTHEME_Release(); return FWL_Error::Succeeded; @@ -386,8 +382,6 @@ FX_BOOL CXFA_FWLTheme::CalcTextRect(CFWL_ThemeText* pParams, CFX_RectF& rect) { m_pTextOut->SetTextColor(pAcc->GetTextColor()); if (!pParams) return FALSE; - if (!m_pTextOut) - return FALSE; m_pTextOut->SetAlignment(pParams->m_iTTOAlign); m_pTextOut->SetStyles(pParams->m_dwTTOStyles); m_pTextOut->CalcLogicSize(pParams->m_wsText.c_str(), diff --git a/xfa/fxfa/app/xfa_fwltheme.h b/xfa/fxfa/app/xfa_fwltheme.h index 7d58b43c9b..319df8dfa5 100644 --- a/xfa/fxfa/app/xfa_fwltheme.h +++ b/xfa/fxfa/app/xfa_fwltheme.h @@ -7,6 +7,8 @@ #ifndef XFA_FXFA_APP_XFA_FWLTHEME_H_ #define XFA_FXFA_APP_XFA_FWLTHEME_H_ +#include + #include "xfa/fwl/core/ifwl_themeprovider.h" #include "xfa/fwl/theme/cfwl_barcodetp.h" #include "xfa/fwl/theme/cfwl_carettp.h" @@ -77,7 +79,7 @@ class CXFA_FWLTheme : public IFWL_ThemeProvider { CFWL_PushButtonTP* m_pPushButtonTP; CFWL_CaretTP* m_pCaretTP; CFWL_BarcodeTP* m_pBarcodeTP; - CFDE_TextOut* m_pTextOut; + std::unique_ptr m_pTextOut; FX_FLOAT m_fCapacity; uint32_t m_dwCapacity; IFX_Font* m_pCalendarFont; diff --git a/xfa/fxfa/app/xfa_textlayout.cpp b/xfa/fxfa/app/xfa_textlayout.cpp index 01b16a93a2..1f34b45670 100644 --- a/xfa/fxfa/app/xfa_textlayout.cpp +++ b/xfa/fxfa/app/xfa_textlayout.cpp @@ -32,11 +32,13 @@ void CXFA_TextParseContext::SetDecls(const CFDE_CSSDeclaration** ppDeclArray, FXSYS_memcpy(m_ppMatchedDecls, ppDeclArray, iDeclCount * sizeof(CFDE_CSSDeclaration*)); } + +CXFA_TextParser::CXFA_TextParser() : m_pAllocator(NULL), m_pUASheet(NULL) {} + CXFA_TextParser::~CXFA_TextParser() { if (m_pUASheet) m_pUASheet->Release(); - if (m_pSelector) - m_pSelector->Release(); + delete m_pAllocator; FX_POSITION ps = m_mapXMLNodeToParseContext.GetStartPosition(); while (ps) { @@ -62,14 +64,14 @@ void CXFA_TextParser::Reset() { m_pAllocator = nullptr; } void CXFA_TextParser::InitCSSData(CXFA_TextProvider* pTextProvider) { - if (pTextProvider == NULL) { + if (!pTextProvider) return; - } - if (m_pSelector == NULL) { + + if (!m_pSelector) { CXFA_FFDoc* pDoc = pTextProvider->GetDocNode(); IFX_FontMgr* pFontMgr = pDoc->GetApp()->GetFDEFontMgr(); ASSERT(pFontMgr); - m_pSelector = new CFDE_CSSStyleSelector; + m_pSelector.reset(new CFDE_CSSStyleSelector); m_pSelector->SetFontMgr(pFontMgr); FX_FLOAT fFontSize = 10; CXFA_Font font = pTextProvider->GetFontNode(); @@ -78,7 +80,7 @@ void CXFA_TextParser::InitCSSData(CXFA_TextProvider* pTextProvider) { } m_pSelector->SetDefFontSize(fFontSize); } - if (m_pUASheet == NULL) { + if (!m_pUASheet) { m_pUASheet = LoadDefaultSheetStyle(); m_pSelector->SetStyleSheet(FDE_CSSSTYLESHEETGROUP_UserAgent, m_pUASheet); m_pSelector->UpdateStyleIndex(FDE_CSSMEDIATYPE_ALL); @@ -1183,13 +1185,13 @@ FX_BOOL CXFA_TextLayout::DrawString(CFX_RenderDevice* pFxDevice, if (!pFxDevice) return FALSE; - CFDE_RenderDevice* pDevice = new CFDE_RenderDevice(pFxDevice, FALSE); + std::unique_ptr pDevice( + new CFDE_RenderDevice(pFxDevice, FALSE)); FDE_HDEVICESTATE state = pDevice->SaveState(); pDevice->SetClipRect(rtClip); - CFDE_Brush* pSolidBrush = new CFDE_Brush; - CFDE_Pen* pPen = new CFDE_Pen; - ASSERT(pDevice); + std::unique_ptr pSolidBrush(new CFDE_Brush); + std::unique_ptr pPen(new CFDE_Pen); if (m_pieceLines.GetSize() == 0) { int32_t iBlockCount = CountBlocks(); for (int32_t i = 0; i < iBlockCount; i++) { @@ -1226,17 +1228,16 @@ FX_BOOL CXFA_TextLayout::DrawString(CFX_RenderDevice* pFxDevice, iCharCount = iChars; } FXSYS_memset(pCharPos, 0, iCharCount * sizeof(FXTEXT_CHARPOS)); - RenderString(pDevice, pSolidBrush, pPieceLine, j, pCharPos, tmDoc2Device); + RenderString(pDevice.get(), pSolidBrush.get(), pPieceLine, j, pCharPos, + tmDoc2Device); } for (j = 0; j < iPieces; j++) { - RenderPath(pDevice, pPen, pPieceLine, j, pCharPos, tmDoc2Device); + RenderPath(pDevice.get(), pPen.get(), pPieceLine, j, pCharPos, + tmDoc2Device); } } pDevice->RestoreState(state); FX_Free(pCharPos); - delete pSolidBrush; - delete pPen; - pDevice->Release(); return iPieceLines; } void CXFA_TextLayout::UpdateAlign(FX_FLOAT fHeight, FX_FLOAT fBottom) { @@ -1817,6 +1818,7 @@ void CXFA_TextLayout::RenderString(CFDE_RenderDevice* pDevice, } pPieceLine->m_charCounts.Add(iCount); } + void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, CFDE_Pen* pPen, CXFA_PieceLine* pPieceLine, @@ -1830,7 +1832,7 @@ void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, return; } pPen->SetColor(pPiece->dwColor); - CFDE_Path* pPath = new CFDE_Path; + std::unique_ptr pPath(new CFDE_Path); int32_t iChars = GetDisplayPos(pPiece, pCharPos); if (iChars > 0) { CFX_PointF pt1, pt2; @@ -1870,7 +1872,7 @@ void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, } else { if (bNoLineThrough && (bNoUnderline || pPiece->iPeriod != XFA_ATTRIBUTEENUM_All)) { - goto XFA_RenderPathRet; + return; } int32_t iCharsTmp = 0; int32_t iPiecePrev = iPiece, iPieceNext = iPiece; @@ -1882,7 +1884,7 @@ void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, } } if (iCharsTmp == 0) { - goto XFA_RenderPathRet; + return; } iCharsTmp = 0; int32_t iPieces = pPieceLine->m_textPieces.GetSize(); @@ -1894,20 +1896,20 @@ void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, } } if (iCharsTmp == 0) { - goto XFA_RenderPathRet; + return; } FX_FLOAT fOrgX = 0.0f, fEndX = 0.0f; pPiece = pPieceLine->m_textPieces.GetAt(iPiecePrev); iChars = GetDisplayPos(pPiece, pCharPos); if (iChars < 1) { - goto XFA_RenderPathRet; + return; } fOrgX = pCharPos[iChars - 1].m_OriginX + pCharPos[iChars - 1].m_FontCharWidth * pPiece->fFontSize / 1000.0f; pPiece = pPieceLine->m_textPieces.GetAt(iPieceNext); iChars = GetDisplayPos(pPiece, pCharPos); if (iChars < 1) { - goto XFA_RenderPathRet; + return; } fEndX = pCharPos[0].m_OriginX; CFX_PointF pt1, pt2; @@ -1926,10 +1928,9 @@ void CXFA_TextLayout::RenderPath(CFDE_RenderDevice* pDevice, fEndY += 2.0f; } } - pDevice->DrawPath(pPen, 1, pPath, &tmDoc2Device); -XFA_RenderPathRet: - pPath->Release(); + pDevice->DrawPath(pPen, 1, pPath.get(), &tmDoc2Device); } + int32_t CXFA_TextLayout::GetDisplayPos(const XFA_TextPiece* pPiece, FXTEXT_CHARPOS* pCharPos, FX_BOOL bCharCode) { diff --git a/xfa/fxfa/app/xfa_textlayout.h b/xfa/fxfa/app/xfa_textlayout.h index 17450ddeab..4d03aee302 100644 --- a/xfa/fxfa/app/xfa_textlayout.h +++ b/xfa/fxfa/app/xfa_textlayout.h @@ -8,6 +8,7 @@ #define XFA_FXFA_APP_XFA_TEXTLAYOUT_H_ #include +#include #include "xfa/fde/css/fde_css.h" #include "xfa/fde/fde_gedevice.h" @@ -79,7 +80,7 @@ class CXFA_TextParseContext : public CFX_Target { class CXFA_TextParser { public: - CXFA_TextParser() : m_pAllocator(NULL), m_pSelector(NULL), m_pUASheet(NULL) {} + CXFA_TextParser(); virtual ~CXFA_TextParser(); void Reset(); void DoParse(CFDE_XMLNode* pXMLContainer, CXFA_TextProvider* pTextProvider); @@ -131,7 +132,7 @@ class CXFA_TextParser { IFDE_CSSStyleSheet* LoadDefaultSheetStyle(); IFDE_CSSComputedStyle* CreateStyle(IFDE_CSSComputedStyle* pParentStyle); IFX_MemoryAllocator* m_pAllocator; - CFDE_CSSStyleSelector* m_pSelector; + std::unique_ptr m_pSelector; IFDE_CSSStyleSheet* m_pUASheet; CFX_MapPtrTemplate m_mapXMLNodeToParseContext; -- cgit v1.2.3