Refactor GpuProcessHost::GetForClient to GpuProcessHost::Get, which takes a GPU process kind rather…

content/browser/gpu/gpu_process_host.h

 // 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.
 
 #ifndef CONTENT_BROWSER_GPU_GPU_PROCESS_HOST_H_
 #define CONTENT_BROWSER_GPU_GPU_PROCESS_HOST_H_
 #pragma once
 
 #include <map>
 #include <queue>
@@ skipping 14 matching lines @@
 struct GPUCreateCommandBufferConfig;
 struct GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params;
 struct GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params;
 
 class BrowserChildProcessHostImpl;
 
 class GpuProcessHost : public content::BrowserChildProcessHostDelegate,
                        public IPC::Message::Sender,
                        public base::NonThreadSafe {
  public:
+  enum Kind {
+    kSandboxed,

[jam, 2012/03/01 01:38:31]

nit KIND_SANDBOXED per the chromium style guide. to make it more readable, I'd
rename the enum to GpuKind and then the values would be

GPU_KIND_SANDBOXED
GPU_KIND_UNSANDBOXED

http://www.chromium.org/developers/coding-style#Naming

[Ami GONE FROM CHROMIUM, 2012/03/01 02:33:04]

If you reversed the order then the old "sandboxed" bool arg value of 0 would
continue to mean "unsandboxed" in the new enum, and lower the odds of missing
changes in this CL biting us in the future.  Just sayin'.

[apatrick_chromium, 2012/03/01 02:57:39]

Good idea. Done.

+    kUnsandboxed
+  };
+
   typedef base::Callback<void(const IPC::ChannelHandle&,
                               base::ProcessHandle,
                               const content::GPUInfo&)>
       EstablishChannelCallback;
 
   typedef base::Callback<void(int32)> CreateCommandBufferCallback;
 
   static bool gpu_enabled() { return gpu_enabled_; }
 
-  // Creates a new GpuProcessHost or gets one for a particular client, resulting
-  // in the launching of a GPU process if required.  Returns null on failure. It
+  // Creates a new GpuProcessHost or gets an existing one, resulting in the
+  // launching of a GPU process if required.  Returns null on failure. It
   // is not safe to store the pointer once control has returned to the message
   // loop as it can be destroyed. Instead store the associated GPU host ID.
   // This could return NULL if GPU access is not allowed (blacklisted).
-  static GpuProcessHost* GetForClient(int client_id,
-                                      content::CauseForGpuLaunch cause);
+  static GpuProcessHost* Get(Kind kind, content::CauseForGpuLaunch cause);
 
   // Helper function to send the given message to the GPU process on the IO
-  // thread.  Calls GetForClient and if a host is returned, sends it.
-  // Can be called from any thread.
-  CONTENT_EXPORT static void SendOnIO(int client_id,
+  // thread.  Calls Get and if a host is returned, sends it.  Can be called from

[Ami GONE FROM CHROMIUM, 2012/03/01 02:33:04]

What a strange contract; Get() returning NULL is silently ignored here?
(obvs not part of your CL; today is just the first time I'm looking at this
code)

[apatrick_chromium, 2012/03/01 02:57:39]

It handles Get returning NULL by deleting the message so it doesn't leak. If Get
returns NULL it should mean the GPU process has crashed. The failure is detected
later when the GpuProcessHost is destroyed.

+  // any thread.
+  CONTENT_EXPORT static void SendOnIO(Kind kind,
                                       content::CauseForGpuLaunch cause,
                                       IPC::Message* message);
 
   // Get the GPU process host for the GPU process with the given ID. Returns
   // null if the process no longer exists.
   static GpuProcessHost* FromID(int host_id);
   int host_id() const { return host_id_; }
 
   // IPC::Message::Sender implementation.
   virtual bool Send(IPC::Message* msg) OVERRIDE;
@@ skipping 10 matching lines @@
   void CreateViewCommandBuffer(
       const gfx::GLSurfaceHandle& compositing_surface,
       int surface_id,
       int client_id,
       const GPUCreateCommandBufferConfig& init_params,
       const CreateCommandBufferCallback& callback);
 
   // Whether this GPU process is set up to use software rendering.
   bool software_rendering();
 
-  // Whether this GPU process is sandboxed.
-  bool sandboxed();
+  // What kind of GPU process, e.g. sandboxed or unsandboxed.
+  Kind kind();
 
   void ForceShutdown();
 
  private:
   static bool HostIsValid(GpuProcessHost* host);
 
-  GpuProcessHost(int host_id, bool sandboxed);
+  GpuProcessHost(int host_id, Kind kind);
   virtual ~GpuProcessHost();
 
   bool Init();
 
   // Post an IPC message to the UI shim's message handler on the UI thread.
   void RouteOnUIThread(const IPC::Message& message);
 
   // BrowserChildProcessHostDelegate implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
   virtual void OnChannelConnected(int32 peer_pid) OVERRIDE;
@@ skipping 50 matching lines @@
   std::queue<IPC::Message*> queued_messages_;
 
   // 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_;
 
   bool software_rendering_;
-  bool sandboxed_;
+  Kind kind_;
 
   scoped_ptr<GpuMainThread> in_process_gpu_thread_;
 
   // Whether we actually launched a GPU process.
   bool process_launched_;
 
   // Master switch for enabling/disabling GPU acceleration for the current
   // browser session. It does not change the acceleration settings for
   // existing tabs, just the future ones.
   static bool gpu_enabled_;
 
   static bool hardware_gpu_enabled_;
 
   scoped_ptr<BrowserChildProcessHostImpl> process_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuProcessHost);
 };
 
 #endif  // CONTENT_BROWSER_GPU_GPU_PROCESS_HOST_H_