From 3cde1a2d144e562b8bf3c8009039ca1cf67a8b44 Mon Sep 17 00:00:00 2001
From: Oliver Chang <ochang@chromium.org>
Date: Mon, 15 Aug 2016 14:28:20 -0700
Subject: Merge to M53: Fix issue when firing TimerProc() destroys timer

We must look the timer up a second time since the callback
may have released it.

BUG=634394
TBR=thestig@chromium.org

Original Review-Url: https://codereview.chromium.org/2214003003
(cherry picked from commit 32e693fe13105fab5baf81b334e932fce62d89b5)

Review URL: https://codereview.chromium.org/2247083002 .
---
 fpdfsdk/fpdfformfill_embeddertest.cpp | 41 +++++++++++++++++++++++++++++++++++
 fpdfsdk/javascript/JS_Object.cpp      | 29 ++++++++++++++++---------
 2 files changed, 60 insertions(+), 10 deletions(-)

(limited to 'fpdfsdk')

diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index c73dc008f7..dfbfc66be5 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -84,4 +84,45 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_551248) {
   EXPECT_EQ(0, alerts[0].type);
   EXPECT_EQ(0, alerts[0].icon);
 }
+
+TEST_F(FPDFFormFillEmbeddertest, BUG_620428) {
+  // Test that timers and intervals are cancelable.
+  EmbedderTestTimerHandlingDelegate delegate;
+  SetDelegate(&delegate);
+
+  EXPECT_TRUE(OpenDocument("bug_620428.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_TRUE(page);
+  DoOpenActions();
+  delegate.AdvanceTime(5000);
+  UnloadPage(page);
+
+  const auto& alerts = delegate.GetAlerts();
+  ASSERT_EQ(1U, alerts.size());
+  EXPECT_STREQ(L"done", alerts[0].message.c_str());
+}
+
+TEST_F(FPDFFormFillEmbeddertest, BUG_634394) {
+  // Cancel timer inside timer callback.
+  EmbedderTestTimerHandlingDelegate delegate;
+  SetDelegate(&delegate);
+
+  EXPECT_TRUE(OpenDocument("bug_634394.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_TRUE(page);
+  DoOpenActions();
+
+  // Timers fire at most once per AdvanceTime(), allow intervals
+  // to fire several times if possible.
+  delegate.AdvanceTime(1000);
+  delegate.AdvanceTime(1000);
+  delegate.AdvanceTime(1000);
+  delegate.AdvanceTime(1000);
+  delegate.AdvanceTime(1000);
+  UnloadPage(page);
+
+  const auto& alerts = delegate.GetAlerts();
+  EXPECT_EQ(2U, alerts.size());
+}
+
 #endif  // PDF_ENABLE_V8
diff --git a/fpdfsdk/javascript/JS_Object.cpp b/fpdfsdk/javascript/JS_Object.cpp
index 0d190a7fa4..9505799048 100644
--- a/fpdfsdk/javascript/JS_Object.cpp
+++ b/fpdfsdk/javascript/JS_Object.cpp
@@ -145,16 +145,25 @@ void CJS_Timer::KillJSTimer() {
 
 // static
 void CJS_Timer::TimerProc(int idEvent) {
-  const auto it = GetGlobalTimerMap()->find(idEvent);
-  if (it != GetGlobalTimerMap()->end()) {
-    CJS_Timer* pTimer = it->second;
-    if (!pTimer->m_bProcessing) {
-      CFX_AutoRestorer<bool> scoped_processing(&pTimer->m_bProcessing);
-      pTimer->m_bProcessing = true;
-      if (pTimer->m_pEmbedObj)
-        pTimer->m_pEmbedObj->TimerProc(pTimer);
-    }
-  }
+  auto it = GetGlobalTimerMap()->find(idEvent);
+  if (it == GetGlobalTimerMap()->end())
+    return;
+
+  CJS_Timer* pTimer = it->second;
+  if (pTimer->m_bProcessing)
+    return;
+
+  pTimer->m_bProcessing = true;
+  if (pTimer->m_pEmbedObj)
+    pTimer->m_pEmbedObj->TimerProc(pTimer);
+
+  // Timer proc may have destroyed timer, find it again.
+  it = GetGlobalTimerMap()->find(idEvent);
+  if (it == GetGlobalTimerMap()->end())
+    return;
+
+  pTimer = it->second;
+  pTimer->m_bProcessing = false;
 }
 
 // static
-- 
cgit v1.2.3