Refactor WebThreadImpl, WebSubThread, and WebMainLoop.

WebThreadImpl is now merely a scoped object that binds a provided
SingleThreadTaskRunner to a WebThread::ID. It no longer subclasses
base::Thread. That functionality has been moved to WebSubThread, which is
effectively only used for the IO thread now.

This is the //web implementation of
https://chromium-review.googlesource.com/c/chromium/src/+/980793. See the
description of that CL for more details.

BUG=826465

Change-Id: I636051b154c4156bc32517d649081131f9bdd14d
Reviewed-on: https://chromium-review.googlesource.com/c/1383140
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618761}

base/threading/thread_restrictions.h

@@ -234,6 +234,10 @@ namespace vr {
 class VrShell;
 }
 
+namespace web {
+class WebSubThread;
+}
+
 namespace webrtc {
 class DesktopConfigurationMonitor;
 }
@@ -317,6 +321,7 @@ class BASE_EXPORT ScopedAllowBlocking {
   friend class mojo::CoreLibraryInitializer;
   friend class resource_coordinator::TabManagerDelegate;  // crbug.com/778703
   friend class ui::MaterialDesignController;
+  friend class web::WebSubThread;
   friend class StackSamplingProfiler;
 
   ScopedAllowBlocking() EMPTY_BODY_IF_DCHECK_IS_OFF;

ios/components/io_thread/DEPS

@@ -7,4 +7,8 @@ include_rules = [
   "+components/version_info",
   "+ios/web/public",
   "+net",
+
+  # Temporarily allow WebThreadImpl until TaskExecutor functionality is moved
+  # into a new file in ios/web/public.
+  "+ios/web/web_thread_impl.h"
 ]

ios/components/io_thread/ios_io_thread_unittest.mm

@@ -4,16 +4,16 @@
 
 #include "ios/components/io_thread/ios_io_thread.h"
 
-#include "base/message_loop/message_loop.h"
-#include "base/run_loop.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/pref_service.h"
-#include "components/prefs/pref_service_factory.h"
-#include "components/prefs/testing_pref_store.h"
+#include <memory>
+
+#include "base/test/scoped_task_environment.h"
+#include "components/prefs/testing_pref_service.h"
 #include "components/proxy_config/pref_proxy_config_tracker_impl.h"
-#include "ios/web/public/test/test_web_thread_bundle.h"
-#include "net/test/url_request/url_request_failed_job.h"
-#include "net/url_request/url_request_filter.h"
+#import "ios/web/public/test/fakes/test_web_client.h"
+#include "ios/web/public/test/scoped_testing_web_client.h"
+#include "ios/web/public/test/test_web_thread.h"
+#include "ios/web/web_thread_impl.h"
+#include "net/base/network_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gtest_mac.h"
 #include "testing/platform_test.h"
@@ -42,32 +42,43 @@ class TestIOThread : public io_thread::IOSIOThread {
 
 class IOSIOThreadTest : public PlatformTest {
  public:
-  IOSIOThreadTest() : thread_bundle_(web::TestWebThreadBundle::IO_MAINLOOP) {
-    net::URLRequestFailedJob::AddUrlHandler();
+  IOSIOThreadTest() : web_client_(std::make_unique<web::TestWebClient>()) {
+    web::WebThreadImpl::CreateTaskExecutor();
+
+    ui_thread_ = std::make_unique<web::TestWebThread>(
+        web::WebThread::UI, scoped_task_environment_.GetMainThreadTaskRunner());
   }
 
   ~IOSIOThreadTest() override {
-    net::URLRequestFilter::GetInstance()->ClearHandlers();
+    web::WebThreadImpl::ResetTaskExecutorForTesting();
   }
 
- private:
-  web::TestWebThreadBundle thread_bundle_;
+ protected:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  web::ScopedTestingWebClient web_client_;
+  std::unique_ptr<web::TestWebThread> ui_thread_;
 };
 
 // Tests the creation of an IOSIOThread and verifies that it returns a system
 // url request context.
 TEST_F(IOSIOThreadTest, AssertSystemUrlRequestContext) {
-  PrefServiceFactory pref_service_factory;
-  pref_service_factory.set_user_prefs(base::MakeRefCounted<TestingPrefStore>());
+  std::unique_ptr<TestingPrefServiceSimple> pref_service(
+      std::make_unique<TestingPrefServiceSimple>());
+  PrefProxyConfigTrackerImpl::RegisterPrefs(pref_service->registry());
 
-  scoped_refptr<PrefRegistrySimple> pref_registry = new PrefRegistrySimple;
-  PrefProxyConfigTrackerImpl::RegisterPrefs(pref_registry.get());
+  // Create the IO thread but do not register it yet.
+  std::unique_ptr<web::TestWebThread> io_thread(
+      std::make_unique<web::TestWebThread>(web::WebThread::IO));
+  io_thread->StartIOThreadUnregistered();
 
-  std::unique_ptr<PrefService> pref_service(
-      pref_service_factory.Create(pref_registry.get()));
-
-  std::unique_ptr<TestIOThread> test_io_thread(
+  // Create the TestIOThread before the IO thread is registered.
+  std::unique_ptr<TestIOThread> ios_io_thread(
       new TestIOThread(pref_service.get(), nullptr));
+  io_thread->RegisterAsWebThread();
+
+  ASSERT_TRUE(ios_io_thread->system_url_request_context_getter());
 
-  ASSERT_TRUE(test_io_thread->system_url_request_context_getter());
+  // Explicitly destroy the IO thread so that it is unregistered before the
+  // TestIOThread is destroyed.
+  io_thread.reset();
 }

ios/web/app/web_main_loop.h

@@ -83,7 +83,7 @@ class WebMainLoop {
   // This must get destroyed after other threads that are created in parts_.
   std::unique_ptr<WebThreadImpl> main_thread_;
 
-  // Members initialized in |RunMainMessageLoopParts()| ------------------------
+  // Members initialized in |CreateThreads()| ------------------------
   std::unique_ptr<WebSubThread> io_thread_;
 
   // Members initialized in |WebThreadsStarted()| --------------------------

ios/web/app/web_main_loop.mm

@@ -130,7 +130,9 @@ int WebMainLoop::CreateThreads(
   base::Thread::Options io_message_loop_options;
   io_message_loop_options.message_loop_type = base::MessageLoop::TYPE_IO;
   io_thread_ = std::make_unique<WebSubThread>(WebThread::IO);
-  io_thread_->StartWithOptions(io_message_loop_options);
+  if (!io_thread_->StartWithOptions(io_message_loop_options))
+    LOG(FATAL) << "Failed to start WebThread::IO";
+  io_thread_->RegisterAsWebThread();
 
   // Only start IO thread above as this is the only WebThread besides UI (which
   // is the main thread).
@@ -196,8 +198,10 @@ void WebMainLoop::InitializeMainThread() {
   base::PlatformThread::SetName("CrWebMain");
 
   // Register the main thread by instantiating it, but don't call any methods.
+  DCHECK(base::ThreadTaskRunnerHandle::IsSet());
   main_thread_.reset(new WebThreadImpl(
-      WebThread::UI, ios_global_state::GetMainThreadMessageLoop()));
+      WebThread::UI,
+      ios_global_state::GetMainThreadMessageLoop()->task_runner()));
 }
 
 int WebMainLoop::WebThreadsStarted() {

ios/web/public/test/test_web_thread.h

@@ -12,18 +12,29 @@
 
 namespace base {
 class MessageLoop;
+class Thread;
 }
 
 namespace web {
 
-class TestWebThreadImpl;
+class WebSubThread;
+class WebThreadImpl;
 
 // DEPRECATED: use TestWebThreadBundle instead.
 // A WebThread for unit tests; this lets unit tests outside of web create
 // WebThread instances.
 class TestWebThread {
  public:
+  // Constructs a TestWebThread with a |real_thread_| and starts it (with a
+  // MessageLoopForIO if |identifier == WebThread::IO|).
   explicit TestWebThread(WebThread::ID identifier);
+
+  // Constructs a TestWebThread "running" on |thread_runner| (no
+  // |real_thread_|).
+  TestWebThread(WebThread::ID identifier,
+                scoped_refptr<base::SingleThreadTaskRunner> thread_runner);
+
+  // Constructs a TestWebThread based on |message_loop| (no |real_thread_|).
   TestWebThread(WebThread::ID identifier, base::MessageLoop* message_loop);
 
   ~TestWebThread();
@@ -33,22 +44,31 @@ class TestWebThread {
   // WebThread, do no provide the full Thread interface.
 
   // Starts the thread with a generic message loop.
-  bool Start();
+  void Start();
 
   // Starts the thread with an IOThread message loop.
-  bool StartIOThread();
+  void StartIOThread();
+
+  // Together these are the same as StartIOThread(). They can be called in
+  // phases to test binding WebThread::IO after its underlying thread was
+  // started.
+  void StartIOThreadUnregistered();
+  void RegisterAsWebThread();
 
   // Stops the thread.
   void Stop();
 
-  // Returns true if the thread is running.
-  bool IsRunning();
-
  private:
-  std::unique_ptr<TestWebThreadImpl> impl_;
-
   const WebThread::ID identifier_;
 
+  // A real thread which represents |identifier_| when constructor #1 is used
+  // (null otherwise).
+  std::unique_ptr<WebSubThread> real_thread_;
+
+  // Binds |identifier_| to |message_loop| when constructor #2 is used (null
+  // otherwise).
+  std::unique_ptr<WebThreadImpl> fake_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(TestWebThread);
 };
 

ios/web/public/web_thread.h

@@ -95,9 +95,9 @@ class WebThread {
   // Only one delegate may be registered at a time. Delegates may be
   // unregistered by providing a nullptr pointer.
   //
-  // If the caller unregisters a delegate before CleanUp has been
-  // called, it must perform its own locking to ensure the delegate is
-  // not deleted while unregistering.
+  // The delegate can only be registered through this call before
+  // WebThreadImpl(WebThread::IO) is created and unregistered after
+  // it was destroyed and its underlying thread shutdown.
   static void SetIOThreadDelegate(WebThreadDelegate* delegate);
 
   // Returns an appropriate error message for when DCHECK_CURRENTLY_ON() fails.

ios/web/public/web_thread_delegate.h

@@ -13,14 +13,17 @@ namespace web {
 // If registered as such, it will schedule to run Init() before the
 // message loop begins, and receive a CleanUp() call right after the message
 // loop ends (and before the WebThread has done its own clean-up).
+
+// A delegate for //web embedders to perform extra initialization/cleanup on
+// WebThread::IO.
 class WebThreadDelegate {
  public:
   virtual ~WebThreadDelegate() {}
 
-  // Called prior to starting the message loop
+  // Called prior to completing initialization of WebThread::IO.
   virtual void Init() = 0;
 
-  // Called just after the message loop ends.
+  // Called during teardown of WebThread::IO.
   virtual void CleanUp() = 0;
 };
 

ios/web/test/test_web_thread.cc

@@ -6,41 +6,36 @@
 
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
+#include "ios/web/web_sub_thread.h"
 #include "ios/web/web_thread_impl.h"
 
 namespace web {
 
-class TestWebThreadImpl : public WebThreadImpl {
- public:
-  TestWebThreadImpl(WebThread::ID identifier) : WebThreadImpl(identifier) {}
-
-  TestWebThreadImpl(WebThread::ID identifier, base::MessageLoop* message_loop)
-      : WebThreadImpl(identifier, message_loop) {}
-
-  ~TestWebThreadImpl() override { Stop(); }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestWebThreadImpl);
-};
-
 TestWebThread::TestWebThread(WebThread::ID identifier)
-    : impl_(new TestWebThreadImpl(identifier)), identifier_(identifier) {}
+    : identifier_(identifier),
+      real_thread_(std::make_unique<WebSubThread>(identifier_)) {
+  real_thread_->AllowBlockingForTesting();
+}
+
+TestWebThread::TestWebThread(
+    WebThread::ID identifier,
+    scoped_refptr<base::SingleThreadTaskRunner> thread_runner)
+    : identifier_(identifier),
+      fake_thread_(new WebThreadImpl(identifier_, thread_runner)) {}
 
 TestWebThread::TestWebThread(WebThread::ID identifier,
                              base::MessageLoop* message_loop)
-    : impl_(new TestWebThreadImpl(identifier, message_loop)),
-      identifier_(identifier) {}
+    : TestWebThread(identifier, message_loop->task_runner()) {}
 
 TestWebThread::~TestWebThread() {
   // The upcoming WebThreadImpl::ResetGlobalsForTesting() call requires that
-  // |impl_| have triggered the shutdown phase for its WebThread::ID. This
-  // either happens when the thread is stopped (if real) or destroyed (when fake
-  // -- i.e. using an externally provided MessageLoop).
-  impl_.reset();
+  // |identifier_| completed its shutdown phase.
+  real_thread_.reset();
+  fake_thread_.reset();
 
-  // Resets WebThreadImpl's globals so that |impl_| is no longer bound to
-  // |identifier_|. This is fine since the underlying MessageLoop has already
-  // been flushed and deleted in Stop(). In the case of an externally provided
+  // Resets WebThreadImpl's globals so that |identifier_| is no longer
+  // bound. This is fine since the underlying MessageLoop has already been
+  // flushed and deleted above. In the case of an externally provided
   // MessageLoop however, this means that TaskRunners obtained through
   // |WebThreadImpl::GetTaskRunnerForThread(identifier_)| will no longer
   // recognize their WebThreadImpl for RunsTasksInCurrentSequence(). This
@@ -54,22 +49,29 @@ TestWebThread::~TestWebThread() {
   WebThreadImpl::ResetGlobalsForTesting(identifier_);
 }
 
-bool TestWebThread::Start() {
-  return impl_->Start();
+void TestWebThread::Start() {
+  CHECK(real_thread_->Start());
+  RegisterAsWebThread();
+}
+
+void TestWebThread::StartIOThread() {
+  StartIOThreadUnregistered();
+  RegisterAsWebThread();
 }
 
-bool TestWebThread::StartIOThread() {
+void TestWebThread::StartIOThreadUnregistered() {
   base::Thread::Options options;
   options.message_loop_type = base::MessageLoop::TYPE_IO;
-  return impl_->StartWithOptions(options);
+  CHECK(real_thread_->StartWithOptions(options));
 }
 
-void TestWebThread::Stop() {
-  impl_->Stop();
+void TestWebThread::RegisterAsWebThread() {
+  real_thread_->RegisterAsWebThread();
 }
 
-bool TestWebThread::IsRunning() {
-  return impl_->IsRunning();
+void TestWebThread::Stop() {
+  if (real_thread_)
+    real_thread_->Stop();
 }
 
 }  // namespace web

ios/web/web_sub_thread.cc

@@ -4,43 +4,122 @@
 
 #include "ios/web/web_sub_thread.h"
 
+#include "base/compiler_specific.h"
+#include "base/debug/alias.h"
 #include "base/threading/thread_restrictions.h"
-#include "ios/web/public/web_thread.h"
+#include "ios/web/public/web_thread_delegate.h"
 #include "ios/web/web_thread_impl.h"
 #include "net/url_request/url_fetcher.h"
 
 namespace web {
 
-WebSubThread::WebSubThread(WebThread::ID identifier)
-    : WebThreadImpl(identifier) {}
+namespace {
+WebThreadDelegate* g_io_thread_delegate = nullptr;
+}  // namespace
+
+// static
+void WebThread::SetIOThreadDelegate(WebThreadDelegate* delegate) {
+  // |delegate| can only be set/unset while WebThread::IO isn't up.
+  DCHECK(!WebThread::IsThreadInitialized(WebThread::IO));
+  // and it cannot be set twice.
+  DCHECK(!g_io_thread_delegate || !delegate);
 
-WebSubThread::WebSubThread(WebThread::ID identifier,
-                           base::MessageLoop* message_loop)
-    : WebThreadImpl(identifier, message_loop) {}
+  g_io_thread_delegate = delegate;
+}
+
+WebSubThread::WebSubThread(WebThread::ID identifier)
+    : base::Thread(WebThreadImpl::GetThreadName(identifier)),
+      identifier_(identifier) {
+  // Not bound to creation thread.
+  DETACH_FROM_THREAD(web_thread_checker_);
+}
 
 WebSubThread::~WebSubThread() {
   Stop();
 }
 
+void WebSubThread::RegisterAsWebThread() {
+  DCHECK(IsRunning());
+
+  DCHECK(!web_thread_);
+  web_thread_.reset(new WebThreadImpl(identifier_, task_runner()));
+
+  // Unretained(this) is safe as |this| outlives its underlying thread.
+  task_runner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&WebSubThread::CompleteInitializationOnWebThread,
+                     Unretained(this)));
+}
+
+void WebSubThread::AllowBlockingForTesting() {
+  DCHECK(!IsRunning());
+  is_blocking_allowed_for_testing_ = true;
+}
+
 void WebSubThread::Init() {
-  WebThreadImpl::Init();
+  DCHECK_CALLED_ON_VALID_THREAD(web_thread_checker_);
 
-  if (WebThread::CurrentlyOn(WebThread::IO)) {
-    // Though this thread is called the "IO" thread, it actually just routes
-    // messages around; it shouldn't be allowed to perform any blocking disk
-    // I/O.
+  if (!is_blocking_allowed_for_testing_) {
     base::DisallowUnresponsiveTasks();
   }
 }
 
+void WebSubThread::Run(base::RunLoop* run_loop) {
+  DCHECK_CALLED_ON_VALID_THREAD(web_thread_checker_);
+
+  switch (identifier_) {
+    case WebThread::UI:
+      // The main thread is usually promoted as the UI thread and doesn't go
+      // through Run() but some tests do run a separate UI thread.
+      UIThreadRun(run_loop);
+      break;
+    case WebThread::IO:
+      IOThreadRun(run_loop);
+      return;
+    case WebThread::ID_COUNT:
+      NOTREACHED();
+      break;
+  }
+}
+
 void WebSubThread::CleanUp() {
+  DCHECK_CALLED_ON_VALID_THREAD(web_thread_checker_);
+
+  // Run extra cleanup if this thread represents WebThread::IO.
   if (WebThread::CurrentlyOn(WebThread::IO))
-    IOThreadPreCleanUp();
+    IOThreadCleanUp();
+
+  if (identifier_ == WebThread::IO && g_io_thread_delegate)
+    g_io_thread_delegate->CleanUp();
 
-  WebThreadImpl::CleanUp();
+  web_thread_.reset();
 }
 
-void WebSubThread::IOThreadPreCleanUp() {
+void WebSubThread::CompleteInitializationOnWebThread() {
+  DCHECK_CALLED_ON_VALID_THREAD(web_thread_checker_);
+
+  if (identifier_ == WebThread::IO && g_io_thread_delegate) {
+    // Allow blocking calls while initializing the IO thread.
+    base::ScopedAllowBlocking allow_blocking_for_init;
+    g_io_thread_delegate->Init();
+  }
+}
+
+void WebSubThread::UIThreadRun(base::RunLoop* run_loop) {
+  const int line_number = __LINE__;
+  Thread::Run(run_loop);
+  base::debug::Alias(&line_number);
+}
+
+void WebSubThread::IOThreadRun(base::RunLoop* run_loop) {
+  const int line_number = __LINE__;
+  Thread::Run(run_loop);
+  base::debug::Alias(&line_number);
+}
+
+void WebSubThread::IOThreadCleanUp() {
+  DCHECK_CALLED_ON_VALID_THREAD(web_thread_checker_);
+
   // Kill all things that might be holding onto
   // net::URLRequest/net::URLRequestContexts.
 

ios/web/web_sub_thread.h

@@ -6,26 +6,65 @@
 #define IOS_WEB_WEB_SUB_THREAD_H_
 
 #include "base/macros.h"
-#include "ios/web/web_thread_impl.h"
+#include "base/threading/thread.h"
+#include "base/threading/thread_checker.h"
+#include "ios/web/public/web_thread.h"
 
 namespace web {
+class WebThreadImpl;
+}
 
-// This simple thread object is used for the specialized threads that are spun
-// up during startup.
-class WebSubThread : public WebThreadImpl {
+namespace web {
+
+// A WebSubThread is a physical thread backing a WebThread.
+class WebSubThread : public base::Thread {
  public:
+  // Constructs a WebSubThread for |identifier|.
   explicit WebSubThread(WebThread::ID identifier);
-  WebSubThread(WebThread::ID identifier, base::MessageLoop* message_loop);
   ~WebSubThread() override;
 
+  // Registers this thread to represent |identifier_| in the web_thread.h
+  // API. This thread must already be running when this is called. This can only
+  // be called once per WebSubThread instance.
+  void RegisterAsWebThread();
+
+  // Ideally there wouldn't be a special blanket allowance to block the
+  // WebThreads in tests but TestWebThreadImpl previously bypassed
+  // WebSubThread and hence wasn't subject to ThreadRestrictions...
+  // Flipping that around in favor of explicit scoped allowances would be
+  // preferable but a non-trivial amount of work. Can only be called before
+  // starting this WebSubThread.
+  void AllowBlockingForTesting();
+
  protected:
   void Init() override;
+  void Run(base::RunLoop* run_loop) override;
   void CleanUp() override;
 
  private:
-  // These methods encapsulate cleanup that needs to happen on the IO thread
-  // before the embedder's |CleanUp()| function is called.
-  void IOThreadPreCleanUp();
+  // Second Init() phase that must happen on this thread but can only happen
+  // after it's promoted to a WebThread in |RegisterAsWebThread()|.
+  void CompleteInitializationOnWebThread();
+
+  // These methods merely forwards to Thread::Run() but are useful to identify
+  // which WebThread this represents in stack traces.
+  void UIThreadRun(base::RunLoop* run_loop);
+  void IOThreadRun(base::RunLoop* run_loop);
+
+  // This method encapsulates cleanup that needs to happen on the IO thread.
+  void IOThreadCleanUp();
+
+  const WebThread::ID identifier_;
+
+  // WebThreads are not allowed to do file I/O nor wait on synchronization
+  // primitives except when explicitly allowed in tests.
+  bool is_blocking_allowed_for_testing_ = false;
+
+  // The WebThread registration for this |identifier_|, initialized in
+  // RegisterAsWebThread().
+  std::unique_ptr<WebThreadImpl> web_thread_;
+
+  THREAD_CHECKER(web_thread_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(WebSubThread);
 };

ios/web/web_thread_impl.h

@@ -6,28 +6,30 @@
 #define IOS_WEB_WEB_THREAD_IMPL_H_
 
 #include "base/callback.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/threading/thread.h"
 #include "ios/web/public/web_thread.h"
 
 namespace web {
 
-class WebTestSuiteListener;
-
-class WebThreadImpl : public WebThread, public base::Thread {
+class TestWebThread;
+class WebMainLoop;
+class WebSubThread;
+
+// WebThreadImpl is a scoped object which maps a SingleThreadTaskRunner to a
+// WebThread::ID. On ~WebThreadImpl() that ID enters a SHUTDOWN state
+// (in which WebThread::IsThreadInitialized() returns false) but the mapping
+// isn't undone to avoid shutdown races (the task runner is free to stop
+// accepting tasks however).
+//
+// Very few users should use this directly. To mock WebThreads, tests should
+// use TestWebThreadBundle instead.
+class WebThreadImpl : public WebThread {
  public:
-  // Construct a WebThreadImpl with the supplied identifier.  It is an error
-  // to construct a WebThreadImpl that already exists.
-  explicit WebThreadImpl(WebThread::ID identifier);
-
-  // Special constructor for the main (UI) thread and unittests. If a
-  // |message_loop| is provied, we use a dummy thread here since the main
-  // thread already exists.
-  WebThreadImpl(WebThread::ID identifier, base::MessageLoop* message_loop);
-  ~WebThreadImpl() override;
+  ~WebThreadImpl();
 
-  bool Start();
-  bool StartWithOptions(const Options& options);
-  bool StartAndWaitForTesting();
+  // Returns the thread name for |identifier|.
+  static const char* GetThreadName(WebThread::ID identifier);
 
   // Creates and registers a TaskExecutor that facilitates posting tasks to a
   // WebThread via //base/task/post_task.h.
@@ -46,29 +48,19 @@ class WebThreadImpl : public WebThread, public base::Thread {
   // Also unregisters and deletes the TaskExecutor.
   static void ResetGlobalsForTesting(WebThread::ID identifier);
 
- protected:
-  void Init() override;
-  void Run(base::RunLoop* run_loop) override;
-  void CleanUp() override;
-
  private:
-  // This class implements all the functionality of the public WebThread
-  // functions, but state is stored in the WebThreadImpl to keep
-  // the API cleaner. Therefore make WebThread a friend class.
-  friend class WebThread;
-
-  // The following are unique function names that makes it possible to tell
-  // the thread id from the callstack alone in crash dumps.
-  void UIThreadRun(base::RunLoop* run_loop);
-  void IOThreadRun(base::RunLoop* run_loop);
-
-  // Common initialization code for the constructors.
-  void Initialize();
-
-  // For testing.
-  friend class TestWebThreadBundle;
-  friend class TestWebThreadBundleImpl;
-  friend class WebTestSuiteListener;
+  // Restrict instantiation to WebSubThread as it performs important
+  // initialization that shouldn't be bypassed (except by WebMainLoop for
+  // the main thread).
+  friend class WebSubThread;
+  friend class WebMainLoop;
+  // TestWebThread is also allowed to construct this when instantiating fake
+  // threads.
+  friend class TestWebThread;
+
+  // Binds |identifier| to |task_runner| for the web_thread.h API.
+  WebThreadImpl(WebThread::ID identifier,
+                scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   // The identifier of this thread.  Only one thread can exist with a given
   // identifier at a given time.