commit a0d16d18d072ce77e639a09ed211340a2ad9034e
author Tom Sepez <tsepez@chromium.org> Tue Mar 14 21:18:45 2023 +0000
committer Lei Zhang <thestig@chromium.org> Tue Mar 14 21:18:45 2023 +0000
tree 6d59731f8ea7ff160647a94c6281573a5100170a
parent b15f1b6966496c72391a0e2a1e20b4e6c28536fc

M110: Observe CPWL_* object destruction across CPDFSDK_Widget methods

This is a simple fix to stop the symptoms while we investigate
how to avoid mutations at these points in the first place.

-- fix some nearby braces and annoying blank lines while at it.

Bug: chromium:1419831
Change-Id: I20c38806b91c7c0c9016bb1b567a04ce319243d8
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104397
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
(cherry picked from commit 63e3719f1ec20ee6db804b2b2d4b00680db18d9c)
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/104833
Auto-Submit: Tom Sepez <tsepez@chromium.org>

fpdfsdk/formfiller/cffl_checkbox.cpp

diff --git a/fpdfsdk/formfiller/cffl_checkbox.cpp b/fpdfsdk/formfiller/cffl_checkbox.cpp
index 9f2f620..527720e 100644
--- a/fpdfsdk/formfiller/cffl_checkbox.cpp
+++ b/fpdfsdk/formfiller/cffl_checkbox.cpp
@@ -65,9 +65,13 @@
       CFFL_FormField::OnChar(pWidget, nChar, nFlags);
 
       CPWL_CheckBox* pWnd = CreateOrUpdatePWLCheckBox(pPageView);
-      if (pWnd && !pWnd->IsReadOnly())
-        pWnd->SetCheck(!pWidget->IsChecked());
-
+      if (pWnd && !pWnd->IsReadOnly()) {
+        ObservedPtr<CPWL_CheckBox> pObservedBox(pWnd);
+        const bool is_checked = pWidget->IsChecked();
+        if (pObservedBox) {
+          pObservedBox->SetCheck(!is_checked);
+        }
+      }
       return CommitData(pPageView, nFlags);
     }
     default:
@@ -80,14 +84,17 @@
                                 Mask<FWL_EVENTFLAG> nFlags,
                                 const CFX_PointF& point) {
   CFFL_Button::OnLButtonUp(pPageView, pWidget, nFlags, point);
-
-  if (!IsValid())
+  if (!IsValid()) {
     return true;
-
+  }
   CPWL_CheckBox* pWnd = CreateOrUpdatePWLCheckBox(pPageView);
-  if (pWnd)
-    pWnd->SetCheck(!pWidget->IsChecked());
-
+  if (pWnd) {
+    ObservedPtr<CPWL_CheckBox> pObservedBox(pWnd);
+    const bool is_checked = pWidget->IsChecked();
+    if (pObservedBox) {
+      pObservedBox->SetCheck(!is_checked);
+    }
+  }
   return CommitData(pPageView, nFlags);
 }
 

fpdfsdk/formfiller/cffl_listbox.cpp

diff --git a/fpdfsdk/formfiller/cffl_listbox.cpp b/fpdfsdk/formfiller/cffl_listbox.cpp
index 88ce209..ea119ec 100644
--- a/fpdfsdk/formfiller/cffl_listbox.cpp
+++ b/fpdfsdk/formfiller/cffl_listbox.cpp
@@ -105,11 +105,15 @@
 
 void CFFL_ListBox::SaveData(const CPDFSDK_PageView* pPageView) {
   CPWL_ListBox* pListBox = GetPWLListBox(pPageView);
-  if (!pListBox)
+  if (!pListBox) {
     return;
-
+  }
   int32_t nNewTopIndex = pListBox->GetTopVisibleIndex();
+  ObservedPtr<CPWL_ListBox> observed_box(pListBox);
   m_pWidget->ClearSelection();
+  if (!observed_box) {
+    return;
+  }
   if (m_pWidget->GetFieldFlags() & pdfium::form_flags::kChoiceMultiSelect) {
     for (int32_t i = 0, sz = pListBox->GetCount(); i < sz; i++) {
       if (pListBox->IsItemSelected(i))
@@ -121,17 +125,17 @@
   ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
   ObservedPtr<CFFL_ListBox> observed_this(this);
   m_pWidget->SetTopVisibleIndex(nNewTopIndex);
-  if (!observed_widget)
+  if (!observed_widget) {
     return;
-
+  }
   m_pWidget->ResetFieldAppearance();
-  if (!observed_widget)
+  if (!observed_widget) {
     return;
-
+  }
   m_pWidget->UpdateField();
-  if (!observed_widget || !observed_this)
+  if (!observed_widget || !observed_this) {
     return;
-
+  }
   SetChangeMark();
 }
 

fpdfsdk/formfiller/cffl_textfield.cpp

diff --git a/fpdfsdk/formfiller/cffl_textfield.cpp b/fpdfsdk/formfiller/cffl_textfield.cpp
index cd6e9a1..8096c82 100644
--- a/fpdfsdk/formfiller/cffl_textfield.cpp
+++ b/fpdfsdk/formfiller/cffl_textfield.cpp
@@ -142,26 +142,29 @@
 }
 
 void CFFL_TextField::SaveData(const CPDFSDK_PageView* pPageView) {
-  CPWL_Edit* pWnd = GetPWLEdit(pPageView);
-  if (!pWnd)
+  ObservedPtr<CPWL_Edit> observed_edit(GetPWLEdit(pPageView));
+  if (!observed_edit) {
     return;
-
+  }
   WideString sOldValue = m_pWidget->GetValue();
-  WideString sNewValue = pWnd->GetText();
+  if (!observed_edit) {
+    return;
+  }
+  WideString sNewValue = observed_edit->GetText();
   ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
   ObservedPtr<CFFL_TextField> observed_this(this);
   m_pWidget->SetValue(sNewValue);
-  if (!observed_widget)
+  if (!observed_widget) {
     return;
-
+  }
   m_pWidget->ResetFieldAppearance();
-  if (!observed_widget)
+  if (!observed_widget) {
     return;
-
+  }
   m_pWidget->UpdateField();
-  if (!observed_widget || !observed_this)
+  if (!observed_widget || !observed_this) {
     return;
-
+  }
   SetChangeMark();
 }
 
@@ -172,9 +175,7 @@
     case CPDF_AAction::kKeyStroke:
       if (CPWL_Edit* pWnd = GetPWLEdit(pPageView)) {
         fa.bFieldFull = pWnd->IsTextFull();
-
         fa.sValue = pWnd->GetText();
-
         if (fa.bFieldFull) {
           fa.sChange.clear();
           fa.sChangeEx.clear();