commit 396e872d872b760813036b7e7dd8bb68a8b61598
author Tom Sepez <tsepez@chromium.org> Wed Sep 09 10:16:08 2015 -0700
committer Tom Sepez <tsepez@chromium.org> Wed Sep 09 10:16:08 2015 -0700
tree 30d129ec8bbc06750d656f3eeef479b6e15936f0
parent d6278baea3dec46fec555f7740bde9087e57d8f1

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 .

fpdfsdk/src/fpdfformfill_embeddertest.cpp

diff --git a/fpdfsdk/src/fpdfformfill_embeddertest.cpp b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
index 6baad11..56710b9 100644
--- a/fpdfsdk/src/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfformfill_embeddertest.cpp
@@ -40,6 +40,18 @@
   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);

fpdfsdk/src/fsdk_mgr.cpp

diff --git a/fpdfsdk/src/fsdk_mgr.cpp b/fpdfsdk/src/fsdk_mgr.cpp
index 4d3d01f..4a55622 100644
--- a/fpdfsdk/src/fsdk_mgr.cpp
+++ b/fpdfsdk/src/fsdk_mgr.cpp
@@ -664,15 +664,22 @@
 }
 
 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();