Don't put timers with ID == 0 into the global timer map.

A return of ID == 0 from the embedder means the timer was not
created (see public/fpdf_formfill.h), although few embedders
actually conform to this convention.

Firing a timer with ID == 0 will thus do nothing since there can't
be such a timer in the map.

BUG=679649

Review-Url: https://codereview.chromium.org/2626863003

5 files changed, 241 insertions, 4 deletions

diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index baf4853f17..47f1a75efb 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -179,4 +179,21 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_634716) {
   EXPECT_EQ(2U, alerts.size());
 }
 
+TEST_F(FPDFFormFillEmbeddertest, BUG_679649) {
+  EmbedderTestTimerHandlingDelegate delegate;
+  SetDelegate(&delegate);
+
+  EXPECT_TRUE(OpenDocument("bug_679649.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_TRUE(page);
+
+  delegate.SetFailNextTimer();
+  DoOpenActions();
+  delegate.AdvanceTime(2000);
+  UnloadPage(page);
+
+  const auto& alerts = delegate.GetAlerts();
+  EXPECT_EQ(0u, alerts.size());
+}
+
 #endif  // PDF_ENABLE_V8
diff --git a/fpdfsdk/javascript/app.cpp b/fpdfsdk/javascript/app.cpp
index 80c952d6ca..6562d1b598 100644
--- a/fpdfsdk/javascript/app.cpp
+++ b/fpdfsdk/javascript/app.cpp
@@ -75,7 +75,8 @@ GlobalTimer::GlobalTimer(app* pObj,
       m_pFormFillEnv(pFormFillEnv) {
   CFX_SystemHandler* pHandler = m_pFormFillEnv->GetSysHandler();
   m_nTimerID = pHandler->SetTimer(dwElapse, Trigger);
-  (*GetGlobalTimerMap())[m_nTimerID] = this;
+  if (m_nTimerID)
+    (*GetGlobalTimerMap())[m_nTimerID] = this;
 }
 
 GlobalTimer::~GlobalTimer() {
diff --git a/testing/embedder_test_timer_handling_delegate.h b/testing/embedder_test_timer_handling_delegate.h
index 709dd22c17..31966159eb 100644
--- a/testing/embedder_test_timer_handling_delegate.h
+++ b/testing/embedder_test_timer_handling_delegate.h
@@ -38,9 +38,11 @@ class EmbedderTestTimerHandlingDelegate : public EmbedderTest::Delegate {
   }
 
   int SetTimer(int msecs, TimerCallback fn) override {
-    expiry_to_timer_map_.insert(std::pair<int, Timer>(
-        msecs + fake_elapsed_msecs_, {++next_timer_id_, msecs, fn}));
-    return next_timer_id_;
+    int id = fail_next_timer_ ? 0 : ++next_timer_id_;
+    expiry_to_timer_map_.insert(
+        std::pair<int, Timer>(msecs + fake_elapsed_msecs_, {id, msecs, fn}));
+    fail_next_timer_ = false;
+    return id;
   }
 
   void KillTimer(int id) override {
@@ -73,8 +75,11 @@ class EmbedderTestTimerHandlingDelegate : public EmbedderTest::Delegate {
 
   const std::vector<AlertRecord>& GetAlerts() const { return alerts_; }
 
+  void SetFailNextTimer() { fail_next_timer_ = true; }
+
  protected:
   std::multimap<int, Timer> expiry_to_timer_map_;  // Keyed by timeout.
+  bool fail_next_timer_ = false;
   int next_timer_id_ = 0;
   int fake_elapsed_msecs_ = 0;
   std::vector<AlertRecord> alerts_;
diff --git a/testing/resources/bug_679649.in b/testing/resources/bug_679649.in
new file mode 100644
index 0000000000..338f910c32
--- /dev/null
+++ b/testing/resources/bug_679649.in
@@ -0,0 +1,80 @@
+{{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 ping() {
+  app.alert("ping");
+}
+var timer = app.setTimeOut("ping()", 100);
+app.clearTimeOut(timer);
+endstream
+endobj
+{{xref}}
+trailer <<
+  /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_679649.pdf b/testing/resources/bug_679649.pdf
new file mode 100644
index 0000000000..f94d463f92
--- /dev/null
+++ b/testing/resources/bug_679649.pdf
@@ -0,0 +1,134 @@
+%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 ping() {
+  app.alert("ping");
+}
+var timer = app.setTimeOut("ping()", 100);
+app.clearTimeOut(timer);
+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
+970
+%%EOF