Merge to M53: Reland of Remove pageview from map immediately

This reverts commit f2cee9894b9f7cf2e50060965ad1eedd90ab55b6.

This CL removes the default parameter from the CPDFSDK_Document::GetPageView
|ReNew| flag and updates the code as needed. In
CFFL_FormFillter::KillFocusForAnnot we flip the flag to |FALSE| as we don't want
to re-create the page view if it is already removed. If we don't do this then
the page view will be re-created in the map, the page associated to the page
view, but then the page can be deleted out from under the pageview as it isn't
owned by the page view.

BUG=chromium:630654

Review-Url: https://codereview.chromium.org/2179163004
(cherry picked from commit 461eeafe191068ac8c32f2717907fc6a22a667d2)

TBR=thestig@chromium.org

Review URL: https://codereview.chromium.org/2200203002 .

11 files changed, 27 insertions, 24 deletions

diff --git a/fpdfsdk/formfiller/cffl_combobox.cpp b/fpdfsdk/formfiller/cffl_combobox.cpp
index 262c485138..9da24fefd6 100644
--- a/fpdfsdk/formfiller/cffl_combobox.cpp
+++ b/fpdfsdk/formfiller/cffl_combobox.cpp
@@ -284,7 +284,7 @@ CFX_WideString CFFL_ComboBox::GetSelectExportText() {
   CFX_WideString swRet;
 
   int nExport = -1;
-  CPDFSDK_PageView* pPageView = GetCurPageView();
+  CPDFSDK_PageView* pPageView = GetCurPageView(true);
   if (CPWL_ComboBox* pComboBox =
           (CPWL_ComboBox*)GetPDFWindow(pPageView, FALSE)) {
     nExport = pComboBox->GetSelect();
diff --git a/fpdfsdk/formfiller/cffl_formfiller.cpp b/fpdfsdk/formfiller/cffl_formfiller.cpp
index d7bc9586a4..479a1bc169 100644
--- a/fpdfsdk/formfiller/cffl_formfiller.cpp
+++ b/fpdfsdk/formfiller/cffl_formfiller.cpp
@@ -220,7 +220,7 @@ FX_BOOL CFFL_FormFiller::OnKeyDown(CPDFSDK_Annot* pAnnot,
                                    FX_UINT nKeyCode,
                                    FX_UINT nFlags) {
   if (IsValid()) {
-    CPDFSDK_PageView* pPageView = GetCurPageView();
+    CPDFSDK_PageView* pPageView = GetCurPageView(true);
     ASSERT(pPageView);
 
     if (CPWL_Wnd* pWnd = GetPDFWindow(pPageView, FALSE)) {
@@ -235,7 +235,7 @@ FX_BOOL CFFL_FormFiller::OnChar(CPDFSDK_Annot* pAnnot,
                                 FX_UINT nChar,
                                 FX_UINT nFlags) {
   if (IsValid()) {
-    CPDFSDK_PageView* pPageView = GetCurPageView();
+    CPDFSDK_PageView* pPageView = GetCurPageView(true);
     ASSERT(pPageView);
 
     if (CPWL_Wnd* pWnd = GetPDFWindow(pPageView, FALSE)) {
@@ -250,7 +250,7 @@ void CFFL_FormFiller::SetFocusForAnnot(CPDFSDK_Annot* pAnnot, FX_UINT nFlag) {
   CPDFSDK_Widget* pWidget = (CPDFSDK_Widget*)pAnnot;
   UnderlyingPageType* pPage = pWidget->GetUnderlyingPage();
   CPDFSDK_Document* pDoc = m_pApp->GetSDKDocument();
-  CPDFSDK_PageView* pPageView = pDoc->GetPageView(pPage);
+  CPDFSDK_PageView* pPageView = pDoc->GetPageView(pPage, true);
   if (CPWL_Wnd* pWnd = GetPDFWindow(pPageView, TRUE))
     pWnd->SetFocus();
 
@@ -263,7 +263,7 @@ void CFFL_FormFiller::KillFocusForAnnot(CPDFSDK_Annot* pAnnot, FX_UINT nFlag) {
   if (!IsValid())
     return;
 
-  CPDFSDK_PageView* pPageView = GetCurPageView();
+  CPDFSDK_PageView* pPageView = GetCurPageView(false);
   if (!pPageView)
     return;
 
@@ -456,10 +456,10 @@ CFX_FloatRect CFFL_FormFiller::GetPDFWindowRect() const {
   return CFX_FloatRect(0, 0, fWidth, fHeight);
 }
 
-CPDFSDK_PageView* CFFL_FormFiller::GetCurPageView() {
+CPDFSDK_PageView* CFFL_FormFiller::GetCurPageView(bool renew) {
   UnderlyingPageType* pPage = m_pAnnot->GetUnderlyingPage();
   CPDFSDK_Document* pSDKDoc = m_pApp->GetSDKDocument();
-  return pSDKDoc ? pSDKDoc->GetPageView(pPage) : nullptr;
+  return pSDKDoc ? pSDKDoc->GetPageView(pPage, renew) : nullptr;
 }
 
 CFX_FloatRect CFFL_FormFiller::GetFocusBox(CPDFSDK_PageView* pPageView) {
diff --git a/fpdfsdk/formfiller/cffl_formfiller.h b/fpdfsdk/formfiller/cffl_formfiller.h
index 7360957256..7ae724d7ff 100644
--- a/fpdfsdk/formfiller/cffl_formfiller.h
+++ b/fpdfsdk/formfiller/cffl_formfiller.h
@@ -141,7 +141,7 @@ class CFFL_FormFiller : public IPWL_Provider, public CPWL_TimerHandler {
   FX_BOOL IsValid() const;
   CFX_FloatRect GetPDFWindowRect() const;
 
-  CPDFSDK_PageView* GetCurPageView();
+  CPDFSDK_PageView* GetCurPageView(bool renew);
   void SetChangeMark();
 
   virtual void InvalidateRect(double left,
diff --git a/fpdfsdk/formfiller/cffl_textfield.cpp b/fpdfsdk/formfiller/cffl_textfield.cpp
index 4f32f42ac7..8442898cf0 100644
--- a/fpdfsdk/formfiller/cffl_textfield.cpp
+++ b/fpdfsdk/formfiller/cffl_textfield.cpp
@@ -106,7 +106,7 @@ FX_BOOL CFFL_TextField::OnChar(CPDFSDK_Annot* pAnnot,
   switch (nChar) {
     case FWL_VKEY_Return:
       if (!(m_pWidget->GetFieldFlags() & FIELDFLAG_MULTILINE)) {
-        CPDFSDK_PageView* pPageView = GetCurPageView();
+        CPDFSDK_PageView* pPageView = GetCurPageView(true);
         ASSERT(pPageView);
         m_bValid = !m_bValid;
         CFX_FloatRect rcAnnot = pAnnot->GetRect();
@@ -126,7 +126,7 @@ FX_BOOL CFFL_TextField::OnChar(CPDFSDK_Annot* pAnnot,
       }
       break;
     case FWL_VKEY_Escape: {
-      CPDFSDK_PageView* pPageView = GetCurPageView();
+      CPDFSDK_PageView* pPageView = GetCurPageView(true);
       ASSERT(pPageView);
       EscapeFiller(pPageView, TRUE);
       return TRUE;
diff --git a/fpdfsdk/fpdfformfill.cpp b/fpdfsdk/fpdfformfill.cpp
index 60e9e52915..35f53c48b3 100644
--- a/fpdfsdk/fpdfformfill.cpp
+++ b/fpdfsdk/fpdfformfill.cpp
@@ -41,7 +41,7 @@ CPDFSDK_PageView* FormHandleToPageView(FPDF_FORMHANDLE hHandle,
     return nullptr;
 
   CPDFSDK_Document* pSDKDoc = CPDFSDK_Document::FromFPDFFormHandle(hHandle);
-  return pSDKDoc ? pSDKDoc->GetPageView(pPage, TRUE) : nullptr;
+  return pSDKDoc ? pSDKDoc->GetPageView(pPage, true) : nullptr;
 }
 
 #ifdef PDF_ENABLE_XFA
@@ -131,7 +131,7 @@ void FFLCommon(FPDF_FORMHANDLE hHandle,
   options.m_AddFlags = flags >> 8;
   options.m_pOCContext = new CPDF_OCContext(pPDFDoc, CPDF_OCContext::View);
 
-  if (CPDFSDK_PageView* pPageView = pFXDoc->GetPageView(pPage))
+  if (CPDFSDK_PageView* pPageView = pFXDoc->GetPageView(pPage, true))
     pPageView->PageView_OnDraw(pDevice.get(), &matrix, &options, clip);
 #endif  // PDF_ENABLE_XFA
 
@@ -655,7 +655,7 @@ DLLEXPORT void STDCALL FORM_OnBeforeClosePage(FPDF_PAGE page,
   if (!pPage)
     return;
 
-  CPDFSDK_PageView* pPageView = pSDKDoc->GetPageView(pPage, FALSE);
+  CPDFSDK_PageView* pPageView = pSDKDoc->GetPageView(pPage, false);
   if (pPageView) {
     pPageView->SetValid(FALSE);
     // RemovePageView() takes care of the delete for us.
@@ -706,7 +706,7 @@ DLLEXPORT void STDCALL FORM_DoPageAAction(FPDF_PAGE page,
   CPDF_Page* pPDFPage = CPDFPageFromFPDFPage(page);
   if (!pPDFPage)
     return;
-  if (pSDKDoc->GetPageView(pPage, FALSE)) {
+  if (pSDKDoc->GetPageView(pPage, false)) {
     CPDFDoc_Environment* pEnv = pSDKDoc->GetEnv();
     CPDFSDK_ActionHandler* pActionHandler = pEnv->GetActionHander();
     CPDF_Dictionary* pPageDict = pPDFPage->m_pFormDict;
diff --git a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
index 42dad4557e..95f0cbe25a 100644
--- a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
+++ b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
@@ -503,7 +503,7 @@ void CPDFXFA_Document::WidgetPostAdd(CXFA_FFWidget* hWidget,
   if (!pXFAPage)
     return;
 
-  m_pSDKDoc->GetPageView(pXFAPage)->AddAnnot(hWidget);
+  m_pSDKDoc->GetPageView(pXFAPage, true)->AddAnnot(hWidget);
 }
 
 void CPDFXFA_Document::WidgetPreRemove(CXFA_FFWidget* hWidget,
@@ -519,7 +519,7 @@ void CPDFXFA_Document::WidgetPreRemove(CXFA_FFWidget* hWidget,
   if (!pXFAPage)
     return;
 
-  CPDFSDK_PageView* pSdkPageView = m_pSDKDoc->GetPageView(pXFAPage);
+  CPDFSDK_PageView* pSdkPageView = m_pSDKDoc->GetPageView(pXFAPage, true);
   if (CPDFSDK_Annot* pAnnot = pSdkPageView->GetAnnotByXFAWidget(hWidget))
     pSdkPageView->DeleteAnnot(pAnnot);
 }
diff --git a/fpdfsdk/fsdk_baseform.cpp b/fpdfsdk/fsdk_baseform.cpp
index 90ff144759..f0ccccec2c 100644
--- a/fpdfsdk/fsdk_baseform.cpp
+++ b/fpdfsdk/fsdk_baseform.cpp
@@ -2281,7 +2281,7 @@ void CPDFSDK_InterForm::UpdateField(CPDF_FormField* pFormField) {
       CPDFDoc_Environment* pEnv = m_pDocument->GetEnv();
       CFFL_IFormFiller* pIFormFiller = pEnv->GetIFormFiller();
       UnderlyingPageType* pPage = pWidget->GetUnderlyingPage();
-      CPDFSDK_PageView* pPageView = m_pDocument->GetPageView(pPage, FALSE);
+      CPDFSDK_PageView* pPageView = m_pDocument->GetPageView(pPage, false);
       FX_RECT rcBBox = pIFormFiller->GetViewBBox(pPageView, pWidget);
 
       pEnv->FFI_Invalidate(pPage, rcBBox.left, rcBBox.top, rcBBox.right,
diff --git a/fpdfsdk/fsdk_mgr.cpp b/fpdfsdk/fsdk_mgr.cpp
index 6500e302ec..17b03e2644 100644
--- a/fpdfsdk/fsdk_mgr.cpp
+++ b/fpdfsdk/fsdk_mgr.cpp
@@ -263,7 +263,7 @@ CPDFSDK_Document::~CPDFSDK_Document() {
 
 CPDFSDK_PageView* CPDFSDK_Document::GetPageView(
     UnderlyingPageType* pUnderlyingPage,
-    FX_BOOL ReNew) {
+    bool ReNew) {
   auto it = m_pageMap.find(pUnderlyingPage);
   if (it != m_pageMap.end())
     return it->second;
@@ -281,7 +281,7 @@ CPDFSDK_PageView* CPDFSDK_Document::GetPageView(
 CPDFSDK_PageView* CPDFSDK_Document::GetCurrentView() {
   UnderlyingPageType* pPage =
       UnderlyingFromFPDFPage(m_pEnv->FFI_GetCurrentPage(m_pDoc));
-  return pPage ? GetPageView(pPage, TRUE) : nullptr;
+  return pPage ? GetPageView(pPage, true) : nullptr;
 }
 
 CPDFSDK_PageView* CPDFSDK_Document::GetPageView(int nIndex) {
@@ -353,9 +353,12 @@ void CPDFSDK_Document::RemovePageView(UnderlyingPageType* pUnderlyingPage) {
   if (pPageView->IsLocked())
     return;
 
+  // Remove the page from the map to make sure we don't accidentally attempt
+  // to use the |pPageView| while we're cleaning it up.
+  m_pageMap.erase(it);
+
   pPageView->KillFocusAnnotIfNeeded();
   delete pPageView;
-  m_pageMap.erase(it);
 }
 
 UnderlyingPageType* CPDFSDK_Document::GetPage(int nIndex) {
diff --git a/fpdfsdk/include/fsdk_mgr.h b/fpdfsdk/include/fsdk_mgr.h
index 84dea23b72..16aef4af4e 100644
--- a/fpdfsdk/include/fsdk_mgr.h
+++ b/fpdfsdk/include/fsdk_mgr.h
@@ -479,8 +479,7 @@ class CPDFSDK_Document {
   int GetPageViewCount() const { return m_pageMap.size(); }
 #endif  // PDF_ENABLE_XFA
 
-  CPDFSDK_PageView* GetPageView(UnderlyingPageType* pPage,
-                                FX_BOOL ReNew = TRUE);
+  CPDFSDK_PageView* GetPageView(UnderlyingPageType* pPage, bool ReNew);
   CPDFSDK_PageView* GetPageView(int nIndex);
   CPDFSDK_PageView* GetCurrentView();
   void RemovePageView(UnderlyingPageType* pPage);
diff --git a/fpdfsdk/javascript/Document.cpp b/fpdfsdk/javascript/Document.cpp
index 258dddbe10..3c7756836c 100644
--- a/fpdfsdk/javascript/Document.cpp
+++ b/fpdfsdk/javascript/Document.cpp
@@ -494,7 +494,7 @@ FX_BOOL Document::removeField(IJS_Context* cc,
     UnderlyingPageType* pPage = pWidget->GetUnderlyingPage();
     ASSERT(pPage);
 
-    CPDFSDK_PageView* pPageView = m_pDocument->GetPageView(pPage);
+    CPDFSDK_PageView* pPageView = m_pDocument->GetPageView(pPage, true);
     pPageView->DeleteAnnot(pWidget);
     pPageView->UpdateRects(aRefresh);
   }
diff --git a/fpdfsdk/javascript/Field.cpp b/fpdfsdk/javascript/Field.cpp
index 8d7fe1b186..c69fc00612 100644
--- a/fpdfsdk/javascript/Field.cpp
+++ b/fpdfsdk/javascript/Field.cpp
@@ -3262,7 +3262,8 @@ FX_BOOL Field::setFocus(IJS_Context* cc,
         pEnv->FFI_GetCurrentPage(m_pDocument->GetUnderlyingDocument()));
     if (!pPage)
       return FALSE;
-    if (CPDFSDK_PageView* pCurPageView = m_pDocument->GetPageView(pPage)) {
+    if (CPDFSDK_PageView* pCurPageView =
+            m_pDocument->GetPageView(pPage, true)) {
       for (int32_t i = 0; i < nCount; i++) {
         if (CPDFSDK_Widget* pTempWidget =
                 pInterForm->GetWidget(pFormField->GetControl(i))) {