From 32e693fe13105fab5baf81b334e932fce62d89b5 Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 4 Aug 2016 12:47:42 -0700 Subject: 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 Review-Url: https://codereview.chromium.org/2214003003 --- fpdfsdk/fpdfformfill_embeddertest.cpp | 23 ++++++ fpdfsdk/javascript/JS_Object.cpp | 29 ++++--- testing/resources/bug_634394.in | 85 +++++++++++++++++++++ testing/resources/bug_634394.pdf | 139 ++++++++++++++++++++++++++++++++++ 4 files changed, 266 insertions(+), 10 deletions(-) create mode 100644 testing/resources/bug_634394.in create mode 100644 testing/resources/bug_634394.pdf diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp index 27b889085c..a1425153a1 100644 --- a/fpdfsdk/fpdfformfill_embeddertest.cpp +++ b/fpdfsdk/fpdfformfill_embeddertest.cpp @@ -134,4 +134,27 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_620428) { 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 b0a307beb1..9ec316303d 100644 --- a/fpdfsdk/javascript/JS_Object.cpp +++ b/fpdfsdk/javascript/JS_Object.cpp @@ -115,16 +115,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 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 diff --git a/testing/resources/bug_634394.in b/testing/resources/bug_634394.in new file mode 100644 index 0000000000..528b4637da --- /dev/null +++ b/testing/resources/bug_634394.in @@ -0,0 +1,85 @@ +{{header}} +{{object 1 0}} << + /Type /Catalog + /Pages 2 0 R + /AcroForm 6 0 R + /Names <> +>> +endobj +{{object 2 0}} << + /Type /Pages + /Count 1 + /Kids [4 0 R] +>> +endobj +{{object 4 0}} << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] + /CropBox [0 0 612 792] + /Resources <<>> +>> +endobj +{{object 6 0}} << + /DR << + /Font <> + >> + /DA (/Helv 0 Tf 0 g) + /Fields [5 0 R] +>> +endobj +{{object 7 0}} << + /Type /Font + /Subtype /Type1 + /BaseFont /Helvetica + /Encoding /WinAnsiEncoding +>> +endobj +{{object 8 0}} << + /Type /XObject + /Subtype /Form + /FormType 1 + /Matrix [1 0 0 1 0 0] + /BBox [0 0 75.907 28.472] + /Resources << + /Font <> + >> +>> +stream +q +Q + + +endstream +endobj +{{object 11 0}} << + /Type /Action + /S /JavaScript + /JS 50 0 R +>> +endobj +{{object 13 0}} << + /Names [(startDelay) 11 0 R] +>> +endobj +{{object 50 0}} << +>> +stream +var timer = app.setTimeOut("fireTimeOut()", 3000); +var interval = app.setInterval("fireInterval()", 1000); +function fireTimeOut() { + app.alert("goodbye world"); + app.clearInterval(interval); +} +function fireInterval() { + app.alert("hello world"); + app.clearInterval(interval); +} +endstream +endobj +{{xref}} +trailer << + /Root 1 0 R +>> +{{startxref}} +%%EOF diff --git a/testing/resources/bug_634394.pdf b/testing/resources/bug_634394.pdf new file mode 100644 index 0000000000..a17a9f213e --- /dev/null +++ b/testing/resources/bug_634394.pdf @@ -0,0 +1,139 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /Type /Catalog + /Pages 2 0 R + /AcroForm 6 0 R + /Names <> +>> +endobj +2 0 obj << + /Type /Pages + /Count 1 + /Kids [4 0 R] +>> +endobj +4 0 obj << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] + /CropBox [0 0 612 792] + /Resources <<>> +>> +endobj +6 0 obj << + /DR << + /Font <> + >> + /DA (/Helv 0 Tf 0 g) + /Fields [5 0 R] +>> +endobj +7 0 obj << + /Type /Font + /Subtype /Type1 + /BaseFont /Helvetica + /Encoding /WinAnsiEncoding +>> +endobj +8 0 obj << + /Type /XObject + /Subtype /Form + /FormType 1 + /Matrix [1 0 0 1 0 0] + /BBox [0 0 75.907 28.472] + /Resources << + /Font <> + >> +>> +stream +q +Q + + +endstream +endobj +11 0 obj << + /Type /Action + /S /JavaScript + /JS 50 0 R +>> +endobj +13 0 obj << + /Names [(startDelay) 11 0 R] +>> +endobj +50 0 obj << +>> +stream +var timer = app.setTimeOut("fireTimeOut()", 3000); +var interval = app.setInterval("fireInterval()", 1000); +function fireTimeOut() { + app.alert("goodbye world"); + app.clearInterval(interval); +} +function fireInterval() { + app.alert("hello world"); + app.clearInterval(interval); +} +endstream +endobj +xref +0 51 +0000000000 65535 f +0000000015 00000 n +0000000118 00000 n +0000000000 65535 f +0000000181 00000 n +0000000000 65535 f +0000000302 00000 n +0000000404 00000 n +0000000509 00000 n +0000000000 65535 f +0000000000 65535 f +0000000701 00000 n +0000000000 65535 f +0000000769 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 +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 +0000000822 00000 n +trailer << + /Root 1 0 R +>> +startxref +1143 +%%EOF -- cgit v1.2.3