Run the later parts of startup as UI thread tasks

content/browser/browser_main_loop.cc

Index: content/browser/browser_main_loop.cc
diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index d95dbb594142047be0deb447367a014e2279b908..4f25676e7810efc08e0497823eba97b7a05930a0 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -19,6 +19,7 @@
 #include "base/system_monitor/system_monitor.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/timer/hi_res_timer_manager.h"
+#include "content/browser/browser_startup_configuration.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/device_orientation/device_motion_service.h"
 #include "content/browser/download/save_file_manager.h"
@@ -34,6 +35,7 @@
 #include "content/browser/renderer_host/media/audio_mirroring_manager.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/speech/speech_recognition_manager_impl.h"
+#include "content/browser/startup_task_runner.h"
 #include "content/browser/tracing/trace_controller_impl.h"
 #include "content/browser/webui/content_web_ui_controller_factory.h"
 #include "content/browser/webui/url_data_manager.h"
@@ -45,6 +47,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/main_function_params.h"
 #include "content/public/common/result_codes.h"
+#include "content/browser/startup_task_runner.h"
 #include "crypto/nss_util.h"
 #include "media/audio/audio_manager.h"
 #include "media/base/media.h"
@@ -483,8 +486,7 @@ void BrowserMainLoop::MainMessageLoopStart() {
 #endif
 }
 
-void BrowserMainLoop::CreateThreads() {
-  TRACE_EVENT0("startup", "BrowserMainLoop::CreateThreads")
+int BrowserMainLoop::PreCreateThreads() {
 
   if (parts_) {
     TRACE_EVENT0("startup",
@@ -509,9 +511,38 @@ void BrowserMainLoop::CreateThreads() {
   if (parsed_command_line_.HasSwitch(switches::kSingleProcess))
     RenderProcessHost::SetRunRendererInProcess(true);
 #endif
+  return result_code_;
+}
+
+void BrowserMainLoop::CreateStartupTasks() {
+  TRACE_EVENT0("startup", "BrowserMainLoop::CreateStartupTasks")
+
+  scoped_refptr<StartupTaskRunner> task_runner =

[Yaron, 2013/07/30 00:06:24]

Again, can't this be owned by BrowserMainRunnerImpl?

[aberent, 2013/07/30 15:01:54]

I assume you are suggesting making this a class member and removing the
scoped_refptr. If not, then please explain further. I am anyway unclear why you
are suggesting BrowserMainRunnerImpl rather than BrowserMainLoop, since
BrowserMainRunnerImpl now needs no knowledge of this, and moving to there would
mean passing it through extra levels.

This would be possible to make this a class variable but it makes lifetime
management more difficult. The StartupTaskRunner object has to stay alive as
long as there are tasks in the queue, but is then redundant. This is handled at
the moment by passing it (within its implementation) to base::Bind(), which then
manages its lifetime using a scoped_refptr, and deletes it once the last task
has run. We could tell Bind not to manage it, but in this case the owning class
would have to delete it. At the moment BrowserMainRunnerImpl doesn't know when
the last startup task has finished, so doesn't know when to delete it (this
could be fixed, but would complicate the code). As such we would probably end up
leaving this object around for the whole life of Chrome, rather than deleting it
at the end of startup.

Please explain further why you think using a class member variable would be
better in this case, if this is what you meant.

 

[Yaron, 2013/07/30 18:53:22]

Err, you're right that I should've said BrowserMainLoop. I guess I have an
unhealthy aversion to refptr. I don't see it as a major problem that the
task_runner is owned until BrowserMainLoop is deleted but it is unnecessary. Ok,
you've convinced me. Thanks for bearing with me

+      new StartupTaskRunner(BrowserMayStartAsynchronously(),
+                            BrowserStartupComplete,
+                            base::MessageLoop::current()->message_loop_proxy());
+
+  StartupTask pre_create_threads =
+      base::Bind(&BrowserMainLoop::PreCreateThreads, base::Unretained(this));
+  task_runner->AddTask(pre_create_threads);
+
+  StartupTask create_threads =
+      base::Bind(&BrowserMainLoop::CreateThreads, base::Unretained(this));
+  task_runner->AddTask(create_threads);
+
+  StartupTask browser_thread_started = base::Bind(
+      &BrowserMainLoop::BrowserThreadsStarted, base::Unretained(this));
+  task_runner->AddTask(browser_thread_started);
 
-  if (result_code_ > 0)
-    return;
+  StartupTask pre_main_message_loop_run = base::Bind(
+      &BrowserMainLoop::PreMainMessageLoopRun, base::Unretained(this));
+  task_runner->AddTask(pre_main_message_loop_run);
+
+  task_runner->StartRunningTasks();
+}
+
+int BrowserMainLoop::CreateThreads() {
+  TRACE_EVENT0("startup", "BrowserMainLoop::CreateThreads");
 
   base::Thread::Options default_options;
   base::Thread::Options io_message_loop_options;
@@ -596,14 +627,10 @@ void BrowserMainLoop::CreateThreads() {
     TRACE_EVENT_END0("startup", "BrowserMainLoop::CreateThreads:start");
 
   }
+  return result_code_;
+}
 
-#if !defined(OS_IOS)
-  indexed_db_thread_.reset(new base::Thread("IndexedDB"));
-  indexed_db_thread_->Start();
-#endif
-
-  BrowserThreadsStarted();
-
+int BrowserMainLoop::PreMainMessageLoopRun() {
   if (parts_) {
     TRACE_EVENT0("startup",
         "BrowserMainLoop::CreateThreads:PreMainMessageLoopRun");
@@ -614,6 +641,7 @@ void BrowserMainLoop::CreateThreads() {
   // Do not allow disk IO from the UI thread.
   base::ThreadRestrictions::SetIOAllowed(false);
   base::ThreadRestrictions::DisallowWaiting();
+  return result_code_;
 }
 
 void BrowserMainLoop::RunMainMessageLoopParts() {
@@ -765,8 +793,14 @@ void BrowserMainLoop::InitializeMainThread() {
       new BrowserThreadImpl(BrowserThread::UI, base::MessageLoop::current()));
 }
 
-void BrowserMainLoop::BrowserThreadsStarted() {
+int BrowserMainLoop::BrowserThreadsStarted() {
   TRACE_EVENT0("startup", "BrowserMainLoop::BrowserThreadsStarted")
+
+#if !defined(OS_IOS)
+  indexed_db_thread_.reset(new base::Thread("IndexedDB"));
+  indexed_db_thread_->Start();
+#endif
+
 #if defined(OS_ANDROID)
   // Up the priority of anything that touches with display tasks
   // (this thread is UI thread, and io_thread_ is for IPCs).
@@ -844,6 +878,7 @@ void BrowserMainLoop::BrowserThreadsStarted() {
             CAUSE_FOR_GPU_LAUNCH_BROWSER_STARTUP));
   }
 #endif  // !defined(OS_IOS)
+  return result_code_;
 }
 
 void BrowserMainLoop::InitializeToolkit() {