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
diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index c73dc00..e2920e0 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -24,7 +24,7 @@
EXPECT_TRUE(OpenDocument("hello_world.pdf"));
FPDF_PAGE page = LoadPage(0);
- EXPECT_NE(nullptr, page);
+ EXPECT_TRUE(page);
UnloadPage(page);
}
@@ -34,7 +34,7 @@
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 @@
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_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 @@
#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();
+
+ const auto& alerts = delegate.GetAlerts();
+ 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);
+
+ 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();
- ASSERT_EQ(1U, alerts.size());
-
- EXPECT_STREQ(L"hello world", 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_EQ(0U, alerts.size());
}
+
#endif // PDF_ENABLE_V8
diff --git a/public/fpdf_formfill.h b/public/fpdf_formfill.h
index 79e9c26..248bcd2 100644
--- a/public/fpdf_formfill.h
+++ b/public/fpdf_formfill.h
@@ -316,8 +316,8 @@
#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 @@
/**
* 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 @@
/**
* 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 cb0c31b..709dd22 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<int, Timer>(
- 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 @@
}
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<int, Timer>(fake_elapsed_msecs_ + t.interval, t));
+ t.fn(t.id); // Fire timer.
}
}
- const std::vector<ReceivedAlert>& GetAlerts() const { return alerts_; }
+ const std::vector<AlertRecord>& GetAlerts() const { return alerts_; }
protected:
- using Timer = std::pair<int, TimerCallback>; // ID, callback pair.
std::multimap<int, Timer> expiry_to_timer_map_; // Keyed by timeout.
int next_timer_id_ = 0;
- int imaginary_elapsed_msecs_ = 0;
- std::vector<ReceivedAlert> alerts_;
+ int fake_elapsed_msecs_ = 0;
+ std::vector<AlertRecord> alerts_;
};
#endif // TESTING_EMBEDDER_TEST_TIMER_HANDLING_DELEGATE_H_
diff --git a/testing/resources/bug_551248.in b/testing/resources/bug_551248.in
index ad10e93..c577824 100644
--- a/testing/resources/bug_551248.in
+++ b/testing/resources/bug_551248.in
@@ -65,11 +65,14 @@
{{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 cca1872..dea541a 100644
--- a/testing/resources/bug_551248.pdf
+++ b/testing/resources/bug_551248.pdf
@@ -66,11 +66,14 @@
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 @@
/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 0000000..1942305
--- /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 <</JavaScript 13 0 R>>
+>>
+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 <</Helv 7 0 R>>
+ >>
+ /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 <</FXF0 7 0 R>>
+ >>
+>>
+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 0000000..ff625b7
--- /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 <</JavaScript 13 0 R>>
+>>
+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 <</Helv 7 0 R>>
+ >>
+ /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 <</FXF0 7 0 R>>
+ >>
+>>
+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
