commit 7d6ec19df654383bcdf418b2d8fc8da47400ae38
author Tom Sepez <tsepez@chromium.org> Tue Jul 02 19:13:21 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Jul 02 19:13:21 2019 +0000
tree 555aa736a30a433a571630961323a76089c32fba
parent f31c479491f7be38e3d7e41c5ea9d254c3fc1e83

Observe CXFA_FFWidgets across OnSetFocus() events.

Although ObservedPtrs are computationally expensive, the
distance between the free and the stale pointer includes a round
trip through JS and back to C++, so returning status through all
the intervening layers would be cumbersome.

Bug: chromium:978575
Change-Id: Id4dcb40fab3bddb9ede58b986569c7cfa91c4b87
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/57110
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

xfa/fxfa/cxfa_ffdocview.cpp

diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp
index 59bc5fe..3308722 100644
--- a/xfa/fxfa/cxfa_ffdocview.cpp
+++ b/xfa/fxfa/cxfa_ffdocview.cpp
@@ -280,17 +280,22 @@
         !pItem->TestStatusBits(XFA_WidgetStatus_Focused)) {
       if (!pOldFocus->IsLoaded())
         pOldFocus->LoadWidget();
-      pOldFocus->OnSetFocus(pOldFocus);
+      if (!pOldFocus->OnSetFocus(pOldFocus))
+        pOldFocus = nullptr;
     }
-    pOldFocus->OnKillFocus(pNewFocus);
   }
+  if (pOldFocus)
+    pOldFocus->OnKillFocus(pNewFocus);
 
   if (pNewFocus) {
     if (pNewFocus->GetLayoutItem()->TestStatusBits(XFA_WidgetStatus_Visible)) {
       if (!pNewFocus->IsLoaded())
         pNewFocus->LoadWidget();
-      pNewFocus->OnSetFocus(pOldFocus);
+      if (!pNewFocus->OnSetFocus(pOldFocus))
+        pNewFocus = nullptr;
     }
+  }
+  if (pNewFocus) {
     CXFA_Node* node = pNewFocus->GetNode();
     m_pFocusNode = node->IsWidgetReady() ? node : nullptr;
     m_pFocusWidget = pNewFocus;

xfa/fxfa/cxfa_fffield.cpp

diff --git a/xfa/fxfa/cxfa_fffield.cpp b/xfa/fxfa/cxfa_fffield.cpp
index 6176b3b..b43fafc 100644
--- a/xfa/fxfa/cxfa_fffield.cpp
+++ b/xfa/fxfa/cxfa_fffield.cpp
@@ -499,7 +499,9 @@
 }
 
 bool CXFA_FFField::OnSetFocus(CXFA_FFWidget* pOldWidget) {
-  CXFA_FFWidget::OnSetFocus(pOldWidget);
+  if (!CXFA_FFWidget::OnSetFocus(pOldWidget))
+    return false;
+
   if (!m_pNormalWidget)
     return false;
 

xfa/fxfa/cxfa_ffwidget.cpp

diff --git a/xfa/fxfa/cxfa_ffwidget.cpp b/xfa/fxfa/cxfa_ffwidget.cpp
index eb50185..ebcac99 100644
--- a/xfa/fxfa/cxfa_ffwidget.cpp
+++ b/xfa/fxfa/cxfa_ffwidget.cpp
@@ -394,16 +394,24 @@
 }
 
 bool CXFA_FFWidget::OnSetFocus(CXFA_FFWidget* pOldWidget) {
+  // 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 (!pWatched)
+    return false;
+
   GetLayoutItem()->SetStatusBits(XFA_WidgetStatus_Focused);
 
   CXFA_EventParam eParam;
   eParam.m_eType = XFA_EVENT_Enter;
   eParam.m_pTarget = m_pNode.Get();
   m_pNode->ProcessEvent(GetDocView(), XFA_AttributeValue::Enter, &eParam);
+  if (!pWatched)
+    return false;
+
   return true;
 }
 

xfa/fxfa/cxfa_ffwidget.h

diff --git a/xfa/fxfa/cxfa_ffwidget.h b/xfa/fxfa/cxfa_ffwidget.h
index 48d9eed..ed868e2 100644
--- a/xfa/fxfa/cxfa_ffwidget.h
+++ b/xfa/fxfa/cxfa_ffwidget.h
@@ -11,6 +11,7 @@
 
 #include "core/fpdfdoc/cpdf_formfield.h"
 #include "core/fxcodec/fx_codec_def.h"
+#include "core/fxcrt/observed_ptr.h"
 #include "core/fxge/cfx_graphstatedata.h"
 #include "xfa/fwl/cfwl_app.h"
 #include "xfa/fwl/cfwl_messagemouse.h"
@@ -62,7 +63,7 @@
   int32_t m_iRefCount;
 };
 
-class CXFA_FFWidget : public CFWL_Widget::AdapterIface {
+class CXFA_FFWidget : public Observable, public CFWL_Widget::AdapterIface {
  public:
   enum FocusOption { kDoNotDrawFocus = 0, kDrawFocus };
   enum HighlightOption { kNoHighlight = 0, kHighlight };