[platform] Use condition variable instead of semaphore

The MessageLoopBehavior might change over time: Sometimes we want to
wait because wasm background compilation is going on, sometimes we
don't. This makes the semaphore go out of sync with the task queue (we
always notify it when a new task is scheduled, but we only sometimes
wait on it).
Using a condition variable instead of a semaphore avoids this problem.

R=ahaas@chromium.org

Change-Id: Ib9850efc634f5988d3f824895b6566bd76475985
Reviewed-on: https://chromium-review.googlesource.com/969122Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52057}

src/libplatform/default-foreground-task-runner.cc

@@ -12,9 +12,7 @@ namespace platform {
 
 DefaultForegroundTaskRunner::DefaultForegroundTaskRunner(
     IdleTaskSupport idle_task_support, TimeFunction time_function)
-    : event_loop_control_(0),
-      idle_task_support_(idle_task_support),
-      time_function_(time_function) {}
+    : idle_task_support_(idle_task_support), time_function_(time_function) {}
 
 void DefaultForegroundTaskRunner::Terminate() {
   base::LockGuard<base::Mutex> guard(&lock_);
@@ -27,10 +25,10 @@ void DefaultForegroundTaskRunner::Terminate() {
 }
 
 void DefaultForegroundTaskRunner::PostTaskLocked(
-    std::unique_ptr<Task> task, const base::LockGuard<base::Mutex>& guard) {
+    std::unique_ptr<Task> task, const base::LockGuard<base::Mutex>&) {
   if (terminated_) return;
   task_queue_.push(std::move(task));
-  event_loop_control_.Signal();
+  event_loop_control_.NotifyOne();
 }
 
 void DefaultForegroundTaskRunner::PostTask(std::unique_ptr<Task> task) {
@@ -62,7 +60,8 @@ bool DefaultForegroundTaskRunner::IdleTasksEnabled() {
   return idle_task_support_ == IdleTaskSupport::kEnabled;
 }
 
-std::unique_ptr<Task> DefaultForegroundTaskRunner::PopTaskFromQueue() {
+std::unique_ptr<Task> DefaultForegroundTaskRunner::PopTaskFromQueue(
+    MessageLoopBehavior wait_for_work) {
   base::LockGuard<base::Mutex> guard(&lock_);
   // Move delayed tasks that hit their deadline to the main queue.
   std::unique_ptr<Task> task = PopTaskFromDelayedQueueLocked(guard);
@@ -71,7 +70,10 @@ std::unique_ptr<Task> DefaultForegroundTaskRunner::PopTaskFromQueue() {
     task = PopTaskFromDelayedQueueLocked(guard);
   }
 
-  if (task_queue_.empty()) return {};
+  while (task_queue_.empty()) {
+    if (wait_for_work == MessageLoopBehavior::kDoNotWait) return {};
+    WaitForTaskLocked(guard);
+  }
 
   task = std::move(task_queue_.front());
   task_queue_.pop();
@@ -81,7 +83,7 @@ std::unique_ptr<Task> DefaultForegroundTaskRunner::PopTaskFromQueue() {
 
 std::unique_ptr<Task>
 DefaultForegroundTaskRunner::PopTaskFromDelayedQueueLocked(
-    const base::LockGuard<base::Mutex>& guard) {
+    const base::LockGuard<base::Mutex>&) {
   if (delayed_task_queue_.empty()) return {};
 
   double now = MonotonicallyIncreasingTime();
@@ -109,7 +111,10 @@ std::unique_ptr<IdleTask> DefaultForegroundTaskRunner::PopTaskFromIdleQueue() {
   return task;
 }
 
-void DefaultForegroundTaskRunner::WaitForTask() { event_loop_control_.Wait(); }
+void DefaultForegroundTaskRunner::WaitForTaskLocked(
+    const base::LockGuard<base::Mutex>&) {
+  event_loop_control_.Wait(&lock_);
+}
 
 }  // namespace platform
 }  // namespace v8

src/libplatform/default-foreground-task-runner.h

@@ -9,8 +9,8 @@
 
 #include "include/libplatform/libplatform.h"
 #include "include/v8-platform.h"
+#include "src/base/platform/condition-variable.h"
 #include "src/base/platform/mutex.h"
-#include "src/base/platform/semaphore.h"
 
 namespace v8 {
 namespace platform {
@@ -25,11 +25,11 @@ class V8_PLATFORM_EXPORT DefaultForegroundTaskRunner
 
   void Terminate();
 
-  std::unique_ptr<Task> PopTaskFromQueue();
+  std::unique_ptr<Task> PopTaskFromQueue(MessageLoopBehavior wait_for_work);
 
   std::unique_ptr<IdleTask> PopTaskFromIdleQueue();
 
-  void WaitForTask();
+  void WaitForTaskLocked(const base::LockGuard<base::Mutex>&);
 
   double MonotonicallyIncreasingTime();
 
@@ -47,16 +47,16 @@ class V8_PLATFORM_EXPORT DefaultForegroundTaskRunner
   // The same as PostTask, but the lock is already held by the caller. The
   // {guard} parameter should make sure that the caller is holding the lock.
   void PostTaskLocked(std::unique_ptr<Task> task,
-                      const base::LockGuard<base::Mutex>& guard);
+                      const base::LockGuard<base::Mutex>&);
 
   // A caller of this function has to hold {lock_}. The {guard} parameter should
   // make sure that the caller is holding the lock.
   std::unique_ptr<Task> PopTaskFromDelayedQueueLocked(
-      const base::LockGuard<base::Mutex>& guard);
+      const base::LockGuard<base::Mutex>&);
 
   bool terminated_ = false;
   base::Mutex lock_;
-  base::Semaphore event_loop_control_;
+  base::ConditionVariable event_loop_control_;
   std::queue<std::unique_ptr<Task>> task_queue_;
   IdleTaskSupport idle_task_support_;
   std::queue<std::unique_ptr<IdleTask>> idle_task_queue_;

src/libplatform/default-platform.cc

@@ -138,8 +138,8 @@ void DefaultPlatform::SetTimeFunctionForTesting(
 }
 
 bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate,
-                                      MessageLoopBehavior behavior) {
-  bool failed_result = behavior == MessageLoopBehavior::kWaitForWork;
+                                      MessageLoopBehavior wait_for_work) {
+  bool failed_result = wait_for_work == MessageLoopBehavior::kWaitForWork;
   std::shared_ptr<DefaultForegroundTaskRunner> task_runner;
   {
     base::LockGuard<base::Mutex> guard(&lock_);
@@ -149,11 +149,8 @@ bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate,
     }
     task_runner = foreground_task_runner_map_[isolate];
   }
-  if (behavior == MessageLoopBehavior::kWaitForWork) {
-    task_runner->WaitForTask();
-  }
 
-  std::unique_ptr<Task> task = task_runner->PopTaskFromQueue();
+  std::unique_ptr<Task> task = task_runner->PopTaskFromQueue(wait_for_work);
   if (!task) return failed_result;
 
   task->Run();