Fix gpu acceleration with --in-process-gpu and --single-process modes.

With recent changes that have moved gpu message handling in the browser to the IO thread (and moved the handling of messages between gpu and renderer, that are mediated by the browser, to GpuProcessHost), the routing for such messages was broken when running the gpu thread (rather than process).

The new approach is to always instantiate GpuProcessHost (even when running a gpu thread only) and have a real IPC channel between host and gpu thread. This makes the 'in-process' GPU code work similar to what the renderer does when running --single-process.

Note that --single-process mode is potentially still a bit fragile with this, since ChildProcess and ChildThread are currently written to only allow a single static instance in one process (it would be better to instantiate GpuProcess and RenderProcess simultaneously), so ambiguous calls to access e.g. the main thread are possible.

Review URL: http://codereview.chromium.org/7054005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86958 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 103 insertions, 173 deletions

diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index e1944bd..fb756ec 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -134,9 +134,6 @@ class BrowserProcess {
   // Returns the thread that is used for background cache operations.
   virtual base::Thread* cache_thread() = 0;
 
-  // Returns the thread that issues GPU calls.
-  virtual base::Thread* gpu_thread() = 0;
-
 #if defined(USE_X11)
   // Returns the thread that is used to process UI requests in cases where
   // we can't route the request to the UI thread. Note that this thread
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 3d7a073..7f8b7e9 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -117,7 +117,6 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line)
       created_db_thread_(false),
       created_process_launcher_thread_(false),
       created_cache_thread_(false),
-      created_gpu_thread_(false),
       created_watchdog_thread_(false),
 #if defined(OS_CHROMEOS)
       created_web_socket_proxy_thread_(false),
@@ -240,7 +239,6 @@ BrowserProcessImpl::~BrowserProcessImpl() {
   // stopping the GPU thread. The GPU thread will close IPC channels to renderer
   // processes so this has to happen before stopping the IO thread.
   GpuProcessHostUIShim::DestroyAll();
-  gpu_thread_.reset();
 
   // Need to stop io_thread_ before resource_dispatcher_host_, since
   // io_thread_ may still deref ResourceDispatcherHost and handle resource
@@ -425,13 +423,6 @@ base::Thread* BrowserProcessImpl::cache_thread() {
   return cache_thread_.get();
 }
 
-base::Thread* BrowserProcessImpl::gpu_thread() {
-  DCHECK(CalledOnValidThread());
-  if (!created_gpu_thread_)
-    CreateGpuThread();
-  return gpu_thread_.get();
-}
-
 #if defined(USE_X11)
 base::Thread* BrowserProcessImpl::background_x11_thread() {
   DCHECK(CalledOnValidThread());
@@ -868,29 +859,6 @@ void BrowserProcessImpl::CreateCacheThread() {
   cache_thread_.swap(thread);
 }
 
-void BrowserProcessImpl::CreateGpuThread() {
-  DCHECK(!created_gpu_thread_ && !gpu_thread_.get());
-  created_gpu_thread_ = true;
-
-  scoped_ptr<base::Thread> thread(new BrowserThread(BrowserThread::GPU));
-
-  base::Thread::Options options;
-#if defined(OS_WIN)
-  // On Windows the GPU thread needs to pump the compositor child window's
-  // message loop. TODO(apatrick): make this an IO thread if / when we get rid
-  // of this child window. Unfortunately it might always be necessary for
-  // Windows XP because we cannot share the backing store textures between
-  // processes.
-  options.message_loop_type = MessageLoop::TYPE_UI;
-#else
-  options.message_loop_type = MessageLoop::TYPE_IO;
-#endif
-
-  if (!thread->StartWithOptions(options))
-    return;
-  gpu_thread_.swap(thread);
-}
-
 void BrowserProcessImpl::CreateWatchdogThread() {
   DCHECK(!created_watchdog_thread_ && watchdog_thread_.get() == NULL);
   created_watchdog_thread_ = true;
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 6274747..6e0def3 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -55,7 +55,6 @@ class BrowserProcessImpl : public BrowserProcess,
   virtual base::Thread* db_thread();
   virtual base::Thread* process_launcher_thread();
   virtual base::Thread* cache_thread();
-  virtual base::Thread* gpu_thread();
 #if defined(USE_X11)
   virtual base::Thread* background_x11_thread();
 #endif
@@ -190,9 +189,6 @@ class BrowserProcessImpl : public BrowserProcess,
   bool created_cache_thread_;
   scoped_ptr<base::Thread> cache_thread_;
 
-  bool created_gpu_thread_;
-  scoped_ptr<base::Thread> gpu_thread_;
-
   bool created_watchdog_thread_;
   scoped_ptr<WatchDogThread> watchdog_thread_;
 
diff --git a/chrome/test/testing_browser_process.cc b/chrome/test/testing_browser_process.cc
index b4b1594..2543f88 100644
--- a/chrome/test/testing_browser_process.cc
+++ b/chrome/test/testing_browser_process.cc
@@ -64,10 +64,6 @@ base::Thread* TestingBrowserProcess::cache_thread() {
   return NULL;
 }
 
-base::Thread* TestingBrowserProcess::gpu_thread() {
-  return NULL;
-}
-
 WatchDogThread* TestingBrowserProcess::watchdog_thread() {
   return NULL;
 }
diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h
index 3fc3c7d..59c4fff 100644
--- a/chrome/test/testing_browser_process.h
+++ b/chrome/test/testing_browser_process.h
@@ -63,8 +63,6 @@ class TestingBrowserProcess : public BrowserProcess {
 
   virtual base::Thread* cache_thread();
 
-  virtual base::Thread* gpu_thread();
-
   virtual WatchDogThread* watchdog_thread();
 
 #if defined(OS_CHROMEOS)
diff --git a/content/browser/browser_thread.cc b/content/browser/browser_thread.cc
index 3809989..31b7e43 100644
--- a/content/browser/browser_thread.cc
+++ b/content/browser/browser_thread.cc
@@ -17,7 +17,6 @@ static const char* browser_thread_names[BrowserThread::ID_COUNT] = {
   "Chrome_ProcessLauncherThread",  // PROCESS_LAUNCHER
   "Chrome_CacheThread",  // CACHE
   "Chrome_IOThread",  // IO
-  "Chrome_GpuThread",  // GPU
 #if defined(USE_X11)
   "Chrome_Background_X11Thread",  // BACKGROUND_X11
 #endif
diff --git a/content/browser/browser_thread.h b/content/browser/browser_thread.h
index c226551..3da4012c9 100644
--- a/content/browser/browser_thread.h
+++ b/content/browser/browser_thread.h
@@ -67,9 +67,6 @@ class BrowserThread : public base::Thread {
     // This is the thread that processes IPC and network messages.
     IO,
 
-    // This thread issues calls to the GPU in the browser process.
-    GPU,
-
 #if defined(USE_X11)
     // This thread has a second connection to the X server and is used to
     // process UI requests when routing the request to the UI thread would risk
diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc
index 3cc8b31..0bc414e 100644
--- a/content/browser/gpu/gpu_process_host.cc
+++ b/content/browser/gpu/gpu_process_host.cc
@@ -10,6 +10,7 @@
 #include "base/metrics/histogram.h"
 #include "base/process_util.h"
 #include "base/string_piece.h"
+#include "base/threading/thread.h"
 #include "chrome/browser/tab_contents/render_view_host_delegate_helper.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/gpu/gpu_data_manager.h"
@@ -18,6 +19,8 @@
 #include "content/browser/renderer_host/render_widget_host_view.h"
 #include "content/common/content_switches.h"
 #include "content/common/gpu/gpu_messages.h"
+#include "content/gpu/gpu_child_thread.h"
+#include "content/gpu/gpu_process.h"
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_switches.h"
 #include "media/base/media_switches.h"
@@ -114,6 +117,51 @@ GpuProcessHost::SurfaceRef::~SurfaceRef() {
 }
 #endif  // defined(OS_LINUX)
 
+// This class creates a GPU thread (instead of a GPU process), when running
+// with --in-process-gpu or --single-process.
+class GpuMainThread : public base::Thread {
+ public:
+  explicit GpuMainThread(const std::string& channel_id)
+      : base::Thread("Chrome_InProcGpuThread"),
+        channel_id_(channel_id),
+        gpu_process_(NULL),
+        child_thread_(NULL) {
+  }
+
+  ~GpuMainThread() {
+    Stop();
+  }
+
+ protected:
+  virtual void Init() {
+    // TODO: Currently, ChildProcess supports only a single static instance,
+    // which is a problem in --single-process mode, where both gpu and renderer
+    // should be able to create separate instances.
+    if (GpuProcess::current()) {
+      child_thread_ = new GpuChildThread(channel_id_);
+    } else {
+      gpu_process_ = new GpuProcess();
+      // The process object takes ownership of the thread object, so do not
+      // save and delete the pointer.
+      gpu_process_->set_main_thread(new GpuChildThread(channel_id_));
+    }
+  }
+
+  virtual void CleanUp() {
+    delete gpu_process_;
+    if (child_thread_)
+      delete child_thread_;
+  }
+
+ private:
+  std::string channel_id_;
+  // Deleted in CleanUp() on the gpu thread, so don't use smart pointers.
+  GpuProcess* gpu_process_;
+  GpuChildThread* child_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(GpuMainThread);
+};
+
 // static
 GpuProcessHost* GpuProcessHost::GetForRenderer(
     int renderer_id, content::CauseForGpuLaunch cause) {
@@ -136,28 +184,6 @@ GpuProcessHost* GpuProcessHost::GetForRenderer(
     return NULL;
 
   int host_id;
-  /* TODO(apatrick): this is currently broken because this function is called on
-     the IO thread from GpuMessageFilter, and we don't have an IO thread object
-     when running the GPU code in the browser at the moment.
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) ||
-      CommandLine::ForCurrentProcess()->HasSwitch(switches::kInProcessGPU)) {
-    if (!g_browser_process->gpu_thread())
-      return NULL;
-
-    // Initialize GL on the GPU thread.
-    // TODO(apatrick): Handle failure to initialize (asynchronously).
-    if (!BrowserThread::PostTask(
-        BrowserThread::GPU,
-        FROM_HERE,
-        NewRunnableFunction(&gfx::GLContext::InitializeOneOff))) {
-      return NULL;
-    }
-
-    host_id = 0;
-  } else {
-    host_id = ++g_last_host_id;
-  }
-  */
   host_id = ++g_last_host_id;
 
   UMA_HISTOGRAM_ENUMERATION("GPU.GPUProcessLaunchCause",
@@ -195,10 +221,17 @@ GpuProcessHost* GpuProcessHost::FromID(int host_id) {
 GpuProcessHost::GpuProcessHost(int host_id)
     : BrowserChildProcessHost(GPU_PROCESS),
       host_id_(host_id),
-      gpu_process_(base::kNullProcessHandle) {
+      gpu_process_(base::kNullProcessHandle),
+      in_process_(false) {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) ||
+      CommandLine::ForCurrentProcess()->HasSwitch(switches::kInProcessGPU))
+    in_process_ = true;
+
+  // If the 'single GPU process' policy ever changes, we still want to maintain
+  // it for 'gpu thread' mode and only create one instance of host and thread.
+  DCHECK(!in_process_ || g_hosts_by_id.IsEmpty());
+
   g_hosts_by_id.AddWithID(this, host_id_);
-  if (host_id == 0)
-    gpu_process_ = base::GetCurrentProcessHandle();
 
   // Post a task to create the corresponding GpuProcessHostUIShim.  The
   // GpuProcessHostUIShim will be destroyed if either the browser exits,
@@ -236,7 +269,27 @@ bool GpuProcessHost::Init() {
   if (!CreateChannel())
     return false;
 
-  if (!LaunchGpuProcess())
+  if (in_process_) {
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kDisableGpuWatchdog);
+
+    in_process_gpu_thread_.reset(new GpuMainThread(channel_id()));
+
+    base::Thread::Options options;
+#if defined(OS_WIN)
+  // On Windows the GPU thread needs to pump the compositor child window's
+  // message loop. TODO(apatrick): make this an IO thread if / when we get rid
+  // of this child window. Unfortunately it might always be necessary for
+  // Windows XP because we cannot share the backing store textures between
+  // processes.
+  options.message_loop_type = MessageLoop::TYPE_UI;
+#else
+  options.message_loop_type = MessageLoop::TYPE_IO;
+#endif
+    in_process_gpu_thread_->StartWithOptions(options);
+
+    OnProcessLaunched();  // Fake a callback that the process is ready.
+  } else if (!LaunchGpuProcess())
     return false;
 
   return Send(new GpuMsg_Initialize());
@@ -426,16 +479,20 @@ void GpuProcessHost::OnProcessLaunched() {
   // Send the GPU process handle to the UI thread before it has to
   // respond to any requests to establish a GPU channel. The response
   // to such requests require that the GPU process handle be known.
+
+  base::ProcessHandle child_handle = in_process_ ?
+      base::GetCurrentProcessHandle() : handle();
+
 #if defined(OS_WIN)
   DuplicateHandle(base::GetCurrentProcessHandle(),
-                  handle(),
+                  child_handle,
                   base::GetCurrentProcessHandle(),
                   &gpu_process_,
                   PROCESS_DUP_HANDLE,
                   FALSE,
                   0);
 #else
-  gpu_process_ = handle();
+  gpu_process_ = child_handle;
 #endif
 }
 
diff --git a/content/browser/gpu/gpu_process_host.h b/content/browser/gpu/gpu_process_host.h
index abf7280..c7c7df7 100644
--- a/content/browser/gpu/gpu_process_host.h
+++ b/content/browser/gpu/gpu_process_host.h
@@ -23,6 +23,8 @@ namespace IPC {
 class Message;
 }
 
+class GpuMainThread;
+
 class GpuProcessHost : public BrowserChildProcessHost,
                        public base::NonThreadSafe {
  public:
@@ -151,6 +153,12 @@ class GpuProcessHost : public BrowserChildProcessHost,
   // The handle for the GPU process or null if it is not known to be launched.
   base::ProcessHandle gpu_process_;
 
+  // Whether we are running a GPU thread inside the browser process instead
+  // of a separate GPU process.
+  bool in_process_;
+
+  scoped_ptr<GpuMainThread> in_process_gpu_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(GpuProcessHost);
 };
 
diff --git a/content/browser/gpu/gpu_process_host_ui_shim.cc b/content/browser/gpu/gpu_process_host_ui_shim.cc
index 276dca4..312ca9f 100644
--- a/content/browser/gpu/gpu_process_host_ui_shim.cc
+++ b/content/browser/gpu/gpu_process_host_ui_shim.cc
@@ -4,18 +4,15 @@
 
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
 
-#include "base/command_line.h"
 #include "base/id_map.h"
 #include "base/process_util.h"
 #include "base/debug/trace_event.h"
-#include "chrome/browser/browser_process.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/gpu/gpu_data_manager.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/renderer_host/render_widget_host_view.h"
-#include "content/common/content_switches.h"
 #include "content/common/gpu/gpu_messages.h"
 
 #if defined(OS_LINUX)
@@ -23,7 +20,6 @@
 #include <gdk/gdkwindow.h>  // NOLINT
 #include <gdk/gdkx.h>  // NOLINT
 #include "ui/base/x/x11_util.h"
-#include "ui/gfx/gtk_native_view_id_manager.h"
 #include "ui/gfx/size.h"
 #endif  // defined(OS_LINUX)
 namespace {
@@ -53,41 +49,6 @@ class SendOnIOThreadTask : public Task {
   scoped_ptr<IPC::Message> msg_;
 };
 
-class UIThreadSender : public IPC::Channel::Sender {
- public:
-  virtual bool Send(IPC::Message* msg) {
-  // The GPU process must never send a synchronous IPC message to the browser
-  // process. This could result in deadlock. Unfortunately linux does this for
-  // GpuHostMsg_ResizeXID. TODO(apatrick): fix this before issuing any GL calls
-  // on the browser process' GPU thread.
-#if !defined(OS_LINUX)
-    DCHECK(!msg->is_sync());
-#endif
-
-    // When the GpuChannelManager sends an IPC, post it to the UI thread without
-    // using IPC.
-    bool success = BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
-        new RouteToGpuProcessHostUIShimTask(0, *msg));
-
-    delete msg;
-    return success;
-  }
-};
-
-void ForwardMessageToGpuThread(GpuChannelManager* gpu_channel_manager,
-                               IPC::Message* msg) {
-  bool success = gpu_channel_manager->OnMessageReceived(*msg);
-
-  // If the message was not handled, it is likely it was intended for the
-  // GpuChildThread, which does not exist in single process and in process GPU
-  // mode.
-  DCHECK(success);
-
-  delete msg;
-}
-
 }  // namespace
 
 RouteToGpuProcessHostUIShimTask::RouteToGpuProcessHostUIShimTask(
@@ -107,18 +68,8 @@ void RouteToGpuProcessHostUIShimTask::Run() {
 }
 
 GpuProcessHostUIShim::GpuProcessHostUIShim(int host_id)
-    : host_id_(host_id),
-      gpu_channel_manager_(NULL),
-      ui_thread_sender_(NULL) {
+    : host_id_(host_id) {
   g_hosts_by_id.AddWithID(this, host_id_);
-  if (host_id == 0) {
-    ui_thread_sender_ = new UIThreadSender;
-    gpu_channel_manager_ = new GpuChannelManager(
-        ui_thread_sender_,
-        NULL,
-        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
-        g_browser_process->shutdown_event());
-  }
 }
 
 // static
@@ -150,24 +101,9 @@ GpuProcessHostUIShim* GpuProcessHostUIShim::FromID(int host_id) {
 
 bool GpuProcessHostUIShim::Send(IPC::Message* msg) {
   DCHECK(CalledOnValidThread());
-
-  bool success;
-
-  if (host_id_ == 0) {
-    success = BrowserThread::PostTask(
-        BrowserThread::GPU,
-        FROM_HERE,
-        NewRunnableFunction(ForwardMessageToGpuThread,
-                            gpu_channel_manager_,
-                            msg));
-  } else {
-    success = BrowserThread::PostTask(
-        BrowserThread::IO,
-        FROM_HERE,
-        new SendOnIOThreadTask(host_id_, msg));
-  }
-
-  return success;
+  return BrowserThread::PostTask(BrowserThread::IO,
+                                 FROM_HERE,
+                                 new SendOnIOThreadTask(host_id_, msg));
 }
 
 bool GpuProcessHostUIShim::OnMessageReceived(const IPC::Message& message) {
@@ -200,20 +136,6 @@ void GpuProcessHostUIShim::SendToGpuHost(int host_id, IPC::Message* msg) {
 GpuProcessHostUIShim::~GpuProcessHostUIShim() {
   DCHECK(CalledOnValidThread());
   g_hosts_by_id.Remove(host_id_);
-
-  // Ensure these are destroyed on the GPU thread.
-  if (gpu_channel_manager_) {
-    BrowserThread::DeleteSoon(BrowserThread::GPU,
-                              FROM_HERE,
-                              gpu_channel_manager_);
-    gpu_channel_manager_ = NULL;
-  }
-  if (ui_thread_sender_) {
-    BrowserThread::DeleteSoon(BrowserThread::GPU,
-                              FROM_HERE,
-                              ui_thread_sender_);
-    ui_thread_sender_ = NULL;
-  }
 }
 
 bool GpuProcessHostUIShim::OnControlMessageReceived(
diff --git a/content/browser/gpu/gpu_process_host_ui_shim.h b/content/browser/gpu/gpu_process_host_ui_shim.h
index fc077fa..f50ee95 100644
--- a/content/browser/gpu/gpu_process_host_ui_shim.h
+++ b/content/browser/gpu/gpu_process_host_ui_shim.h
@@ -14,13 +14,10 @@
 #include <queue>
 
 #include "base/callback.h"
+#include "base/task.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/non_thread_safe.h"
-#include "content/common/gpu/gpu_channel_manager.h"
-#include "content/common/gpu/gpu_feature_flags.h"
-#include "content/common/gpu/gpu_info.h"
-#include "content/common/gpu/gpu_process_launch_causes.h"
 #include "content/common/message_router.h"
 
 namespace gfx {
@@ -109,15 +106,6 @@ class GpuProcessHostUIShim
 
   // The serial number of the GpuProcessHost / GpuProcessHostUIShim pair.
   int host_id_;
-
-  // In single process and in process GPU mode, this references the
-  // GpuChannelManager or null otherwise. It must be called and deleted on the
-  // GPU thread.
-  GpuChannelManager* gpu_channel_manager_;
-
-  // This is likewise single process / in process GPU specific. This is a Sender
-  // implementation that forwards IPC messages to this UI shim on the UI thread.
-  IPC::Channel::Sender* ui_thread_sender_;
 };
 
 #endif  // CONTENT_BROWSER_GPU_GPU_PROCESS_HOST_UI_SHIM_H_
diff --git a/content/gpu/gpu_child_thread.cc b/content/gpu/gpu_child_thread.cc
index 8612a66..d3e6e94 100644
--- a/content/gpu/gpu_child_thread.cc
+++ b/content/gpu/gpu_child_thread.cc
@@ -116,7 +116,11 @@ bool GpuChildThread::OnControlMessageReceived(const IPC::Message& msg) {
 }
 
 void GpuChildThread::OnInitialize() {
-  logging::SetLogMessageHandler(GpuProcessLogMessageHandler);
+  // We don't need to pipe log messages if we are running the GPU thread in
+  // the browser process.
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) &&
+      !CommandLine::ForCurrentProcess()->HasSwitch(switches::kInProcessGPU))
+    logging::SetLogMessageHandler(GpuProcessLogMessageHandler);
 
   // Load the GL implementation and locate the bindings before starting the GPU
   // watchdog because this can take a lot of time and the GPU watchdog might