From 8e12029407cf1cca95b7f79bf5e5a5fec5bea9cb Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 3 Aug 2016 14:03:35 -0700 Subject: Add test for bug 620428 (setinterval cancellation) While we're at it, beef up existing test for non-cancellation. In turn, fix test harness to implement intervals properly. In turn, fix public documentation to be clearer about timers. Also rename a few identifiers that sounded "off". Review-Url: https://codereview.chromium.org/2211513002 --- fpdfsdk/fpdfformfill_embeddertest.cpp | 67 ++++++++++-- public/fpdf_formfill.h | 29 +++-- testing/embedder_test_timer_handling_delegate.h | 41 ++++--- testing/resources/bug_551248.in | 11 +- testing/resources/bug_551248.pdf | 13 ++- testing/resources/bug_620428.in | 85 +++++++++++++++ testing/resources/bug_620428.pdf | 139 ++++++++++++++++++++++++ 7 files changed, 331 insertions(+), 54 deletions(-) create mode 100644 testing/resources/bug_620428.in create mode 100644 testing/resources/bug_620428.pdf diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp index c73dc008f7..e2920e041b 100644 --- a/fpdfsdk/fpdfformfill_embeddertest.cpp +++ b/fpdfsdk/fpdfformfill_embeddertest.cpp @@ -24,7 +24,7 @@ TEST_F(FPDFFormFillEmbeddertest, FirstTest) { EXPECT_TRUE(OpenDocument("hello_world.pdf")); FPDF_PAGE page = LoadPage(0); - EXPECT_NE(nullptr, page); + EXPECT_TRUE(page); UnloadPage(page); } @@ -34,7 +34,7 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_487928) { EXPECT_TRUE(OpenDocument("bug_487928.pdf")); FPDF_PAGE page = LoadPage(0); - EXPECT_NE(nullptr, page); + EXPECT_TRUE(page); DoOpenActions(); delegate.AdvanceTime(5000); UnloadPage(page); @@ -46,7 +46,7 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_507316) { EXPECT_TRUE(OpenDocument("bug_507316.pdf")); FPDF_PAGE page = LoadAndCachePage(2); - EXPECT_NE(nullptr, page); + EXPECT_TRUE(page); DoOpenActions(); delegate.AdvanceTime(4000); UnloadPage(page); @@ -55,7 +55,7 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_507316) { TEST_F(FPDFFormFillEmbeddertest, BUG_514690) { EXPECT_TRUE(OpenDocument("hello_world.pdf")); FPDF_PAGE page = LoadPage(0); - EXPECT_NE(nullptr, page); + EXPECT_TRUE(page); // Test that FORM_OnMouseMove() etc. permit null HANDLES and PAGES. FORM_OnMouseMove(nullptr, page, 0, 10.0, 10.0); @@ -66,22 +66,71 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_514690) { #ifdef PDF_ENABLE_V8 TEST_F(FPDFFormFillEmbeddertest, BUG_551248) { + // Test that timers fire once and intervals fire repeatedly. EmbedderTestTimerHandlingDelegate delegate; SetDelegate(&delegate); EXPECT_TRUE(OpenDocument("bug_551248.pdf")); FPDF_PAGE page = LoadPage(0); - EXPECT_NE(nullptr, page); + EXPECT_TRUE(page); DoOpenActions(); - delegate.AdvanceTime(5000); - UnloadPage(page); const auto& alerts = delegate.GetAlerts(); - ASSERT_EQ(1U, alerts.size()); + EXPECT_EQ(0U, alerts.size()); + + delegate.AdvanceTime(1000); + EXPECT_EQ(0U, alerts.size()); // nothing fired. + delegate.AdvanceTime(1000); + EXPECT_EQ(1U, alerts.size()); // interval fired. + delegate.AdvanceTime(1000); + EXPECT_EQ(2U, alerts.size()); // timer fired. + delegate.AdvanceTime(1000); + EXPECT_EQ(3U, alerts.size()); // interval fired again. + delegate.AdvanceTime(1000); + EXPECT_EQ(3U, alerts.size()); // nothing fired. + delegate.AdvanceTime(1000); + EXPECT_EQ(4U, alerts.size()); // interval fired again. + delegate.AdvanceTime(1000); + EXPECT_EQ(4U, alerts.size()); // nothing fired. + UnloadPage(page); - EXPECT_STREQ(L"hello world", alerts[0].message.c_str()); + ASSERT_EQ(4U, alerts.size()); // nothing else fired. + + EXPECT_STREQ(L"interval fired", alerts[0].message.c_str()); EXPECT_STREQ(L"Alert", alerts[0].title.c_str()); EXPECT_EQ(0, alerts[0].type); EXPECT_EQ(0, alerts[0].icon); + + EXPECT_STREQ(L"timer fired", alerts[1].message.c_str()); + EXPECT_STREQ(L"Alert", alerts[1].title.c_str()); + EXPECT_EQ(0, alerts[1].type); + EXPECT_EQ(0, alerts[1].icon); + + EXPECT_STREQ(L"interval fired", alerts[2].message.c_str()); + EXPECT_STREQ(L"Alert", alerts[2].title.c_str()); + EXPECT_EQ(0, alerts[2].type); + EXPECT_EQ(0, alerts[2].icon); + + EXPECT_STREQ(L"interval fired", alerts[3].message.c_str()); + EXPECT_STREQ(L"Alert", alerts[3].title.c_str()); + EXPECT_EQ(0, alerts[3].type); + EXPECT_EQ(0, alerts[3].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(); + EXPECT_EQ(0U, alerts.size()); +} + #endif // PDF_ENABLE_V8 diff --git a/public/fpdf_formfill.h b/public/fpdf_formfill.h index 79e9c26961..248bcd2b02 100644 --- a/public/fpdf_formfill.h +++ b/public/fpdf_formfill.h @@ -316,8 +316,8 @@ typedef struct _IPDF_JsPlatform { #define FXCT_HAND 5 /** - * Declares of a pointer type to the callback function for the FFI_SetTimer - *method. + * Function signature for the callback function passed to the FFI_SetTimer + * method. * Parameters: * idEvent - Identifier of the timer. * Return value: @@ -490,19 +490,18 @@ typedef struct _FPDF_FORMFILLINFO { /** * Method: FFI_SetTimer - * This method installs a system timer. A time-out value is - * specified, - * and every time a time-out occurs, the system passes a message to - * the TimerProc callback function. + * This method installs a system timer. An interval value is specified, + * and every time that interval elapses, the system must call into the + * callback function with the timer ID as returned by this function. * Interface Version: - * 1 + * 1 * Implementation Required: - * yes + * yes * Parameters: * pThis - Pointer to the interface structure itself. * uElapse - Specifies the time-out value, in milliseconds. * lpTimerFunc - A pointer to the callback function-TimerCallback. - * Return value: + * Return value: * The timer identifier of the new timer if the function is successful. * An application passes this value to the FFI_KillTimer method to kill * the timer. Nonzero if it is successful; otherwise, it is zero. @@ -513,16 +512,16 @@ typedef struct _FPDF_FORMFILLINFO { /** * Method: FFI_KillTimer - * This method kills the timer event identified by nIDEvent, set by - * an earlier call to FFI_SetTimer. + * This method uninstalls a system timer identified by nIDEvent, as + * set by an earlier call to FFI_SetTimer. * Interface Version: - * 1 + * 1 * Implementation Required: - * yes + * yes * Parameters: * pThis - Pointer to the interface structure itself. - * nTimerID - The timer ID return by FFI_SetTimer function. - * Return value: + * nTimerID - The timer ID returned by FFI_SetTimer function. + * Return value: * None. * */ void (*FFI_KillTimer)(struct _FPDF_FORMFILLINFO* pThis, int nTimerID); diff --git a/testing/embedder_test_timer_handling_delegate.h b/testing/embedder_test_timer_handling_delegate.h index cb0c31bd95..709dd22c17 100644 --- a/testing/embedder_test_timer_handling_delegate.h +++ b/testing/embedder_test_timer_handling_delegate.h @@ -15,40 +15,38 @@ class EmbedderTestTimerHandlingDelegate : public EmbedderTest::Delegate { public: - struct ReceivedAlert { - ReceivedAlert(FPDF_WIDESTRING message_in, - FPDF_WIDESTRING title_in, - int type_in, - int icon_in) - : type(type_in), icon(icon_in) { - message = GetPlatformWString(message_in); - title = GetPlatformWString(title_in); - } - + struct AlertRecord { std::wstring message; std::wstring title; int type; int icon; }; + struct Timer { + int id; + int interval; + TimerCallback fn; + }; + int Alert(FPDF_WIDESTRING message, FPDF_WIDESTRING title, int type, int icon) override { - alerts_.push_back(ReceivedAlert(message, title, type, icon)); + alerts_.push_back( + {GetPlatformWString(message), GetPlatformWString(title), type, icon}); return 0; } int SetTimer(int msecs, TimerCallback fn) override { expiry_to_timer_map_.insert(std::pair( - msecs + imaginary_elapsed_msecs_, Timer(++next_timer_id_, fn))); + msecs + fake_elapsed_msecs_, {++next_timer_id_, msecs, fn})); return next_timer_id_; } void KillTimer(int id) override { for (auto iter = expiry_to_timer_map_.begin(); iter != expiry_to_timer_map_.end(); ++iter) { - if (iter->second.first == id) { + if (iter->second.id == id) { expiry_to_timer_map_.erase(iter); break; } @@ -56,29 +54,30 @@ class EmbedderTestTimerHandlingDelegate : public EmbedderTest::Delegate { } void AdvanceTime(int increment_msecs) { - imaginary_elapsed_msecs_ += increment_msecs; + fake_elapsed_msecs_ += increment_msecs; while (1) { auto iter = expiry_to_timer_map_.begin(); if (iter == expiry_to_timer_map_.end()) { break; } - Timer t = iter->second; - if (t.first > imaginary_elapsed_msecs_) { + if (iter->first > fake_elapsed_msecs_) { break; } + Timer t = iter->second; expiry_to_timer_map_.erase(iter); - t.second(t.first); // Fire timer. + expiry_to_timer_map_.insert( + std::pair(fake_elapsed_msecs_ + t.interval, t)); + t.fn(t.id); // Fire timer. } } - const std::vector& GetAlerts() const { return alerts_; } + const std::vector& GetAlerts() const { return alerts_; } protected: - using Timer = std::pair; // ID, callback pair. std::multimap expiry_to_timer_map_; // Keyed by timeout. int next_timer_id_ = 0; - int imaginary_elapsed_msecs_ = 0; - std::vector alerts_; + int fake_elapsed_msecs_ = 0; + std::vector alerts_; }; #endif // TESTING_EMBEDDER_TEST_TIMER_HANDLING_DELEGATE_H_ diff --git a/testing/resources/bug_551248.in b/testing/resources/bug_551248.in index ad10e93912..c577824873 100644 --- a/testing/resources/bug_551248.in +++ b/testing/resources/bug_551248.in @@ -65,11 +65,14 @@ endobj {{object 50 0}} << >> stream -function startDelay() -{ - app.alert("hello world"); +function fireTimeOut() { + app.alert("timer fired"); } -app.setTimeOut("startDelay()", 3000); +function fireInterval() { + app.alert("interval fired"); +} +app.setTimeOut("fireTimeOut()", 3000); +app.setInterval("fireInterval()", 2000); endstream endobj {{xref}} diff --git a/testing/resources/bug_551248.pdf b/testing/resources/bug_551248.pdf index cca1872ee7..dea541a8de 100644 --- a/testing/resources/bug_551248.pdf +++ b/testing/resources/bug_551248.pdf @@ -66,11 +66,14 @@ endobj 50 0 obj << >> stream -function startDelay() -{ - app.alert("hello world"); +function fireTimeOut() { + app.alert("timer fired"); } -app.setTimeOut("startDelay()", 3000); +function fireInterval() { + app.alert("interval fired"); +} +app.setTimeOut("fireTimeOut()", 3000); +app.setInterval("fireInterval()", 2000); endstream endobj xref @@ -130,5 +133,5 @@ trailer << /Root 1 0 R >> startxref -954 +1055 %%EOF diff --git a/testing/resources/bug_620428.in b/testing/resources/bug_620428.in new file mode 100644 index 0000000000..1942305173 --- /dev/null +++ b/testing/resources/bug_620428.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 +function fireTimeOut() { + app.alert("hello world"); +} +function fireInterval() { + app.alert("goodbye world"); +} +var timer = app.setTimeOut("fireTimeOut()", 3000); +var interval = app.setInterval("fireInterval()", 1000); +app.clearTimeOut(timer); +app.clearInterval(interval); +endstream +endobj +{{xref}} +trailer << + /Root 1 0 R +>> +{{startxref}} +%%EOF diff --git a/testing/resources/bug_620428.pdf b/testing/resources/bug_620428.pdf new file mode 100644 index 0000000000..ff625b7a2e --- /dev/null +++ b/testing/resources/bug_620428.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 +function fireTimeOut() { + app.alert("hello world"); +} +function fireInterval() { + app.alert("goodbye world"); +} +var timer = app.setTimeOut("fireTimeOut()", 3000); +var interval = app.setInterval("fireInterval()", 1000); +app.clearTimeOut(timer); +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 +1135 +%%EOF -- cgit v1.2.3