commit 2ffc628f4573a533f9a674ba2aa8e5b423e0669c
author Lei Zhang <thestig@chromium.org> Fri Aug 16 16:59:23 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Fri Aug 16 16:59:23 2019 +0000
tree 91fa3368102a89af20af2d0758a9a64cdb3d741a
parent b3f93d4417c75f97f19e07aed8fa0243b8a6a0d3

Clean up CPDFSDK_AnnotHandlerMgr.

- Rename the XFA version of NewAnnot() to NewXFAAnnot().
- Make both GetAnnotHandler() methods private.
- Rename one GetAnnotHandler() method to GetAnnotHandlerOfType().
- Do not call GetAnnotHandlerOfType() when the callers wants a specific
  handler. Just access the handler directly.
- Remove impossible check for nullptr with the XFA annot handler.
- Mark unique_ptrs holding annot handlers const.

Change-Id: I5cc65f001ba71bb94e7cb320db0d60d86a8eb1fa
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/59550
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

fpdfsdk/cpdfsdk_annothandlermgr.cpp

diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
index 42cb5f0..48daf0f 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -40,22 +40,22 @@
 {
 }
 
-CPDFSDK_AnnotHandlerMgr::~CPDFSDK_AnnotHandlerMgr() {}
+CPDFSDK_AnnotHandlerMgr::~CPDFSDK_AnnotHandlerMgr() = default;
 
 CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::NewAnnot(CPDF_Annot* pAnnot,
                                                  CPDFSDK_PageView* pPageView) {
   ASSERT(pPageView);
-  return GetAnnotHandler(pAnnot->GetSubtype())->NewAnnot(pAnnot, pPageView);
+  return GetAnnotHandlerOfType(pAnnot->GetSubtype())
+      ->NewAnnot(pAnnot, pPageView);
 }
 
 #ifdef PDF_ENABLE_XFA
-CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::NewAnnot(CXFA_FFWidget* pAnnot,
-                                                 CPDFSDK_PageView* pPageView) {
+CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::NewXFAAnnot(
+    CXFA_FFWidget* pAnnot,
+    CPDFSDK_PageView* pPageView) {
   ASSERT(pAnnot);
   ASSERT(pPageView);
-  auto* pHandler = static_cast<CPDFXFA_WidgetHandler*>(
-      GetAnnotHandler(CPDF_Annot::Subtype::XFAWIDGET));
-  return pHandler->NewAnnotForXFA(pAnnot, pPageView);
+  return m_pXFAWidgetHandler->NewAnnotForXFA(pAnnot, pPageView);
 }
 #endif  // PDF_ENABLE_XFA
 
@@ -102,10 +102,10 @@
 
 IPDFSDK_AnnotHandler* CPDFSDK_AnnotHandlerMgr::GetAnnotHandler(
     CPDFSDK_Annot* pAnnot) const {
-  return GetAnnotHandler(pAnnot->GetAnnotSubtype());
+  return GetAnnotHandlerOfType(pAnnot->GetAnnotSubtype());
 }
 
-IPDFSDK_AnnotHandler* CPDFSDK_AnnotHandlerMgr::GetAnnotHandler(
+IPDFSDK_AnnotHandler* CPDFSDK_AnnotHandlerMgr::GetAnnotHandlerOfType(
     CPDF_Annot::Subtype nAnnotSubtype) const {
   if (nAnnotSubtype == CPDF_Annot::Subtype::WIDGET)
     return m_pWidgetHandler.get();
@@ -280,14 +280,7 @@
     ObservedPtr<CPDFSDK_Annot>* pKillAnnot) {
   bool bXFA = (*pSetAnnot && (*pSetAnnot)->GetXFAWidget()) ||
               (*pKillAnnot && (*pKillAnnot)->GetXFAWidget());
-
-  if (bXFA) {
-    auto* pXFAAnnotHandler = static_cast<CPDFXFA_WidgetHandler*>(
-        GetAnnotHandler(CPDF_Annot::Subtype::XFAWIDGET));
-    if (pXFAAnnotHandler)
-      return pXFAAnnotHandler->OnXFAChangedFocus(pKillAnnot, pSetAnnot);
-  }
-  return true;
+  return !bXFA || m_pXFAWidgetHandler->OnXFAChangedFocus(pKillAnnot, pSetAnnot);
 }
 #endif  // PDF_ENABLE_XFA
 

fpdfsdk/cpdfsdk_annothandlermgr.h

diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.h b/fpdfsdk/cpdfsdk_annothandlermgr.h
index 0444c7e..5c3c73a 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.h
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.h
@@ -33,7 +33,8 @@
 
   CPDFSDK_Annot* NewAnnot(CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView);
 #ifdef PDF_ENABLE_XFA
-  CPDFSDK_Annot* NewAnnot(CXFA_FFWidget* pAnnot, CPDFSDK_PageView* pPageView);
+  CPDFSDK_Annot* NewXFAAnnot(CXFA_FFWidget* pAnnot,
+                             CPDFSDK_PageView* pPageView);
 #endif  // PDF_ENABLE_XFA
   void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot);
 
@@ -48,7 +49,6 @@
   bool Annot_Undo(CPDFSDK_Annot* pAnnot);
   bool Annot_Redo(CPDFSDK_Annot* pAnnot);
 
-  IPDFSDK_AnnotHandler* GetAnnotHandler(CPDFSDK_Annot* pAnnot) const;
   void Annot_OnDraw(CPDFSDK_PageView* pPageView,
                     CPDFSDK_Annot* pAnnot,
                     CFX_RenderDevice* pDevice,
@@ -111,14 +111,15 @@
                        const CFX_PointF& point);
 
  private:
-  IPDFSDK_AnnotHandler* GetAnnotHandler(
+  IPDFSDK_AnnotHandler* GetAnnotHandler(CPDFSDK_Annot* pAnnot) const;
+  IPDFSDK_AnnotHandler* GetAnnotHandlerOfType(
       CPDF_Annot::Subtype nAnnotSubtype) const;
   CPDFSDK_Annot* GetNextAnnot(CPDFSDK_Annot* pSDKAnnot, bool bNext);
 
-  std::unique_ptr<CPDFSDK_BAAnnotHandler> m_pBAAnnotHandler;
-  std::unique_ptr<CPDFSDK_WidgetHandler> m_pWidgetHandler;
+  std::unique_ptr<CPDFSDK_BAAnnotHandler> const m_pBAAnnotHandler;
+  std::unique_ptr<CPDFSDK_WidgetHandler> const m_pWidgetHandler;
 #ifdef PDF_ENABLE_XFA
-  std::unique_ptr<CPDFXFA_WidgetHandler> m_pXFAWidgetHandler;
+  std::unique_ptr<CPDFXFA_WidgetHandler> const m_pXFAWidgetHandler;
 #endif  // PDF_ENABLE_XFA
 };
 

fpdfsdk/cpdfsdk_pageview.cpp

diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp
index 45ef17a..4d4bdea 100644
--- a/fpdfsdk/cpdfsdk_pageview.cpp
+++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -130,7 +130,7 @@
     return pSDKAnnot;
 
   CPDFSDK_AnnotHandlerMgr* pAnnotHandler = m_pFormFillEnv->GetAnnotHandlerMgr();
-  pSDKAnnot = pAnnotHandler->NewAnnot(pPDFAnnot, this);
+  pSDKAnnot = pAnnotHandler->NewXFAAnnot(pPDFAnnot, this);
   if (!pSDKAnnot)
     return nullptr;
 
@@ -488,7 +488,7 @@
     }
 
     while (CXFA_FFWidget* pXFAAnnot = pWidgetHandler->MoveToNext()) {
-      CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pXFAAnnot, this);
+      CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewXFAAnnot(pXFAAnnot, this);
       if (!pAnnot)
         continue;
       m_SDKAnnotArray.push_back(pAnnot);