commit a0d85587ff7212e24d1df8a6451c49f5eaa171d6
author Tom Sepez <tsepez@chromium.org> Mon May 20 22:25:24 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon May 20 22:25:24 2024 +0000
tree 2e374863d31da45e7739587360cc7cc90e365ea6
parent b67b048d671fdbd9a3674a4d1cf8ba4069994bb1

Defensive programming around ObservedPtr<CPDFSDK_Annot>().

Such an argument is a strong hint to the caller that the object may
be destroyed somewhere in the called function, but be careful even
within the called function to not extract a raw pointer from it.

-- Change some x.Reset(y.Get()) usage to assignment to cut down on
   number of (often dubious) Get() calls.

Bug: 341313077
Change-Id: I572a2c5093f110ee04dbd2e82e3994a8266d5422
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119350
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>

fpdfsdk/cpdfsdk_formfillenvironment.cpp

diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index ec4ca48..f7a26ba 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -399,20 +399,19 @@
 
 void CPDFSDK_FormFillEnvironment::OnCalculate(
     ObservedPtr<CPDFSDK_Annot>& pAnnot) {
-  CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot.Get());
-  if (pWidget)
+  ObservedPtr<CPDFSDK_Widget> pWidget(ToCPDFSDKWidget(pAnnot.Get()));
+  if (pWidget) {
     m_pInteractiveForm->OnCalculate(pWidget->GetFormField());
+  }
 }
 
 void CPDFSDK_FormFillEnvironment::OnFormat(ObservedPtr<CPDFSDK_Annot>& pAnnot) {
-  CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot.Get());
-  DCHECK(pWidget);
-
+  ObservedPtr<CPDFSDK_Widget> pWidget(ToCPDFSDKWidget(pAnnot.Get()));
   std::optional<WideString> sValue =
       m_pInteractiveForm->OnFormat(pWidget->GetFormField());
-  if (!pAnnot)
+  if (!pWidget) {
     return;
-
+  }
   if (sValue.has_value()) {
     m_pInteractiveForm->ResetFieldAppearance(pWidget->GetFormField(), sValue);
     m_pInteractiveForm->UpdateField(pWidget->GetFormField());
@@ -757,12 +756,13 @@
     return false;
 #endif  // PDF_ENABLE_XFA
 
-  if (!CPDFSDK_Annot::OnSetFocus(pAnnot, {}))
+  if (!CPDFSDK_Annot::OnSetFocus(pAnnot, {})) {
     return false;
-  if (m_pFocusAnnot)
+  }
+  if (m_pFocusAnnot) {
     return false;
-
-  m_pFocusAnnot.Reset(pAnnot.Get());
+  }
+  m_pFocusAnnot = pAnnot;
 
   // If we are not able to inform the client about the focus change, it
   // shouldn't be considered as failure.
@@ -778,7 +778,7 @@
   m_pFocusAnnot.Reset();
 
   if (!CPDFSDK_Annot::OnKillFocus(pFocusAnnot, nFlags)) {
-    m_pFocusAnnot.Reset(pFocusAnnot.Get());
+    m_pFocusAnnot = pFocusAnnot;
     return false;
   }
 
@@ -787,10 +787,10 @@
     return false;
 
   if (pFocusAnnot->GetAnnotSubtype() == CPDF_Annot::Subtype::WIDGET) {
-    CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pFocusAnnot.Get());
-    FormFieldType fieldType = pWidget->GetFieldType();
-    if (fieldType == FormFieldType::kTextField ||
-        fieldType == FormFieldType::kComboBox) {
+    const FormFieldType field_type =
+        ToCPDFSDKWidget(pFocusAnnot.Get())->GetFieldType();
+    if (field_type == FormFieldType::kTextField ||
+        field_type == FormFieldType::kComboBox) {
       OnSetFieldInputFocusInternal(WideString(), false);
     }
   }