Cleanup parts of CPDFSDK_AnnotIterator and CPDFSDK_PageView.

fpdfsdk/src/fsdk_mgr.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../public/fpdf_ext.h"
 #include "../../third_party/base/nonstd_unique_ptr.h"
 #include "../include/formfiller/FFL_FormFiller.h"
 #include "../include/fsdk_define.h"
 #include "../include/fsdk_mgr.h"
 #include "../include/javascript/IJavaScript.h"
 
 #if _FX_OS_ == _FX_ANDROID_
 #include "time.h"
 #else
 #include <ctime>
 #endif
 
 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:
   // IFX_SystemHandler
   void InvalidateRect(FX_HWND hWnd, FX_RECT rect) override;
   void OutputSelectedRect(void* pFormFiller, CPDF_Rect& rect) override;
   FX_BOOL IsSelectionImplemented() override;
   CFX_WideString GetClipboardText(FX_HWND hWnd) override { return L""; }
   FX_BOOL SetClipboardText(FX_HWND hWnd, CFX_WideString string) override {
     return FALSE;
@@ skipping 570 matching lines @@
   return m_pEnv->JS_docGetFilePath();
 }
 
 CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc, CPDF_Page* page)
     : m_page(page), m_pSDKDoc(pSDKDoc) {
   CPDFSDK_InterForm* pInterForm = pSDKDoc->GetInterForm();
   if (pInterForm) {
     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, esp since we're passing
|this| to some method above?

[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;
 }
 
 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;
       }
     }
   }
 
   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) {
     delete m_page;
   }
 }
 
 void CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice* pDevice,
                                        CPDF_Matrix* pUser2Device,
                                        CPDF_RenderOptions* pOptions) {
   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);
   }
 }
 
 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;
     pAnnot->GetRect(annotRect);
     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;
       pAnnot->GetRect(annotRect);
       if (annotRect.Contains(pageX, pageY))
         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;
   }
 
   return NULL;
 }
 
 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);
       if (pAnnotMgr->Annot_OnHitTest(this, pSDKAnnot, point))
         return pSDKAnnot;
     }
   }
 
   return NULL;
 }
 
 FX_BOOL CPDFSDK_PageView::Annot_HasAppearance(CPDF_Annot* pAnnot) {
   CPDF_Dictionary* pAnnotDic = pAnnot->GetAnnotDict();
   if (pAnnotDic)
     return pAnnotDic->KeyExist("AS");
   return FALSE;
 }
 
 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;
+  CPDFSDK_Annot* pSDKAnnot = pAnnotHandler->NewAnnot(pPDFAnnot, this);
+  if (!pSDKAnnot)
+    return nullptr;
 
-  if (pAnnotHandler) {
-    pSDKAnnot = pAnnotHandler->NewAnnot(pPDFAnnot, this);
-  }
-  if (!pSDKAnnot)
-    return NULL;
-
-  m_fxAnnotArray.Add(pSDKAnnot);
-
-  if (pAnnotHandler) {
-    pAnnotHandler->Annot_OnCreate(pSDKAnnot);
-  }
-
+  m_fxAnnotArray.push_back(pSDKAnnot);
+  pAnnotHandler->Annot_OnCreate(pSDKAnnot);
   return pSDKAnnot;
 }
 
 CPDFSDK_Annot* CPDFSDK_PageView::AddAnnot(CPDF_Dictionary* pDict) {
   return pDict ? AddAnnot(pDict->GetString("Subtype"), pDict) : nullptr;
 }
 
 CPDFSDK_Annot* CPDFSDK_PageView::AddAnnot(const FX_CHAR* lpSubType,
                                           CPDF_Dictionary* pDict) {
   return NULL;
 }
 
 FX_BOOL CPDFSDK_PageView::DeleteAnnot(CPDFSDK_Annot* pAnnot) {
   return FALSE;
 }
 
 CPDF_Document* CPDFSDK_PageView::GetPDFDocument() {
   if (m_page) {
     return m_page->m_pDocument;
   }
   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,
                                         FX_UINT nFlag) {
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   ASSERT(pEnv);
   CPDFSDK_Annot* pFXAnnot = GetFXWidgetAtPoint(point.x, point.y);
   if (!pFXAnnot) {
     KillFocusAnnot(nFlag);
   } else {
@@ skipping 99 matching lines @@
 FX_BOOL CPDFSDK_PageView::OnKeyUp(int nKeyCode, int nFlag) {
   return FALSE;
 }
 
 void CPDFSDK_PageView::LoadFXAnnots() {
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
 
   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();
 
     CheckUnSupportAnnot(pDoc, pPDFAnnot);
 
     CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
     ASSERT(pAnnotHandlerMgr != NULL);
 
     if (pAnnotHandlerMgr) {
       CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pPDFAnnot, this);
       if (!pAnnot)
         continue;
-      m_fxAnnotArray.Add(pAnnot);
+      m_fxAnnotArray.push_back(pAnnot);
 
       pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
     }
   }
   SetLock(FALSE);
 }
 
 void CPDFSDK_PageView::UpdateRects(CFX_RectArray& rects) {
   for (int i = 0; i < rects.GetSize(); i++) {
     CPDF_Rect rc = rects.GetAt(i);
@@ skipping 13 matching lines @@
   if (m_page) {
     CPDF_Dictionary* pDic = m_page->m_pFormDict;
     CPDF_Document* pDoc = m_pSDKDoc->GetDocument();
     if (pDoc && pDic) {
       return pDoc->GetPageIndex(pDic->GetObjNum());
     }
   }
   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;
   }
   return FALSE;
 }
 
 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;
 }