commit 6f960347f8474a202d8dd99063bf8ce584896baf
author Dan Sinclair <dsinclair@chromium.org> Tue Sep 19 13:46:08 2017 -0400
committer Chromium commit bot <commit-bot@chromium.org> Tue Sep 19 17:57:30 2017 +0000
tree bddfcc4e3cfe629d769807965cbf28145f874f4f
parent 2d510f7dc4d6c30cdecef09c5e68c47e98de6ffb

Setting focus on a widget may destroy the widget

When a widget has focus set, this can trigger an Invalidation call which
can trigger a page and annotation reload. This reload can destroy the
current widget we're handling.

This CL adds ObservedPtrs as needed so we can make sure the widgets are
still alive after we've done the Invalidation.

Bug: chromium:765921
Change-Id: I51cd24aa1ebd96abe9478efef5130a4e568dac1a
Reviewed-on: https://pdfium-review.googlesource.com/14290
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

fpdfsdk/pwl/cpwl_caret.cpp

diff --git a/fpdfsdk/pwl/cpwl_caret.cpp b/fpdfsdk/pwl/cpwl_caret.cpp
index b1040bc2..99d2e2e 100644
--- a/fpdfsdk/pwl/cpwl_caret.cpp
+++ b/fpdfsdk/pwl/cpwl_caret.cpp
@@ -59,6 +59,8 @@
   } else {
     m_bFlash = !m_bFlash;
     InvalidateRect(nullptr);
+    // Note, |this| may no longer be viable at this point. If more work needs
+    // to be done, add an observer.
   }
 }
 
@@ -77,15 +79,24 @@
         m_ptFoot = ptFoot;
         m_bFlash = true;
         Move(m_rcInvalid, false, true);
+        // Note, |this| may no longer be viable at this point. If more work
+        // needs to be done, add an observer.
       }
     } else {
       m_ptHead = ptHead;
       m_ptFoot = ptFoot;
       EndTimer();
       BeginTimer(PWL_CARET_FLASHINTERVAL);
+
+      ObservedPtr observer(this);
       CPWL_Wnd::SetVisible(true);
+      if (!observer)
+        return;
+
       m_bFlash = true;
       Move(m_rcInvalid, false, true);
+      // Note, |this| may no longer be viable at this point. If more work needs
+      // to be done, add an observer.
     }
   } else {
     m_ptHead = CFX_PointF();
@@ -94,6 +105,8 @@
     if (IsVisible()) {
       EndTimer();
       CPWL_Wnd::SetVisible(false);
+      // Note, |this| may no longer be viable at this point. If more work needs
+      // to be done, add an observer.
     }
   }
 }
@@ -111,4 +124,6 @@
   } else {
     CPWL_Wnd::InvalidateRect(pRect);
   }
+  // Note, |this| may no longer be viable at this point. If more work needs
+  // to be done, add an observer.
 }

fpdfsdk/pwl/cpwl_edit.cpp

diff --git a/fpdfsdk/pwl/cpwl_edit.cpp b/fpdfsdk/pwl/cpwl_edit.cpp
index c71dbe4..0b74a18 100644
--- a/fpdfsdk/pwl/cpwl_edit.cpp
+++ b/fpdfsdk/pwl/cpwl_edit.cpp
@@ -331,16 +331,24 @@
 }
 
 void CPWL_Edit::OnSetFocus() {
+  ObservedPtr observed_ptr(this);
   SetEditCaret(true);
+  if (!observed_ptr)
+    return;
+
   if (!IsReadOnly()) {
-    if (CPWL_Wnd::FocusHandlerIface* pFocusHandler = GetFocusHandler())
+    if (CPWL_Wnd::FocusHandlerIface* pFocusHandler = GetFocusHandler()) {
       pFocusHandler->OnSetFocus(this);
+      if (!observed_ptr)
+        return;
+    }
   }
   m_bFocus = true;
 }
 
 void CPWL_Edit::OnKillFocus() {
-  ObservedPtr observed_ptr = ObservedPtr(this);
+  ObservedPtr observed_ptr(this);
+
   CPWL_ScrollBar* pScroll = GetVScrollBar();
   if (pScroll && pScroll->IsVisible()) {
     pScroll->SetVisible(false);

fpdfsdk/pwl/cpwl_wnd.cpp

diff --git a/fpdfsdk/pwl/cpwl_wnd.cpp b/fpdfsdk/pwl/cpwl_wnd.cpp
index 1c1512e..4e4abd2 100644
--- a/fpdfsdk/pwl/cpwl_wnd.cpp
+++ b/fpdfsdk/pwl/cpwl_wnd.cpp
@@ -86,19 +86,21 @@
 
   void SetFocus(CPWL_Wnd* pWnd) {
     m_aKeyboardPath.clear();
-    if (pWnd) {
-      m_pMainKeyboardWnd = pWnd;
-      CPWL_Wnd* pParent = pWnd;
-      while (pParent) {
-        m_aKeyboardPath.push_back(pParent);
-        pParent = pParent->GetParentWindow();
-      }
-      pWnd->OnSetFocus();
+    if (!pWnd)
+      return;
+
+    m_pMainKeyboardWnd = pWnd;
+    CPWL_Wnd* pParent = pWnd;
+    while (pParent) {
+      m_aKeyboardPath.push_back(pParent);
+      pParent = pParent->GetParentWindow();
     }
+    // Note, pWnd may get destroyed in the OnSetFocus call.
+    pWnd->OnSetFocus();
   }
 
   void KillFocus() {
-    ObservedPtr observed_ptr = ObservedPtr(this);
+    ObservedPtr observed_ptr(this);
     if (!m_aKeyboardPath.empty())
       if (CPWL_Wnd* pWnd = m_aKeyboardPath[0])
         pWnd->OnKillFocus();