Timer: Separate code path for single and periodic

* Add helper RunSingleTask() and RunPeriodicTask() to handle two cases
  separately
* In RunTask(), check that it must run on specified message loop thread

Bug: 116081383
Test: Run bluetooth_test_common
Change-Id: Idc41b2a509f43dbe946b2e26f2afd775726514aa

diff --git a/common/timer.cc b/common/timer.cc
index b4d126c..423b6d5 100644 (file)
--- a/common/timer.cc
+++ b/common/timer.cc
@@ -24,6 +24,7 @@ namespace common {
 
 constexpr base::TimeDelta kMinimumPeriod = base::TimeDelta::FromMicroseconds(1);
 
+// This runs on user thread
 Timer::~Timer() {
   std::lock_guard<std::recursive_mutex> api_lock(api_mutex_);
   if (message_loop_thread_ != nullptr && message_loop_thread_->IsRunning()) {
@@ -83,34 +84,34 @@ bool Timer::ScheduleTaskHelper(const base::WeakPtr<MessageLoopThread>& thread,
 
 // This runs on user thread
 void Timer::Cancel() {
-  std::lock_guard<std::recursive_mutex> api_lock(api_mutex_);
-  CancelHelper(false);
+  std::promise<void> promise;
+  CancelHelper(std::move(promise));
 }
 
 // This runs on user thread
 void Timer::CancelAndWait() {
-  std::lock_guard<std::recursive_mutex> api_lock(api_mutex_);
-  CancelHelper(true);
+  std::promise<void> promise;
+  auto future = promise.get_future();
+  CancelHelper(std::move(promise));
+  future.wait();
 }
 
 // This runs on user thread
-void Timer::CancelHelper(bool is_synchronous) {
+void Timer::CancelHelper(std::promise<void> promise) {
+  std::lock_guard<std::recursive_mutex> api_lock(api_mutex_);
   if (message_loop_thread_ == nullptr) {
+    promise.set_value();
     return;
   }
-  std::promise<void> promise;
-  auto future = promise.get_future();
-  if (message_loop_thread_->GetThreadId() ==
-      base::PlatformThread::CurrentId()) {
+  if (!message_loop_thread_->IsRunning() ||
+      message_loop_thread_->GetThreadId() ==
+          base::PlatformThread::CurrentId()) {
     CancelClosure(std::move(promise));
     return;
   }
   message_loop_thread_->DoInThread(
       FROM_HERE, base::BindOnce(&Timer::CancelClosure, base::Unretained(this),
                                 std::move(promise)));
-  if (is_synchronous) {
-    future.wait();
-  }
 }
 
 // This runs on message loop thread
@@ -123,50 +124,63 @@ void Timer::CancelClosure(std::promise<void> promise) {
   promise.set_value();
 }
 
-// This runs in user thread
+// This runs on user thread
 bool Timer::IsScheduled() const {
   std::lock_guard<std::recursive_mutex> api_lock(api_mutex_);
   return message_loop_thread_ != nullptr && message_loop_thread_->IsRunning();
 }
 
-// This runs in message loop thread
+// This runs on message loop thread
 void Timer::RunTask() {
   if (message_loop_thread_ == nullptr || !message_loop_thread_->IsRunning()) {
     LOG(ERROR) << __func__
                << ": message_loop_thread_ is null or is not running";
     return;
   }
+  CHECK_EQ(message_loop_thread_->GetThreadId(),
+           base::PlatformThread::CurrentId())
+      << ": task must run on message loop thread";
   if (is_periodic_) {
-    int64_t period_us = period_.InMicroseconds();
-    expected_time_next_task_us_ += period_us;
-    uint64_t time_now_us = time_get_os_boottime_us();
-    int64_t remaining_time_us = expected_time_next_task_us_ - time_now_us;
-    if (remaining_time_us < 0) {
-      // if remaining_time_us is negative, schedule the task to the nearest
-      // multiple of period
-      remaining_time_us =
-          (remaining_time_us % period_us + period_us) % period_us;
-    }
-    message_loop_thread_->DoInThreadDelayed(
-        FROM_HERE, task_wrapper_,
-        base::TimeDelta::FromMicroseconds(remaining_time_us));
+    RunPeriodicTask();
+  } else {
+    RunSingleTask();
   }
+}
+
+// This runs on message loop thread
+void Timer::RunPeriodicTask() {
+  int64_t period_us = period_.InMicroseconds();
+  expected_time_next_task_us_ += period_us;
+  uint64_t time_now_us = time_get_os_boottime_us();
+  int64_t remaining_time_us = expected_time_next_task_us_ - time_now_us;
+  if (remaining_time_us < 0) {
+    // if remaining_time_us is negative, schedule the task to the nearest
+    // multiple of period
+    remaining_time_us = (remaining_time_us % period_us + period_us) % period_us;
+  }
+  message_loop_thread_->DoInThreadDelayed(
+      FROM_HERE, task_wrapper_,
+      base::TimeDelta::FromMicroseconds(remaining_time_us));
+
   uint64_t time_before_task_us = time_get_os_boottime_us();
   task_.Run();
   uint64_t time_after_task_us = time_get_os_boottime_us();
-  int64_t task_time_us =
+  auto task_time_us =
       static_cast<int64_t>(time_after_task_us - time_before_task_us);
-  if (is_periodic_ && task_time_us > period_.InMicroseconds()) {
+  if (task_time_us > period_.InMicroseconds()) {
     LOG(ERROR) << __func__ << ": Periodic task execution took " << task_time_us
                << " microseconds, longer than interval "
                << period_.InMicroseconds() << " microseconds";
   }
-  if (!is_periodic_) {
-    message_loop_thread_ = nullptr;
-    task_.Reset();
-    period_ = base::TimeDelta();
-    expected_time_next_task_us_ = 0;
-  }
+}
+
+// This runs on message loop thread
+void Timer::RunSingleTask() {
+  task_.Run();
+  message_loop_thread_ = nullptr;
+  task_.Reset();
+  period_ = base::TimeDelta();
+  expected_time_next_task_us_ = 0;
 }
 
 }  // namespace common

diff --git a/common/timer.h b/common/timer.h
index e8532a5..3b8f9a0 100644 (file)
--- a/common/timer.h
+++ b/common/timer.h
@@ -105,13 +105,15 @@ class Timer final {
                           const tracked_objects::Location& from_here,
                           base::Closure task, base::TimeDelta delay,
                           bool is_periodic);
-  void CancelHelper(bool is_synchronous);
+  void CancelHelper(std::promise<void> promise);
   void CancelClosure(std::promise<void> promise);
 
   /**
    * Wraps a task. It posts another same task if the scheduled task is periodic.
    */
   void RunTask();
+  void RunSingleTask();
+  void RunPeriodicTask();
 };
 
 }  // namespace common