Remove MessageLoop from content::RenderThreadImpl.

It only needs main thread scheduler to initialize,
MessageLoop was unnecessary there.

Bug: 828835
Change-Id: I70235bbd06af1ca6c576cc77e3e4722a27225069
Reviewed-on: https://chromium-review.googlesource.com/1169211Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Greg Kraynov <kraynov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582125}

content/renderer/in_process_renderer_thread.cc

@@ -8,6 +8,7 @@
 #include "content/renderer/render_process.h"
 #include "content/renderer/render_process_impl.h"
 #include "content/renderer/render_thread_impl.h"
+#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/jni_android.h"
@@ -46,8 +47,14 @@ void InProcessRendererThread::Init() {
   // Android. Temporary CHECK() to debug http://crbug.com/514141
   CHECK(!render_process_);
 #endif
+  std::unique_ptr<blink::scheduler::WebThreadScheduler> main_thread_scheduler =
+      blink::scheduler::WebThreadScheduler::CreateMainThreadScheduler();
+
   render_process_ = RenderProcessImpl::Create();
-  RenderThreadImpl::Create(params_, message_loop());
+  // RenderThreadImpl doesn't currently support a proper shutdown sequence
+  // and it's okay when we're running in multi-process mode because renderers
+  // get killed by the OS. In-process mode is used for test and debug only.
+  new RenderThreadImpl(params_, std::move(main_thread_scheduler));
 }
 
 void InProcessRendererThread::CleanUp() {

content/renderer/render_thread_impl.cc

@@ -19,7 +19,6 @@
 #include "base/memory/discardable_memory_allocator.h"
 #include "base/memory/memory_coordinator_client_registry.h"
 #include "base/memory/shared_memory.h"
-#include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
@@ -637,28 +636,6 @@ bool RenderThreadImpl::HistogramCustomizer::IsAlexaTop10NonGoogleSite(
   return false;
 }
 
-// static
-RenderThreadImpl* RenderThreadImpl::Create(
-    const InProcessChildThreadParams& params,
-    base::MessageLoop* unowned_message_loop) {
-  TRACE_EVENT0("startup", "RenderThreadImpl::Create");
-  std::unique_ptr<blink::scheduler::WebThreadScheduler> main_thread_scheduler =
-      blink::scheduler::WebThreadScheduler::CreateMainThreadScheduler();
-  scoped_refptr<base::SingleThreadTaskRunner> test_task_counter;
-  return new RenderThreadImpl(params, std::move(main_thread_scheduler),
-                              test_task_counter, unowned_message_loop);
-}
-
-// static
-RenderThreadImpl* RenderThreadImpl::Create(
-    std::unique_ptr<base::MessageLoop> main_message_loop,
-    std::unique_ptr<blink::scheduler::WebThreadScheduler>
-        main_thread_scheduler) {
-  TRACE_EVENT0("startup", "RenderThreadImpl::Create");
-  return new RenderThreadImpl(std::move(main_message_loop),
-                              std::move(main_thread_scheduler));
-}
-
 // static
 RenderThreadImpl* RenderThreadImpl::current() {
   return lazy_tls.Pointer()->Get();
@@ -703,62 +680,49 @@ RenderThreadImpl::DeprecatedGetMainTaskRunner() {
 // the browser
 RenderThreadImpl::RenderThreadImpl(
     const InProcessChildThreadParams& params,
-    std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
-    const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
-    base::MessageLoop* unowned_message_loop)
-    : ChildThreadImpl(
-          Options::Builder()
-              .InBrowserProcess(params)
-              .AutoStartServiceManagerConnection(false)
-              .ConnectToBrowser(true)
-              .IPCTaskRunner(scheduler ? scheduler->IPCTaskRunner() : nullptr)
-              .Build()),
+    std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler)
+    : ChildThreadImpl(Options::Builder()
+                          .InBrowserProcess(params)
+                          .AutoStartServiceManagerConnection(false)
+                          .ConnectToBrowser(true)
+                          .IPCTaskRunner(scheduler->IPCTaskRunner())
+                          .Build()),
       main_thread_scheduler_(std::move(scheduler)),
-      main_message_loop_(unowned_message_loop),
       categorized_worker_pool_(new CategorizedWorkerPool()),
       renderer_binding_(this),
       client_id_(1),
       compositing_mode_watcher_binding_(this),
       weak_factory_(this) {
-  Init(resource_task_queue);
+  TRACE_EVENT0("startup", "RenderThreadImpl::Create");
+  Init();
 }
 
-// When we run plugins in process, we actually run them on the render thread,
-// which means that we need to make the render thread pump UI events.
+// Multi-process mode.
 RenderThreadImpl::RenderThreadImpl(
-    std::unique_ptr<base::MessageLoop> owned_message_loop,
     std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler)
-    : ChildThreadImpl(
-          Options::Builder()
-              .AutoStartServiceManagerConnection(false)
-              .ConnectToBrowser(true)
-              .IPCTaskRunner(scheduler ? scheduler->IPCTaskRunner() : nullptr)
-              .Build()),
+    : ChildThreadImpl(Options::Builder()
+                          .AutoStartServiceManagerConnection(false)
+                          .ConnectToBrowser(true)
+                          .IPCTaskRunner(scheduler->IPCTaskRunner())
+                          .Build()),
       main_thread_scheduler_(std::move(scheduler)),
-      owned_message_loop_(std::move(owned_message_loop)),
-      main_message_loop_(owned_message_loop_.get()),
       categorized_worker_pool_(new CategorizedWorkerPool()),
       is_scroll_animator_enabled_(false),
       renderer_binding_(this),
       compositing_mode_watcher_binding_(this),
       weak_factory_(this) {
-  scoped_refptr<base::SingleThreadTaskRunner> test_task_counter;
+  TRACE_EVENT0("startup", "RenderThreadImpl::Create");
   DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kRendererClientId));
   base::StringToInt(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
                         switches::kRendererClientId),
                     &client_id_);
-  Init(test_task_counter);
+  Init();
 }
 
-void RenderThreadImpl::Init(
-    const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue) {
+void RenderThreadImpl::Init() {
   TRACE_EVENT0("startup", "RenderThreadImpl::Init");
 
-  // Whether owned or unowned, |main_message_loop_| needs to be initialized in
-  // all constructors.
-  DCHECK(main_message_loop_);
-
   GetContentClient()->renderer()->PostIOThreadCreated(GetIOTaskRunner().get());
 
   base::trace_event::TraceLog::GetInstance()->SetThreadSortIndex(
@@ -771,7 +735,7 @@ void RenderThreadImpl::Init(
 #endif
 
   lazy_tls.Pointer()->Set(this);
-  g_main_task_runner.Get() = main_message_loop_->task_runner();
+  g_main_task_runner.Get() = base::ThreadTaskRunnerHandle::Get();
 
   // Register this object as the main thread.
   ChildProcess::current()->set_main_thread(this);
@@ -792,7 +756,7 @@ void RenderThreadImpl::Init(
           URLLoaderThrottleProviderType::kFrame);
 
   auto registry = std::make_unique<service_manager::BinderRegistry>();
-  InitializeWebKit(resource_task_queue, registry.get());
+  InitializeWebKit(registry.get());
 
   // In single process the single process is all there is.
   webkit_shared_timer_suspended_ = false;
@@ -1230,7 +1194,6 @@ void RenderThreadImpl::InitializeCompositorThread() {
 }
 
 void RenderThreadImpl::InitializeWebKit(
-    const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
     service_manager::BinderRegistry* registry) {
   DCHECK(!blink_platform_impl_);
 

content/renderer/render_thread_impl.h

@@ -170,12 +170,6 @@ class CONTENT_EXPORT RenderThreadImpl
       public viz::mojom::CompositingModeWatcher,
       public CompositorDependencies {
  public:
-  static RenderThreadImpl* Create(const InProcessChildThreadParams& params,
-                                  base::MessageLoop* unowned_message_loop);
-  static RenderThreadImpl* Create(
-      std::unique_ptr<base::MessageLoop> main_message_loop,
-      std::unique_ptr<blink::scheduler::WebThreadScheduler>
-          main_thread_scheduler);
   static RenderThreadImpl* current();
   static mojom::RenderMessageFilter* current_render_message_filter();
   static RendererBlinkPlatformImpl* current_blink_platform_impl();
@@ -189,6 +183,11 @@ class CONTENT_EXPORT RenderThreadImpl
   static scoped_refptr<base::SingleThreadTaskRunner>
   DeprecatedGetMainTaskRunner();
 
+  explicit RenderThreadImpl(
+      std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);
+  RenderThreadImpl(
+      const InProcessChildThreadParams& params,
+      std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);
   ~RenderThreadImpl() override;
   void Shutdown() override;
   bool ShouldBeDestroyed() override;
@@ -515,16 +514,6 @@ class CONTENT_EXPORT RenderThreadImpl
   // Sets the current pipeline rendering color space.
   void SetRenderingColorSpace(const gfx::ColorSpace& color_space);
 
- protected:
-  RenderThreadImpl(
-      const InProcessChildThreadParams& params,
-      std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
-      const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
-      base::MessageLoop* unowned_message_loop);
-  RenderThreadImpl(
-      std::unique_ptr<base::MessageLoop> main_message_loop,
-      std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);
-
  private:
   void OnProcessFinalRelease() override;
   // IPC::Listener
@@ -543,14 +532,9 @@ class CONTENT_EXPORT RenderThreadImpl
 
   void RecordPurgeMemory(RendererMemoryMetrics before);
 
-  void Init(
-      const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue);
-
+  void Init();
   void InitializeCompositorThread();
-
-  void InitializeWebKit(
-      const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
-      service_manager::BinderRegistry* registry);
+  void InitializeWebKit(service_manager::BinderRegistry* registry);
 
   void OnTransferBitmap(const SkBitmap& bitmap, int resource_id);
   void OnGetAccessibilityTree();
@@ -683,15 +667,6 @@ class CONTENT_EXPORT RenderThreadImpl
   // software-based.
   bool is_gpu_compositing_disabled_ = false;
 
-  // The message loop of the renderer main thread.
-  // This message loop should be destructed before the RenderThreadImpl
-  // shuts down Blink.
-  // Some test users (e.g. InProcessRenderThread) own the MessageLoop used by
-  // their RenderThreadImpls. |main_message_loop_| is always non-nulll,
-  // |owned_message_loop_| is non-null if handed in at creation.
-  const std::unique_ptr<base::MessageLoop> owned_message_loop_;
-  base::MessageLoop* const main_message_loop_;
-
   // May be null if overridden by ContentRendererClient.
   std::unique_ptr<blink::scheduler::WebThreadBase> compositor_thread_;
 

content/renderer/render_thread_impl_browsertest.cc

@@ -122,31 +122,6 @@ class TestTaskCounter : public base::SingleThreadTaskRunner {
   int count_;
 };
 
-#if defined(COMPILER_MSVC)
-// See explanation for other RenderViewHostImpl which is the same issue.
-#pragma warning(push)
-#pragma warning(disable: 4250)
-#endif
-
-class RenderThreadImplForTest : public RenderThreadImpl {
- public:
-  RenderThreadImplForTest(
-      const InProcessChildThreadParams& params,
-      std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
-      scoped_refptr<base::SingleThreadTaskRunner>& test_task_counter,
-      base::MessageLoop* unowned_message_loop)
-      : RenderThreadImpl(params,
-                         std::move(scheduler),
-                         test_task_counter,
-                         unowned_message_loop) {}
-
-  ~RenderThreadImplForTest() override {}
-};
-
-#if defined(COMPILER_MSVC)
-#pragma warning(pop)
-#endif
-
 class QuitOnTestMsgFilter : public IPC::MessageFilter {
  public:
   explicit QuitOnTestMsgFilter(base::OnceClosure quit_closure)
@@ -256,11 +231,10 @@ class RenderThreadImplBrowserTest : public testing::Test {
 
     base::FieldTrialList::CreateTrialsFromCommandLine(
         *cmd, switches::kFieldTrialHandle, -1);
-    thread_ = new RenderThreadImplForTest(
+    thread_ = new RenderThreadImpl(
         InProcessChildThreadParams(io_task_runner, &invitation,
                                    child_connection_->service_token()),
-        std::move(main_thread_scheduler), test_task_counter,
-        main_message_loop_.get());
+        std::move(main_thread_scheduler));
     cmd->InitFromArgv(old_argv);
 
     run_loop_ = std::make_unique<base::RunLoop>();
@@ -297,7 +271,11 @@ class RenderThreadImplBrowserTest : public testing::Test {
 
   std::unique_ptr<MockRenderProcess> mock_process_;
   scoped_refptr<QuitOnTestMsgFilter> test_msg_filter_;
-  RenderThreadImplForTest* thread_;  // Owned by mock_process_.
+
+  // RenderThreadImpl doesn't currently support a proper shutdown sequence
+  // and it's okay when we're running in multi-process mode because renderers
+  // get killed by the OS. Memory leaks aren't nice but it's test-only.
+  RenderThreadImpl* thread_;
 
   base::FieldTrialList field_trial_list_;
 

content/renderer/renderer_main.cc

@@ -182,9 +182,10 @@ int RendererMain(const MainFunctionParams& parameters) {
     }
 #endif
 
-    auto render_process = RenderProcessImpl::Create();
-    RenderThreadImpl::Create(std::move(main_message_loop),
-                             std::move(main_thread_scheduler));
+    std::unique_ptr<RenderProcess> render_process = RenderProcessImpl::Create();
+    // It's not a memory leak since RenderThread has the same lifetime
+    // as a renderer process.
+    new RenderThreadImpl(std::move(main_thread_scheduler));
 
     if (need_sandbox)
       run_loop = platform.EnableSandbox();