Revert "[TaskScheduler] Label SchedulerWorker threads stacks with an identifying frame"

This reverts commit 5b0cb536.

Reason for revert: find it identified flakes, I'll investigate on Monday

Original change's description:
> [TaskScheduler] Label SchedulerWorker threads stacks with an identifying frame
> 
> I intentionally didn't add "Foreground" to non "Background" workers as
> they may be used for background tasks in some configurations and I don't
> want it to be confusing for developers.
> 
> R=​fdoray@chromium.org
> 
> Bug: 839525
> Change-Id: I8c1e928914aa6d2e35fedc1db4e8a639d39116b2
> Reviewed-on: https://chromium-review.googlesource.com/1044501
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556169}

TBR=gab@chromium.org,fdoray@chromium.org

Change-Id: I660f43ba88eade2ac3e8fedf4d60495eab708054
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 839525
Reviewed-on: https://chromium-review.googlesource.com/1045747Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556306}

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

@@ -76,9 +76,8 @@ class AtomicThreadRefChecker {
 
 class SchedulerWorkerDelegate : public SchedulerWorker::Delegate {
  public:
-  SchedulerWorkerDelegate(const std::string& thread_name,
-                          SchedulerWorker::ThreadLabel thread_label)
-      : thread_name_(thread_name), thread_label_(thread_label) {}
+  SchedulerWorkerDelegate(const std::string& thread_name)
+      : thread_name_(thread_name) {}
 
   void set_worker(SchedulerWorker* worker) {
     DCHECK(!worker_);
@@ -92,10 +91,6 @@ class SchedulerWorkerDelegate : public SchedulerWorker::Delegate {
     worker_->WakeUp();
   }
 
-  SchedulerWorker::ThreadLabel GetThreadLabel() const final {
-    return thread_label_;
-  }
-
   void OnMainEntry(const SchedulerWorker* /* worker */) override {
     thread_ref_checker_.Set();
     PlatformThread::SetName(thread_name_);
@@ -154,7 +149,6 @@ class SchedulerWorkerDelegate : public SchedulerWorker::Delegate {
 
  private:
   const std::string thread_name_;
-  const SchedulerWorker::ThreadLabel thread_label_;
 
   // The SchedulerWorker that has |this| as a delegate. Must be set before
   // starting or posting a task to the SchedulerWorker, because it's used in
@@ -177,9 +171,8 @@ class SchedulerWorkerDelegate : public SchedulerWorker::Delegate {
 class SchedulerWorkerCOMDelegate : public SchedulerWorkerDelegate {
  public:
   SchedulerWorkerCOMDelegate(const std::string& thread_name,
-                             SchedulerWorker::ThreadLabel thread_label,
                              TrackedRef<TaskTracker> task_tracker)
-      : SchedulerWorkerDelegate(thread_name, thread_label),
+      : SchedulerWorkerDelegate(thread_name),
         task_tracker_(std::move(task_tracker)) {}
 
   ~SchedulerWorkerCOMDelegate() override { DCHECK(!scoped_com_initializer_); }
@@ -487,7 +480,7 @@ SchedulerSingleThreadTaskRunnerManager::CreateTaskRunnerWithTraitsImpl(
         worker_name += "Shared";
       worker_name += environment_params.name_suffix;
       worker = CreateAndRegisterSchedulerWorker<DelegateType>(
-          worker_name, thread_mode, environment_params.priority_hint);
+          worker_name, environment_params.priority_hint);
       new_worker = true;
     }
     started = started_;
@@ -526,28 +519,18 @@ void SchedulerSingleThreadTaskRunnerManager::JoinForTesting() {
 template <>
 std::unique_ptr<SchedulerWorkerDelegate>
 SchedulerSingleThreadTaskRunnerManager::CreateSchedulerWorkerDelegate<
-    SchedulerWorkerDelegate>(const std::string& name,
-                             int id,
-                             SingleThreadTaskRunnerThreadMode thread_mode) {
+    SchedulerWorkerDelegate>(const std::string& name, int id) {
   return std::make_unique<SchedulerWorkerDelegate>(
-      StringPrintf("TaskSchedulerSingleThread%s%d", name.c_str(), id),
-      thread_mode == SingleThreadTaskRunnerThreadMode::DEDICATED
-          ? SchedulerWorker::ThreadLabel::DEDICATED
-          : SchedulerWorker::ThreadLabel::SHARED);
+      StringPrintf("TaskSchedulerSingleThread%s%d", name.c_str(), id));
 }
 
 #if defined(OS_WIN)
 template <>
 std::unique_ptr<SchedulerWorkerDelegate>
 SchedulerSingleThreadTaskRunnerManager::CreateSchedulerWorkerDelegate<
-    SchedulerWorkerCOMDelegate>(const std::string& name,
-                                int id,
-                                SingleThreadTaskRunnerThreadMode thread_mode) {
+    SchedulerWorkerCOMDelegate>(const std::string& name, int id) {
   return std::make_unique<SchedulerWorkerCOMDelegate>(
       StringPrintf("TaskSchedulerSingleThreadCOMSTA%s%d", name.c_str(), id),
-      thread_mode == SingleThreadTaskRunnerThreadMode::DEDICATED
-          ? SchedulerWorker::ThreadLabel::DEDICATED_COM
-          : SchedulerWorker::ThreadLabel::SHARED_COM,
       task_tracker_);
 }
 #endif  // defined(OS_WIN)
@@ -556,12 +539,11 @@ template <typename DelegateType>
 SchedulerWorker*
 SchedulerSingleThreadTaskRunnerManager::CreateAndRegisterSchedulerWorker(
     const std::string& name,
-    SingleThreadTaskRunnerThreadMode thread_mode,
     ThreadPriority priority_hint) {
   lock_.AssertAcquired();
   int id = next_worker_id_++;
   std::unique_ptr<SchedulerWorkerDelegate> delegate =
-      CreateSchedulerWorkerDelegate<DelegateType>(name, id, thread_mode);
+      CreateSchedulerWorkerDelegate<DelegateType>(name, id);
   SchedulerWorkerDelegate* delegate_raw = delegate.get();
   scoped_refptr<SchedulerWorker> worker = MakeRefCounted<SchedulerWorker>(
       priority_hint, std::move(delegate), task_tracker_);

base/task_scheduler/scheduler_single_thread_task_runner_manager.h

@@ -98,13 +98,11 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
   template <typename DelegateType>
   std::unique_ptr<SchedulerWorkerDelegate> CreateSchedulerWorkerDelegate(
       const std::string& name,
-      int id,
-      SingleThreadTaskRunnerThreadMode thread_mode);
+      int id);
 
   template <typename DelegateType>
   SchedulerWorker* CreateAndRegisterSchedulerWorker(
       const std::string& name,
-      SingleThreadTaskRunnerThreadMode thread_mode,
       ThreadPriority priority_hint);
 
   template <typename DelegateType>

base/task_scheduler/scheduler_worker.cc

@@ -8,8 +8,6 @@
 
 #include <utility>
 
-#include "base/compiler_specific.h"
-#include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/task_scheduler/task_tracker.h"
 #include "base/trace_event/trace_event.h"
@@ -162,112 +160,6 @@ void SchedulerWorker::UpdateThreadPriority(
 }
 
 void SchedulerWorker::ThreadMain() {
-  if (priority_hint_ == ThreadPriority::BACKGROUND) {
-    switch (delegate_->GetThreadLabel()) {
-      case ThreadLabel::POOLED:
-        RunBackgroundPooledWorker();
-        return;
-      case ThreadLabel::SHARED:
-        RunBackgroundSharedWorker();
-        return;
-      case ThreadLabel::DEDICATED:
-        RunBackgroundDedicatedWorker();
-        return;
-#if defined(OS_WIN)
-      case ThreadLabel::SHARED_COM:
-        RunBackgroundSharedCOMWorker();
-        return;
-      case ThreadLabel::DEDICATED_COM:
-        RunBackgroundDedicatedCOMWorker();
-        return;
-#endif  // defined(OS_WIN)
-    }
-  }
-
-  switch (delegate_->GetThreadLabel()) {
-    case ThreadLabel::POOLED:
-      RunPooledWorker();
-      return;
-    case ThreadLabel::SHARED:
-      RunSharedWorker();
-      return;
-    case ThreadLabel::DEDICATED:
-      RunDedicatedWorker();
-      return;
-#if defined(OS_WIN)
-    case ThreadLabel::SHARED_COM:
-      RunSharedCOMWorker();
-      return;
-    case ThreadLabel::DEDICATED_COM:
-      RunDedicatedCOMWorker();
-      return;
-#endif  // defined(OS_WIN)
-  }
-}
-
-NOINLINE void SchedulerWorker::RunPooledWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunBackgroundPooledWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunSharedWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunBackgroundSharedWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunDedicatedWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunBackgroundDedicatedWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-#if defined(OS_WIN)
-NOINLINE void SchedulerWorker::RunSharedCOMWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunBackgroundSharedCOMWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunDedicatedCOMWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-
-NOINLINE void SchedulerWorker::RunBackgroundDedicatedCOMWorker() {
-  const int line_number = __LINE__;
-  RunWorker();
-  base::debug::Alias(&line_number);
-}
-#endif  // defined(OS_WIN)
-
-void SchedulerWorker::RunWorker() {
   DCHECK_EQ(self_, this);
   TRACE_EVENT_BEGIN0("task_scheduler", "SchedulerWorkerThread active");
 

base/task_scheduler/scheduler_worker.h

@@ -44,19 +44,6 @@ class BASE_EXPORT SchedulerWorker
     : public RefCountedThreadSafe<SchedulerWorker>,
       public PlatformThread::Delegate {
  public:
-  // Labels this SchedulerWorker's association. This doesn't affect any logic
-  // but will add a stack frame labeling this thread for ease of stack trace
-  // identification.
-  enum class ThreadLabel {
-    POOLED,
-    SHARED,
-    DEDICATED,
-#if defined(OS_WIN)
-    SHARED_COM,
-    DEDICATED_COM,
-#endif  // defined(OS_WIN)
-  };
-
   // Delegate interface for SchedulerWorker. All methods except
   // OnCanScheduleSequence() (inherited from CanScheduleSequenceObserver) are
   // called from the thread managed by the SchedulerWorker instance.
@@ -64,10 +51,6 @@ class BASE_EXPORT SchedulerWorker
    public:
     ~Delegate() override = default;
 
-    // Returns the ThreadLabel the Delegate wants its SchedulerWorkers' stacks
-    // to be labeled with.
-    virtual ThreadLabel GetThreadLabel() const = 0;
-
     // Called by |worker|'s thread when it enters its main function.
     virtual void OnMainEntry(const SchedulerWorker* worker) = 0;
 
@@ -173,27 +156,6 @@ class BASE_EXPORT SchedulerWorker
   // PlatformThread::Delegate:
   void ThreadMain() override;
 
-  // Dummy frames to act as "RunLabeledWorker()" (see RunMain() below). Their
-  // impl is aliased to prevent compiler/linker from optimizing them out.
-  void RunPooledWorker();
-  void RunBackgroundPooledWorker();
-  void RunSharedWorker();
-  void RunBackgroundSharedWorker();
-  void RunDedicatedWorker();
-  void RunBackgroundDedicatedWorker();
-#if defined(OS_WIN)
-  void RunSharedCOMWorker();
-  void RunBackgroundSharedCOMWorker();
-  void RunDedicatedCOMWorker();
-  void RunBackgroundDedicatedCOMWorker();
-#endif  // defined(OS_WIN)
-
-  // The real main, invoked through :
-  //     ThreadMain() -> RunLabeledWorker() -> RunWorker().
-  // "RunLabeledWorker()" is a dummy frame based on ThreadLabel+ThreadPriority
-  // and used to easily identify threads in stack traces.
-  void RunWorker();
-
   // Self-reference to prevent destruction of |this| while the thread is alive.
   // Set in Start() before creating the thread. Reset in ThreadMain() before the
   // thread exits. No lock required because the first access occurs before the

base/task_scheduler/scheduler_worker_pool_impl.cc

@@ -73,7 +73,6 @@ class SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl
 
   // SchedulerWorker::Delegate:
   void OnCanScheduleSequence(scoped_refptr<Sequence> sequence) override;
-  SchedulerWorker::ThreadLabel GetThreadLabel() const override;
   void OnMainEntry(const SchedulerWorker* worker) override;
   scoped_refptr<Sequence> GetWork(SchedulerWorker* worker) override;
   void DidRunTask() override;
@@ -385,11 +384,6 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::
   outer_->OnCanScheduleSequence(std::move(sequence));
 }
 
-SchedulerWorker::ThreadLabel
-SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetThreadLabel() const {
-  return SchedulerWorker::ThreadLabel::POOLED;
-}
-
 void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::OnMainEntry(
     const SchedulerWorker* worker) {
   DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);

base/task_scheduler/scheduler_worker_stack_unittest.cc

@@ -25,9 +25,6 @@ class MockSchedulerWorkerDelegate : public SchedulerWorker::Delegate {
   void OnCanScheduleSequence(scoped_refptr<Sequence> sequence) override {
     ADD_FAILURE() << "Unexpected call to OnCanScheduleSequence().";
   }
-  SchedulerWorker::ThreadLabel GetThreadLabel() const override {
-    return SchedulerWorker::ThreadLabel::DEDICATED;
-  }
   void OnMainEntry(const SchedulerWorker* worker) override {}
   scoped_refptr<Sequence> GetWork(SchedulerWorker* worker) override {
     return nullptr;

base/task_scheduler/scheduler_worker_unittest.cc

@@ -53,9 +53,6 @@ class SchedulerWorkerDefaultDelegate : public SchedulerWorker::Delegate {
   void OnCanScheduleSequence(scoped_refptr<Sequence> sequence) override {
     ADD_FAILURE() << "Unexpected call to OnCanScheduleSequence().";
   }
-  SchedulerWorker::ThreadLabel GetThreadLabel() const override {
-    return SchedulerWorker::ThreadLabel::DEDICATED;
-  }
   void OnMainEntry(const SchedulerWorker* worker) override {}
   scoped_refptr<Sequence> GetWork(SchedulerWorker* worker) override {
     return nullptr;

base/task_scheduler/task_scheduler_impl_unittest.cc

@@ -13,7 +13,6 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
-#include "base/debug/stack_trace.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
@@ -642,83 +641,5 @@ TEST_F(TaskSchedulerImplTest, FlushAsyncNoTasks) {
   EXPECT_TRUE(called_back);
 }
 
-namespace {
-
-// Verifies that |query| is found on the current stack. Ignores failures if this
-// configuration doesn't have symbols.
-void VerifyHasStringOnStack(const std::string& query) {
-  const std::string stack = debug::StackTrace().ToString();
-  SCOPED_TRACE(stack);
-  const bool found_on_stack = stack.find(query) != std::string::npos;
-  const bool stack_has_symbols =
-      stack.find("SchedulerWorker") != std::string::npos;
-  EXPECT_TRUE(found_on_stack || !stack_has_symbols) << query;
-}
-
-}  // namespace
-
-// Integration test that verifies that workers have a frame on their stacks
-// which easily identifies the type of worker (useful to diagnose issues from
-// logs without memory dumps).
-TEST_F(TaskSchedulerImplTest, IdentifiableStacks) {
-  StartTaskScheduler();
-
-  scheduler_.CreateSequencedTaskRunnerWithTraits({})->PostTask(
-      FROM_HERE, BindOnce(&VerifyHasStringOnStack, "RunPooledWorker"));
-  scheduler_.CreateSequencedTaskRunnerWithTraits({TaskPriority::BACKGROUND})
-      ->PostTask(FROM_HERE, BindOnce(&VerifyHasStringOnStack,
-                                     "RunBackgroundPooledWorker"));
-
-  scheduler_
-      .CreateSingleThreadTaskRunnerWithTraits(
-          {}, SingleThreadTaskRunnerThreadMode::SHARED)
-      ->PostTask(FROM_HERE,
-                 BindOnce(&VerifyHasStringOnStack, "RunSharedWorker"));
-  scheduler_
-      .CreateSingleThreadTaskRunnerWithTraits(
-          {TaskPriority::BACKGROUND}, SingleThreadTaskRunnerThreadMode::SHARED)
-      ->PostTask(FROM_HERE, BindOnce(&VerifyHasStringOnStack,
-                                     "RunBackgroundSharedWorker"));
-
-  scheduler_
-      .CreateSingleThreadTaskRunnerWithTraits(
-          {}, SingleThreadTaskRunnerThreadMode::DEDICATED)
-      ->PostTask(FROM_HERE,
-                 BindOnce(&VerifyHasStringOnStack, "RunDedicatedWorker"));
-  scheduler_
-      .CreateSingleThreadTaskRunnerWithTraits(
-          {TaskPriority::BACKGROUND},
-          SingleThreadTaskRunnerThreadMode::DEDICATED)
-      ->PostTask(FROM_HERE, BindOnce(&VerifyHasStringOnStack,
-                                     "RunBackgroundDedicatedWorker"));
-
-#if defined(OS_WIN)
-  scheduler_
-      .CreateCOMSTATaskRunnerWithTraits(
-          {}, SingleThreadTaskRunnerThreadMode::SHARED)
-      ->PostTask(FROM_HERE,
-                 BindOnce(&VerifyHasStringOnStack, "RunSharedCOMWorker"));
-  scheduler_
-      .CreateCOMSTATaskRunnerWithTraits(
-          {TaskPriority::BACKGROUND}, SingleThreadTaskRunnerThreadMode::SHARED)
-      ->PostTask(FROM_HERE, BindOnce(&VerifyHasStringOnStack,
-                                     "RunBackgroundSharedCOMWorker"));
-
-  scheduler_
-      .CreateCOMSTATaskRunnerWithTraits(
-          {}, SingleThreadTaskRunnerThreadMode::DEDICATED)
-      ->PostTask(FROM_HERE,
-                 BindOnce(&VerifyHasStringOnStack, "RunDedicatedCOMWorker"));
-  scheduler_
-      .CreateCOMSTATaskRunnerWithTraits(
-          {TaskPriority::BACKGROUND},
-          SingleThreadTaskRunnerThreadMode::DEDICATED)
-      ->PostTask(FROM_HERE, BindOnce(&VerifyHasStringOnStack,
-                                     "RunBackgroundDedicatedCOMWorker"));
-#endif  // defined(OS_WIN)
-
-  scheduler_.FlushForTesting();
-}
-
 }  // namespace internal
 }  // namespace base