More BrowserThread cleanups after removal of PROCESS_LAUNCHER thread.

Follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/941264

Further cleanup of WithBaseSyncPrimitives() is done in
https://chromium-review.googlesource.com/c/chromium/src/+/961455
as that may have side-effects that risk a revert.

Bug: 815225, 689520
Change-Id: I8ba8bb6709b7d8cc88e07e0f915c3c00d0a80cc2
Reviewed-on: https://chromium-review.googlesource.com/961451
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543311}

components/metrics/call_stack_profile_metrics_provider.cc

@@ -478,8 +478,6 @@ Thread ToExecutionContextThread(CallStackProfileParams::Thread thread) {
       return UNKNOWN_THREAD;
     case CallStackProfileParams::UI_THREAD:
       return UI_THREAD;
-    case CallStackProfileParams::PROCESS_LAUNCHER_THREAD:
-      return PROCESS_LAUNCHER_THREAD;
     case CallStackProfileParams::IO_THREAD:
       return IO_THREAD;
     case CallStackProfileParams::GPU_MAIN_THREAD:

components/metrics/call_stack_profile_params.h

@@ -30,7 +30,6 @@ struct CallStackProfileParams {
 
     // Browser process threads, some of which occur in other processes as well.
     UI_THREAD,
-    PROCESS_LAUNCHER_THREAD,
     IO_THREAD,
 
     // GPU process thread.

components/metrics/public/cpp/call_stack_profile_struct_traits.h

@@ -230,8 +230,6 @@ struct EnumTraits<metrics::mojom::Thread,
         return metrics::mojom::Thread::UNKNOWN_THREAD;
       case metrics::CallStackProfileParams::Thread::UI_THREAD:
         return metrics::mojom::Thread::UI_THREAD;
-      case metrics::CallStackProfileParams::Thread::PROCESS_LAUNCHER_THREAD:
-        return metrics::mojom::Thread::PROCESS_LAUNCHER_THREAD;
       case metrics::CallStackProfileParams::Thread::IO_THREAD:
         return metrics::mojom::Thread::IO_THREAD;
       case metrics::CallStackProfileParams::Thread::GPU_MAIN_THREAD:
@@ -256,9 +254,6 @@ struct EnumTraits<metrics::mojom::Thread,
       case metrics::mojom::Thread::UI_THREAD:
         *out = metrics::CallStackProfileParams::Thread::UI_THREAD;
         return true;
-      case metrics::mojom::Thread::PROCESS_LAUNCHER_THREAD:
-        *out = metrics::CallStackProfileParams::Thread::PROCESS_LAUNCHER_THREAD;
-        return true;
       case metrics::mojom::Thread::IO_THREAD:
         *out = metrics::CallStackProfileParams::Thread::IO_THREAD;
         return true;

components/metrics/public/cpp/call_stack_profile_struct_traits_unittest.cc

@@ -306,8 +306,6 @@ TEST_F(CallStackProfileStructTraitsTest, Thread) {
   EXPECT_TRUE(proxy_->BounceThread(Thread::UI_THREAD, &out));
   EXPECT_EQ(Thread::UI_THREAD, out);
 
-  EXPECT_TRUE(proxy_->BounceThread(Thread::PROCESS_LAUNCHER_THREAD, &out));
-  EXPECT_EQ(Thread::PROCESS_LAUNCHER_THREAD, out);
 
   EXPECT_TRUE(proxy_->BounceThread(Thread::IO_THREAD, &out));
   EXPECT_EQ(Thread::IO_THREAD, out);

components/metrics/public/interfaces/call_stack_profile_collector.mojom

@@ -48,7 +48,6 @@ enum Thread {
   UNKNOWN_THREAD,
 
   UI_THREAD,
-  PROCESS_LAUNCHER_THREAD,
   IO_THREAD,
 
   GPU_MAIN_THREAD,

content/browser/browser_main_loop.cc

@@ -37,10 +37,6 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/system_monitor/system_monitor.h"
 #include "base/task_scheduler/initialization_util.h"
-#include "base/task_scheduler/post_task.h"
-#include "base/task_scheduler/single_thread_task_runner_thread_mode.h"
-#include "base/task_scheduler/task_scheduler.h"
-#include "base/task_scheduler/task_traits.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"

content/browser/browser_main_loop.h

@@ -336,17 +336,7 @@ class CONTENT_EXPORT BrowserMainLoop {
 #endif
 
   // Members initialized in |CreateThreads()| ----------------------------------
-  // Only the IO thread is a real thread by default, other BrowserThreads are
-  // redirected to TaskScheduler under the hood.
   std::unique_ptr<BrowserProcessSubThread> io_thread_;
-#if defined(OS_WIN)
-  // TaskScheduler doesn't support async I/O on Windows as CACHE thread is
-  // the only user and this use case is going away in
-  // https://codereview.chromium.org/2216583003/.
-  // TODO(gavinp): Remove this (and thus enable redirection of the CACHE thread
-  // on Windows) once that CL lands.
-  std::unique_ptr<BrowserProcessSubThread> cache_thread_;
-#endif
 
   // Members initialized in |BrowserThreadsStarted()| --------------------------
   std::unique_ptr<ServiceManagerContext> service_manager_context_;

content/browser/browser_thread_impl.cc

@@ -15,7 +15,6 @@
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_thread_delegate.h"
@@ -117,8 +116,8 @@ struct BrowserThreadGlobals {
   base::Lock lock;
 
   // This array is filled either as the underlying threads start and invoke
-  // Init() or in RedirectThreadIDToTaskRunner() for threads that are being
+  // Init() or in BrowserThreadImpl() when a MessageLoop* is provided at
-  // redirected. It is not emptied during shutdown in order to support
+  // construction. It is not emptied during shutdown in order to support
   // RunsTasksInCurrentSequence() until the very end.
   scoped_refptr<base::SingleThreadTaskRunner>
       task_runners[BrowserThread::ID_COUNT];
@@ -363,69 +362,6 @@ bool BrowserThreadImpl::StartAndWaitForTesting() {
   return true;
 }
 
-// static
-void BrowserThreadImpl::RedirectThreadIDToTaskRunner(
-    BrowserThread::ID identifier,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
-  DCHECK(task_runner);
-
-  BrowserThreadGlobals& globals = g_globals.Get();
-  base::AutoLock lock(globals.lock);
-
-  DCHECK(!globals.task_runners[identifier]);
-  DCHECK_EQ(globals.states[identifier], BrowserThreadState::UNINITIALIZED);
-
-  globals.task_runners[identifier] = std::move(task_runner);
-  globals.states[identifier] = BrowserThreadState::RUNNING;
-}
-
-// static
-void BrowserThreadImpl::StopRedirectionOfThreadID(
-    BrowserThread::ID identifier) {
-  BrowserThreadGlobals& globals = g_globals.Get();
-  base::AutoLock auto_lock(globals.lock);
-
-  DCHECK(globals.task_runners[identifier]);
-
-  // Change the state to SHUTDOWN to stop accepting new tasks. Note: this is
-  // different from non-redirected threads which continue accepting tasks while
-  // being joined and only quit when idle. However, any tasks for which this
-  // difference matters was already racy as any thread posting a task after the
-  // Signal task below can't be synchronized with the joining thread. Therefore,
-  // that task could already come in before or after the join had completed in
-  // the non-redirection world. Entering SHUTDOWN early merely skews this race
-  // towards making it less likely such a task is accepted by the joined thread
-  // which is fine.
-  DCHECK_EQ(globals.states[identifier], BrowserThreadState::RUNNING);
-  globals.states[identifier] = BrowserThreadState::SHUTDOWN;
-
-  // Wait for all pending tasks to complete.
-  base::WaitableEvent flushed(base::WaitableEvent::ResetPolicy::MANUAL,
-                              base::WaitableEvent::InitialState::NOT_SIGNALED);
-  globals.task_runners[identifier]->PostTask(
-      FROM_HERE,
-      base::BindOnce(&base::WaitableEvent::Signal, base::Unretained(&flushed)));
-  {
-    base::AutoUnlock auto_unlock(globals.lock);
-    flushed.Wait();
-  }
-
-  // Only reset the task runner after running pending tasks so that
-  // BrowserThread::CurrentlyOn() works in their scope.
-  globals.task_runners[identifier] = nullptr;
-
-  // Note: it's still possible for tasks to be posted to that task runner after
-  // this point (e.g. through a previously obtained ThreadTaskRunnerHandle or by
-  // one of the last tasks re-posting to its ThreadTaskRunnerHandle) but the
-  // BrowserThread API itself won't accept tasks. Such tasks are ultimately
-  // guaranteed to run before TaskScheduler::Shutdown() returns but may break
-  // the assumption in PostTaskHelper that BrowserThread::ID A > B will always
-  // succeed to post to B. This is pretty much the only observable difference
-  // between a redirected thread and a real one and is one we're willing to live
-  // with for this experiment. TODO(gab): fix this before enabling the
-  // experiment by default on trunk, http://crbug.com/653916.
-}
-
 // static
 bool BrowserThreadImpl::PostTaskHelper(BrowserThread::ID identifier,
                                        const base::Location& from_here,

content/browser/browser_thread_impl.h

@@ -6,7 +6,6 @@
 #define CONTENT_BROWSER_BROWSER_THREAD_IMPL_H_
 
 #include "base/callback.h"
-#include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
@@ -48,16 +47,6 @@ class CONTENT_EXPORT BrowserThreadImpl : public BrowserThread,
   // https://crbug.com/729596.
   void InitIOThreadDelegate();
 
-  // Redirects tasks posted to |identifier| to |task_runner|.
-  static void RedirectThreadIDToTaskRunner(
-      BrowserThread::ID identifier,
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
-
-  // Makes this |identifier| no longer accept tasks and synchronously flushes
-  // any tasks previously posted to it.
-  // Can only be called after a matching RedirectThreadIDToTaskRunner call.
-  static void StopRedirectionOfThreadID(BrowserThread::ID identifier);
-
   // Resets globals for |identifier|. Used in tests to clear global state that
   // would otherwise leak to the next test. Globals are not otherwise fully
   // cleaned up in ~BrowserThreadImpl() as there are subtle differences between

content/browser/child_process_launcher_helper.cc

@@ -207,6 +207,7 @@ base::SingleThreadTaskRunner* GetProcessLauncherTaskRunner() {
       launcher_task_runner(
           android::LauncherThread::GetMessageLoop()->task_runner());
 #else   // defined(OS_ANDROID)
+  // TODO(gab): WithBaseSyncPrimitives() is likely not required here.
   constexpr base::TaskTraits task_traits = {
       base::MayBlock(), base::WithBaseSyncPrimitives(),
       base::TaskPriority::USER_BLOCKING,

content/public/browser/browser_thread.h

@@ -13,17 +13,12 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/task_runner_util.h"
 #include "base/time/time.h"
 #include "content/common/content_export.h"
 
-namespace base {
-class MessageLoop;
-class Thread;
-}
-
 namespace content {
 
 class BrowserThreadDelegate;
@@ -68,13 +63,11 @@ class CONTENT_EXPORT BrowserThread {
     UI,
 
     // This is the thread that processes non-blocking IO, i.e. IPC and network.
-    // Blocking IO should happen in TaskScheduler.
+    // Blocking I/O should happen in TaskScheduler.
     IO,
 
-    // NOTE: do not add new threads here that are only used by a small number of
+    // NOTE: do not add new threads here. Instead you should just use
-    // files. Instead you should just use a Thread class and pass its
+    // base::Create*TaskRunnerWithTraits.
-    // task runner around. Named threads there are only for threads that
-    // are used in many places.
 
     // This identifier does not represent a thread.  Instead it counts the
     // number of well-known threads.  Insert new well-known threads before this

docs/threading_and_tasks.md

@@ -596,28 +596,6 @@ TEST(MyTest, MyTest) {
 }
 ```
 
-## Legacy Post Task APIs
-
-The Chrome browser process has a few legacy named threads (aka
-“BrowserThreads”). Each of these threads runs a specific type of task (e.g. the
-`FILE` thread handles low priority file operations, the `FILE_USER_BLOCKING`
-thread handles high priority file operations, the `CACHE` thread handles cache
-operations…). Usage of these named threads is now discouraged. New code should
-post tasks to task scheduler via
-[`base/task_scheduler/post_task.h`](https://cs.chromium.org/chromium/src/base/task_scheduler/post_task.h)
-instead.
-
-If for some reason you absolutely need to post a task to a legacy named thread
-(e.g. because it needs mutual exclusion with a task running on one of these
-threads), this is how you do it:
-
-```cpp
-content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::[IDENTIFIER])
-    ->PostTask(FROM_HERE, base::BindOnce(&Task));
-```
-
-Where `IDENTIFIER` is one of: `DB`, `FILE`, `FILE_USER_BLOCKING`, `PROCESS_LAUNCHER`, `CACHE`.
-
 ## Using TaskScheduler in a New Process
 
 TaskScheduler needs to be initialized in a process before the functions in

third_party/metrics_proto/execution_context.proto

@@ -36,7 +36,7 @@ enum Thread {
   UI_THREAD = 1;
   FILE_THREAD = 2;                // Deprecated.
   FILE_USER_BLOCKING_THREAD = 3;  // Deprecated.
-  PROCESS_LAUNCHER_THREAD = 4;
+  PROCESS_LAUNCHER_THREAD = 4;    // Deprecated.
   CACHE_THREAD = 5;  // Deprecated.
   IO_THREAD = 6;
   DB_THREAD = 7;  // Deprecated.