Fix incorrect CPDFSDK_PageView::CountAnnots(). (try 2)
authorLei Zhang <thestig@chromium.org>
Fri, 30 Oct 2015 18:18:29 +0000 (11:18 -0700)
committerLei Zhang <thestig@chromium.org>
Fri, 30 Oct 2015 18:18:29 +0000 (11:18 -0700)
The original XFA version was correct, and the master version here is
wrong. The two versions are now in sync, but incorrect.
So we need to fix this here and then merge to XFA again.

Also fix what are now incorrect uses of CountAnnots() and do some
cleanups.

BUG=543049
R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1413973011 .

fpdfsdk/include/fsdk_mgr.h
fpdfsdk/src/fsdk_baseform.cpp
fpdfsdk/src/fsdk_mgr.cpp

index fa2860a..2c063ed 100644 (file)
@@ -308,7 +308,7 @@ class CPDFSDK_PageView final {
   CPDFSDK_Annot* AddAnnot(const FX_CHAR* lpSubType, CPDF_Dictionary* pDict);
   CPDFSDK_Annot* AddAnnot(CPDF_Annot* pPDFAnnot);
   FX_BOOL DeleteAnnot(CPDFSDK_Annot* pAnnot);
-  int CountAnnots() const;
+  size_t CountAnnots() const;
   CPDFSDK_Annot* GetAnnot(size_t nIndex);
   CPDFSDK_Annot* GetAnnotByDict(CPDF_Dictionary* pDict);
   CPDF_Page* GetPDFPage() { return m_page; }
index d1f1aed..fc45cd9 100644 (file)
@@ -2427,58 +2427,45 @@ int CBA_AnnotIterator::CompareByTop(CPDFSDK_Annot* p1, CPDFSDK_Annot* p2) {
 }
 
 void CBA_AnnotIterator::GenerateResults() {
-  ASSERT(m_pPageView != NULL);
-
   switch (m_nTabs) {
     case BAI_STRUCTURE: {
       for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
         CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        ASSERT(pAnnot != NULL);
-
         if (pAnnot->GetType() == m_sType && pAnnot->GetSubType() == m_sSubType)
           m_Annots.Add(pAnnot);
       }
-    } break;
+      break;
+    }
     case BAI_ROW: {
       CPDFSDK_SortAnnots sa;
-
-      {
-        for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-          CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-          ASSERT(pAnnot != NULL);
-
-          if (pAnnot->GetType() == m_sType &&
-              pAnnot->GetSubType() == m_sSubType)
-            sa.Add(pAnnot);
-        }
+      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
+        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
+        if (pAnnot->GetType() == m_sType && pAnnot->GetSubType() == m_sSubType)
+          sa.Add(pAnnot);
       }
 
-      if (sa.GetSize() > 0) {
+      if (sa.GetSize() > 0)
         sa.Sort(CBA_AnnotIterator::CompareByLeft);
-      }
 
       while (sa.GetSize() > 0) {
         int nLeftTopIndex = -1;
+        FX_FLOAT fTop = 0.0f;
 
-        {
-          FX_FLOAT fTop = 0.0f;
+        for (int i = sa.GetSize() - 1; i >= 0; i--) {
+          CPDFSDK_Annot* pAnnot = sa.GetAt(i);
+          ASSERT(pAnnot);
 
-          for (int i = sa.GetSize() - 1; i >= 0; i--) {
-            CPDFSDK_Annot* pAnnot = sa.GetAt(i);
-            ASSERT(pAnnot != NULL);
+          CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
 
-            CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
-
-            if (rcAnnot.top > fTop) {
-              nLeftTopIndex = i;
-              fTop = rcAnnot.top;
-            }
+          if (rcAnnot.top > fTop) {
+            nLeftTopIndex = i;
+            fTop = rcAnnot.top;
           }
         }
 
         if (nLeftTopIndex >= 0) {
           CPDFSDK_Annot* pLeftTopAnnot = sa.GetAt(nLeftTopIndex);
-          ASSERT(pLeftTopAnnot != NULL);
+          ASSERT(pLeftTopAnnot);
 
           CPDF_Rect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
 
@@ -2487,80 +2474,61 @@ void CBA_AnnotIterator::GenerateResults() {
 
           CFX_ArrayTemplate<int> aSelect;
 
-          {
-            for (int i = 0, sz = sa.GetSize(); i < sz; i++) {
-              CPDFSDK_Annot* pAnnot = sa.GetAt(i);
-              ASSERT(pAnnot != NULL);
-
-              CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
-
-              FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
+          for (int i = 0, sz = sa.GetSize(); i < sz; ++i) {
+            CPDFSDK_Annot* pAnnot = sa.GetAt(i);
+            ASSERT(pAnnot);
 
-              if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
-                aSelect.Add(i);
-            }
+            CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
+            FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
+            if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
+              aSelect.Add(i);
           }
 
-          {
-            for (int i = 0, sz = aSelect.GetSize(); i < sz; i++) {
-              m_Annots.Add(sa[aSelect[i]]);
-            }
-          }
+          for (int i = 0, sz = aSelect.GetSize(); i < sz; ++i)
+            m_Annots.Add(sa[aSelect[i]]);
 
-          {
-            for (int i = aSelect.GetSize() - 1; i >= 0; i--) {
+          for (int i = aSelect.GetSize() - 1; i >= 0; --i)
               sa.RemoveAt(aSelect[i]);
-            }
-          }
 
           aSelect.RemoveAll();
         }
       }
       sa.RemoveAll();
-    } break;
+      break;
+    }
     case BAI_COLUMN: {
       CPDFSDK_SortAnnots sa;
-
-      {
-        for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-          CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-          ASSERT(pAnnot != NULL);
-
-          if (pAnnot->GetType() == m_sType &&
-              pAnnot->GetSubType() == m_sSubType)
-            sa.Add(pAnnot);
-        }
+      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
+        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
+        if (pAnnot->GetType() == m_sType && pAnnot->GetSubType() == m_sSubType)
+          sa.Add(pAnnot);
       }
 
-      if (sa.GetSize() > 0) {
+      if (sa.GetSize() > 0)
         sa.Sort(CBA_AnnotIterator::CompareByTop, FALSE);
-      }
 
       while (sa.GetSize() > 0) {
         int nLeftTopIndex = -1;
+        FX_FLOAT fLeft = -1.0f;
 
-        {
-          FX_FLOAT fLeft = -1.0f;
+        for (int i = sa.GetSize() - 1; i >= 0; --i) {
+          CPDFSDK_Annot* pAnnot = sa.GetAt(i);
+          ASSERT(pAnnot);
 
-          for (int i = sa.GetSize() - 1; i >= 0; i--) {
-            CPDFSDK_Annot* pAnnot = sa.GetAt(i);
-            ASSERT(pAnnot != NULL);
-
-            CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
+          CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
 
-            if (fLeft < 0) {
-              nLeftTopIndex = 0;
-              fLeft = rcAnnot.left;
-            } else if (rcAnnot.left < fLeft) {
-              nLeftTopIndex = i;
-              fLeft = rcAnnot.left;
-            }
+          if (fLeft < 0) {
+            nLeftTopIndex = 0;
+            fLeft = rcAnnot.left;
+          } else if (rcAnnot.left < fLeft) {
+            nLeftTopIndex = i;
+            fLeft = rcAnnot.left;
           }
         }
 
         if (nLeftTopIndex >= 0) {
           CPDFSDK_Annot* pLeftTopAnnot = sa.GetAt(nLeftTopIndex);
-          ASSERT(pLeftTopAnnot != NULL);
+          ASSERT(pLeftTopAnnot);
 
           CPDF_Rect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
 
@@ -2568,49 +2536,33 @@ void CBA_AnnotIterator::GenerateResults() {
           sa.RemoveAt(nLeftTopIndex);
 
           CFX_ArrayTemplate<int> aSelect;
+          for (int i = 0, sz = sa.GetSize(); i < sz; ++i) {
+            CPDFSDK_Annot* pAnnot = sa.GetAt(i);
+            ASSERT(pAnnot);
 
-          {
-            for (int i = 0, sz = sa.GetSize(); i < sz; i++) {
-              CPDFSDK_Annot* pAnnot = sa.GetAt(i);
-              ASSERT(pAnnot != NULL);
-
-              CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
-
-              FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
-
-              if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
-                aSelect.Add(i);
-            }
+            CPDF_Rect rcAnnot = GetAnnotRect(pAnnot);
+            FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
+            if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
+              aSelect.Add(i);
           }
 
-          {
-            for (int i = 0, sz = aSelect.GetSize(); i < sz; i++) {
-              m_Annots.Add(sa[aSelect[i]]);
-            }
-          }
+          for (int i = 0, sz = aSelect.GetSize(); i < sz; ++i)
+            m_Annots.Add(sa[aSelect[i]]);
 
-          {
-            for (int i = aSelect.GetSize() - 1; i >= 0; i--) {
-              sa.RemoveAt(aSelect[i]);
-            }
-          }
+          for (int i = aSelect.GetSize() - 1; i >= 0; --i)
+            sa.RemoveAt(aSelect[i]);
 
           aSelect.RemoveAll();
         }
       }
       sa.RemoveAll();
-    } break;
+      break;
+    }
   }
 }
 
 CPDF_Rect CBA_AnnotIterator::GetAnnotRect(CPDFSDK_Annot* pAnnot) {
-  ASSERT(pAnnot != NULL);
-
-  CPDF_Annot* pPDFAnnot = pAnnot->GetPDFAnnot();
-  ASSERT(pPDFAnnot != NULL);
-
   CPDF_Rect rcAnnot;
-  pPDFAnnot->GetRect(rcAnnot);
-
+  pAnnot->GetPDFAnnot()->GetRect(rcAnnot);
   return rcAnnot;
 }
index 53a774e..0b67701 100644 (file)
@@ -664,8 +664,8 @@ void CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice* pDevice,
 
 CPDF_Annot* CPDFSDK_PageView::GetPDFAnnotAtPoint(FX_FLOAT pageX,
                                                  FX_FLOAT pageY) {
-  int nCount = CountAnnots();
-  for (int i = 0; i < nCount; i++) {
+  const int nCount = m_pAnnotList->Count();
+  for (int i = 0; i < nCount; ++i) {
     CPDF_Annot* pAnnot = m_pAnnotList->GetAt(i);
     CFX_FloatRect annotRect;
     pAnnot->GetRect(annotRect);
@@ -677,7 +677,7 @@ CPDF_Annot* CPDFSDK_PageView::GetPDFAnnotAtPoint(FX_FLOAT pageX,
 
 CPDF_Annot* CPDFSDK_PageView::GetPDFWidgetAtPoint(FX_FLOAT pageX,
                                                   FX_FLOAT pageY) {
-  int nCount = CountAnnots();
+  const int nCount = m_pAnnotList->Count();
   for (int i = 0; i < nCount; ++i) {
     CPDF_Annot* pAnnot = m_pAnnotList->GetAt(i);
     if (pAnnot->GetSubType() == "Widget") {
@@ -764,8 +764,8 @@ CPDF_Document* CPDFSDK_PageView::GetPDFDocument() {
   return NULL;
 }
 
-int CPDFSDK_PageView::CountAnnots() const {
-  return m_pAnnotList->Count();
+size_t CPDFSDK_PageView::CountAnnots() const {
+  return m_fxAnnotArray.size();
 }
 
 CPDFSDK_Annot* CPDFSDK_PageView::GetAnnot(size_t nIndex) {
@@ -899,7 +899,7 @@ void CPDFSDK_PageView::LoadFXAnnots() {
   CPDF_InterForm::EnableUpdateAP(FALSE);
   m_pAnnotList.reset(new CPDF_AnnotList(m_page));
   CPDF_InterForm::EnableUpdateAP(enableAPUpdate);
-  int nCount = CountAnnots();
+  const int nCount = m_pAnnotList->Count();
   SetLock(TRUE);
   for (int i = 0; i < nCount; ++i) {
     CPDF_Annot* pPDFAnnot = m_pAnnotList->GetAt(i);
@@ -908,16 +908,12 @@ void CPDFSDK_PageView::LoadFXAnnots() {
     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.push_back(pAnnot);
+    CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pPDFAnnot, this);
+    if (!pAnnot)
+      continue;
+    m_fxAnnotArray.push_back(pAnnot);
 
-      pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
-    }
+    pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
   }
   SetLock(FALSE);
 }
@@ -952,7 +948,7 @@ FX_BOOL CPDFSDK_PageView::IsValidAnnot(CPDF_Annot* p) const {
   if (!p)
     return FALSE;
 
-  int nCount = CountAnnots();
+  const int nCount = m_pAnnotList->Count();
   for (int i = 0; i < nCount; ++i) {
     if (m_pAnnotList->GetAt(i) == p)
       return TRUE;