Delete GpuVideoService and move GpuVideoDecodeAccelerator ownership to stubs

content/common/gpu/gpu_channel.cc

Index: content/common/gpu/gpu_channel.cc
diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc
index e28e14d985946e2542f1b340fdb5d8573d9da1ba..89706dc37bad9567f5f0bc112018fa502c2ebd2c 100644
--- a/content/common/gpu/gpu_channel.cc
+++ b/content/common/gpu/gpu_channel.cc
@@ -17,7 +17,6 @@
 #include "content/common/content_switches.h"
 #include "content/common/gpu/gpu_channel_manager.h"
 #include "content/common/gpu/gpu_messages.h"
-#include "content/common/gpu/media/gpu_video_service.h"
 #include "content/common/gpu/transport_texture.h"
 #include "ui/gfx/gl/gl_context.h"
 #include "ui/gfx/gl/gl_surface.h"
@@ -338,12 +337,6 @@ void GpuChannel::OnDestroySurface(int route_id) {
 void GpuChannel::OnCreateVideoDecoder(int32 decoder_host_id,
                                       uint32 command_buffer_route_id,
                                       const std::vector<uint32>& configs) {
-  GpuVideoService* service = GpuVideoService::GetInstance();
-  if (service == NULL) {
-    // TODO(hclam): Need to send a failure message.
-    return;
-  }
-
   GpuCommandBufferStub* stub = stubs_.Lookup(command_buffer_route_id);
   // TODO(vrk): Need to notify renderer that given route is invalid.
   if (!stub)
@@ -351,23 +344,33 @@ void GpuChannel::OnCreateVideoDecoder(int32 decoder_host_id,
 
   int32 decoder_id = GenerateRouteID();
 
-  // TODO(fischman): this is a BUG.  We hand off stub to be baked into the
-  // resulting GpuVideoDecodeAccelerator, but we don't own that GVDA, and we
-  // make no attempt to tear it down if/when stub is destroyed.  GpuVideoService
-  // should be subsumed into this class and GpuVideoDecodeAccelerator should be
-  // owned by GpuCommandBufferStub.
-  bool ret = service->CreateVideoDecoder(
-      this, &router_, decoder_host_id, decoder_id, stub, configs);
-  DCHECK(ret) << "Failed to create a GpuVideoDecodeAccelerator";
+  // Create GpuVideoDecodeAccelerator and add to map.
+  scoped_ptr<GpuVideoDecodeAccelerator> decoder(
+      new GpuVideoDecodeAccelerator(this, decoder_host_id, decoder_id, stub));
+  router_.AddRoute(decoder_id, decoder.get());
+  stub->set_video_decoder(decoder.release());
+  bool result = stubs_by_video_decoder_id_.insert(std::make_pair(

[apatrick_chromium, 2011/06/30 21:11:16]

Rather than maintaining a map of video decoder id to stub in the channel and
handling the IPCs there, can you instead route OnCreateVideoDecoder and
OnDestroyVideoDecoder directly the stub's route id?

Also, I don't know that the video decoder needs to have its own route ID?
GpuCommandBufferStub::OnMessageReceived could just pass IPCs on to the video
decoder if it doesn't handle them.

[vrk (LEFT CHROMIUM), 2011/07/01 18:12:14]

D'oh.

Emphatic yes to everything! These suggestions cleaned up a lot of unnecessary
indirection. Thanks!

+      decoder_id, stub)).second;
+  DCHECK(result);
+
+  // Tell client that creation is complete.
+  Send(new AcceleratedVideoDecoderHostMsg_CreateDone(
+      decoder_host_id, decoder_id));
 }
 
 void GpuChannel::OnDestroyVideoDecoder(int32 decoder_id) {
 #if defined(ENABLE_GPU)
   LOG(ERROR) << "GpuChannel::OnDestroyVideoDecoder";
-  GpuVideoService* service = GpuVideoService::GetInstance();
-  if (service == NULL)
+  router_.RemoveRoute(decoder_id);
+  std::map<int32, GpuCommandBufferStub*>::iterator it =
+      stubs_by_video_decoder_id_.find(decoder_id);
+  DCHECK(it != stubs_by_video_decoder_id_.end());
+  GpuCommandBufferStub* stub = it->second;
+  // TODO(fischman/vrk): Throw error.
+  if (!stub)
     return;
-  service->DestroyVideoDecoder(&router_, decoder_id);
+  stubs_by_video_decoder_id_.erase(decoder_id);
+  stub->DestroyVideoDecoder();
 #endif
 }