OnBeforeKeystroke may invalidate the widget's window.

And along with it any data that the window may have been carrying.
Tidy some return codes while we're at it.

Bug: 765384
Change-Id: Id16ec5f82b3d9273ba8f2edb1e4645a9145df4f6
Reviewed-on: https://pdfium-review.googlesource.com/14050
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

2 files changed, 51 insertions, 39 deletions

diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
index eabb899c27..10cefd204b 100644
--- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
+++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
@@ -863,33 +863,33 @@ std::pair<bool, bool> CFFL_InteractiveFormFiller::OnBeforeKeyStroke(
     int nSelEnd,
     bool bKeyDown,
     uint32_t nFlag) {
-  bool bRC = true;
-  bool bExit = false;
-  CFFL_PrivateData* pData = reinterpret_cast<CFFL_PrivateData*>(pPrivateData);
-  ASSERT(pData->pWidget);
+  // Copy the private data since the window owning it may not survive.
+  CFFL_PrivateData privateData =
+      *reinterpret_cast<CFFL_PrivateData*>(pPrivateData);
+  ASSERT(privateData.pWidget);
 
-  CFFL_FormFiller* pFormFiller = GetFormFiller(pData->pWidget, false);
+  CFFL_FormFiller* pFormFiller = GetFormFiller(privateData.pWidget, false);
 
 #ifdef PDF_ENABLE_XFA
-  if (pFormFiller->IsFieldFull(pData->pPageView)) {
-    CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget);
-    if (OnFull(&pObserved, pData->pPageView, nFlag) || !pObserved)
-      return {bRC, true};
+  if (pFormFiller->IsFieldFull(privateData.pPageView)) {
+    CPDFSDK_Annot::ObservedPtr pObserved(privateData.pWidget);
+    if (OnFull(&pObserved, privateData.pPageView, nFlag) || !pObserved)
+      return {true, true};
   }
 #endif  // PDF_ENABLE_XFA
 
   if (m_bNotifying ||
-      !pData->pWidget->GetAAction(CPDF_AAction::KeyStroke).GetDict()) {
-    return {bRC, bExit};
+      !privateData.pWidget->GetAAction(CPDF_AAction::KeyStroke).GetDict()) {
+    return {true, false};
   }
 
   CFX_AutoRestorer<bool> restorer(&m_bNotifying);
   m_bNotifying = true;
-  int nAge = pData->pWidget->GetAppearanceAge();
-  int nValueAge = pData->pWidget->GetValueAge();
 
+  int nAge = privateData.pWidget->GetAppearanceAge();
+  int nValueAge = privateData.pWidget->GetValueAge();
   CPDFSDK_FormFillEnvironment* pFormFillEnv =
-      pData->pPageView->GetFormFillEnv();
+      privateData.pPageView->GetFormFillEnv();
 
   PDFSDK_FieldAction fa;
   fa.bModifier = CPDFSDK_FormFillEnvironment::IsCTRLKeyDown(nFlag);
@@ -901,37 +901,38 @@ std::pair<bool, bool> CFFL_InteractiveFormFiller::OnBeforeKeyStroke(
   fa.bRC = true;
   fa.nSelStart = nSelStart;
   fa.nSelEnd = nSelEnd;
+  pFormFiller->GetActionData(privateData.pPageView, CPDF_AAction::KeyStroke,
+                             fa);
+  pFormFiller->SaveState(privateData.pPageView);
 
-  pFormFiller->GetActionData(pData->pPageView, CPDF_AAction::KeyStroke, fa);
-  pFormFiller->SaveState(pData->pPageView);
+  CPDFSDK_Annot::ObservedPtr pObserved(privateData.pWidget);
+  bool action_status = privateData.pWidget->OnAAction(
+      CPDF_AAction::KeyStroke, fa, privateData.pPageView);
 
-  CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget);
-  if (!pData->pWidget->OnAAction(CPDF_AAction::KeyStroke, fa,
-                                 pData->pPageView)) {
-    if (!IsValidAnnot(pData->pPageView, pData->pWidget))
-      bExit = true;
-    return {bRC, bExit};
-  }
+  if (!pObserved || !IsValidAnnot(privateData.pPageView, privateData.pWidget))
+    return {true, true};
 
-  if (!pObserved || !IsValidAnnot(pData->pPageView, pData->pWidget))
-    return {bRC, true};
+  if (!action_status)
+    return {true, false};
 
-  if (nAge != pData->pWidget->GetAppearanceAge()) {
+  bool bExit = false;
+  if (nAge != privateData.pWidget->GetAppearanceAge()) {
     CPWL_Wnd* pWnd = pFormFiller->ResetPDFWindow(
-        pData->pPageView, nValueAge == pData->pWidget->GetValueAge());
-    pData = reinterpret_cast<CFFL_PrivateData*>(pWnd->GetAttachedData());
+        privateData.pPageView, nValueAge == privateData.pWidget->GetValueAge());
+    if (!pWnd)
+      return {true, true};
+    privateData = *reinterpret_cast<CFFL_PrivateData*>(pWnd->GetAttachedData());
     bExit = true;
   }
+  if (fa.bRC) {
+    pFormFiller->SetActionData(privateData.pPageView, CPDF_AAction::KeyStroke,
+                               fa);
+  } else {
+    pFormFiller->RestoreState(privateData.pPageView);
+  }
+  if (pFormFillEnv->GetFocusAnnot() == privateData.pWidget)
+    return {false, bExit};
 
-  if (fa.bRC)
-    pFormFiller->SetActionData(pData->pPageView, CPDF_AAction::KeyStroke, fa);
-  else
-    pFormFiller->RestoreState(pData->pPageView);
-  bRC = false;
-
-  if (pFormFillEnv->GetFocusAnnot() == pData->pWidget)
-    return {bRC, bExit};
-
-  pFormFiller->CommitData(pData->pPageView, nFlag);
-  return {bRC, true};
+  pFormFiller->CommitData(privateData.pPageView, nFlag);
+  return {false, true};
 }
diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index 93350874dc..56e166252e 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -506,6 +506,17 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_707673) {
   EXPECT_EQ(0u, alerts.size());
 }
 
+TEST_F(FPDFFormFillEmbeddertest, BUG_765384) {
+  EXPECT_TRUE(OpenDocument("bug_765384.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_TRUE(page);
+
+  DoOpenActions();
+  FORM_OnLButtonDown(form_handle(), page, 0, 140, 590);
+  FORM_OnLButtonUp(form_handle(), page, 0, 140, 590);
+  UnloadPage(page);
+}
+
 #endif  // PDF_ENABLE_V8
 
 TEST_F(FPDFFormFillEmbeddertest, FormText) {