Make in-process gpu init thread safe

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 64e3446bcce54b75ed116e3820d246929c916dcc..4bf2ee6c00e77e9c1bd8144cac5b4c85500f7c55 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -394,8 +394,7 @@ void BrowserMainLoop::EarlyInitialization() {
 #endif
 
 #if defined(USE_X11)
-  if (parsed_command_line_.HasSwitch(switches::kSingleProcess) ||
-      parsed_command_line_.HasSwitch(switches::kInProcessGPU)) {
+  if (UsingInProcessGpu()) {
     if (!gfx::InitializeThreadedX11()) {
       LOG(ERROR) << "Failed to put Xlib into threaded mode.";
     }
@@ -617,6 +616,26 @@ int BrowserMainLoop::PreCreateThreads() {
   if (parsed_command_line_.HasSwitch(switches::kSingleProcess))
     RenderProcessHost::SetRunRendererInProcess(true);
 #endif
+
+  // Need to initialize in-process GpuDataManager before creating threads.
+  // It's unsafe to append the gpu command line switches to the global
+  // CommandLine::ForCurrentProcess object after threads are created.
+  if (UsingInProcessGpu()) {
+    bool initialize_gpu_data_manager = true;
+#if defined(OS_ANDROID)
+    if (!gfx::GLSurface::InitializeOneOff()) {

[no sievers, 2015/01/27 20:42:04]

Actually you can comment before this line that this is not thread-safe either
and has to happen before we start threads (for the case where we start a gpu
thread, i.e. in-process, and will call InitializeOneOff() from there also).

[boliu, 2015/01/27 21:20:15]

Done.

+      // Single-process Android WebView supports no gpu.
+      LOG(ERROR) << "GLSurface::InitializeOneOff failed";
+      initialize_gpu_data_manager = false;
+    }
+#endif
+
+    // Initialize the GpuDataManager before we set up the MessageLoops because
+    // otherwise we'll trigger the assertion about doing IO on the UI thread.

[no sievers, 2015/01/27 20:42:04]

And then maybe this comment can go.

[boliu, 2015/01/27 21:20:15]

Done.

+    if (initialize_gpu_data_manager)
+      GpuDataManagerImpl::GetInstance()->Initialize();
+  }
+
   return result_code_;
 }
 
@@ -1012,6 +1031,12 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   indexed_db_thread_->Start();
 #endif
 
+#if !defined(OS_IOS)
+  HistogramSynchronizer::GetInstance();
+
+
+  // GpuDataManager for in-process initialized in PreCreateThreads.
+  bool initialize_gpu_data_manager = !UsingInProcessGpu();
 #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).
@@ -1019,18 +1044,15 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   base::PlatformThread::SetThreadPriority(
       base::PlatformThread::CurrentHandle(),
       base::kThreadPriority_Display);
-#endif
-
-#if !defined(OS_IOS)
-  HistogramSynchronizer::GetInstance();
 
-  bool initialize_gpu_data_manager = true;
-#if defined(OS_ANDROID)
-  // On Android, GLSurface::InitializeOneOff() must be called before initalizing
-  // the GpuDataManagerImpl as it uses the GL bindings. crbug.com/326295
-  if (!gfx::GLSurface::InitializeOneOff()) {
-    LOG(ERROR) << "GLSurface::InitializeOneOff failed";
-    initialize_gpu_data_manager = false;
+  // On Android, GLSurface::InitializeOneOff() must be called before
+  // initalizing the GpuDataManagerImpl as it uses the GL bindings.
+  // TODO(sievers): Shouldn't need to init full bindings to determine GL
+  // version/vendor strings? crbug.com/326295
+  if (initialize_gpu_data_manager) {
+    if (!gfx::GLSurface::InitializeOneOff()) {
+      LOG(FATAL) << "GLSurface::InitializeOneOff failed";
+    }
   }
 #endif
 
@@ -1118,8 +1140,7 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   if (GpuDataManagerImpl::GetInstance()->GpuAccessAllowed(NULL) &&
       !established_gpu_channel &&
       always_uses_gpu &&
-      !parsed_command_line_.HasSwitch(switches::kSingleProcess) &&
-      !parsed_command_line_.HasSwitch(switches::kInProcessGPU)) {
+      !UsingInProcessGpu()) {
     TRACE_EVENT_INSTANT0("gpu", "Post task to launch GPU process",
                          TRACE_EVENT_SCOPE_THREAD);
     BrowserThread::PostTask(
@@ -1144,6 +1165,11 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   return result_code_;
 }
 
+bool BrowserMainLoop::UsingInProcessGpu() const {
+  return parsed_command_line_.HasSwitch(switches::kSingleProcess) ||
+         parsed_command_line_.HasSwitch(switches::kInProcessGPU);
+}
+
 bool BrowserMainLoop::InitializeToolkit() {
   TRACE_EVENT0("startup", "BrowserMainLoop::InitializeToolkit");
   // TODO(evan): this function is rather subtle, due to the variety