M62: OnBeforeKeystroke may invalidate the widget's window.chromium/3202

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>
(cherry picked from commit b1f9205bb1a0671c31e44e7362784c770bf2a948)
Reviewed-on: https://pdfium-review.googlesource.com/15111
Commit-Queue: Lei Zhang <thestig@chromium.org>

4 files changed, 315 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) {
diff --git a/testing/resources/bug_765384.in b/testing/resources/bug_765384.in
new file mode 100644
index 0000000000..1de85d31ba
--- /dev/null
+++ b/testing/resources/bug_765384.in
@@ -0,0 +1,105 @@
+{{header}}
+{{object 1 0}} <<
+  /Type /Catalog
+  /Pages 2 0 R
+  /AcroForm 4 0 R
+  /OpenAction 10 0 R
+>>
+endobj
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 1
+  /Kids [
+    3 0 R
+  ]
+>>
+endobj
+% Page number 0.
+{{object 3 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /Annots [
+    8 0 R
+    9 0 R
+    ]
+>>
+endobj
+% Forms
+{{object 4 0}} <<
+  /XFA [
+    (xdp:xdp) 43 0 R
+    (form) 49 0 R
+    (</xdp:xdp>) 50 0 R
+  ]
+  /Fields [
+    8 0 R
+    9 0 R
+    ]
+>>
+endobj
+% Fields
+{{object 8 0}} <<
+  /FT /Ch
+  /Ff 0
+  /T (MyField)
+  /Type /Annot
+  /Subtype /Widget
+  /Rect [0 0 800 800]
+  /Opt [(test 1) (test 2)]
+  /AA << /K 20 0 R >>
+>>
+endobj
+% Fields
+{{object 9 0}} <<
+  /FT /Tx
+  /T (MyField2)
+  /Type /Annot
+  /Subtype /Widget
+  /V (myfield_2)
+  /Rect [0 0 1 1]
+>>
+endobj
+% JS Action
+{{object 20 0}} <<
+  /Type /Action
+  /S /JavaScript
+  /JS 21 0 R
+>>
+endobj
+% JS program to exexute
+{{object 21 0}} <<
+>>
+stream
+function gc() {
+  tmp = [];
+  for (var i = 0; i < 0x200000; i++)
+    tmp.push(new Uint8Array(10));
+  tmp = null;
+}
+this.getField("MyField2").setFocus();
+this.getField("MyField").borderStyle="dashed";
+this.getField("MyField").setFocus();
+gc();
+endstream
+endobj
+{{object 43 0}} <<
+>>stream
+<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
+endstream
+endobj
+{{object 49 0}} <<
+>>stream
+<config></config>
+<template></template>
+endstream
+endobj
+{{object 50 0}} <<
+>>stream
+</xdp:xdp>
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_765384.pdf b/testing/resources/bug_765384.pdf
new file mode 100644
index 0000000000..fcaa5f4305
--- /dev/null
+++ b/testing/resources/bug_765384.pdf
@@ -0,0 +1,159 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+  /Type /Catalog
+  /Pages 2 0 R
+  /AcroForm 4 0 R
+  /OpenAction 10 0 R
+>>
+endobj
+2 0 obj <<
+  /Type /Pages
+  /Count 1
+  /Kids [
+    3 0 R
+  ]
+>>
+endobj
+% Page number 0.
+3 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /Annots [
+    8 0 R
+    9 0 R
+    ]
+>>
+endobj
+% Forms
+4 0 obj <<
+  /XFA [
+    (xdp:xdp) 43 0 R
+    (form) 49 0 R
+    (</xdp:xdp>) 50 0 R
+  ]
+  /Fields [
+    8 0 R
+    9 0 R
+    ]
+>>
+endobj
+% Fields
+8 0 obj <<
+  /FT /Ch
+  /Ff 0
+  /T (MyField)
+  /Type /Annot
+  /Subtype /Widget
+  /Rect [0 0 800 800]
+  /Opt [(test 1) (test 2)]
+  /AA << /K 20 0 R >>
+>>
+endobj
+% Fields
+9 0 obj <<
+  /FT /Tx
+  /T (MyField2)
+  /Type /Annot
+  /Subtype /Widget
+  /V (myfield_2)
+  /Rect [0 0 1 1]
+>>
+endobj
+% JS Action
+20 0 obj <<
+  /Type /Action
+  /S /JavaScript
+  /JS 21 0 R
+>>
+endobj
+% JS program to exexute
+21 0 obj <<
+>>
+stream
+function gc() {
+  tmp = [];
+  for (var i = 0; i < 0x200000; i++)
+    tmp.push(new Uint8Array(10));
+  tmp = null;
+}
+this.getField("MyField2").setFocus();
+this.getField("MyField").borderStyle="dashed";
+this.getField("MyField").setFocus();
+gc();
+endstream
+endobj
+43 0 obj <<
+>>stream
+<?xml version="1.0" encoding="UTF-8"?>
+<xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
+endstream
+endobj
+49 0 obj <<
+>>stream
+<config></config>
+<template></template>
+endstream
+endobj
+50 0 obj <<
+>>stream
+</xdp:xdp>
+endstream
+endobj
+xref
+0 51
+0000000000 65535 f 
+0000000015 00000 n 
+0000000107 00000 n 
+0000000195 00000 n 
+0000000292 00000 n 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000436 00000 n 
+0000000604 00000 n 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000732 00000 n 
+0000000824 00000 n 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000001106 00000 n 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000000000 65535 f 
+0000001230 00000 n 
+0000001308 00000 n 
+trailer<< /Root 1 0 R /Size 51 >>
+startxref
+1357
+%%EOF