Observe CFXA_FFWidget, part 2
Related to https://crbug.com/978575, but fires event upon losing
focus. Now that we can fix these with observables, it is a matter
of ensuring that the return is checked by all callers.
Bug: chromium:949032
Change-Id: Ia9ab0f84e2a06d93177008de9bf1316045b58c2e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/57190
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fxfa/cxfa_ffcombobox.cpp b/xfa/fxfa/cxfa_ffcombobox.cpp
index e943991..2efbdf8 100644
--- a/xfa/fxfa/cxfa_ffcombobox.cpp
+++ b/xfa/fxfa/cxfa_ffcombobox.cpp
@@ -106,8 +106,7 @@
if (!ProcessCommittedData())
UpdateFWLData();
- CXFA_FFField::OnKillFocus(pNewWidget);
- return true;
+ return CXFA_FFField::OnKillFocus(pNewWidget);
}
void CXFA_FFComboBox::OpenDropDownList() {
diff --git a/xfa/fxfa/cxfa_ffcombobox.h b/xfa/fxfa/cxfa_ffcombobox.h
index 96ed66c..78b7eb5 100644
--- a/xfa/fxfa/cxfa_ffcombobox.h
+++ b/xfa/fxfa/cxfa_ffcombobox.h
@@ -22,7 +22,7 @@
bool LoadWidget() override;
void UpdateWidgetProperty() override;
bool OnRButtonUp(uint32_t dwFlags, const CFX_PointF& point) override;
- bool OnKillFocus(CXFA_FFWidget* pNewWidget) override;
+ bool OnKillFocus(CXFA_FFWidget* pNewWidget) override WARN_UNUSED_RESULT;
bool CanUndo() override;
bool CanRedo() override;
bool Undo() override;
diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp
index 3308722..9016c55 100644
--- a/xfa/fxfa/cxfa_ffdocview.cpp
+++ b/xfa/fxfa/cxfa_ffdocview.cpp
@@ -284,8 +284,10 @@
pOldFocus = nullptr;
}
}
- if (pOldFocus)
- pOldFocus->OnKillFocus(pNewFocus);
+ if (pOldFocus) {
+ if (!pOldFocus->OnKillFocus(pNewFocus))
+ return false;
+ }
if (pNewFocus) {
if (pNewFocus->GetLayoutItem()->TestStatusBits(XFA_WidgetStatus_Visible)) {
diff --git a/xfa/fxfa/cxfa_fffield.cpp b/xfa/fxfa/cxfa_fffield.cpp
index b43fafc..7a6f845 100644
--- a/xfa/fxfa/cxfa_fffield.cpp
+++ b/xfa/fxfa/cxfa_fffield.cpp
@@ -513,15 +513,13 @@
}
bool CXFA_FFField::OnKillFocus(CXFA_FFWidget* pNewWidget) {
- if (!m_pNormalWidget)
- return CXFA_FFWidget::OnKillFocus(pNewWidget);
-
- CFWL_MessageKillFocus ms(nullptr, m_pNormalWidget.get());
- TranslateFWLMessage(&ms);
- GetLayoutItem()->ClearStatusBits(XFA_WidgetStatus_Focused);
- InvalidateRect();
- CXFA_FFWidget::OnKillFocus(pNewWidget);
- return true;
+ if (m_pNormalWidget) {
+ CFWL_MessageKillFocus ms(nullptr, m_pNormalWidget.get());
+ TranslateFWLMessage(&ms);
+ GetLayoutItem()->ClearStatusBits(XFA_WidgetStatus_Focused);
+ InvalidateRect();
+ }
+ return CXFA_FFWidget::OnKillFocus(pNewWidget);
}
bool CXFA_FFField::OnKeyDown(uint32_t dwKeyCode, uint32_t dwFlags) {
diff --git a/xfa/fxfa/cxfa_fffield.h b/xfa/fxfa/cxfa_fffield.h
index 4e89eb6b..1e2b9db 100644
--- a/xfa/fxfa/cxfa_fffield.h
+++ b/xfa/fxfa/cxfa_fffield.h
@@ -49,9 +49,8 @@
void OnRButtonDown(uint32_t dwFlags, const CFX_PointF& point) override;
bool OnRButtonUp(uint32_t dwFlags, const CFX_PointF& point) override;
bool OnRButtonDblClk(uint32_t dwFlags, const CFX_PointF& point) override;
-
- bool OnSetFocus(CXFA_FFWidget* pOldWidget) override;
- bool OnKillFocus(CXFA_FFWidget* pNewWidget) override;
+ bool OnSetFocus(CXFA_FFWidget* pOldWidget) override WARN_UNUSED_RESULT;
+ bool OnKillFocus(CXFA_FFWidget* pNewWidget) override WARN_UNUSED_RESULT;
bool OnKeyDown(uint32_t dwKeyCode, uint32_t dwFlags) override;
bool OnKeyUp(uint32_t dwKeyCode, uint32_t dwFlags) override;
bool OnChar(uint32_t dwChar, uint32_t dwFlags) override;
diff --git a/xfa/fxfa/cxfa_fflistbox.cpp b/xfa/fxfa/cxfa_fflistbox.cpp
index 2fa9a7c..4643de3 100644
--- a/xfa/fxfa/cxfa_fflistbox.cpp
+++ b/xfa/fxfa/cxfa_fflistbox.cpp
@@ -75,8 +75,7 @@
if (!ProcessCommittedData())
UpdateFWLData();
- CXFA_FFField::OnKillFocus(pNewFocus);
- return true;
+ return CXFA_FFField::OnKillFocus(pNewFocus);
}
bool CXFA_FFListBox::CommitData() {
diff --git a/xfa/fxfa/cxfa_fflistbox.h b/xfa/fxfa/cxfa_fflistbox.h
index 47d87d9..f9fa801 100644
--- a/xfa/fxfa/cxfa_fflistbox.h
+++ b/xfa/fxfa/cxfa_fflistbox.h
@@ -15,9 +15,9 @@
explicit CXFA_FFListBox(CXFA_Node* pNode);
~CXFA_FFListBox() override;
- // CXFA_FFField
+ // CXFA_FFField:
bool LoadWidget() override;
- bool OnKillFocus(CXFA_FFWidget* pNewWidget) override;
+ bool OnKillFocus(CXFA_FFWidget* pNewWidget) override WARN_UNUSED_RESULT;
void OnProcessMessage(CFWL_Message* pMessage) override;
void OnProcessEvent(CFWL_Event* pEvent) override;
void OnDrawWidget(CXFA_Graphics* pGraphics,
diff --git a/xfa/fxfa/cxfa_fftextedit.cpp b/xfa/fxfa/cxfa_fftextedit.cpp
index 907ce99..da52cf5 100644
--- a/xfa/fxfa/cxfa_fftextedit.cpp
+++ b/xfa/fxfa/cxfa_fftextedit.cpp
@@ -164,22 +164,28 @@
UpdateFWLData();
InvalidateRect();
}
- CXFA_FFWidget::OnSetFocus(pOldWidget);
+ if (!CXFA_FFWidget::OnSetFocus(pOldWidget))
+ return false;
+
CFWL_MessageSetFocus ms(nullptr, m_pNormalWidget.get());
TranslateFWLMessage(&ms);
return true;
}
bool CXFA_FFTextEdit::OnKillFocus(CXFA_FFWidget* pNewWidget) {
- CFWL_MessageKillFocus ms(nullptr, m_pNormalWidget.get());
- TranslateFWLMessage(&ms);
+ {
+ // Message can't outlive the OnKillFocus call.
+ CFWL_MessageKillFocus ms(nullptr, m_pNormalWidget.get());
+ TranslateFWLMessage(&ms);
+ }
GetLayoutItem()->ClearStatusBits(XFA_WidgetStatus_Focused);
SetEditScrollOffset();
ProcessCommittedData();
UpdateFWLData();
InvalidateRect();
- CXFA_FFWidget::OnKillFocus(pNewWidget);
+ if (!CXFA_FFWidget::OnKillFocus(pNewWidget))
+ return false;
GetLayoutItem()->ClearStatusBits(XFA_WidgetStatus_TextEditValueChanged);
return true;
diff --git a/xfa/fxfa/cxfa_fftextedit.h b/xfa/fxfa/cxfa_fftextedit.h
index 37604c1..ad49626 100644
--- a/xfa/fxfa/cxfa_fftextedit.h
+++ b/xfa/fxfa/cxfa_fftextedit.h
@@ -33,8 +33,8 @@
void OnLButtonDown(uint32_t dwFlags, const CFX_PointF& point) override;
void OnRButtonDown(uint32_t dwFlags, const CFX_PointF& point) override;
bool OnRButtonUp(uint32_t dwFlags, const CFX_PointF& point) override;
- bool OnSetFocus(CXFA_FFWidget* pOldWidget) override;
- bool OnKillFocus(CXFA_FFWidget* pNewWidget) override;
+ bool OnSetFocus(CXFA_FFWidget* pOldWidget) override WARN_UNUSED_RESULT;
+ bool OnKillFocus(CXFA_FFWidget* pNewWidget) override WARN_UNUSED_RESULT;
void OnProcessMessage(CFWL_Message* pMessage) override;
void OnProcessEvent(CFWL_Event* pEvent) override;
void OnDrawWidget(CXFA_Graphics* pGraphics,
diff --git a/xfa/fxfa/cxfa_ffwidget.cpp b/xfa/fxfa/cxfa_ffwidget.cpp
index ebcac99..6bc684d 100644
--- a/xfa/fxfa/cxfa_ffwidget.cpp
+++ b/xfa/fxfa/cxfa_ffwidget.cpp
@@ -397,9 +397,10 @@
// OnSetFocus event may remove this widget.
ObservedPtr<CXFA_FFWidget> pWatched(this);
CXFA_FFWidget* pParent = GetFFWidget(ToContentLayoutItem(GetParent()));
- if (pParent && !pParent->IsAncestorOf(pOldWidget))
- pParent->OnSetFocus(pOldWidget);
-
+ if (pParent && !pParent->IsAncestorOf(pOldWidget)) {
+ if (!pParent->OnSetFocus(pOldWidget))
+ return false;
+ }
if (!pWatched)
return false;
@@ -416,14 +417,23 @@
}
bool CXFA_FFWidget::OnKillFocus(CXFA_FFWidget* pNewWidget) {
+ // OnKillFocus event may remove this widget.
+ ObservedPtr<CXFA_FFWidget> pWatched(this);
GetLayoutItem()->ClearStatusBits(XFA_WidgetStatus_Focused);
EventKillFocus();
+ if (!pWatched)
+ return false;
+
if (!pNewWidget)
return true;
CXFA_FFWidget* pParent = GetFFWidget(ToContentLayoutItem(GetParent()));
- if (pParent && !pParent->IsAncestorOf(pNewWidget))
- pParent->OnKillFocus(pNewWidget);
+ if (pParent && !pParent->IsAncestorOf(pNewWidget)) {
+ if (!pParent->OnKillFocus(pNewWidget))
+ return false;
+ }
+ if (!pWatched)
+ return false;
return true;
}
diff --git a/xfa/fxfa/cxfa_ffwidget.h b/xfa/fxfa/cxfa_ffwidget.h
index ed868e2..c58e084 100644
--- a/xfa/fxfa/cxfa_ffwidget.h
+++ b/xfa/fxfa/cxfa_ffwidget.h
@@ -102,8 +102,10 @@
virtual bool OnRButtonUp(uint32_t dwFlags, const CFX_PointF& point);
virtual bool OnRButtonDblClk(uint32_t dwFlags, const CFX_PointF& point);
- virtual bool OnSetFocus(CXFA_FFWidget* pOldWidget);
- virtual bool OnKillFocus(CXFA_FFWidget* pNewWidget);
+ // Returning false may imply |this| is gone, continue with caution.
+ virtual bool OnSetFocus(CXFA_FFWidget* pOldWidget) WARN_UNUSED_RESULT;
+ virtual bool OnKillFocus(CXFA_FFWidget* pNewWidget) WARN_UNUSED_RESULT;
+
virtual bool OnKeyDown(uint32_t dwKeyCode, uint32_t dwFlags);
virtual bool OnKeyUp(uint32_t dwKeyCode, uint32_t dwFlags);
virtual bool OnChar(uint32_t dwChar, uint32_t dwFlags);
