Clean up CPDFSDK_PageView methods.

fpdfsdk/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 "fpdfsdk/include/fsdk_mgr.h"
 
 #include <algorithm>
 #include <memory>
@@ skipping 702 matching lines @@
   if (m_page) {
 #ifdef PDF_ENABLE_XFA
     return m_page->GetDocument()->GetPDFDoc();
 #else   // PDF_ENABLE_XFA
     return m_page->m_pDocument;
 #endif  // PDF_ENABLE_XFA
   }
   return nullptr;
 }
 
+CPDF_Page* CPDFSDK_PageView::GetPDFPage() const {
+  if (m_page) {
 #ifdef PDF_ENABLE_XFA

[Lei Zhang, 2016/08/25 22:41:31]

A bit weird here. How about:

#ifdef PDF_ENABLE_XFA
return m_page ? m_page->GetPDFPage() : nullptr;
#else
return m_page;
#endif

[jaepark, 2016/08/25 23:14:14]

Done.

-CPDF_Page* CPDFSDK_PageView::GetPDFPage() {
-  if (m_page) {
     return m_page->GetPDFPage();
+#else   // PDF_ENABLE_XFA
+    return m_page;
+#endif  // PDF_ENABLE_XFA
   }
   return nullptr;
 }
-#endif  // PDF_ENABLE_XFA
 
 size_t CPDFSDK_PageView::CountAnnots() const {
   return m_fxAnnotArray.size();
 }
 
 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) {
@@ skipping 41 matching lines @@
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   ASSERT(pEnv);
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
   ASSERT(pAnnotHandlerMgr);
 
   CPDFSDK_Annot* pFXAnnot = GetFXWidgetAtPoint(point.x, point.y);
 
   if (!pFXAnnot)
     return FALSE;
 
-  FX_BOOL bRet =
-      pAnnotHandlerMgr->Annot_OnRButtonDown(this, pFXAnnot, nFlag, point);
-  if (bRet) {
+  if (pAnnotHandlerMgr->Annot_OnRButtonDown(this, pFXAnnot, nFlag, point))
     SetFocusAnnot(pFXAnnot);
-  }
+
   return TRUE;
 }
 
 FX_BOOL CPDFSDK_PageView::OnRButtonUp(const CFX_FloatPoint& point,
                                       FX_UINT nFlag) {
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   ASSERT(pEnv);
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
 
   CPDFSDK_Annot* pFXAnnot = GetFXWidgetAtPoint(point.x, point.y);
 
   if (!pFXAnnot)
     return FALSE;
 
-  FX_BOOL bRet =
-      pAnnotHandlerMgr->Annot_OnRButtonUp(this, pFXAnnot, nFlag, point);
-  if (bRet) {
+  if (pAnnotHandlerMgr->Annot_OnRButtonUp(this, pFXAnnot, nFlag, point))
     SetFocusAnnot(pFXAnnot);
-  }
+
   return TRUE;
 }
 #endif  // PDF_ENABLE_XFA
 
 FX_BOOL CPDFSDK_PageView::OnLButtonUp(const CFX_FloatPoint& point,
                                       FX_UINT nFlag) {
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   ASSERT(pEnv);
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
   CPDFSDK_Annot* pFXAnnot = GetFXWidgetAtPoint(point.x, point.y);
@@ skipping 70 matching lines @@
   }
   return FALSE;
 }
 
 FX_BOOL CPDFSDK_PageView::OnKeyUp(int nKeyCode, int nFlag) {
   return FALSE;
 }
 
 void CPDFSDK_PageView::LoadFXAnnots() {
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
-#ifdef PDF_ENABLE_XFA
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
-#else
-  FX_BOOL bUpdateAP = CPDF_InterForm::IsUpdateAPEnabled();
-  // Disable the default AP construction.
-  CPDF_InterForm::SetUpdateAP(FALSE);
-  m_pAnnotList.reset(new CPDF_AnnotList(m_page));
-  CPDF_InterForm::SetUpdateAP(bUpdateAP);
-  const size_t nCount = m_pAnnotList->Count();
-#endif  // PDF_ENABLE_XFA
 
   SetLock(TRUE);
 
 #ifdef PDF_ENABLE_XFA
   CFX_RetainPtr<CPDFXFA_Page> protector(m_page);
   if (m_pSDKDoc->GetXFADocument()->GetDocType() == DOCTYPE_DYNAMIC_XFA) {
     CXFA_FFPageView* pageView = m_page->GetXFAPageView();
     std::unique_ptr<IXFA_WidgetIterator> pWidgetHander(
         pageView->CreateWidgetIterator(
             XFA_TRAVERSEWAY_Form,
             XFA_WidgetStatus_Visible | XFA_WidgetStatus_Viewable));
     if (!pWidgetHander) {
       SetLock(FALSE);
       return;
     }
 
     while (CXFA_FFWidget* pXFAAnnot = pWidgetHander->MoveToNext()) {
       CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pXFAAnnot, this);
       if (!pAnnot)
         continue;
       m_fxAnnotArray.push_back(pAnnot);
       pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
     }
-  } else {
-    CPDF_Page* pPage = m_page->GetPDFPage();
-    ASSERT(pPage);
-    FX_BOOL bUpdateAP = CPDF_InterForm::IsUpdateAPEnabled();
-    // Disable the default AP construction.
-    CPDF_InterForm::SetUpdateAP(FALSE);
-    m_pAnnotList.reset(new CPDF_AnnotList(pPage));
-    CPDF_InterForm::SetUpdateAP(bUpdateAP);
 
-    const size_t nCount = m_pAnnotList->Count();
-    for (size_t i = 0; i < nCount; ++i) {
-      CPDF_Annot* pPDFAnnot = m_pAnnotList->GetAt(i);
-      CheckUnSupportAnnot(GetPDFDocument(), pPDFAnnot);
+    SetLock(FALSE);
+    return;
+  }
+#endif  // PDF_ENABLE_XFA
 
-      CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pPDFAnnot, this);
-      if (!pAnnot)
-        continue;
-      m_fxAnnotArray.push_back(pAnnot);
-      pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
-    }
-  }
-#else   // PDF_ENABLE_XFA
+  CPDF_Page* pPage = GetPDFPage();

[Lei Zhang, 2016/08/25 22:41:31]

For the non-XFA code path, previously, this code gets called before
SetLock(TRUE). Is that an issue?

[jaepark, 2016/08/25 23:14:14]

I don't think this will be an issue. m_bLocked is only used to defer deleting
CPDFSDK_PageView.

+  ASSERT(pPage);
+  FX_BOOL bUpdateAP = CPDF_InterForm::IsUpdateAPEnabled();
+  // Disable the default AP construction.
+  CPDF_InterForm::SetUpdateAP(FALSE);
+  m_pAnnotList.reset(new CPDF_AnnotList(pPage));
+  CPDF_InterForm::SetUpdateAP(bUpdateAP);
+
+  const size_t nCount = m_pAnnotList->Count();
   for (size_t i = 0; i < nCount; ++i) {
     CPDF_Annot* pPDFAnnot = m_pAnnotList->GetAt(i);
-    CPDF_Document* pDoc = GetPDFDocument();
-    CheckUnSupportAnnot(pDoc, pPDFAnnot);
-    CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
+    CheckUnSupportAnnot(GetPDFDocument(), pPDFAnnot);
     CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pPDFAnnot, this);
     if (!pAnnot)
       continue;
     m_fxAnnotArray.push_back(pAnnot);
     pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
   }
-#endif  // PDF_ENABLE_XFA
 
   SetLock(FALSE);
 }
 
 void CPDFSDK_PageView::ClearFXAnnots() {
   SetLock(TRUE);
   if (m_pSDKDoc && GetFocusAnnot())
     m_pSDKDoc->SetFocusAnnot(nullptr);
   m_CaptureWidget = nullptr;
   for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray)
@@ skipping 56 matching lines @@
     return nullptr;
 
   for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray) {
     if (pAnnot == pFocusAnnot)
       return pAnnot;
   }
   return nullptr;
 }
 
 int CPDFSDK_PageView::GetPageIndexForStaticPDF() const {
-#ifdef PDF_ENABLE_XFA
-  CPDF_Page* pPage = m_page->GetPDFPage();
-#else   // PDF_ENABLE_XFA
-  CPDF_Page* pPage = m_page;
-#endif  // PDF_ENABLE_XFA
-  CPDF_Dictionary* pDict = pPage->m_pFormDict;
+  CPDF_Dictionary* pDict = GetPDFPage()->m_pFormDict;
   CPDF_Document* pDoc = m_pSDKDoc->GetPDFDocument();
   return (pDoc && pDict) ? pDoc->GetPageIndex(pDict->GetObjNum()) : -1;
 }