From a548b1d3e2444f256bcbf6c2fa2165798e33ba8d Mon Sep 17 00:00:00 2001
From: Oliver Chang <ochang@chromium.org>
Date: Fri, 30 Oct 2015 12:48:49 -0700
Subject: Rip out the KillFocusAnnot call from CPDFSDK_PageView's destructor

Previously, blur event actions could potentially touch deleted PageViews
as CPDFSDK_Document deletes the PageViews one by one.

This also fixes a related issue: CPDFSDK_Document::SetFocusAnnot no
longer does anything if the document is being destroyed. Otherwise, it
eventually tries to use m_pEnv->GetSDKDocument() at which point has
already been set to NULL by FPDFDOC_ExitFormFillEnvironment.

R=tsepez@chromium.org, thestig@chromium.org
BUG=512445

Review URL: https://codereview.chromium.org/1414353007 .
---
 fpdfsdk/include/fsdk_mgr.h |  2 ++
 fpdfsdk/src/fsdk_mgr.cpp   | 31 ++++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/fpdfsdk/include/fsdk_mgr.h b/fpdfsdk/include/fsdk_mgr.h
index 2c063ed5a8..2cc528265d 100644
--- a/fpdfsdk/include/fsdk_mgr.h
+++ b/fpdfsdk/include/fsdk_mgr.h
@@ -283,6 +283,7 @@ class CPDFSDK_Document {
   CPDFDoc_Environment* m_pEnv;
   CPDF_OCContext* m_pOccontent;
   FX_BOOL m_bChangeMask;
+  FX_BOOL m_bBeingDestroyed;
 };
 class CPDFSDK_PageView final {
  public:
@@ -302,6 +303,7 @@ class CPDFSDK_PageView final {
   FX_BOOL KillFocusAnnot(FX_UINT nFlag = 0) {
     return m_pSDKDoc->KillFocusAnnot(nFlag);
   }
+  void KillFocusAnnotIfNeeded();
   FX_BOOL Annot_HasAppearance(CPDF_Annot* pAnnot);
 
   CPDFSDK_Annot* AddAnnot(CPDF_Dictionary* pDict);
diff --git a/fpdfsdk/src/fsdk_mgr.cpp b/fpdfsdk/src/fsdk_mgr.cpp
index 0b6770174b..7dba8d22b8 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -408,9 +408,16 @@ CPDFSDK_Document::CPDFSDK_Document(CPDF_Document* pDoc,
       m_pFocusAnnot(nullptr),
       m_pEnv(pEnv),
       m_pOccontent(nullptr),
-      m_bChangeMask(FALSE) {}
+      m_bChangeMask(FALSE),
+      m_bBeingDestroyed(FALSE) {
+}
 
 CPDFSDK_Document::~CPDFSDK_Document() {
+  m_bBeingDestroyed = TRUE;
+
+  for (auto& it : m_pageMap)
+    it.second->KillFocusAnnotIfNeeded();
+
   for (auto& it : m_pageMap)
     delete it.second;
   m_pageMap.clear();
@@ -509,6 +516,7 @@ void CPDFSDK_Document::ReMovePageView(CPDF_Page* pPDFPage) {
   if (pPageView->IsLocked())
     return;
 
+  pPageView->KillFocusAnnotIfNeeded();
   delete pPageView;
   m_pageMap.erase(it);
 }
@@ -541,6 +549,9 @@ CPDFSDK_Annot* CPDFSDK_Document::GetFocusAnnot() {
 }
 
 FX_BOOL CPDFSDK_Document::SetFocusAnnot(CPDFSDK_Annot* pAnnot, FX_UINT nFlag) {
+  if (m_bBeingDestroyed)
+    return FALSE;
+
   if (m_pFocusAnnot == pAnnot)
     return TRUE;
 
@@ -627,14 +638,6 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc, CPDF_Page* page)
 }
 
 CPDFSDK_PageView::~CPDFSDK_PageView() {
-  // if there is a focused annot on the page, we should kill the focus first.
-  if (CPDFSDK_Annot* focusedAnnot = m_pSDKDoc->GetFocusAnnot()) {
-    auto it =
-        std::find(m_fxAnnotArray.begin(), m_fxAnnotArray.end(), focusedAnnot);
-    if (it != m_fxAnnotArray.end())
-      KillFocusAnnot();
-  }
-
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
   for (CPDFSDK_Annot* pAnnot : m_fxAnnotArray)
@@ -721,6 +724,16 @@ CPDFSDK_Annot* CPDFSDK_PageView::GetFXWidgetAtPoint(FX_FLOAT pageX,
   return nullptr;
 }
 
+void CPDFSDK_PageView::KillFocusAnnotIfNeeded() {
+  // if there is a focused annot on the page, we should kill the focus first.
+  if (CPDFSDK_Annot* focusedAnnot = m_pSDKDoc->GetFocusAnnot()) {
+    auto it =
+        std::find(m_fxAnnotArray.begin(), m_fxAnnotArray.end(), focusedAnnot);
+    if (it != m_fxAnnotArray.end())
+      KillFocusAnnot();
+  }
+}
+
 FX_BOOL CPDFSDK_PageView::Annot_HasAppearance(CPDF_Annot* pAnnot) {
   CPDF_Dictionary* pAnnotDic = pAnnot->GetAnnotDict();
   if (pAnnotDic)
-- 
cgit v1.2.3