commit 0844b9a3b6259ff2af9e77425aa4bd3dcf71d861
author Lei Zhang <thestig@chromium.org> Wed Feb 12 18:10:05 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Feb 12 18:10:05 2020 +0000
tree c9df875cd12aef0da35307326d0f98b80b0dbcd8
parent 5930498d7fe26d0e0c2525a0323aac2f896dc71d

Prevent destruction of CXFA_FFWidget across FWL events.

The current strategy of using ObservedPtr lets the destruction happen,
and then dealing with the destruction. Instead, use RetainPtr on the
CXFA_ContentLayoutItem that owns CXFA_FFWidget to prevent destruction
altogether for the duration of the events.

Bug: chromium:1003501,chromium:1039629
Change-Id: I5d185c752b93904fafb060e13fc18bb5e3ddee52
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/66370
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

xfa/fxfa/cxfa_ffcheckbutton.cpp

diff --git a/xfa/fxfa/cxfa_ffcheckbutton.cpp b/xfa/fxfa/cxfa_ffcheckbutton.cpp
index 1d39e2f..e9eb2bc 100644
--- a/xfa/fxfa/cxfa_ffcheckbutton.cpp
+++ b/xfa/fxfa/cxfa_ffcheckbutton.cpp
@@ -303,6 +303,9 @@
 }
 
 void CXFA_FFCheckButton::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   CXFA_FFField::OnProcessEvent(pEvent);
   switch (pEvent->GetType()) {
     case CFWL_Event::Type::CheckStateChanged: {

xfa/fxfa/cxfa_ffcombobox.cpp

diff --git a/xfa/fxfa/cxfa_ffcombobox.cpp b/xfa/fxfa/cxfa_ffcombobox.cpp
index 61fa659..79a750f 100644
--- a/xfa/fxfa/cxfa_ffcombobox.cpp
+++ b/xfa/fxfa/cxfa_ffcombobox.cpp
@@ -346,7 +346,9 @@
 }
 
 void CXFA_FFComboBox::OnProcessEvent(CFWL_Event* pEvent) {
-  ObservedPtr<CXFA_FFComboBox> watched(this);
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   CXFA_FFField::OnProcessEvent(pEvent);
   switch (pEvent->GetType()) {
     case CFWL_Event::Type::SelectChanged: {
@@ -370,8 +372,7 @@
     default:
       break;
   }
-  if (watched)
-    m_pOldDelegate->OnProcessEvent(pEvent);
+  m_pOldDelegate->OnProcessEvent(pEvent);
 }
 
 void CXFA_FFComboBox::OnDrawWidget(CXFA_Graphics* pGraphics,

xfa/fxfa/cxfa_ffdatetimeedit.cpp

diff --git a/xfa/fxfa/cxfa_ffdatetimeedit.cpp b/xfa/fxfa/cxfa_ffdatetimeedit.cpp
index d8d57f5..5f74ec0 100644
--- a/xfa/fxfa/cxfa_ffdatetimeedit.cpp
+++ b/xfa/fxfa/cxfa_ffdatetimeedit.cpp
@@ -208,6 +208,9 @@
 }
 
 void CXFA_FFDateTimeEdit::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   if (pEvent->GetType() == CFWL_Event::Type::SelectChanged) {
     auto* event = static_cast<CFWL_EventSelectChanged*>(pEvent);
     OnSelectChanged(GetNormalWidget(), event->iYear, event->iMonth,

xfa/fxfa/cxfa_ffimageedit.cpp

diff --git a/xfa/fxfa/cxfa_ffimageedit.cpp b/xfa/fxfa/cxfa_ffimageedit.cpp
index 60f584a..5d5a69f 100644
--- a/xfa/fxfa/cxfa_ffimageedit.cpp
+++ b/xfa/fxfa/cxfa_ffimageedit.cpp
@@ -137,6 +137,9 @@
 }
 
 void CXFA_FFImageEdit::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   CXFA_FFField::OnProcessEvent(pEvent);
   m_pOldDelegate->OnProcessEvent(pEvent);
 }

xfa/fxfa/cxfa_fflistbox.cpp

diff --git a/xfa/fxfa/cxfa_fflistbox.cpp b/xfa/fxfa/cxfa_fflistbox.cpp
index 2720514..df7955e 100644
--- a/xfa/fxfa/cxfa_fflistbox.cpp
+++ b/xfa/fxfa/cxfa_fflistbox.cpp
@@ -193,6 +193,9 @@
 }
 
 void CXFA_FFListBox::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   CXFA_FFField::OnProcessEvent(pEvent);
   switch (pEvent->GetType()) {
     case CFWL_Event::Type::SelectChanged:

xfa/fxfa/cxfa_ffnumericedit.cpp

diff --git a/xfa/fxfa/cxfa_ffnumericedit.cpp b/xfa/fxfa/cxfa_ffnumericedit.cpp
index 8d1b3bb..ae6a749 100644
--- a/xfa/fxfa/cxfa_ffnumericedit.cpp
+++ b/xfa/fxfa/cxfa_ffnumericedit.cpp
@@ -69,6 +69,9 @@
 }
 
 void CXFA_FFNumericEdit::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   if (pEvent->GetType() == CFWL_Event::Type::Validate) {
     CFWL_EventValidate* event = static_cast<CFWL_EventValidate*>(pEvent);
     event->bValidate = OnValidate(GetNormalWidget(), event->wsInsert);

xfa/fxfa/cxfa_ffpushbutton.cpp

diff --git a/xfa/fxfa/cxfa_ffpushbutton.cpp b/xfa/fxfa/cxfa_ffpushbutton.cpp
index 7f2a06e..d7e1f34 100644
--- a/xfa/fxfa/cxfa_ffpushbutton.cpp
+++ b/xfa/fxfa/cxfa_ffpushbutton.cpp
@@ -195,6 +195,9 @@
 }
 
 void CXFA_FFPushButton::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   m_pOldDelegate->OnProcessEvent(pEvent);
   CXFA_FFField::OnProcessEvent(pEvent);
 }

xfa/fxfa/cxfa_fftextedit.cpp

diff --git a/xfa/fxfa/cxfa_fftextedit.cpp b/xfa/fxfa/cxfa_fftextedit.cpp
index 7b1c93b..52ce7ab 100644
--- a/xfa/fxfa/cxfa_fftextedit.cpp
+++ b/xfa/fxfa/cxfa_fftextedit.cpp
@@ -357,6 +357,9 @@
 }
 
 void CXFA_FFTextEdit::OnProcessEvent(CFWL_Event* pEvent) {
+  // Prevents destruction of the CXFA_ContentLayoutItem that owns |this|.
+  RetainPtr<CXFA_ContentLayoutItem> retain_layout(m_pLayoutItem.Get());
+
   CXFA_FFField::OnProcessEvent(pEvent);
   switch (pEvent->GetType()) {
     case CFWL_Event::Type::TextWillChange: