Once observed, stop using underlying raw members.
Small bit of defensive programming, i.e. consider the value borrowed.
Observe as early as possible to future-proof the code.
Change-Id: I96eb04a98a0088b549b763c12096f5219c9c0faa
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119319
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp
index 0aab9db..02b8e0f 100644
--- a/fpdfsdk/cpdfsdk_pageview.cpp
+++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -154,31 +154,31 @@
}
void CPDFSDK_PageView::DeleteAnnotForFFWidget(CXFA_FFWidget* pWidget) {
- CPDFSDK_Annot* pAnnot = GetAnnotForFFWidget(pWidget);
- if (!pAnnot)
+ ObservedPtr<CPDFSDK_Annot> pAnnot(GetAnnotForFFWidget(pWidget));
+ if (!pAnnot) {
return;
-
+ }
IPDF_Page* pPage = pAnnot->GetXFAPage();
- if (!pPage)
+ if (!pPage) {
return;
-
+ }
CPDF_Document::Extension* pContext = pPage->GetDocument()->GetExtension();
- if (pContext && !pContext->ContainsExtensionForm())
+ if (pContext && !pContext->ContainsExtensionForm()) {
return;
-
- ObservedPtr<CPDFSDK_Annot> pObserved(pAnnot);
- if (GetFocusAnnot() == pAnnot)
- m_pFormFillEnv->KillFocusAnnot({}); // May invoke JS, invalidating pAnnot.
-
- if (pObserved) {
+ }
+ if (GetFocusAnnot() == pAnnot) {
+ // May invoke JS, invalidating pAnnot.
+ m_pFormFillEnv->KillFocusAnnot({});
+ }
+ if (pAnnot) {
auto it = std::find(m_SDKAnnotArray.begin(), m_SDKAnnotArray.end(),
- fxcrt::MakeFakeUniquePtr(pAnnot));
+ fxcrt::MakeFakeUniquePtr(pAnnot.Get()));
if (it != m_SDKAnnotArray.end())
m_SDKAnnotArray.erase(it);
}
-
- if (m_pCaptureWidget.Get() == pAnnot)
+ if (m_pCaptureWidget.Get() == pAnnot) {
m_pCaptureWidget.Reset();
+ }
}
CPDFXFA_Page* CPDFSDK_PageView::XFAPageIfNotBackedByPDFPage() {
diff --git a/fpdfsdk/formfiller/cffl_checkbox.cpp b/fpdfsdk/formfiller/cffl_checkbox.cpp
index 241ad4c..3366067 100644
--- a/fpdfsdk/formfiller/cffl_checkbox.cpp
+++ b/fpdfsdk/formfiller/cffl_checkbox.cpp
@@ -87,12 +87,12 @@
if (!IsValid()) {
return true;
}
- CPWL_CheckBox* pWnd = CreateOrUpdatePWLCheckBox(pPageView);
+ ObservedPtr<CPWL_CheckBox> pWnd(CreateOrUpdatePWLCheckBox(pPageView));
if (pWnd) {
- ObservedPtr<CPWL_CheckBox> pObservedBox(pWnd);
+ // IsChecked() may invalidate check box.
const bool is_checked = pWidget->IsChecked();
- if (pObservedBox) {
- pObservedBox->SetCheck(!is_checked);
+ if (pWnd) {
+ pWnd->SetCheck(!is_checked);
}
}
return CommitData(pPageView, nFlags);
@@ -105,20 +105,22 @@
void CFFL_CheckBox::SaveData(const CPDFSDK_PageView* pPageView) {
CPWL_CheckBox* pWnd = GetPWLCheckBox(pPageView);
- if (!pWnd)
+ if (!pWnd) {
return;
-
+ }
bool bNewChecked = pWnd->IsChecked();
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
ObservedPtr<CFFL_CheckBox> observed_this(this);
- m_pWidget->SetCheck(bNewChecked);
- if (!observed_widget)
+ observed_widget->SetCheck(bNewChecked);
+ if (!observed_widget) {
return;
-
- m_pWidget->UpdateField();
- if (!observed_widget || !observed_this)
- return;
-
+ }
+ observed_widget->UpdateField();
+ {
+ if (!observed_widget || !observed_this) {
+ return;
+ }
+ }
SetChangeMark();
}
diff --git a/fpdfsdk/formfiller/cffl_combobox.cpp b/fpdfsdk/formfiller/cffl_combobox.cpp
index a95574b..163c3ca 100644
--- a/fpdfsdk/formfiller/cffl_combobox.cpp
+++ b/fpdfsdk/formfiller/cffl_combobox.cpp
@@ -81,31 +81,32 @@
void CFFL_ComboBox::SaveData(const CPDFSDK_PageView* pPageView) {
CPWL_ComboBox* pWnd = GetPWLComboBox(pPageView);
- if (!pWnd)
+ if (!pWnd) {
return;
-
+ }
WideString swText = pWnd->GetText();
int32_t nCurSel = pWnd->GetSelect();
bool bSetValue = false;
- if (m_pWidget->GetFieldFlags() & pdfium::form_flags::kChoiceEdit)
- bSetValue = (nCurSel < 0) || (swText != m_pWidget->GetOptionLabel(nCurSel));
-
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
+ if (observed_widget->GetFieldFlags() & pdfium::form_flags::kChoiceEdit) {
+ bSetValue =
+ (nCurSel < 0) || (swText != observed_widget->GetOptionLabel(nCurSel));
+ }
if (bSetValue) {
- m_pWidget->SetValue(swText);
+ observed_widget->SetValue(swText);
} else {
- m_pWidget->GetSelectedIndex(0);
- m_pWidget->SetOptionSelection(nCurSel);
+ observed_widget->GetSelectedIndex(0);
+ observed_widget->SetOptionSelection(nCurSel);
}
if (!observed_widget) {
return;
}
ObservedPtr<CFFL_ComboBox> observed_this(this);
- m_pWidget->ResetFieldAppearance();
+ observed_widget->ResetFieldAppearance();
if (!observed_widget) {
return;
}
- m_pWidget->UpdateField();
+ observed_widget->UpdateField();
if (!observed_widget || !observed_this) {
return;
}
diff --git a/fpdfsdk/formfiller/cffl_listbox.cpp b/fpdfsdk/formfiller/cffl_listbox.cpp
index c9e65d0..156ce57 100644
--- a/fpdfsdk/formfiller/cffl_listbox.cpp
+++ b/fpdfsdk/formfiller/cffl_listbox.cpp
@@ -111,35 +111,36 @@
int32_t nNewTopIndex = pListBox->GetTopVisibleIndex();
ObservedPtr<CPWL_ListBox> observed_box(pListBox);
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
- m_pWidget->ClearSelection();
+ observed_widget->ClearSelection();
if (!observed_box || !observed_widget) {
return;
}
- if (m_pWidget->GetFieldFlags() & pdfium::form_flags::kChoiceMultiSelect) {
- for (int32_t i = 0, sz = pListBox->GetCount(); i < sz; i++) {
- if (pListBox->IsItemSelected(i)) {
- m_pWidget->SetOptionSelection(i);
+ if (observed_widget->GetFieldFlags() &
+ pdfium::form_flags::kChoiceMultiSelect) {
+ for (int32_t i = 0, sz = observed_box->GetCount(); i < sz; i++) {
+ if (observed_box->IsItemSelected(i)) {
+ observed_widget->SetOptionSelection(i);
if (!observed_box || !observed_widget) {
return;
}
}
}
} else {
- m_pWidget->SetOptionSelection(pListBox->GetCurSel());
+ observed_widget->SetOptionSelection(observed_box->GetCurSel());
if (!observed_box || !observed_widget) {
return;
}
}
ObservedPtr<CFFL_ListBox> observed_this(this);
- m_pWidget->SetTopVisibleIndex(nNewTopIndex);
+ observed_widget->SetTopVisibleIndex(nNewTopIndex);
if (!observed_widget) {
return;
}
- m_pWidget->ResetFieldAppearance();
+ observed_widget->ResetFieldAppearance();
if (!observed_widget) {
return;
}
- m_pWidget->UpdateField();
+ observed_widget->UpdateField();
if (!observed_widget || !observed_this) {
return;
}
diff --git a/fpdfsdk/formfiller/cffl_radiobutton.cpp b/fpdfsdk/formfiller/cffl_radiobutton.cpp
index 5ced9d1..aeb1287 100644
--- a/fpdfsdk/formfiller/cffl_radiobutton.cpp
+++ b/fpdfsdk/formfiller/cffl_radiobutton.cpp
@@ -97,11 +97,11 @@
bool bNewChecked = pWnd->IsChecked();
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
ObservedPtr<CFFL_RadioButton> observed_this(this);
- m_pWidget->SetCheck(bNewChecked);
+ observed_widget->SetCheck(bNewChecked);
if (!observed_widget)
return;
- m_pWidget->UpdateField();
+ observed_widget->UpdateField();
if (!observed_widget || !observed_this)
return;
diff --git a/fpdfsdk/formfiller/cffl_textfield.cpp b/fpdfsdk/formfiller/cffl_textfield.cpp
index 15a3227..862ca5e 100644
--- a/fpdfsdk/formfiller/cffl_textfield.cpp
+++ b/fpdfsdk/formfiller/cffl_textfield.cpp
@@ -153,15 +153,15 @@
WideString sNewValue = observed_edit->GetText();
ObservedPtr<CPDFSDK_Widget> observed_widget(m_pWidget);
ObservedPtr<CFFL_TextField> observed_this(this);
- m_pWidget->SetValue(sNewValue);
+ observed_widget->SetValue(sNewValue);
if (!observed_widget) {
return;
}
- m_pWidget->ResetFieldAppearance();
+ observed_widget->ResetFieldAppearance();
if (!observed_widget) {
return;
}
- m_pWidget->UpdateField();
+ observed_widget->UpdateField();
if (!observed_widget || !observed_this) {
return;
}
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp
index 784b572..59efa8e 100644
--- a/fxjs/cjs_field.cpp
+++ b/fxjs/cjs_field.cpp
@@ -101,24 +101,25 @@
bool bResetAP) {
DCHECK(pFormControl);
CPDFSDK_InteractiveForm* pForm = pFormFillEnv->GetInteractiveForm();
- CPDFSDK_Widget* pWidget = pForm->GetWidget(pFormControl);
- if (pWidget) {
- ObservedPtr<CPDFSDK_Widget> observed_widget(pWidget);
+ ObservedPtr<CPDFSDK_Widget> observed_widget(pForm->GetWidget(pFormControl));
+ if (observed_widget) {
if (bResetAP) {
- FormFieldType fieldType = pWidget->GetFieldType();
+ FormFieldType fieldType = observed_widget->GetFieldType();
if (fieldType == FormFieldType::kComboBox ||
fieldType == FormFieldType::kTextField) {
- std::optional<WideString> sValue = pWidget->OnFormat();
+ std::optional<WideString> sValue = observed_widget->OnFormat();
if (!observed_widget)
return;
- pWidget->ResetAppearance(sValue, CPDFSDK_Widget::kValueUnchanged);
+ observed_widget->ResetAppearance(sValue,
+ CPDFSDK_Widget::kValueUnchanged);
} else {
- pWidget->ResetAppearance(std::nullopt, CPDFSDK_Widget::kValueUnchanged);
+ observed_widget->ResetAppearance(std::nullopt,
+ CPDFSDK_Widget::kValueUnchanged);
}
if (!observed_widget)
return;
}
- pFormFillEnv->UpdateAllViews(pWidget);
+ pFormFillEnv->UpdateAllViews(observed_widget.Get());
}
pFormFillEnv->SetChangeMark();
}
