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..b06631abc8c3a0bf8a1fb9d380a5913e7547c4cc 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,24 @@ int BrowserMainLoop::PreCreateThreads() {
   if (parsed_command_line_.HasSwitch(switches::kSingleProcess))
     RenderProcessHost::SetRunRendererInProcess(true);
 #endif
+
+  // Need to initialize in-process GpuDataManager before creating threads
+  // since it's unsafe to manipulate the global CommandLine after.

[no sievers, 2015/01/27 19:43:58]

nit: I'd say more specifically that we need to append the gpu command line flags
(workarounds etc.) here early when running in-process.

[boliu, 2015/01/27 20:06:35]

Done.

+  if (UsingInProcessGpu()) {
+    bool initialize_gpu_data_manager = true;
+#if defined(OS_ANDROID)
+    if (!gfx::GLSurface::InitializeOneOff()) {
+      LOG(ERROR) << "GLSurface::InitializeOneOff failed";
+      initialize_gpu_data_manager = false;

[no sievers, 2015/01/27 19:43:58]

Do we really support this or is this fatal?

[boliu, 2015/01/27 20:06:35]

Webview supports this!

This used be fatal and then someone tried running webview on an emulator with no
gpu support which crashed immediately. So we turned this into a LOG(ERROR).

Now that they are separate, I can make the cross-process one call fatal again.

+    }
+#endif
+
+    // Initialize the GpuDataManager before we set up the MessageLoops because
+    // otherwise we'll trigger the assertion about doing IO on the UI thread.
+    if (initialize_gpu_data_manager)
+      GpuDataManagerImpl::GetInstance()->Initialize();
+  }
+
   return result_code_;
 }
 
@@ -1012,6 +1029,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,16 +1042,11 @@ 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()) {
+  if (!UsingInProcessGpu() && !gfx::GLSurface::InitializeOneOff()) {

[no sievers, 2015/01/27 19:43:58]

nit: use |initialize_gpu_data_manager|

[boliu, 2015/01/27 20:06:35]

Done.

+    // On Android, GLSurface::InitializeOneOff() must be called before
+    // initalizing the GpuDataManagerImpl as it uses the GL bindings.
+    // crbug.com/326295

[no sievers, 2015/01/27 19:43:58]

Pull this comment out to line 1045 and can you add a TODO(sievers): Shouldn't
need to init full bindings to determine GL version/vendor strings?

[boliu, 2015/01/27 20:06:35]

Done.

     LOG(ERROR) << "GLSurface::InitializeOneOff failed";
     initialize_gpu_data_manager = false;
   }
@@ -1118,8 +1136,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 +1161,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