PR fixes

5 files changed, 68 insertions, 38 deletions

diff --git a/routing/async_router.hpp b/routing/async_router.hpp
index db5adbe623..148df81826 100644
--- a/routing/async_router.hpp
+++ b/routing/async_router.hpp
@@ -49,9 +49,6 @@ public:
   /// Interrupt routing and clear buffers
   virtual void ClearState();
 
-  /// Wait routing process. For testing use.
-  void WaitRoutingForTesting() { lock_guard<mutex> routingGuard(m_routingMutex); }
-
 private:
   /// This function is called in async mode
   void CalculateRouteImpl(TReadyCallback const & readyCallback, uint32_t timeoutSec);
diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp
index 410f038476..8fa2e10ddf 100644
--- a/routing/osrm_router.cpp
+++ b/routing/osrm_router.cpp
@@ -32,18 +32,25 @@
 #include "3party/osrm/osrm-backend/data_structures/internal_route_result.hpp"
 #include "3party/osrm/osrm-backend/descriptors/description_factory.hpp"
 
-#define INTERRUPT_WHEN_CANCELLED() \
+#define INTERRUPT_WHEN_CANCELLED(DELEGATE) \
   do                               \
   {                                \
-    if (delegate.IsCancelled())    \
+    if (DELEGATE.IsCancelled())    \
       return Cancelled;            \
   } while (false)
 
 namespace routing
 {
+
+namespace
+{
 size_t constexpr kMaxNodeCandidatesCount = 10;
 double constexpr kFeatureFindingRectSideRadiusMeters = 1000.0;
-
+double constexpr kMwmLoadedProgress = 10.0f;
+double constexpr kPointsFoundProgress = 15.0f;
+double constexpr kCrossPathFoundProgress = 50.0f;
+double constexpr kPathFoundProgress = 70.0f;
+} //  namespace
 // TODO (ldragunov) Switch all RawRouteData and incapsulate to own omim types.
 using RawRouteData = InternalRouteResult;
 
@@ -554,7 +561,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint,
   LOG(LINFO, ("Duration of the MWM loading", timer.ElapsedNano()));
   timer.Reset();
 
-  delegate.OnProgress(10.0f);
+  delegate.OnProgress(kMwmLoadedProgress);
 
   // 3. Find start/end nodes.
   TFeatureGraphNodeVec startTask;
@@ -576,11 +583,11 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint,
       m_cachedTargetPoint = finalPoint;
     }
   }
-  INTERRUPT_WHEN_CANCELLED();
+  INTERRUPT_WHEN_CANCELLED(delegate);
 
   LOG(LINFO, ("Duration of the start/stop points lookup", timer.ElapsedNano()));
   timer.Reset();
-  delegate.OnProgress(15.0f);
+  delegate.OnProgress(kPointsFoundProgress);
 
   // 4. Find route.
   RawRoutingResult routingResult;
@@ -598,8 +605,8 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint,
     {
       return RouteNotFound;
     }
-    INTERRUPT_WHEN_CANCELLED();
-    delegate.OnProgress(70.0f);
+    INTERRUPT_WHEN_CANCELLED(delegate);
+    delegate.OnProgress(kPathFoundProgress);
 
     // 5. Restore route.
 
@@ -624,7 +631,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint,
     ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, delegate,
                                             finalPath);
     timer.Reset();
-    delegate.OnProgress(50.0f);
+    delegate.OnProgress(kCrossPathFoundProgress);
 
     // 5. Make generate answer
     if (code == NoError)
@@ -692,7 +699,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation(
 
   for (auto const & segment : routingResult.unpackedPathSegments)
   {
-    INTERRUPT_WHEN_CANCELLED();
+    INTERRUPT_WHEN_CANCELLED(delegate);
 
     // Get all the coordinates for the computed route
     size_t const n = segment.size();
diff --git a/routing/router_delegate.cpp b/routing/router_delegate.cpp
index 42b25209dc..80784539a4 100644
--- a/routing/router_delegate.cpp
+++ b/routing/router_delegate.cpp
@@ -16,12 +16,16 @@ RouterDelegate::RouterDelegate()
 
 void RouterDelegate::SetProgressCallback(TProgressCallback const & progressCallback)
 {
-  m_progressCallback = (progressCallback) ? progressCallback : DefaultProgressFn;
+  m_progressCallback = progressCallback ? progressCallback : DefaultProgressFn;
 }
 
 void RouterDelegate::SetPointCheckCallback(TPointCheckCallback const & pointCallback)
 {
-  m_pointCallback = (pointCallback) ? pointCallback : DefaultPointFn;
+  m_pointCallback = pointCallback ? pointCallback : DefaultPointFn;
+}
+
+TimeoutCancellable::TimeoutCancellable() : m_timeoutSec(0)
+{
 }
 
 bool TimeoutCancellable::IsCancelled() const
@@ -31,6 +35,12 @@ bool TimeoutCancellable::IsCancelled() const
   return Cancellable::IsCancelled();
 }
 
+void TimeoutCancellable::Reset()
+{
+  m_timeoutSec = 0;
+  Cancellable::Reset();
+}
+
 void TimeoutCancellable::SetTimeout(uint32_t timeoutSec)
 {
   m_timeoutSec = timeoutSec;
diff --git a/routing/router_delegate.hpp b/routing/router_delegate.hpp
index ba82340ed4..f2373c816e 100644
--- a/routing/router_delegate.hpp
+++ b/routing/router_delegate.hpp
@@ -12,13 +12,14 @@ namespace routing
 class TimeoutCancellable : public my::Cancellable
 {
 public:
-  TimeoutCancellable() : m_timeoutSec(0) {}
+  TimeoutCancellable();
 
   /// Sets timeout before cancellation. 0 means an infinite timeout.
   void SetTimeout(uint32_t timeoutSec);
 
   // Cancellable overrides:
   bool IsCancelled() const override;
+  void Reset() override;
 
 private:
   my::Timer m_timer;
diff --git a/routing/routing_tests/async_router_test.cpp b/routing/routing_tests/async_router_test.cpp
index f6920c725c..8f826800b3 100644
--- a/routing/routing_tests/async_router_test.cpp
+++ b/routing/routing_tests/async_router_test.cpp
@@ -8,6 +8,8 @@
 
 #include "base/timer.hpp"
 
+#include "std/condition_variable.hpp"
+#include "std/mutex.hpp"
 #include "std/string.hpp"
 #include "std/vector.hpp"
 
@@ -59,54 +61,67 @@ struct DummyResultCallback
 {
   vector<ResultCode> m_codes;
   vector<vector<string>> m_absent;
+  condition_variable m_cv;
+  mutex m_lock;
+  uint32_t const m_expected;
+  uint32_t m_called;
+
+  DummyResultCallback(uint32_t expectedCalls) : m_expected(expectedCalls), m_called(0) {}
 
   void operator()(Route & route, ResultCode code)
   {
     m_codes.push_back(code);
     auto const & absent = route.GetAbsentCountries();
     m_absent.emplace_back(absent.begin(), absent.end());
+    {
+      lock_guard<mutex> calledLock(m_lock);
+      ++m_called;
+      TEST_LESS_OR_EQUAL(m_called, m_expected,
+                         ("The result callback called more times than expected."));
+    }
+    m_cv.notify_all();
+  }
+
+  void WaitFinish()
+  {
+    unique_lock<mutex> lk(m_lock);
+    return m_cv.wait(lk, [this] { return m_called == m_expected; });
   }
 };
 
 UNIT_TEST(NeedMoreMapsSignalTest)
 {
-  unique_ptr<IOnlineFetcher> fetcher(new DummyFetcher({"test1", "test2"}));
+  vector<string> const absentData({"test1", "test2"});
+  unique_ptr<IOnlineFetcher> fetcher(new DummyFetcher(absentData));
   unique_ptr<IRouter> router(new DummyRouter(ResultCode::NoError, {}));
-  DummyResultCallback resultCallback;
-  AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, nullptr);
-  resultCallback.m_codes.clear();
-  resultCallback.m_absent.clear();
-  async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), nullptr, 0);
-
-  // Wait async process start.
-  while (resultCallback.m_codes.empty())
-    ;
+  DummyResultCallback resultCallback(2 /* expectedCalls */);
+  AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback,
+                    nullptr /* pointCheckCallback */);
+  async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2),
+                       nullptr /* progressCallback */, 0 /* timeoutSec */);
 
-  async.WaitRoutingForTesting();
+  resultCallback.WaitFinish();
 
   TEST_EQUAL(resultCallback.m_codes.size(), 2, ());
   TEST_EQUAL(resultCallback.m_codes[0], ResultCode::NoError, ());
   TEST_EQUAL(resultCallback.m_codes[1], ResultCode::NeedMoreMaps, ());
   TEST_EQUAL(resultCallback.m_absent.size(), 2, ());
   TEST(resultCallback.m_absent[0].empty(), ());
-  TEST_EQUAL(resultCallback.m_absent[1].size(), 2, ())
+  TEST_EQUAL(resultCallback.m_absent[1].size(), 2, ());
+  TEST_EQUAL(resultCallback.m_absent[1], absentData, ());
 }
 
 UNIT_TEST(StandartAsyncFogTest)
 {
   unique_ptr<IOnlineFetcher> fetcher(new DummyFetcher({}));
   unique_ptr<IRouter> router(new DummyRouter(ResultCode::NoError, {}));
-  DummyResultCallback resultCallback;
-  AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback, nullptr);
-  resultCallback.m_codes.clear();
-  resultCallback.m_absent.clear();
-  async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2), nullptr, 0);
-
-  // Wait async process start.
-  while (resultCallback.m_codes.empty())
-    ;
+  DummyResultCallback resultCallback(1 /* expectedCalls */);
+  AsyncRouter async(move(router), move(fetcher), DummyStatisticsCallback,
+                    nullptr /* pointCheckCallback */);
+  async.CalculateRoute({1, 2}, {3, 4}, {5, 6}, bind(ref(resultCallback), _1, _2),
+                       nullptr /* progressCallback */, 0 /* timeoutSec */);
 
-  async.WaitRoutingForTesting();
+  resultCallback.WaitFinish();
 
   TEST_EQUAL(resultCallback.m_codes.size(), 1, ());
   TEST_EQUAL(resultCallback.m_codes[0], ResultCode::NoError, ());