Start TaskScheduler before creating BrowserMainLoop.

This is a follow up CL for: https://crrev.com/c/1115783.

In this CL, we use ScopedExecutionFence to prevents tasks running between the
peorid from BrowserMainRunner is instantiated to
BrowserMainLoop::CreateThreads() is called.

Bug: 846846
Change-Id: I9dec8415e45f901c5883c196ab867c0ea4928594
Reviewed-on: https://chromium-review.googlesource.com/c/1174955
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597255}

content/app/content_main_runner_impl.cc

@@ -874,18 +874,11 @@ int ContentMainRunnerImpl::Run(bool start_service_manager_only) {
     main_params.startup_data = startup_data_.get();
 
     if (GetContentClient()->browser()->ShouldCreateTaskScheduler()) {
-      // Create the TaskScheduler early to allow upcoming code to use
-      // the post_task.h API. Note: This is okay because RunBrowserProcessMain()
-      // will soon result in invoking TaskScheduler::GetInstance()->Start().
-      // The TaskScheduler being started soon is a strict requirement (delaying
-      // this start would result in posted tasks not running).
+      // Create and start the TaskScheduler early to allow upcoming code to use
+      // the post_task.h API.
       base::TaskScheduler::Create("Browser");
     }
 
-    // Register the TaskExecutor for posting task to the BrowserThreads. It is
-    // incorrect to post to a BrowserThread before this point.
-    BrowserTaskExecutor::Create();
-
     delegate_->PreCreateMainMessageLoop();
 #if defined(OS_WIN)
     if (l10n_util::GetLocaleOverrides().empty()) {
@@ -909,6 +902,15 @@ int ContentMainRunnerImpl::Run(bool start_service_manager_only) {
 
     delegate_->PostEarlyInitialization();
 
+    if (GetContentClient()->browser()->ShouldCreateTaskScheduler()) {
+      // The FeatureList needs to create before starting the TaskScheduler.
+      StartBrowserTaskScheduler();
+    }
+
+    // Register the TaskExecutor for posting task to the BrowserThreads. It is
+    // incorrect to post to a BrowserThread before this point.
+    BrowserTaskExecutor::Create();
+
     return RunBrowserProcessMain(main_params, delegate_);
   }
 #endif  // !defined(CHROME_MULTIPLE_DLL_CHILD)

content/browser/browser_main_loop.cc

@@ -346,46 +346,6 @@ enum WorkerPoolType : size_t {
   WORKER_POOL_COUNT  // Always last.
 };
 
-std::unique_ptr<base::TaskScheduler::InitParams>
-GetDefaultTaskSchedulerInitParams() {
-#if defined(OS_ANDROID)
-  // Mobile config, for iOS see ios/web/app/web_main_loop.cc.
-  return std::make_unique<base::TaskScheduler::InitParams>(
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0),
-          base::TimeDelta::FromSeconds(30)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0),
-          base::TimeDelta::FromSeconds(30)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0),
-          base::TimeDelta::FromSeconds(30)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0),
-          base::TimeDelta::FromSeconds(60)));
-#else
-  // Desktop config.
-  return std::make_unique<base::TaskScheduler::InitParams>(
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0),
-          base::TimeDelta::FromSeconds(30)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0),
-          base::TimeDelta::FromSeconds(40)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0),
-          base::TimeDelta::FromSeconds(30)),
-      base::SchedulerWorkerPoolParams(
-          base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0),
-          base::TimeDelta::FromSeconds(60))
-#if defined(OS_WIN)
-          ,
-      base::TaskScheduler::InitParams::SharedWorkerPoolEnvironment::COM_MTA
-#endif  // defined(OS_WIN)
-      );
-#endif
-}
-
 #if !defined(OS_FUCHSIA)
 // Time between updating and recording swap rates.
 constexpr base::TimeDelta kSwapMetricsInterval =
@@ -569,12 +529,18 @@ media::AudioManager* BrowserMainLoop::GetAudioManager() {
   return g_current_browser_main_loop->audio_manager();
 }
 
-BrowserMainLoop::BrowserMainLoop(const MainFunctionParams& parameters)
+BrowserMainLoop::BrowserMainLoop(
+    const MainFunctionParams& parameters,
+    std::unique_ptr<base::TaskScheduler::ScopedExecutionFence>
+        scoped_execution_fence)
     : parameters_(parameters),
       parsed_command_line_(parameters.command_line),
       result_code_(service_manager::RESULT_CODE_NORMAL_EXIT),
-      created_threads_(false) {
+      created_threads_(false),
+      scoped_execution_fence_(std::move(scoped_execution_fence)) {
   DCHECK(!g_current_browser_main_loop);
+  DCHECK(scoped_execution_fence_)
+      << "TaskScheduler must be halted before kicking off content.";
   g_current_browser_main_loop = this;
 
   if (GetContentClient()->browser()->ShouldCreateTaskScheduler()) {
@@ -977,31 +943,8 @@ void BrowserMainLoop::SynchronouslyFlushStartupTasks() {
 int BrowserMainLoop::CreateThreads() {
   TRACE_EVENT0("startup,rail", "BrowserMainLoop::CreateThreads");
 
-  {
-    auto task_scheduler_init_params =
-        GetContentClient()->browser()->GetTaskSchedulerInitParams();
-    if (!task_scheduler_init_params)
-      task_scheduler_init_params = GetDefaultTaskSchedulerInitParams();
-    DCHECK(task_scheduler_init_params);
-
-    // If a renderer lives in the browser process, adjust the number of threads
-    // in the foreground pool.
-    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kSingleProcess)) {
-      const base::SchedulerWorkerPoolParams&
-          current_foreground_worker_pool_params(
-              task_scheduler_init_params->foreground_worker_pool_params);
-      task_scheduler_init_params->foreground_worker_pool_params =
-          base::SchedulerWorkerPoolParams(
-              std::max(GetMinThreadsInRendererTaskSchedulerForegroundPool(),
-                       current_foreground_worker_pool_params.max_tasks()),
-              current_foreground_worker_pool_params.suggested_reclaim_time(),
-              current_foreground_worker_pool_params.backward_compatibility());
-    }
-
-    base::TaskScheduler::GetInstance()->Start(
-        *task_scheduler_init_params.get());
-  }
+  // Release the TaskScheduler's threads.
+  scoped_execution_fence_.reset();
 
   // The |io_thread| can have optionally been injected into Init(), but if not,
   // create it here. Thre thread is only tagged as BrowserThread::IO here in

content/browser/browser_main_loop.h

@@ -11,6 +11,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/task/task_scheduler/task_scheduler.h"
 #include "base/timer/timer.h"
 #include "build/build_config.h"
 #include "content/browser/browser_process_sub_thread.h"
@@ -129,7 +130,9 @@ class CONTENT_EXPORT BrowserMainLoop {
 
   // The TaskScheduler instance must exist but not to be started when building
   // BrowserMainLoop.
-  explicit BrowserMainLoop(const MainFunctionParams& parameters);
+  explicit BrowserMainLoop(
+      const MainFunctionParams& parameters,
+      std::unique_ptr<base::TaskScheduler::ScopedExecutionFence> fence);
   virtual ~BrowserMainLoop();
 
   void Init();
@@ -285,6 +288,15 @@ class CONTENT_EXPORT BrowserMainLoop {
   const base::CommandLine& parsed_command_line_;
   int result_code_;
   bool created_threads_;  // True if the non-UI threads were created.
+  // //content must be initialized single-threaded until
+  // BrowserMainLoop::CreateThreads() as things initialized before it require an
+  // initialize-once happens-before relationship with all eventual content tasks
+  // running on other threads. This ScopedExecutionFence ensures that no tasks
+  // posted to TaskScheduler gets to run before CreateThreads(); satisfying this
+  // requirement even though the TaskScheduler is created and started before
+  // content is entered.
+  std::unique_ptr<base::TaskScheduler::ScopedExecutionFence>
+      scoped_execution_fence_;
 
   // Members initialized in |MainMessageLoopStart()| ---------------------------
   std::unique_ptr<base::MessageLoop> main_message_loop_;

content/browser/browser_main_loop_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/test/scoped_command_line.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/scheduler/browser_task_executor.h"
+#include "content/browser/startup_helper.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/main_function_params.h"
@@ -22,14 +23,17 @@ namespace content {
 // the number of cores in its foreground pool.
 TEST(BrowserMainLoopTest, CreateThreadsInSingleProcess) {
   {
-    base::TaskScheduler::Create("Browser");
-    BrowserTaskExecutor::Create();
     base::test::ScopedCommandLine scoped_command_line;
     scoped_command_line.GetProcessCommandLine()->AppendSwitch(
         switches::kSingleProcess);
+    base::TaskScheduler::Create("Browser");
+    StartBrowserTaskScheduler();
+    BrowserTaskExecutor::Create();
     MainFunctionParams main_function_params(
         *scoped_command_line.GetProcessCommandLine());
-    BrowserMainLoop browser_main_loop(main_function_params);
+    BrowserMainLoop browser_main_loop(
+        main_function_params,
+        std::make_unique<base::TaskScheduler::ScopedExecutionFence>());
     browser_main_loop.MainMessageLoopStart();
     browser_main_loop.Init();
     browser_main_loop.CreateThreads();

content/browser/browser_main_runner_impl.cc

@@ -54,7 +54,10 @@ BrowserMainRunnerImpl* BrowserMainRunnerImpl::Create() {
 }
 
 BrowserMainRunnerImpl::BrowserMainRunnerImpl()
-    : initialization_started_(false), is_shutdown_(false) {}
+    : initialization_started_(false),
+      is_shutdown_(false),
+      scoped_execution_fence_(
+          std::make_unique<base::TaskScheduler::ScopedExecutionFence>()) {}
 
 BrowserMainRunnerImpl::~BrowserMainRunnerImpl() {
   if (initialization_started_ && !is_shutdown_)
@@ -107,7 +110,8 @@ int BrowserMainRunnerImpl::Initialize(const MainFunctionParams& parameters) {
     gfx::win::MaybeInitializeDirectWrite();
 #endif  // OS_WIN
 
-    main_loop_.reset(new BrowserMainLoop(parameters));
+    main_loop_.reset(
+        new BrowserMainLoop(parameters, std::move(scoped_execution_fence_)));
 
     main_loop_->Init();
 

content/browser/browser_main_runner_impl.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/task/task_scheduler/task_scheduler.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_main_runner.h"
 
@@ -44,6 +45,12 @@ class BrowserMainRunnerImpl : public BrowserMainRunner {
   // True if the runner has been shut down.
   bool is_shutdown_;
 
+  // Prevents execution of TaskScheduler tasks from the moment content is
+  // entered. Handed off to |main_loop_| later so it can decide when to release
+  // worker threads again.
+  std::unique_ptr<base::TaskScheduler::ScopedExecutionFence>
+      scoped_execution_fence_;
+
   std::unique_ptr<NotificationServiceImpl> notification_service_;
   std::unique_ptr<BrowserMainLoop> main_loop_;
 #if defined(OS_WIN)

content/browser/startup_helper.cc

@@ -6,9 +6,60 @@
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
+#include "base/sys_info.h"
+#include "base/task/task_scheduler/initialization_util.h"
+#include "base/task/task_scheduler/task_scheduler.h"
+#include "build/build_config.h"
+#include "content/common/task_scheduler.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/common/content_switches.h"
 
 namespace content {
 
+namespace {
+
+std::unique_ptr<base::TaskScheduler::InitParams>
+GetDefaultTaskSchedulerInitParams() {
+#if defined(OS_ANDROID)
+  // Mobile config, for iOS see ios/web/app/web_main_loop.cc.
+  return std::make_unique<base::TaskScheduler::InitParams>(
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0),
+          base::TimeDelta::FromSeconds(30)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(2, 8, 0.1, 0),
+          base::TimeDelta::FromSeconds(30)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0),
+          base::TimeDelta::FromSeconds(30)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.3, 0),
+          base::TimeDelta::FromSeconds(60)));
+#else
+  // Desktop config.
+  return std::make_unique<base::TaskScheduler::InitParams>(
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0),
+          base::TimeDelta::FromSeconds(30)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(3, 8, 0.1, 0),
+          base::TimeDelta::FromSeconds(40)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0),
+          base::TimeDelta::FromSeconds(30)),
+      base::SchedulerWorkerPoolParams(
+          base::RecommendedMaxNumberOfThreadsInPool(8, 32, 0.3, 0),
+          base::TimeDelta::FromSeconds(60))
+#if defined(OS_WIN)
+          ,
+      base::TaskScheduler::InitParams::SharedWorkerPoolEnvironment::COM_MTA
+#endif  // defined(OS_WIN)
+      );
+#endif
+}
+
+}  // namespace
+
 std::unique_ptr<base::FieldTrialList> SetUpFieldTrialsAndFeatureList() {
   auto field_trial_list = std::make_unique<base::FieldTrialList>(nullptr);
   const base::CommandLine* command_line =
@@ -31,4 +82,29 @@ std::unique_ptr<base::FieldTrialList> SetUpFieldTrialsAndFeatureList() {
   return field_trial_list;
 }
 
+void StartBrowserTaskScheduler() {
+  auto task_scheduler_init_params =
+      GetContentClient()->browser()->GetTaskSchedulerInitParams();
+  if (!task_scheduler_init_params)
+    task_scheduler_init_params = GetDefaultTaskSchedulerInitParams();
+  DCHECK(task_scheduler_init_params);
+
+  // If a renderer lives in the browser process, adjust the number of
+  // threads in the foreground pool.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSingleProcess)) {
+    const base::SchedulerWorkerPoolParams&
+        current_foreground_worker_pool_params(
+            task_scheduler_init_params->foreground_worker_pool_params);
+    task_scheduler_init_params->foreground_worker_pool_params =
+        base::SchedulerWorkerPoolParams(
+            std::max(GetMinThreadsInRendererTaskSchedulerForegroundPool(),
+                     current_foreground_worker_pool_params.max_tasks()),
+            current_foreground_worker_pool_params.suggested_reclaim_time(),
+            current_foreground_worker_pool_params.backward_compatibility());
+  }
+
+  base::TaskScheduler::GetInstance()->Start(*task_scheduler_init_params.get());
+}
+
 }  // namespace content

content/browser/startup_helper.h

@@ -15,6 +15,9 @@ namespace content {
 std::unique_ptr<base::FieldTrialList> CONTENT_EXPORT
 SetUpFieldTrialsAndFeatureList();
 
+// Starts the task scheduler.
+void CONTENT_EXPORT StartBrowserTaskScheduler();
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_STARTUP_HELPER_H_

content/public/test/browser_test_base.cc

@@ -329,6 +329,7 @@ void BrowserTestBase::SetUp() {
   base::TaskScheduler::Create("Browser");
   DCHECK(!field_trial_list_);
   field_trial_list_ = SetUpFieldTrialsAndFeatureList();
+  StartBrowserTaskScheduler();
   BrowserTaskExecutor::Create();
   // TODO(phajdan.jr): Check return code, http://crbug.com/374738 .
   BrowserMain(params);