Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(216)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: fpdfsdk/src/fsdk_mgr.cpp

Issue 1411203007: Cleanup parts of CPDFSDK_AnnotIterator and CPDFSDK_PageView. (Closed) Base URL: https://pdfium.googlesource.com/pdfium@master
Patch Set: fix bounds check Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« fpdfsdk/src/fsdk_annothandler.cpp ('K') | « fpdfsdk/src/fsdk_baseform.cpp ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: fpdfsdk/src/fsdk_mgr.cpp
diff --git a/fpdfsdk/src/fsdk_mgr.cpp b/fpdfsdk/src/fsdk_mgr.cpp
index 4cf05a2d79d35ee4a15f047a4531c8b7e53c40a7..8e213e70e07e2180ba67f440da8bb2956906f7aa 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -19,7 +19,8 @@
class CFX_SystemHandler : public IFX_SystemHandler {
public:
- CFX_SystemHandler(CPDFDoc_Environment* pEnv) : m_pEnv(pEnv), m_nCharSet(-1) {}
+ explicit CFX_SystemHandler(CPDFDoc_Environment* pEnv)
+ : m_pEnv(pEnv), m_nCharSet(-1) {}
~CFX_SystemHandler() override {}
public:
@@ -610,13 +611,12 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc, CPDF_Page* page)
CPDF_InterForm* pPDFInterForm = pInterForm->GetInterForm();
pPDFInterForm->FixPageFields(page);
}
- m_page->SetPrivateData((void*)m_page, (void*)this, NULL);
- m_fxAnnotArray.RemoveAll();
+ m_page->SetPrivateData((void*)m_page, (void*)this, nullptr);
m_bEnterWidget = FALSE;
Tom Sepez 2015/10/21 19:32:59 Can we make these part of the initialization list,
Lei Zhang 2015/10/22 06:56:22 Done.
m_bExitWidget = FALSE;
m_bOnWidget = FALSE;
- m_CaptureWidget = NULL;
+ m_CaptureWidget = nullptr;
m_bValid = FALSE;
m_bLocked = FALSE;
m_bTakeOverPage = FALSE;
@@ -625,8 +625,7 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc, CPDF_Page* page)
CPDFSDK_PageView::~CPDFSDK_PageView() {
// if there is a focused annot on the page, we should kill the focus first.
if (CPDFSDK_Annot* focusedAnnot = m_pSDKDoc->GetFocusAnnot()) {
- for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
- CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
+ for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray) {
Tom Sepez 2015/10/21 19:32:59 find() of some sort?
Lei Zhang 2015/10/22 06:56:22 Done.
if (pAnnot == focusedAnnot) {
KillFocusAnnot();
break;
@@ -636,15 +635,11 @@ CPDFSDK_PageView::~CPDFSDK_PageView() {
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
- ASSERT(pAnnotHandlerMgr);
- for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
- CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
+ for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray)
pAnnotHandlerMgr->ReleaseAnnot(pAnnot);
- }
- m_fxAnnotArray.RemoveAll();
+ m_fxAnnotArray.clear();
- delete m_pAnnotList;
- m_pAnnotList = NULL;
+ m_pAnnotList.reset();
m_page->RemovePrivateData((void*)m_page);
if (m_bTakeOverPage) {
@@ -658,9 +653,9 @@ void CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice* pDevice,
m_curMatrix = *pUser2Device;
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
- CPDFSDK_AnnotIterator annotIterator(this, TRUE);
- int index = -1;
- while (CPDFSDK_Annot* pSDKAnnot = annotIterator.Next(index)) {
+ CPDFSDK_AnnotIterator annotIterator(this, true);
+ size_t index = 0;
+ while (CPDFSDK_Annot* pSDKAnnot = annotIterator.Next(&index)) {
CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
pAnnotHandlerMgr->Annot_OnDraw(this, pSDKAnnot, pDevice, pUser2Device, 0);
}
@@ -668,7 +663,7 @@ void CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice* pDevice,
CPDF_Annot* CPDFSDK_PageView::GetPDFAnnotAtPoint(FX_FLOAT pageX,
FX_FLOAT pageY) {
- int nCount = m_pAnnotList->Count();
+ int nCount = CountAnnots();
for (int i = 0; i < nCount; i++) {
CPDF_Annot* pAnnot = m_pAnnotList->GetAt(i);
CFX_FloatRect annotRect;
@@ -676,13 +671,13 @@ CPDF_Annot* CPDFSDK_PageView::GetPDFAnnotAtPoint(FX_FLOAT pageX,
if (annotRect.Contains(pageX, pageY))
return pAnnot;
}
- return NULL;
+ return nullptr;
}
CPDF_Annot* CPDFSDK_PageView::GetPDFWidgetAtPoint(FX_FLOAT pageX,
FX_FLOAT pageY) {
- int nCount = m_pAnnotList->Count();
- for (int i = 0; i < nCount; i++) {
+ int nCount = CountAnnots();
+ for (int i = 0; i < nCount; ++i) {
CPDF_Annot* pAnnot = m_pAnnotList->GetAt(i);
if (pAnnot->GetSubType() == "Widget") {
CFX_FloatRect annotRect;
@@ -691,17 +686,17 @@ CPDF_Annot* CPDFSDK_PageView::GetPDFWidgetAtPoint(FX_FLOAT pageX,
return pAnnot;
}
}
- return NULL;
+ return nullptr;
}
CPDFSDK_Annot* CPDFSDK_PageView::GetFXAnnotAtPoint(FX_FLOAT pageX,
FX_FLOAT pageY) {
- CPDFSDK_AnnotIterator annotIterator(this, FALSE);
+ CPDFSDK_AnnotIterator annotIterator(this, false);
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
CPDFSDK_AnnotHandlerMgr* pAnnotMgr = pEnv->GetAnnotHandlerMgr();
CPDFSDK_Annot* pSDKAnnot = NULL;
- int index = -1;
- while ((pSDKAnnot = annotIterator.Next(index))) {
+ size_t index = 0;
+ while ((pSDKAnnot = annotIterator.Next(&index))) {
CPDF_Rect rc = pAnnotMgr->Annot_OnGetViewBBox(this, pSDKAnnot);
if (rc.Contains(pageX, pageY))
return pSDKAnnot;
@@ -712,12 +707,12 @@ CPDFSDK_Annot* CPDFSDK_PageView::GetFXAnnotAtPoint(FX_FLOAT pageX,
CPDFSDK_Annot* CPDFSDK_PageView::GetFXWidgetAtPoint(FX_FLOAT pageX,
FX_FLOAT pageY) {
- CPDFSDK_AnnotIterator annotIterator(this, FALSE);
+ CPDFSDK_AnnotIterator annotIterator(this, false);
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
CPDFSDK_AnnotHandlerMgr* pAnnotMgr = pEnv->GetAnnotHandlerMgr();
CPDFSDK_Annot* pSDKAnnot = NULL;
- int index = -1;
- while ((pSDKAnnot = annotIterator.Next(index))) {
+ size_t index = 0;
+ while ((pSDKAnnot = annotIterator.Next(&index))) {
if (pSDKAnnot->GetType() == "Widget") {
pAnnotMgr->Annot_OnGetViewBBox(this, pSDKAnnot);
CPDF_Point point(pageX, pageY);
@@ -740,21 +735,15 @@ CPDFSDK_Annot* CPDFSDK_PageView::AddAnnot(CPDF_Annot* pPDFAnnot) {
CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
ASSERT(pEnv);
CPDFSDK_AnnotHandlerMgr* pAnnotHandler = pEnv->GetAnnotHandlerMgr();
+ if (!pAnnotHandler)
+ return nullptr;
- CPDFSDK_Annot* pSDKAnnot = NULL;
-
- if (pAnnotHandler) {
- pSDKAnnot = pAnnotHandler->NewAnnot(pPDFAnnot, this);
- }
+ CPDFSDK_Annot* pSDKAnnot = pAnnotHandler->NewAnnot(pPDFAnnot, this);
if (!pSDKAnnot)
- return NULL;
-
- m_fxAnnotArray.Add(pSDKAnnot);
-
- if (pAnnotHandler) {
- pAnnotHandler->Annot_OnCreate(pSDKAnnot);
- }
+ return nullptr;
+ m_fxAnnotArray.push_back(pSDKAnnot);
+ pAnnotHandler->Annot_OnCreate(pSDKAnnot);
return pSDKAnnot;
}
@@ -778,27 +767,20 @@ CPDF_Document* CPDFSDK_PageView::GetPDFDocument() {
return NULL;
}
-int CPDFSDK_PageView::CountAnnots() {
+int CPDFSDK_PageView::CountAnnots() const {
return m_pAnnotList->Count();
}
-CPDFSDK_Annot* CPDFSDK_PageView::GetAnnot(int nIndex) {
- int nCount = m_fxAnnotArray.GetSize();
- if (nIndex < 0 || nIndex >= nCount) {
- return NULL;
- }
-
- return (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(nIndex);
+CPDFSDK_Annot* CPDFSDK_PageView::GetAnnot(size_t nIndex) {
+ return nIndex < m_fxAnnotArray.size() ? m_fxAnnotArray[nIndex] : nullptr;
}
CPDFSDK_Annot* CPDFSDK_PageView::GetAnnotByDict(CPDF_Dictionary* pDict) {
- int nCount = m_fxAnnotArray.GetSize();
- for (int i = 0; i < nCount; i++) {
- CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
- if (pDict == pAnnot->GetPDFAnnot()->GetAnnotDict())
+ for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray) {
+ if (pAnnot->GetPDFAnnot()->GetAnnotDict() == pDict)
return pAnnot;
}
- return NULL;
+ return nullptr;
}
FX_BOOL CPDFSDK_PageView::OnLButtonDown(const CPDF_Point& point,
@@ -918,11 +900,11 @@ void CPDFSDK_PageView::LoadFXAnnots() {
FX_BOOL enableAPUpdate = CPDF_InterForm::UpdatingAPEnabled();
// Disable the default AP construction.
CPDF_InterForm::EnableUpdateAP(FALSE);
- m_pAnnotList = new CPDF_AnnotList(m_page);
+ m_pAnnotList.reset(new CPDF_AnnotList(m_page));
CPDF_InterForm::EnableUpdateAP(enableAPUpdate);
- int nCount = m_pAnnotList->Count();
+ int nCount = CountAnnots();
SetLock(TRUE);
- for (int i = 0; i < nCount; i++) {
+ for (int i = 0; i < nCount; ++i) {
CPDF_Annot* pPDFAnnot = m_pAnnotList->GetAt(i);
CPDF_Document* pDoc = GetPDFDocument();
@@ -935,7 +917,7 @@ void CPDFSDK_PageView::LoadFXAnnots() {
CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pPDFAnnot, this);
if (!pAnnot)
continue;
- m_fxAnnotArray.Add(pAnnot);
+ m_fxAnnotArray.push_back(pAnnot);
pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
}
@@ -969,11 +951,12 @@ int CPDFSDK_PageView::GetPageIndex() {
return -1;
}
-FX_BOOL CPDFSDK_PageView::IsValidAnnot(void* p) {
- if (p == NULL)
+FX_BOOL CPDFSDK_PageView::IsValidAnnot(void* p) const {
Tom Sepez 2015/10/21 19:32:59 Why does this take a void?
Lei Zhang 2015/10/22 06:56:22 Dunno, fixed.
+ if (!p)
return FALSE;
- int iCount = m_pAnnotList->Count();
- for (int i = 0; i < iCount; i++) {
+
+ int nCount = CountAnnots();
+ for (int i = 0; i < nCount; ++i) {
if (m_pAnnotList->GetAt(i) == p)
return TRUE;
}
@@ -983,12 +966,11 @@ FX_BOOL CPDFSDK_PageView::IsValidAnnot(void* p) {
CPDFSDK_Annot* CPDFSDK_PageView::GetFocusAnnot() {
CPDFSDK_Annot* pFocusAnnot = m_pSDKDoc->GetFocusAnnot();
if (!pFocusAnnot)
- return NULL;
+ return nullptr;
- for (int i = 0; i < m_fxAnnotArray.GetSize(); i++) {
- CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
+ for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray) {
if (pAnnot == pFocusAnnot)
return pAnnot;
}
- return NULL;
+ return nullptr;
}
« fpdfsdk/src/fsdk_annothandler.cpp ('K') | « fpdfsdk/src/fsdk_baseform.cpp ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698