Merge to XFA:Fix heap use after free in CPDFSDK_Annot::GetPDFAnnot.

(cherry picked from commit 9241e5a43990859f6f9a94aaa2c488d0451039e3)
Original Review URL: https://codereview.chromium.org/1312313006 .

(cherry picked from commit 343dbb841f4c12e819932e2b66dd70f817337d97)
Original Review URL: https://codereview.chromium.org/1325533004 .

BUG=507316
TBR=thestig@chromium.org

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

6 files changed, 347 insertions, 7 deletions

diff --git a/fpdfsdk/src/fpdfformfill_embeddertest.cpp b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
index 6baad11531..56710b9f83 100644
--- a/fpdfsdk/src/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
@@ -40,6 +40,18 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_487928) {
   UnloadPage(page);
 }
 
+TEST_F(FPDFFormFillEmbeddertest, BUG_507316) {
+  EmbedderTestTimerHandlingDelegate delegate;
+  SetDelegate(&delegate);
+
+  EXPECT_TRUE(OpenDocument("testing/resources/bug_507316.pdf"));
+  FPDF_PAGE page = LoadAndCachePage(2);
+  EXPECT_NE(nullptr, page);
+  DoOpenActions();
+  delegate.AdvanceTime(4000);
+  UnloadPage(page);
+}
+
 TEST_F(FPDFFormFillEmbeddertest, BUG_514690) {
   EXPECT_TRUE(OpenDocument("testing/resources/hello_world.pdf"));
   FPDF_PAGE page = LoadPage(0);
diff --git a/fpdfsdk/src/fsdk_mgr.cpp b/fpdfsdk/src/fsdk_mgr.cpp
index 4d3d01f6f4..4a55622e57 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -664,15 +664,22 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_Document* pSDKDoc,
 }
 
 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()) {
+    for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
+      CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
+      if (pAnnot == focusedAnnot) {
+        KillFocusAnnot();
+        break;
+      }
+    }
+  }
+
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
-  int nAnnotCount = m_fxAnnotArray.GetSize();
-  for (int i = 0; i < nAnnotCount; i++) {
+  CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
+  ASSERT(pAnnotHandlerMgr);
+  for (int i = 0, count = m_fxAnnotArray.GetSize(); i < count; i++) {
     CPDFSDK_Annot* pAnnot = (CPDFSDK_Annot*)m_fxAnnotArray.GetAt(i);
-    // if there is a focused annot on the page, we should kill the focus first.
-    if (pAnnot == m_pSDKDoc->GetFocusAnnot())
-      KillFocusAnnot();
-    CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = pEnv->GetAnnotHandlerMgr();
-    ASSERT(pAnnotHandlerMgr);
     pAnnotHandlerMgr->ReleaseAnnot(pAnnot);
   }
   m_fxAnnotArray.RemoveAll();
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index d62810d087..151987fbb2 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -232,6 +232,7 @@ bool EmbedderTest::OpenDocument(const std::string& filename) {
   formfillinfo->version = 1;
   formfillinfo->FFI_SetTimer = SetTimerTrampoline;
   formfillinfo->FFI_KillTimer = KillTimerTrampoline;
+  formfillinfo->FFI_GetPage = GetPageTrampoline;
   formfillinfo->m_pJsPlatform = platform;
 
   form_handle_ = FPDFDOC_InitFormFillEnvironment(document_, formfillinfo);
@@ -270,6 +271,15 @@ FPDF_PAGE EmbedderTest::LoadPage(int page_number) {
   return page;
 }
 
+FPDF_PAGE EmbedderTest::LoadAndCachePage(int page_number) {
+  FPDF_PAGE page = delegate_->GetPage(form_handle_, document_, page_number);
+  if (!page) {
+    return nullptr;
+  }
+  FORM_DoPageAAction(page, form_handle_, FPDFPAGE_AACTION_OPEN);
+  return page;
+}
+
 FPDF_BITMAP EmbedderTest::RenderPage(FPDF_PAGE page) {
   int width = static_cast<int>(FPDF_GetPageWidth(page));
   int height = static_cast<int>(FPDF_GetPageHeight(page));
@@ -286,6 +296,22 @@ void EmbedderTest::UnloadPage(FPDF_PAGE page) {
   FPDF_ClosePage(page);
 }
 
+FPDF_PAGE EmbedderTest::Delegate::GetPage(FPDF_FORMHANDLE form_handle,
+                                          FPDF_DOCUMENT document,
+                                          int page_index) {
+  auto it = m_pageMap.find(page_index);
+  if (it != m_pageMap.end()) {
+    return it->second;
+  }
+  FPDF_PAGE page = FPDF_LoadPage(document, page_index);
+  if (!page) {
+    return nullptr;
+  }
+  m_pageMap[page_index] = page;
+  FORM_OnAfterLoadPage(page, form_handle);
+  return page;
+}
+
 // static
 void EmbedderTest::UnsupportedHandlerTrampoline(UNSUPPORT_INFO* info,
                                                 int type) {
@@ -317,6 +343,14 @@ void EmbedderTest::KillTimerTrampoline(FPDF_FORMFILLINFO* info, int id) {
   return test->delegate_->KillTimer(id);
 }
 
+// static
+FPDF_PAGE EmbedderTest::GetPageTrampoline(FPDF_FORMFILLINFO* info,
+                                          FPDF_DOCUMENT document,
+                                          int page_index) {
+  EmbedderTest* test = static_cast<EmbedderTest*>(info);
+  return test->delegate_->GetPage(test->m_pFormfillinfo, document, page_index);
+}
+
 // Can't use gtest-provided main since we need to stash the path to the
 // executable in order to find the external V8 binary data files.
 int main(int argc, char** argv) {
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 4bd9d9713e..f490f82536 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -5,6 +5,7 @@
 #ifndef TESTING_EMBEDDER_TEST_H_
 #define TESTING_EMBEDDER_TEST_H_
 
+#include <map>
 #include <string>
 
 #include "../public/fpdf_dataavail.h"
@@ -43,6 +44,14 @@ class EmbedderTest : public ::testing::Test,
 
     // Equivalent to FPDF_FORMFILLINFO::FFI_KillTimer().
     virtual void KillTimer(int id) {}
+
+    // Equivalent to FPDF_FORMFILLINFO::FFI_GetPage().
+    virtual FPDF_PAGE GetPage(FPDF_FORMHANDLE form_handle,
+                              FPDF_DOCUMENT document,
+                              int page_index);
+
+   private:
+    std::map<int, FPDF_PAGE> m_pageMap;
   };
 
   EmbedderTest();
@@ -72,6 +81,10 @@ class EmbedderTest : public ::testing::Test,
   // Load a specific page of the open document.
   virtual FPDF_PAGE LoadPage(int page_number);
 
+  // Load a specific page of the open document using delegate_->GetPage.
+  // delegate_->GetPage also caches loaded page.
+  virtual FPDF_PAGE LoadAndCachePage(int page_number);
+
   // Convert a loaded page into a bitmap.
   virtual FPDF_BITMAP RenderPage(FPDF_PAGE page);
 
@@ -106,6 +119,9 @@ class EmbedderTest : public ::testing::Test,
                                 int msecs,
                                 TimerCallback fn);
   static void KillTimerTrampoline(FPDF_FORMFILLINFO* info, int id);
+  static FPDF_PAGE GetPageTrampoline(FPDF_FORMFILLINFO* info,
+                                     FPDF_DOCUMENT document,
+                                     int page_index);
 };
 
 #endif  // TESTING_EMBEDDER_TEST_H_
diff --git a/testing/resources/bug_507316.in b/testing/resources/bug_507316.in
new file mode 100644
index 0000000000..cd3f81eeea
--- /dev/null
+++ b/testing/resources/bug_507316.in
@@ -0,0 +1,126 @@
+{{header}}
+{{object 1 0}} <<
+  /Type /Catalog
+  /Pages 2 0 R
+  /OpenAction 4 0 R
+  /AcroForm 3 0 R
+>>
+endobj
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 3
+  /Kids [6 0 R 7 0 R 8 0 R ]
+>>
+endobj
+{{object 3 0}} <<
+  /CO [11 0 R]
+  /Fields [11 0 R 9 0 R]
+>>
+endobj
+{{object 4 0}} <<
+  /Type /Action
+  /S /JavaScript
+  /JS 5 0 R
+>>
+endobj
+{{object 5 0}} <<>>
+stream
+var i = 0;
+function run()
+{
+  t = this.getField('txtName1');
+  t2 = this.getField('txtName2');
+  t2.setFocus();
+  t.setFocus();
+  t.value='G';
+}
+
+function remove(){
+  if (i==1){
+    this.removeField('txtName2');
+  }
+  i++;
+}
+
+app.setTimeOut('run()',2000);
+endstream
+endobj
+{{object 6 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+>>
+endobj
+{{object 7 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+>>
+endobj
+{{object 8 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+  /Annots [9 0 R 11 0 R]
+>>
+endobj
+{{object 9 0}} <<
+  /FT /Tx
+  /Type /Annot
+  /Subtype /Widget
+  /T (txtName2)
+  /F 4
+  /AP <</N 10 0 R>>
+  /Rect [20 20 400 60]
+>>
+endobj
+{{object 10 0}} <<
+  /Type /XObject
+  /Subtype /Form
+  /FormType 1
+>>
+endobj
+{{object 11 0}} <<
+  /FT /Tx
+  /Type /Annot
+  /Subtype /Widget
+  /T (txtName1)
+  /F 4
+  /AP <</N 12 0 R>>
+  /Rect [200 200 400 260]
+  /AA 13 0 R
+>>
+endobj
+{{object 12 0}} <<
+  /Type /XObject
+  /Subtype /Form
+  /FormType 1
+>>
+endobj
+{{object 13 0}} <<
+  /C 14 0 R
+>>
+endobj
+{{object 14 0}} <<
+  /Type /Action
+  /S /JavaScript
+  /JS 15 0 R
+>>
+endobj
+{{object 15 0}} <<>>
+stream
+this.getField('txtName2').value='B';
+this.pageNum=1;
+remove();
+endstream
+endobj
+{{xref}}
+trailer <<
+  /Size 15
+  /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_507316.pdf b/testing/resources/bug_507316.pdf
new file mode 100644
index 0000000000..13c70830f8
--- /dev/null
+++ b/testing/resources/bug_507316.pdf
@@ -0,0 +1,145 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+  /Type /Catalog
+  /Pages 2 0 R
+  /OpenAction 4 0 R
+  /AcroForm 3 0 R
+>>
+endobj
+2 0 obj <<
+  /Type /Pages
+  /Count 3
+  /Kids [6 0 R 7 0 R 8 0 R ]
+>>
+endobj
+3 0 obj <<
+  /CO [11 0 R]
+  /Fields [11 0 R 9 0 R]
+>>
+endobj
+4 0 obj <<
+  /Type /Action
+  /S /JavaScript
+  /JS 5 0 R
+>>
+endobj
+5 0 obj <<>>
+stream
+var i = 0;
+function run()
+{
+  t = this.getField('txtName1');
+  t2 = this.getField('txtName2');
+  t2.setFocus();
+  t.setFocus();
+  t.value='G';
+}
+
+function remove(){
+  if (i==1){
+    this.removeField('txtName2');
+  }
+  i++;
+}
+
+app.setTimeOut('run()',2000);
+endstream
+endobj
+6 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+>>
+endobj
+7 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+>>
+endobj
+8 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 612 792]
+  /Resources <<>>
+  /Annots [9 0 R 11 0 R]
+>>
+endobj
+9 0 obj <<
+  /FT /Tx
+  /Type /Annot
+  /Subtype /Widget
+  /T (txtName2)
+  /F 4
+  /AP <</N 10 0 R>>
+  /Rect [20 20 400 60]
+>>
+endobj
+10 0 obj <<
+  /Type /XObject
+  /Subtype /Form
+  /FormType 1
+>>
+endobj
+11 0 obj <<
+  /FT /Tx
+  /Type /Annot
+  /Subtype /Widget
+  /T (txtName1)
+  /F 4
+  /AP <</N 12 0 R>>
+  /Rect [200 200 400 260]
+  /AA 13 0 R
+>>
+endobj
+12 0 obj <<
+  /Type /XObject
+  /Subtype /Form
+  /FormType 1
+>>
+endobj
+13 0 obj <<
+  /C 14 0 R
+>>
+endobj
+14 0 obj <<
+  /Type /Action
+  /S /JavaScript
+  /JS 15 0 R
+>>
+endobj
+15 0 obj <<>>
+stream
+this.getField('txtName2').value='B';
+this.pageNum=1;
+remove();
+endstream
+endobj
+xref
+0 16
+0000000000 65535 f 
+0000000015 00000 n 
+0000000106 00000 n 
+0000000182 00000 n 
+0000000243 00000 n 
+0000000309 00000 n 
+0000000602 00000 n 
+0000000697 00000 n 
+0000000792 00000 n 
+0000000912 00000 n 
+0000001043 00000 n 
+0000001113 00000 n 
+0000001261 00000 n 
+0000001331 00000 n 
+0000001365 00000 n 
+0000001433 00000 n 
+trailer <<
+  /Size 15
+  /Root 1 0 R
+>>
+startxref
+1534
+%%EOF