Refactor GPU init to allow ARM sandboxing.

content/gpu/gpu_main.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 <stdlib.h>
 
 #if defined(OS_WIN)
 #include <windows.h>
 #endif
 
@@ skipping 36 matching lines @@
 #endif
 
 #if defined(OS_LINUX)
 #include "content/public/common/sandbox_init.h"
 #endif
 
 const int kGpuTimeout = 10000;
 
 namespace content {
 namespace {
-void WarmUpSandbox(const GPUInfo&, bool);
+void WarmUpSandboxInitial();
+void WarmUpSandboxComplete(const GPUInfo&, bool);

[sheu, 2013/04/16 21:47:39]

WarmUpSandboxComplete() is only used (and only defined) on the OS_LINUX path,
but it's declared here unconditionally.  Move the declaration inside the
OS_LINUX block?

+#if defined(OS_LINUX)
+bool StartSandboxLinux(GpuWatchdogThread*);
+#elif defined(OS_WIN)
+bool StartSandboxWindows();
+#endif
 }
 
 // Main function for starting the Gpu process.
 int GpuMain(const MainFunctionParams& parameters) {
   TRACE_EVENT0("gpu", "GpuMain");
 
   base::Time start_time = base::Time::Now();
 
   const CommandLine& command_line = parameters.command_line;
   if (command_line.HasSwitch(switches::kGpuStartupDialog)) {
@@ skipping 95 matching lines @@
   success = base::HexStringToInt(
       command_line.GetSwitchValueASCII(switches::kGpuDeviceID),
       reinterpret_cast<int*>(&(gpu_info.gpu.device_id)));
   DCHECK(success);
   gpu_info.driver_vendor =
       command_line.GetSwitchValueASCII(switches::kGpuDriverVendor);
   gpu_info.driver_version =
       command_line.GetSwitchValueASCII(switches::kGpuDriverVersion);
   GetContentClient()->SetGpuInfo(gpu_info);
 
-  // We need to track that information for the WarmUpSandbox function.
+  // Warm up resources that don't need access to GPUInfo.
+  WarmUpSandboxInitial();
+
+  bool initialized_sandbox = false;
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)

[jln (very slow on Chromium), 2013/04/16 21:32:22]

Maybe owners can suggest something better to do here to detect that
configuration, I'll leave this up to them.

+  // On Chrome OS ARM, GPU driver userspace creates threads when initializing
+  // a GL context, so start the sandbox early.
+  gpu_info.sandboxed = StartSandboxLinux(watchdog_thread.get());
+  initialized_sandbox = true;
+#endif
+
+  // We need to track this information for the WarmUpSandboxComplete function.
   bool initialized_gl_context = false;
   // Load and initialize the GL implementation and locate the GL entry points.
   if (gfx::GLSurface::InitializeOneOff()) {
     // We need to collect GL strings (VENDOR, RENDERER) for blacklisting
-    // purpose. However, on Mac we don't actually use them. As documented in
+    // purposes. However, on Mac we don't actually use them. As documented in
     // crbug.com/222934, due to some driver issues, glGetString could take
     // multiple seconds to finish, which in turn cause the GPU process to crash.
     // By skipping the following code on Mac, we don't really lose anything,
     // because the basic GPU information is passed down from browser process
     // and we already registered them through SetGpuInfo() above.
 #if !defined(OS_MACOSX)
     if (!gpu_info_collector::CollectContextGraphicsInfo(&gpu_info))
       VLOG(1) << "gpu_info_collector::CollectGraphicsInfo failed";
     GetContentClient()->SetGpuInfo(gpu_info);
 
@@ skipping 26 matching lines @@
 
   // OSMesa is expected to run very slowly, so disable the watchdog in that
   // case.
   if (enable_watchdog &&
       gfx::GetGLImplementation() == gfx::kGLImplementationOSMesaGL) {
     watchdog_thread->Stop();
 
     watchdog_thread = NULL;
   }
 
-  {
-    const bool should_initialize_gl_context = !initialized_gl_context &&
-                                              !dead_on_arrival;
-    // Warm up the current process before enabling the sandbox.
-    WarmUpSandbox(gpu_info, should_initialize_gl_context);
+#if defined(OS_LINUX)
+  const bool should_initialize_gl_context = !initialized_gl_context &&
+                                            !dead_on_arrival;
+
+  if (!initialized_sandbox) {
+    // Finish warming up the current process.
+    WarmUpSandboxComplete(gpu_info, should_initialize_gl_context);
+    gpu_info.sandboxed = StartSandboxLinux(watchdog_thread);
   }
-
-#if defined(OS_LINUX)
-  {
-    TRACE_EVENT0("gpu", "Initialize sandbox");
-    bool do_init_sandbox = true;
-
-#if defined(OS_CHROMEOS) && defined(NDEBUG)
-    // On Chrome OS and when not on a debug build, initialize
-    // the GPU process' sandbox only for Intel GPUs.
-    do_init_sandbox = gpu_info.gpu.vendor_id == 0x8086;   // Intel GPU.
-#endif
-
-    if (do_init_sandbox) {
-      if (watchdog_thread.get())
-        watchdog_thread->Stop();
-      gpu_info.sandboxed = LinuxSandbox::InitializeSandbox();
-      if (watchdog_thread.get())
-        watchdog_thread->Start();
-    }
-  }
-#endif
-
-#if defined(OS_WIN)
-  {
-    TRACE_EVENT0("gpu", "Lower token");
-    // For windows, if the target_services interface is not zero, the process
-    // is sandboxed and we must call LowerToken() before rendering untrusted
-    // content.
-    sandbox::TargetServices* target_services =
-        parameters.sandbox_info->target_services;
-    if (target_services) {
-      target_services->LowerToken();
-      gpu_info.sandboxed = true;
-    }
-  }
+#elif defined(OS_WIN)
+  gpu_info.sandboxed = StartSandboxWindows();
 #endif
 
   GpuProcess gpu_process;
 
   GpuChildThread* child_thread = new GpuChildThread(watchdog_thread.get(),
                                                     dead_on_arrival, gpu_info);
 
   child_thread->Init(start_time);
 
   gpu_process.set_main_thread(child_thread);
@@ skipping 32 matching lines @@
 
   // Similarly, this is needed for /dev/nvidia0.
   if (context->MakeCurrent(surface)) {
     context->ReleaseCurrent(surface.get());
   } else {
     VLOG(1)  << "gfx::GLContext::MakeCurrent failed";
   }
 }
 #endif
 
-void WarmUpSandbox(const GPUInfo& gpu_info,
-                   bool should_initialize_gl_context) {
+void WarmUpSandboxInitial() {
   {
     TRACE_EVENT0("gpu", "Warm up rand");
     // Warm up the random subsystem, which needs to be done pre-sandbox on all
     // platforms.
     (void) base::RandUint64();
   }
   {
     TRACE_EVENT0("gpu", "Warm up HMAC");
     // Warm up the crypto subsystem, which needs to done pre-sandbox on all
     // platforms.
     crypto::HMAC hmac(crypto::HMAC::SHA256);
     unsigned char key = '\0';
     bool ret = hmac.Init(&key, sizeof(key));
     (void) ret;
   }
 
 #if defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL) && defined(USE_X11)
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUseExynosVda))
     ExynosVideoDecodeAccelerator::PreSandboxInitialization();
   else
     OmxVideoDecodeAccelerator::PreSandboxInitialization();
 #elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
   VaapiVideoDecodeAccelerator::PreSandboxInitialization();
 #endif
 
+#if defined(OS_WIN)
+  {
+    TRACE_EVENT0("gpu", "Preload setupapi.dll");
+    // Preload this DLL because the sandbox prevents it from loading.
+    LoadLibrary(L"setupapi.dll");
+  }
+
+  {
+    TRACE_EVENT0("gpu", "Initialize DXVA");
+    // Initialize H/W video decoding stuff which fails in the sandbox.
+    DXVAVideoDecodeAccelerator::PreSandboxInitialization();
+  }
+#endif
+}
+
 #if defined(OS_LINUX)
+void WarmUpSandboxComplete(const GPUInfo& gpu_info,
+                           bool should_initialize_gl_context) {

[sheu, 2013/04/16 21:47:39]

WarmUpSandboxComplete() seems to be NVIDIA-specific logic, so calling
WarmUpSandboxInitial() and then not calling WarmUpSandboxComplete() on
non-NVIDIA hardware is not an error.  By the way the functions are named though,
the names seem to imply that calling the -Initial() without calling a
-Complete() would be an error.

Could you rename this function, or otherwise refactor it such that you don't
need to break it up?

[Jorge Lucangeli Obes, 2013/04/16 22:39:57]

Before making it Linux-only due to its code, the point was splitting the
original WarmUpSandbox into code that needed GPUInfo and code that didn't. Turns
out that the only code needing GPUInfo was NVIDIA-related, so this can be an
NVIDIA-specific sandbox warmup.

   // We special case Optimus since the vendor_id we see may not be Nvidia.
   bool uses_nvidia_driver = (gpu_info.gpu.vendor_id == 0x10de &&  // NVIDIA.
                              gpu_info.driver_vendor == "NVIDIA") ||
                             gpu_info.optimus;
   if (uses_nvidia_driver && should_initialize_gl_context) {
     // We need this on Nvidia to pre-open /dev/nvidiactl and /dev/nvidia0.
     CreateDummyGlContext();
   }
+}
+
+bool StartSandboxLinux(GpuWatchdogThread* watchdog_thread) {
+  TRACE_EVENT0("gpu", "Initialize sandbox");
+
+  bool res = false;
+
+  if (watchdog_thread)
+    watchdog_thread->Stop();
+  // LinuxSandbox::InitializeSandbox() must always be called
+  // with only one thread.
+  res = LinuxSandbox::InitializeSandbox();
+  if (watchdog_thread)
+    watchdog_thread->Start();
+
+  return res;
+}
 #endif

[jln (very slow on Chromium), 2013/04/16 21:32:22]

style: add  // defined(OS_LINUX)

[Jorge Lucangeli Obes, 2013/04/16 21:40:31]

Done.

 
 #if defined(OS_WIN)
-  {
-    TRACE_EVENT0("gpu", "Preload setupapi.dll");
-    // Preload this DLL because the sandbox prevents it from loading.
-    LoadLibrary(L"setupapi.dll");
+bool StartSandboxWindows() {
+  TRACE_EVENT0("gpu", "Lower token");
+
+  // For Windows, if the target_services interface is not zero, the process
+  // is sandboxed and we must call LowerToken() before rendering untrusted
+  // content.
+  sandbox::TargetServices* target_services =
+      parameters.sandbox_info->target_services;

[jln (very slow on Chromium), 2013/04/16 21:33:52]

Looks like you'll need to take this as a parameter.

[Jorge Lucangeli Obes, 2013/04/16 21:40:31]

Done.

+  if (target_services) {
+    target_services->LowerToken();
+    return true:
   }
 
-  {
-    TRACE_EVENT0("gpu", "Initialize DXVA");
-    // Initialize H/W video decoding stuff which fails in the sandbox.
-    DXVAVideoDecodeAccelerator::PreSandboxInitialization();
-  }
+  return false;
+}
 #endif

[jln (very slow on Chromium), 2013/04/16 21:32:22]

style: add // defined(OS_WIN)

[Jorge Lucangeli Obes, 2013/04/16 21:40:31]

Done.

-}
 
 }  // namespace.
 
 }  // namespace content