From 4f7f6fb6bd586d7fa1d8fa9b798fc7ece4ffd4c4 Mon Sep 17 00:00:00 2001
From: vng <viktor.govako@gmail.com>
Date: Wed, 10 Jul 2013 20:44:53 +0300
Subject: [PATCH] Fix ScheduledTask tests with blocking Cancel.

---
 android/jni/com/mapswithme/maps/Framework.cpp |  2 +-
 base/base_tests/scheduled_task_test.cpp       | 22 ++++++++++++-----
 base/scheduled_task.cpp                       | 24 ++++++++++++-------
 base/scheduled_task.hpp                       | 10 +++++---
 qt/draw_widget.cpp                            |  8 +++----
 qt/draw_widget.hpp                            |  2 +-
 6 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp
index 2802bfa8ab..b17b5e7740 100644
--- a/android/jni/com/mapswithme/maps/Framework.cpp
+++ b/android/jni/com/mapswithme/maps/Framework.cpp
@@ -247,7 +247,7 @@ namespace android
   {
     if (m_scheduledTask)
     {
-      if (!m_scheduledTask->Cancel())
+      if (!m_scheduledTask->CancelNoBlocking())
       {
         // The task is already running - skip new task.
         return false;
diff --git a/base/base_tests/scheduled_task_test.cpp b/base/base_tests/scheduled_task_test.cpp
index f9a877d5dd..26a06af88c 100644
--- a/base/base_tests/scheduled_task_test.cpp
+++ b/base/base_tests/scheduled_task_test.cpp
@@ -1,8 +1,10 @@
-#include "../scheduled_task.hpp"
-#include "../thread.hpp"
 #include "../../testing/testing.hpp"
+
+#include "../scheduled_task.hpp"
+
 #include "../../std/bind.hpp"
 
+
 namespace
 {
   void add_int(int & val, int a)
@@ -17,12 +19,16 @@ namespace
 }
 
 
+/// @todo Next tests are based on assumptions that some delays are suitable for
+/// performing needed checks, before a task will fire.
+
 UNIT_TEST(ScheduledTask_Smoke)
 {
   int val = 0;
 
   ScheduledTask t(bind(&add_int, ref(val), 10), 1000);
 
+  // Assume that t thread isn't fired yet.
   TEST_EQUAL(val, 0, ());
 
   threads::Sleep(1100);
@@ -34,9 +40,11 @@ UNIT_TEST(ScheduledTask_CancelInfinite)
 {
   int val = 2;
 
-  ScheduledTask t0(bind(&add_int, ref(val), 10), -1);
+  ScheduledTask t0(bind(&add_int, ref(val), 10), static_cast<unsigned>(-1));
 
-  t0.Cancel();
+  t0.CancelBlocking();
+
+  TEST_EQUAL(val, 2, ());
 }
 
 UNIT_TEST(ScheduledTask_Cancel)
@@ -48,7 +56,8 @@ UNIT_TEST(ScheduledTask_Cancel)
 
   TEST_EQUAL(val, 2, ());
 
-  t0.Cancel();
+  // Assume that t0 thread isn't fired yet.
+  t0.CancelBlocking();
 
   threads::Sleep(1100);
 
@@ -62,8 +71,9 @@ UNIT_TEST(ScheduledTask_NoWaitInCancel)
   ScheduledTask t0(bind(&add_int, ref(val), 10), 1000);
   ScheduledTask t1(bind(&mul_int, ref(val), 3), 500);
 
-  t0.Cancel();
+  t0.CancelBlocking();
 
+  // Assume that t1 thread isn't fired yet.
   val += 3;
 
   threads::Sleep(600);
diff --git a/base/scheduled_task.cpp b/base/scheduled_task.cpp
index 56643778b1..ba5ba78372 100644
--- a/base/scheduled_task.cpp
+++ b/base/scheduled_task.cpp
@@ -31,24 +31,32 @@ void ScheduledTask::Routine::Do()
   m_pCond->Unlock();
 }
 
+void ScheduledTask::Routine::Cancel()
+{
+  threads::IRoutine::Cancel();
+
+  m_pCond->Signal();
+  m_pCond->Unlock();
+}
+
 ScheduledTask::ScheduledTask(fn_t const & fn, unsigned ms)
   : m_routine(new Routine(fn, ms, &m_cond))
 {
   m_thread.Create(m_routine.get());
 }
 
-bool ScheduledTask::Cancel()
+bool ScheduledTask::CancelNoBlocking()
 {
   if (m_cond.TryLock())
   {
-    m_routine->Cancel();
-
-    m_cond.Signal();
-    m_cond.Unlock();
-
-    m_thread.Join();
+    m_thread.Cancel();
     return true;
   }
-
   return false;
 }
+
+void ScheduledTask::CancelBlocking()
+{
+  m_cond.Lock();
+  m_thread.Cancel();
+}
diff --git a/base/scheduled_task.hpp b/base/scheduled_task.hpp
index 86a885c1a1..f3db1df0e6 100644
--- a/base/scheduled_task.hpp
+++ b/base/scheduled_task.hpp
@@ -22,6 +22,7 @@ class ScheduledTask
   public:
     Routine(fn_t const & fn, unsigned ms, threads::Condition * cond);
     virtual void Do();
+    virtual void Cancel();
   };
 
   scoped_ptr<Routine> m_routine;
@@ -32,7 +33,10 @@ public:
   /// Constructor by function and time in miliseconds.
   ScheduledTask(fn_t const & fn, unsigned ms);
 
-  /// Task could be cancelled before time elapses. This function is NON-blocking.
-  /// @return false If the task is already running.
-  bool Cancel();
+  /// @name Task could be cancelled before time elapses.
+  //@{
+  /// @return false If the task is already running or in some intermediate state.
+  bool CancelNoBlocking();
+  void CancelBlocking();
+  //@}
 };
diff --git a/qt/draw_widget.cpp b/qt/draw_widget.cpp
index 42c1260f2c..1ba822ddc0 100644
--- a/qt/draw_widget.cpp
+++ b/qt/draw_widget.cpp
@@ -336,20 +336,20 @@ namespace qt
   void DrawWidget::StartPressTask(m2::PointD const & pt, unsigned ms)
   {
     if (KillPressTask())
-      m_scheduledTasks.reset(new ScheduledTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), ms));
+      m_scheduledTask.reset(new ScheduledTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), ms));
   }
 
   bool DrawWidget::KillPressTask()
   {
-    if (m_scheduledTasks)
+    if (m_scheduledTask)
     {
-      if (!m_scheduledTasks->Cancel())
+      if (!m_scheduledTask->CancelNoBlocking())
       {
         // The task is already running - skip new task.
         return false;
       }
 
-      m_scheduledTasks.reset();
+      m_scheduledTask.reset();
     }
     return true;
   }
diff --git a/qt/draw_widget.hpp b/qt/draw_widget.hpp
index 7c67194a31..86b26a3947 100644
--- a/qt/draw_widget.hpp
+++ b/qt/draw_widget.hpp
@@ -142,7 +142,7 @@ namespace qt
 
     QScaleSlider * m_pScale;
 
-    scoped_ptr<ScheduledTask> m_scheduledTasks;
+    scoped_ptr<ScheduledTask> m_scheduledTask;
     m2::PointD m_taskPoint;
     bool m_wasLongClick, m_isCleanSingleClick;