[threadpool] Support multiple fences active at the same time.

The goal is to support having a BEST_EFFORT execution fence prior to
main message loop start in BrowserMainLoop, and a fence when visible
tabs are loading in //chrome/browser/resource_coordinator. The lifetime
of both fences would overlap. The reason for not having a single fence
is that the first fence is controlled by //content whereas the second
fence is controlled by //chrome (precise tracking of whether a tab is
loading is a //chrome feature).

Bug: 839110, 1016825
Change-Id: Icaceee4cf9e5a1d4c52f9bb91e69e9f8125cee2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872807
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711688}

base/task/thread_pool/thread_pool_impl.cc

@@ -350,17 +350,29 @@ void ThreadPoolImpl::JoinForTesting() {
 #endif
 }
 
-void ThreadPoolImpl::SetHasFence(bool has_fence) {
+void ThreadPoolImpl::BeginFence() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK_NE(has_fence_, has_fence);
-  has_fence_ = has_fence;
+  ++num_fences_;
   UpdateCanRunPolicy();
 }
 
-void ThreadPoolImpl::SetHasBestEffortFence(bool has_best_effort_fence) {
+void ThreadPoolImpl::EndFence() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK_NE(has_best_effort_fence_, has_best_effort_fence);
-  has_best_effort_fence_ = has_best_effort_fence;
+  DCHECK_GT(num_fences_, 0);
+  --num_fences_;
+  UpdateCanRunPolicy();
+}
+
+void ThreadPoolImpl::BeginBestEffortFence() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  ++num_best_effort_fences_;
+  UpdateCanRunPolicy();
+}
+
+void ThreadPoolImpl::EndBestEffortFence() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_GT(num_best_effort_fences_, 0);
+  --num_best_effort_fences_;
   UpdateCanRunPolicy();
 }
 
@@ -511,14 +523,14 @@ void ThreadPoolImpl::UpdateCanRunPolicy() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   CanRunPolicy can_run_policy;
-  if ((!has_fence_ && !has_best_effort_fence_ &&
+  if ((num_fences_ == 0 && num_best_effort_fences_ == 0 &&
        !has_disable_best_effort_switch_) ||
       task_tracker_->HasShutdownStarted()) {
     can_run_policy = CanRunPolicy::kAll;
-  } else if (has_fence_) {
+  } else if (num_fences_ != 0) {
     can_run_policy = CanRunPolicy::kNone;
   } else {
-    DCHECK(has_best_effort_fence_ || has_disable_best_effort_switch_);
+    DCHECK(num_best_effort_fences_ > 0 || has_disable_best_effort_switch_);
     can_run_policy = CanRunPolicy::kForegroundOnly;
   }
 

base/task/thread_pool/thread_pool_impl.h

@@ -79,8 +79,10 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance,
   void FlushForTesting() override;
   void FlushAsyncForTesting(OnceClosure flush_callback) override;
   void JoinForTesting() override;
-  void SetHasFence(bool has_fence) override;
-  void SetHasBestEffortFence(bool has_best_effort_fence) override;
+  void BeginFence() override;
+  void EndFence() override;
+  void BeginBestEffortFence() override;
+  void EndBestEffortFence() override;
 
   // TaskExecutor:
   bool PostDelayedTask(const Location& from_here,
@@ -120,8 +122,8 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance,
   void ProcessRipeDelayedTasksForTesting();
 
  private:
-  // Invoked after |has_fence_| or |has_best_effort_fence_| is updated. Sets the
-  // CanRunPolicy in TaskTracker and wakes up workers as appropriate.
+  // Invoked after |num_fences_| or |num_best_effort_fences_| is updated. Sets
+  // the CanRunPolicy in TaskTracker and wakes up workers as appropriate.
   void UpdateCanRunPolicy();
 
   // Returns |traits|, with priority set to TaskPriority::USER_BLOCKING if
@@ -170,10 +172,10 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance,
   // BEST_EFFORT tasks until shutdown.
   const bool has_disable_best_effort_switch_;
 
-  // Whether a fence is preventing execution of tasks of any/BEST_EFFORT
-  // priority. Access controlled by |sequence_checker_|.
-  bool has_fence_ = false;
-  bool has_best_effort_fence_ = false;
+  // Number of fences preventing execution of tasks of any/BEST_EFFORT priority.
+  // Access controlled by |sequence_checker_|.
+  int num_fences_ = 0;
+  int num_best_effort_fences_ = 0;
 
 #if DCHECK_IS_ON()
   // Set once JoinForTesting() has returned.

base/task/thread_pool/thread_pool_instance.cc

@@ -29,24 +29,24 @@ ThreadPoolInstance::InitParams::~InitParams() = default;
 
 ThreadPoolInstance::ScopedExecutionFence::ScopedExecutionFence() {
   DCHECK(g_thread_pool);
-  g_thread_pool->SetHasFence(true);
+  g_thread_pool->BeginFence();
 }
 
 ThreadPoolInstance::ScopedExecutionFence::~ScopedExecutionFence() {
   DCHECK(g_thread_pool);
-  g_thread_pool->SetHasFence(false);
+  g_thread_pool->EndFence();
 }
 
 ThreadPoolInstance::ScopedBestEffortExecutionFence::
     ScopedBestEffortExecutionFence() {
   DCHECK(g_thread_pool);
-  g_thread_pool->SetHasBestEffortFence(true);
+  g_thread_pool->BeginBestEffortFence();
 }
 
 ThreadPoolInstance::ScopedBestEffortExecutionFence::
     ~ScopedBestEffortExecutionFence() {
   DCHECK(g_thread_pool);
-  g_thread_pool->SetHasBestEffortFence(false);
+  g_thread_pool->EndBestEffortFence();
 }
 
 #if !defined(OS_NACL)

base/task/thread_pool/thread_pool_instance.h

@@ -100,11 +100,12 @@ class BASE_EXPORT ThreadPoolInstance {
   };
 
   // A Scoped(BestEffort)ExecutionFence prevents new tasks of any/BEST_EFFORT
-  // priority from being scheduled in ThreadPoolInstance within its scope. Upon
-  // its destruction, tasks that were preeempted are released. Note: the
-  // constructor of Scoped(BestEffort)ExecutionFence will not wait for currently
-  // running tasks (as they were posted before entering this scope and do not
-  // violate the contract; some of them could be CONTINUE_ON_SHUTDOWN and
+  // priority from being scheduled in ThreadPoolInstance within its scope.
+  // Multiple fences can exist at the same time. Upon destruction of all
+  // Scoped(BestEffort)ExecutionFences, tasks that were preeempted are released.
+  // Note: the constructor of Scoped(BestEffort)ExecutionFence will not wait for
+  // currently running tasks (as they were posted before entering this scope and
+  // do not violate the contract; some of them could be CONTINUE_ON_SHUTDOWN and
   // waiting for them to complete is ill-advised).
   class BASE_EXPORT ScopedExecutionFence {
    public:
@@ -244,10 +245,12 @@ class BASE_EXPORT ThreadPoolInstance {
   virtual int GetMaxConcurrentNonBlockedTasksWithTraitsDeprecated(
       const TaskTraits& traits) const = 0;
 
-  // Sets whether a fence prevents execution of tasks of any / BEST_EFFORT
+  // Starts/stops a fence that prevents execution of tasks of any / BEST_EFFORT
   // priority.
-  virtual void SetHasFence(bool can_run) = 0;
-  virtual void SetHasBestEffortFence(bool can_run) = 0;
+  virtual void BeginFence() = 0;
+  virtual void EndFence() = 0;
+  virtual void BeginBestEffortFence() = 0;
+  virtual void EndBestEffortFence() = 0;
 };
 
 }  // namespace base

base/test/thread_pool_test_helpers_android.cc

@@ -12,25 +12,28 @@ namespace base {
 class ThreadPoolTestHelpers {
  public:
   // Enables/disables an execution fence that prevents tasks from running.
-  static void SetThreadPoolExecutionFenceEnabledForTesting(bool has_fence);
+  static void BeginFenceForTesting();
+  static void EndFenceForTesting();
 };
 
 // static
-void ThreadPoolTestHelpers::SetThreadPoolExecutionFenceEnabledForTesting(
-    bool has_fence) {
-  ThreadPoolInstance::Get()->SetHasFence(has_fence);
+void ThreadPoolTestHelpers::BeginFenceForTesting() {
+  ThreadPoolInstance::Get()->BeginFence();
+}
+
+// static
+void ThreadPoolTestHelpers::EndFenceForTesting() {
+  ThreadPoolInstance::Get()->EndFence();
 }
 
 }  // namespace base
 
 void JNI_ThreadPoolTestHelpers_EnableThreadPoolExecutionForTesting(
     JNIEnv* env) {
-  base::ThreadPoolTestHelpers::
-      SetThreadPoolExecutionFenceEnabledForTesting(false);
+  base::ThreadPoolTestHelpers::EndFenceForTesting();
 }
 
 void JNI_ThreadPoolTestHelpers_DisableThreadPoolExecutionForTesting(
     JNIEnv* env) {
-  base::ThreadPoolTestHelpers::
-      SetThreadPoolExecutionFenceEnabledForTesting(true);
-}
+  base::ThreadPoolTestHelpers::BeginFenceForTesting();
+}
\ No newline at end of file