gpu: Some code cleanups.

. Remove unused BrowserGpuChannelHostFactory::AddFilterOnIO().
. Use a weak-ptr to bind callbacks to GpuProcessHost, instead of looking
  up the GpuProcessHost from the host-id (using GpuProcessHost::FromID()).
. Remove some null checks for |gpu_channel_manager_| in GpuService,
  because it is created during initialization, and destroyed only in the
  dtor. None of the other callbacks should trigger before initialization.
  So the null checks should not be necessary.

BUG=none

Review-Url: https://codereview.chromium.org/2801943002
Cr-Commit-Position: refs/heads/master@{#463049}

content/browser/gpu/browser_gpu_channel_host_factory.cc

@@ -364,17 +364,6 @@ void BrowserGpuChannelHostFactory::GpuChannelEstablished() {
     callback.Run(gpu_channel_);
 }
 
-// static
-void BrowserGpuChannelHostFactory::AddFilterOnIO(
-    int host_id,
-    scoped_refptr<IPC::MessageFilter> filter) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  GpuProcessHost* host = GpuProcessHost::FromID(host_id);
-  if (host)
-    host->AddFilter(filter.get());
-}
-
 // static
 void BrowserGpuChannelHostFactory::InitializeShaderDiskCacheOnIO(
     int gpu_client_id,

content/browser/gpu/browser_gpu_channel_host_factory.h

@@ -64,8 +64,6 @@ class CONTENT_EXPORT BrowserGpuChannelHostFactory
 
   void GpuChannelEstablished();
 
-  static void AddFilterOnIO(int gpu_host_id,
-                            scoped_refptr<IPC::MessageFilter> filter);
   static void InitializeShaderDiskCacheOnIO(int gpu_client_id,
                                             const base::FilePath& cache_dir);
 

content/browser/gpu/gpu_process_host.cc

@@ -34,8 +34,6 @@
 #include "content/browser/gpu/gpu_main_thread_factory.h"
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
 #include "content/browser/gpu/shader_cache_factory.h"
-#include "content/browser/renderer_host/render_widget_host_impl.h"
-#include "content/browser/renderer_host/render_widget_host_view_frame_subscriber.h"
 #include "content/browser/service_manager/service_manager_context.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/common/in_process_child_thread_params.h"
@@ -44,8 +42,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/gpu_utils.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/common/connection_filter.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
@@ -198,15 +194,14 @@ void RunCallbackOnIO(GpuProcessHost::GpuProcessKind kind,
 }
 
 #if defined(USE_OZONE)
-void SendGpuProcessMessageByHostId(int host_id, IPC::Message* message) {
-  GpuProcessHost* host = GpuProcessHost::FromID(host_id);
-  if (host) {
+void SendGpuProcessMessage(base::WeakPtr<GpuProcessHost> host,
+                           IPC::Message* message) {
+  if (host)
     host->Send(message);
-  } else {
+  else
     delete message;
-  }
 }
-#endif
+#endif  // defined(USE_OZONE)
 
 // NOTE: changes to this class need to be reviewed by the security team.
 class GpuSandboxedProcessLauncherDelegate
@@ -311,14 +306,6 @@ class GpuSandboxedProcessLauncherDelegate
 #endif  // OS_WIN
 };
 
-void HostLoadedShader(int host_id,
-                      const std::string& key,
-                      const std::string& data) {
-  GpuProcessHost* host = GpuProcessHost::FromID(host_id);
-  if (host)
-    host->LoadedShader(key, data);
-}
-
 }  // anonymous namespace
 
 class GpuProcessHost::ConnectionFilterImpl : public ConnectionFilter {
@@ -631,8 +618,8 @@ bool GpuProcessHost::Init() {
   ui::OzonePlatform::GetInstance()
       ->GetGpuPlatformSupportHost()
       ->OnGpuProcessLaunched(
-          host_id_, BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
-          base::Bind(&SendGpuProcessMessageByHostId, host_id_));
+          host_id_, base::ThreadTaskRunnerHandle::Get(),
+          base::Bind(&SendGpuProcessMessage, weak_ptr_factory_.GetWeakPtr()));
 #endif
 
   return true;
@@ -662,11 +649,6 @@ bool GpuProcessHost::Send(IPC::Message* msg) {
   return result;
 }
 
-void GpuProcessHost::AddFilter(IPC::MessageFilter* filter) {
-  DCHECK(CalledOnValidThread());
-  process_->GetHost()->AddFilter(filter);
-}
-
 bool GpuProcessHost::OnMessageReceived(const IPC::Message& message) {
   DCHECK(CalledOnValidThread());
   RouteOnUIThread(message);
@@ -1193,7 +1175,8 @@ void GpuProcessHost::CreateChannelCache(int32_t client_id) {
   if (!cache.get())
     return;
 
-  cache->set_shader_loaded_callback(base::Bind(&HostLoadedShader, host_id_));
+  cache->set_shader_loaded_callback(base::Bind(&GpuProcessHost::LoadedShader,
+                                               weak_ptr_factory_.GetWeakPtr()));
 
   client_id_to_shader_cache_[client_id] = cache;
 }

content/browser/gpu/gpu_process_host.h

@@ -108,9 +108,6 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
   // IPC::Sender implementation.
   bool Send(IPC::Message* msg) override;
 
-  // Adds a message filter to the GpuProcessHost's channel.
-  void AddFilter(IPC::MessageFilter* filter);
-
   // Tells the GPU process to create a new channel for communication with a
   // client. Once the GPU process responds asynchronously with the IPC handle
   // and GPUInfo, we call the callback.

content/gpu/gpu_child_thread.cc

@@ -165,7 +165,6 @@ void GpuChildThread::CreateGpuService(
     ui::mojom::GpuHostPtr gpu_host,
     const gpu::GpuPreferences& gpu_preferences,
     mojo::ScopedSharedBufferHandle activity_flags) {
-  gpu_service_->Bind(std::move(request));
   gpu_service_->UpdateGPUInfoFromPreferences(gpu_preferences);
   for (const LogMessage& log : deferred_messages_)
     gpu_host->RecordLogMessage(log.severity, log.header, log.message);
@@ -179,6 +178,10 @@ void GpuChildThread::CreateGpuService(
     return;
   }
 
+  // Bind should happen only if initialization succeeds (i.e. not dead on
+  // arrival), because otherwise, it can receive requests from the host while in
+  // an uninitialized state.
+  gpu_service_->Bind(std::move(request));
   gpu::SyncPointManager* sync_point_manager = nullptr;
   // Note SyncPointManager from ContentGpuClient cannot be owned by this.
   if (GetContentClient()->gpu())

services/ui/gpu/gpu_service.cc

@@ -232,8 +232,7 @@ void GpuService::DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,
                               id, client_id, sync_token));
     return;
   }
-  if (gpu_channel_manager_)
-    gpu_channel_manager_->DestroyGpuMemoryBuffer(id, client_id, sync_token);
+  gpu_channel_manager_->DestroyGpuMemoryBuffer(id, client_id, sync_token);
 }
 
 void GpuService::GetVideoMemoryUsageStats(
@@ -246,10 +245,8 @@ void GpuService::GetVideoMemoryUsageStats(
     return;
   }
   gpu::VideoMemoryUsageStats video_memory_usage_stats;
-  if (gpu_channel_manager_) {
-    gpu_channel_manager_->gpu_memory_manager()->GetVideoMemoryUsageStats(
-        &video_memory_usage_stats);
-  }
+  gpu_channel_manager_->gpu_memory_manager()->GetVideoMemoryUsageStats(
+      &video_memory_usage_stats);
   callback.Run(video_memory_usage_stats);
 }
 
@@ -384,11 +381,6 @@ void GpuService::EstablishGpuChannel(
     return;
   }
 
-  if (!gpu_channel_manager_) {
-    callback.Run(mojo::ScopedMessagePipeHandle());
-    return;
-  }
-
   gpu::GpuChannel* gpu_channel = gpu_channel_manager_->EstablishChannel(
       client_id, client_tracing_id, is_gpu_host);
 
@@ -407,8 +399,6 @@ void GpuService::CloseChannel(int32_t client_id) {
         FROM_HERE, base::Bind(&GpuService::CloseChannel, weak_ptr_, client_id));
     return;
   }
-  if (!gpu_channel_manager_)
-    return;
   gpu_channel_manager_->RemoveChannel(client_id);
 }
 
@@ -418,8 +408,6 @@ void GpuService::LoadedShader(const std::string& data) {
         FROM_HERE, base::Bind(&GpuService::LoadedShader, weak_ptr_, data));
     return;
   }
-  if (!gpu_channel_manager_)
-    return;
   gpu_channel_manager_->PopulateShaderCache(data);
 }
 
@@ -449,8 +437,6 @@ void GpuService::WakeUpGpu() {
     return;
   }
 #if defined(OS_ANDROID)
-  if (!gpu_channel_manager_)
-    return;
   gpu_channel_manager_->WakeUpGpu();
 #else
   NOTREACHED() << "WakeUpGpu() not supported on this platform.";
@@ -469,8 +455,6 @@ void GpuService::DestroyAllChannels() {
         FROM_HERE, base::Bind(&GpuService::DestroyAllChannels, weak_ptr_));
     return;
   }
-  if (!gpu_channel_manager_)
-    return;
   DVLOG(1) << "GPU: Removing all contexts";
   gpu_channel_manager_->DestroyAllChannels();
 }