commit 51d02661898ba339cae6562b061670d27b0d3501
author Lei Zhang <thestig@chromium.org> Thu Oct 31 18:28:51 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Oct 31 18:28:51 2019 +0000
tree 31d64721061c1554f08cad678195d5fe3d1669b0
parent e397fef547557aae1a01aff2e37f3f33d45ae313

Make CPDFXFA_WidgetHandler::NewAnnotForXFA() return a std::unique_ptr.

Fix its call stack to better indicate ownership, and remove impossible
checks as well.

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

fpdfsdk/cpdfsdk_annothandlermgr.cpp

diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
index b077532..2a45b0d 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -58,7 +58,7 @@
 }
 
 #ifdef PDF_ENABLE_XFA
-CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::NewXFAAnnot(
+std::unique_ptr<CPDFSDK_Annot> CPDFSDK_AnnotHandlerMgr::NewXFAAnnot(
     CXFA_FFWidget* pAnnot,
     CPDFSDK_PageView* pPageView) {
   ASSERT(pAnnot);

fpdfsdk/cpdfsdk_annothandlermgr.h

diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.h b/fpdfsdk/cpdfsdk_annothandlermgr.h
index 5d70f00..2136555 100644
--- a/fpdfsdk/cpdfsdk_annothandlermgr.h
+++ b/fpdfsdk/cpdfsdk_annothandlermgr.h
@@ -38,8 +38,8 @@
 
   CPDFSDK_Annot* NewAnnot(CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView);
 #ifdef PDF_ENABLE_XFA
-  CPDFSDK_Annot* NewXFAAnnot(CXFA_FFWidget* pAnnot,
-                             CPDFSDK_PageView* pPageView);
+  std::unique_ptr<CPDFSDK_Annot> NewXFAAnnot(CXFA_FFWidget* pAnnot,
+                                             CPDFSDK_PageView* pPageView);
 #endif  // PDF_ENABLE_XFA
   void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot);
 

fpdfsdk/cpdfsdk_pageview.cpp

diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp
index 7b878f7..9c7159e 100644
--- a/fpdfsdk/cpdfsdk_pageview.cpp
+++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -131,11 +131,13 @@
     return pSDKAnnot;
 
   CPDFSDK_AnnotHandlerMgr* pAnnotHandler = m_pFormFillEnv->GetAnnotHandlerMgr();
-  pSDKAnnot = pAnnotHandler->NewXFAAnnot(pPDFAnnot, this);
-  if (!pSDKAnnot)
-    return nullptr;
-
-  m_SDKAnnotArray.push_back(pSDKAnnot);
+  std::unique_ptr<CPDFSDK_Annot> pNewAnnot =
+      pAnnotHandler->NewXFAAnnot(pPDFAnnot, this);
+  ASSERT(pNewAnnot);
+  pSDKAnnot = pNewAnnot.get();
+  // TODO(thestig): See if |m_SDKAnnotArray|, which takes ownership of
+  // |pNewAnnot|, can hold std::unique_ptrs instead of raw pointers.
+  m_SDKAnnotArray.push_back(pNewAnnot.release());
   return pSDKAnnot;
 }
 
@@ -492,10 +494,11 @@
     }
 
     while (CXFA_FFWidget* pXFAAnnot = pWidgetHandler->MoveToNext()) {
-      CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewXFAAnnot(pXFAAnnot, this);
-      if (!pAnnot)
-        continue;
-      m_SDKAnnotArray.push_back(pAnnot);
+      std::unique_ptr<CPDFSDK_Annot> pNewAnnot =
+          pAnnotHandlerMgr->NewXFAAnnot(pXFAAnnot, this);
+      ASSERT(pNewAnnot);
+      CPDFSDK_Annot* pAnnot = pNewAnnot.get();
+      m_SDKAnnotArray.push_back(pNewAnnot.release());
       pAnnotHandlerMgr->Annot_OnLoad(pAnnot);
     }
 

fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp

diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
index b2bc251..8251b5d 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
@@ -219,10 +219,11 @@
   return nullptr;
 }
 
-CPDFSDK_Annot* CPDFXFA_WidgetHandler::NewAnnotForXFA(CXFA_FFWidget* pAnnot,
-                                                     CPDFSDK_PageView* pPage) {
+std::unique_ptr<CPDFSDK_Annot> CPDFXFA_WidgetHandler::NewAnnotForXFA(
+    CXFA_FFWidget* pAnnot,
+    CPDFSDK_PageView* pPage) {
   CPDFSDK_InteractiveForm* pForm = m_pFormFillEnv->GetInteractiveForm();
-  return new CPDFXFA_Widget(pAnnot, pPage, pForm);
+  return pdfium::MakeUnique<CPDFXFA_Widget>(pAnnot, pPage, pForm);
 }
 
 void CPDFXFA_WidgetHandler::OnDraw(CPDFSDK_PageView* pPageView,

fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h

diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h
index 9926052..e18e0ca 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h
@@ -100,7 +100,8 @@
                         bool selected) override;
   bool IsIndexSelected(ObservedPtr<CPDFSDK_Annot>* pAnnot, int index) override;
 
-  CPDFSDK_Annot* NewAnnotForXFA(CXFA_FFWidget* pAnnot, CPDFSDK_PageView* pPage);
+  std::unique_ptr<CPDFSDK_Annot> NewAnnotForXFA(CXFA_FFWidget* pAnnot,
+                                                CPDFSDK_PageView* pPage);
   bool OnXFAChangedFocus(ObservedPtr<CPDFSDK_Annot>* pOldAnnot,
                          ObservedPtr<CPDFSDK_Annot>* pNewAnnot);