Make in-process gpu init thread safe

content/browser/browser_main_loop.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/browser_main_loop.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
@@ skipping 376 matching lines @@
 void BrowserMainLoop::EarlyInitialization() {
   TRACE_EVENT0("startup", "BrowserMainLoop::EarlyInitialization");
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   // No thread should be created before this call, as SetupSandbox()
   // will end-up using fork().
   SetupSandbox(parsed_command_line_);
 #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.";
     }
   }
 #endif
 
   // GLib's spawning of new processes is buggy, so it's important that at this
   // point GLib does not need to start DBUS. Chrome should always start with
   // DBUS_SESSION_BUS_ADDRESS properly set. See crbug.com/309093.
 #if defined(USE_GLIB)
@@ skipping 201 matching lines @@
     PluginService::GetInstance()->Init();
   }
 #endif
 
 #if !defined(OS_IOS) && (!defined(GOOGLE_CHROME_BUILD) || defined(OS_ANDROID))
   // Single-process is an unsupported and not fully tested mode, so
   // don't enable it for official Chrome builds (except on Android).
   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_;
 }
 
 void BrowserMainLoop::CreateStartupTasks() {
   TRACE_EVENT0("startup", "BrowserMainLoop::CreateStartupTasks");
 
   // First time through, we really want to create all the tasks
   if (!startup_task_runner_.get()) {
 #if defined(OS_ANDROID)
     startup_task_runner_ = make_scoped_ptr(new StartupTaskRunner(
@@ skipping 375 matching lines @@
 }
 
 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_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).
   io_thread_->SetPriority(base::kThreadPriority_Display);
   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;
   }
 #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();
 
@@ skipping 69 matching lines @@
   allowed_clipboard_threads.push_back(io_thread_->thread_id());
 #endif
   ui::Clipboard::SetAllowedThreads(allowed_clipboard_threads);
 
   // When running the GPU thread in-process, avoid optimistically starting it
   // since creating the GPU thread races against creation of the one-and-only
   // ChildProcess instance which is created by the renderer thread.
   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(
         BrowserThread::IO, FROM_HERE, base::Bind(
             base::IgnoreResult(&GpuProcessHost::Get),
             GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
             CAUSE_FOR_GPU_LAUNCH_BROWSER_STARTUP));
   }
 
 #if defined(OS_MACOSX)
   ThemeHelperMac::GetInstance();
   SystemHotkeyHelperMac::GetInstance()->DeferredLoadSystemHotkeys();
   if (ShouldEnableBootstrapSandbox()) {
     TRACE_EVENT0("startup",
         "BrowserMainLoop::BrowserThreadsStarted:BootstrapSandbox");
     CHECK(GetBootstrapSandbox());
   }
 #endif  // defined(OS_MACOSX)
 
 #endif  // !defined(OS_IOS)
 
   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
   // of intersecting ifdefs we have.  To keep it easy to follow, there
   // are no #else branches on any #ifs.
   // TODO(stevenjb): Move platform specific code into platform specific Parts
   // (Need to add InitializeToolkit stage to BrowserParts).
   // See also GTK setup in EarlyInitialization, above, and associated comments.
 
 #if defined(OS_WIN)
@@ skipping 83 matching lines @@
 
 void BrowserMainLoop::EndStartupTracing() {
   is_tracing_startup_ = false;
   TracingController::GetInstance()->DisableRecording(
       TracingController::CreateFileSink(
           startup_trace_file_,
           base::Bind(OnStoppedStartupTracing, startup_trace_file_)));
 }
 
 }  // namespace content