From 5104885bbcb6132636f29c11157172f68a38b3b3 Mon Sep 17 00:00:00 2001
From: Arsentiy Milchakov <milcars@mapswithme.com>
Date: Fri, 29 May 2020 18:53:33 +0300
Subject: [PATCH] [base] thread pool delayed review fixes

---
 base/base_tests/linked_map_tests.cpp          |  2 +-
 base/base_tests/thread_pool_delayed_tests.cpp | 14 +++++++-------
 base/linked_map.hpp                           |  2 +-
 base/thread_pool_delayed.cpp                  |  8 ++++----
 base/thread_pool_delayed.hpp                  |  6 +++---
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/base/base_tests/linked_map_tests.cpp b/base/base_tests/linked_map_tests.cpp
index e56fe204d1..c08b804160 100644
--- a/base/base_tests/linked_map_tests.cpp
+++ b/base/base_tests/linked_map_tests.cpp
@@ -27,7 +27,7 @@ UNIT_TEST(LinkedMap_Smoke)
   TEST_EQUAL(getResult, "world", ());
 
   TEST_EQUAL(container.Front(), "hello", ());
-  container.Pop();
+  container.PopFront();
   TEST_EQUAL(container.Front(), "world", ());
 
   TEST_EQUAL(container.Size(), 2, ());
diff --git a/base/base_tests/thread_pool_delayed_tests.cpp b/base/base_tests/thread_pool_delayed_tests.cpp
index d0014c43e5..f4ea43e97a 100644
--- a/base/base_tests/thread_pool_delayed_tests.cpp
+++ b/base/base_tests/thread_pool_delayed_tests.cpp
@@ -151,19 +151,19 @@ UNIT_TEST(ThreadPoolDelayed_CancelImmediate)
         ++value;
         testing::Wait();
       });
-      TEST_EQUAL(id, ThreadPool::kImmediateMinId + 1, ());
+      TEST_EQUAL(id, ThreadPool::kImmediateMinId, ());
     }
 
     {
       cancelTaskId = thread.Push([&]() { value += 1023; });
 
-      TEST_EQUAL(cancelTaskId, ThreadPool::kImmediateMinId + 2, ());
+      TEST_EQUAL(cancelTaskId, ThreadPool::kImmediateMinId + 1, ());
     }
 
     {
       auto const id = thread.Push([&]() { ++value; });
 
-      TEST_EQUAL(id, ThreadPool::kImmediateMinId + 3, ());
+      TEST_EQUAL(id, ThreadPool::kImmediateMinId + 2, ());
     }
 
     TEST(thread.Cancel(cancelTaskId), ());
@@ -185,22 +185,22 @@ UNIT_TEST(ThreadPoolDelayed_CancelDelayed)
     ThreadPool thread;
     {
       auto const id = thread.Push([]() { testing::Wait(); });
-      TEST_EQUAL(id, ThreadPool::kImmediateMinId + 1, ());
+      TEST_EQUAL(id, ThreadPool::kImmediateMinId, ());
     }
 
     {
       auto const delayedId = thread.PushDelayed(milliseconds(1), [&value]() { ++value; });
-      TEST_EQUAL(delayedId, ThreadPool::kDelayedMinId + 1, ());
+      TEST_EQUAL(delayedId, ThreadPool::kDelayedMinId, ());
     }
 
     {
       cancelTaskId = thread.PushDelayed(milliseconds(2), [&]() { value += 1023; });
-      TEST_EQUAL(cancelTaskId, ThreadPool::kDelayedMinId + 2, ());
+      TEST_EQUAL(cancelTaskId, ThreadPool::kDelayedMinId + 1, ());
     }
 
     {
       auto const delayedId = thread.PushDelayed(milliseconds(3), [&value]() { ++value; });
-      TEST_EQUAL(delayedId, ThreadPool::kDelayedMinId + 3, ());
+      TEST_EQUAL(delayedId, ThreadPool::kDelayedMinId + 2, ());
     }
 
     TEST(thread.Cancel(cancelTaskId), ());
diff --git a/base/linked_map.hpp b/base/linked_map.hpp
index 838073813d..a69ae9c7fb 100644
--- a/base/linked_map.hpp
+++ b/base/linked_map.hpp
@@ -27,7 +27,7 @@ public:
     return true;
   }
 
-  void Pop()
+  void PopFront()
   {
     CHECK(!m_map.empty(), ());
     m_map.erase(m_list.front().first);
diff --git a/base/thread_pool_delayed.cpp b/base/thread_pool_delayed.cpp
index b5622c89dd..5bfca0fb7e 100644
--- a/base/thread_pool_delayed.cpp
+++ b/base/thread_pool_delayed.cpp
@@ -23,8 +23,8 @@ TaskLoop::TaskId MakeNextId(TaskLoop::TaskId id, TaskLoop::TaskId minId, TaskLoo
 
 ThreadPool::ThreadPool(size_t threadsCount /* = 1 */, Exit e /* = Exit::SkipPending */)
   : m_exit(e)
-  , m_immediateLastId(kImmediateMinId)
-  , m_delayedLastId(kDelayedMinId)
+  , m_immediateLastId(kImmediateMaxId)
+  , m_delayedLastId(kDelayedMaxId)
 {
   for (size_t i = 0; i < threadsCount; ++i)
     m_threads.emplace_back(threads::SimpleThread(&ThreadPool::ProcessTasks, this));
@@ -144,7 +144,7 @@ void ThreadPool::ProcessTasks()
       if (canExecImmediate)
       {
         tasks[QUEUE_TYPE_IMMEDIATE] = move(m_immediate.Front());
-        m_immediate.Pop();
+        m_immediate.PopFront();
       }
 
       if (canExecDelayed)
@@ -161,7 +161,7 @@ void ThreadPool::ProcessTasks()
     }
   }
 
-  for (; !pendingImmediate.IsEmpty(); pendingImmediate.Pop())
+  for (; !pendingImmediate.IsEmpty(); pendingImmediate.PopFront())
     pendingImmediate.Front()();
 
   while (!pendingDelayed.empty())
diff --git a/base/thread_pool_delayed.hpp b/base/thread_pool_delayed.hpp
index 340d0b5801..a040b1bd18 100644
--- a/base/thread_pool_delayed.hpp
+++ b/base/thread_pool_delayed.hpp
@@ -47,7 +47,7 @@ public:
   ~ThreadPool() override;
 
   // Pushes task to the end of the thread's queue of immediate tasks.
-  // Returns task id or empty string when any error occurs
+  // Returns task id or |TaskLoop::kIncorrectId| when any error occurs
   // or the thread is shut down.
   //
   // The task |t| is going to be executed after all immediate tasks
@@ -56,7 +56,7 @@ public:
   TaskId Push(Task const & t) override;
 
   // Pushes task to the thread's queue of delayed tasks.
-  // Returns task id or empty string when any error occurs
+  // Returns task id or |TaskLoop::kIncorrectId| when any error occurs
   // or the thread is shut down.
   //
   // The task |t| is going to be executed not earlier than after
@@ -110,7 +110,7 @@ private:
     bool operator<(DelayedTask const & rhs) const { return m_when < rhs.m_when; }
     bool operator>(DelayedTask const & rhs) const { return rhs < *this; }
 
-    TaskId m_id;
+    TaskId m_id = kIncorrectId;
     TimePoint m_when = {};
     Task m_task = {};
   };