Adding GpuMemoryManager to track GpuCommandBufferStub visibility and last_used_time and dictate mem…

content/common/gpu/gpu_command_buffer_stub.cc

Index: content/common/gpu/gpu_command_buffer_stub.cc
diff --git a/content/common/gpu/gpu_command_buffer_stub.cc b/content/common/gpu/gpu_command_buffer_stub.cc
index ff485a74dcf36996da8fe032202a001bbca35d47..eac480b2bece3ca74c5ff4b98b2318ff27e94e1f 100644
--- a/content/common/gpu/gpu_command_buffer_stub.cc
+++ b/content/common/gpu/gpu_command_buffer_stub.cc
@@ -9,10 +9,13 @@
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/shared_memory.h"
+#include "base/time.h"
 #include "build/build_config.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "content/common/gpu/gpu_channel_manager.h"
 #include "content/common/gpu/gpu_command_buffer_stub.h"
+#include "content/common/gpu/gpu_memory_allocation.h"
+#include "content/common/gpu/gpu_memory_manager.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "content/common/gpu/gpu_watchdog.h"
 #include "content/common/gpu/image_transport_surface.h"
@@ -20,6 +23,14 @@
 #include "ui/gfx/gl/gl_bindings.h"
 #include "ui/gfx/gl/gl_switches.h"
 
+GpuCommandBufferStub::SurfaceState::SurfaceState(int32 surface_id,
+                                                 bool visible,
+                                                 base::TimeTicks last_used_time)
+    : surface_id(surface_id),
+      visible(visible),
+      last_used_time(last_used_time) {
+}
+
 GpuCommandBufferStub::GpuCommandBufferStub(
     GpuChannel* channel,
     GpuCommandBufferStub* share_group,
@@ -43,7 +54,6 @@ GpuCommandBufferStub::GpuCommandBufferStub(
       route_id_(route_id),
       software_(software),
       last_flush_count_(0),
-      surface_id_(surface_id),
       parent_stub_for_initialization_(),
       parent_texture_for_initialization_(0),
       watchdog_(watchdog) {
@@ -54,13 +64,16 @@ GpuCommandBufferStub::GpuCommandBufferStub(
     bool bind_generates_resource = true;
     context_group_ = new gpu::gles2::ContextGroup(bind_generates_resource);
   }
+  if (surface_id != 0)
+    surface_state_.reset(new GpuCommandBufferStubBase::SurfaceState(
+        surface_id, true, base::TimeTicks::Now()));
 }
 
 GpuCommandBufferStub::~GpuCommandBufferStub() {
   Destroy();
 
   GpuChannelManager* gpu_channel_manager = channel_->gpu_channel_manager();
-  gpu_channel_manager->Send(new GpuHostMsg_DestroyCommandBuffer(surface_id_));
+  gpu_channel_manager->Send(new GpuHostMsg_DestroyCommandBuffer(surface_id()));
 }
 
 bool GpuCommandBufferStub::OnMessageReceived(const IPC::Message& message) {
@@ -153,6 +166,8 @@ void GpuCommandBufferStub::Destroy() {
 
   context_ = NULL;
   surface_ = NULL;
+
+  channel_->gpu_channel_manager()->gpu_memory_manager()->ScheduleManage();
 }
 
 void GpuCommandBufferStub::OnInitializeFailed(IPC::Message* reply_message) {
@@ -274,6 +289,8 @@ void GpuCommandBufferStub::OnInitialize(
 
   GpuCommandBufferMsg_Initialize::WriteReplyParams(reply_message, true);
   Send(reply_message);
+
+  channel_->gpu_channel_manager()->gpu_memory_manager()->ScheduleManage();
 }
 
 void GpuCommandBufferStub::OnSetGetBuffer(
@@ -496,6 +513,10 @@ void GpuCommandBufferStub::OnDestroyVideoDecoder(int decoder_route_id) {
 
 void GpuCommandBufferStub::OnSetSurfaceVisible(bool visible) {
   surface_->SetVisible(visible);
+  DCHECK(surface_state_.get());
+  surface_state_->visible = visible;
+  surface_state_->last_used_time = base::TimeTicks::Now();
+  channel_->gpu_channel_manager()->gpu_memory_manager()->ScheduleManage();
 }
 
 void GpuCommandBufferStub::SendConsoleMessage(
@@ -510,4 +531,19 @@ void GpuCommandBufferStub::SendConsoleMessage(
   Send(msg);
 }
 
+bool GpuCommandBufferStub::has_surface_state() {
+  return !!surface_state_.get();

[jonathan.backer, 2012/02/02 14:03:53]

Correct me if I'm wrong, I think that !! is redundant. AFAIK, ! is usually to
coerce a smart pointer into a bool. But we don't have that here: get() is
returning a raw pointer.

Also, why not in the header?

[mmocny, 2012/02/02 14:58:10]

Its not redundant, but its needlessly clever.  I'll just change it to
surface_state_.get() != NULL; which is far more clear.
(Long answer: its not redundant because some compilers will warn on cast from
int types to bool, since the only technically valid values for bool are
true/false. Consider the case of non-0 int value but where the low order 8 bits
are 0..)
On 2012/02/02 14:03:53, jonathan.backer wrote:

[mmocny, 2012/02/02 14:58:10]

Done.

[mmocny, 2012/02/02 15:22:29]

Update: These functions are virtual and should not be inlined.
On 2012/02/02 14:58:10, mmocny wrote:

+}
+
+const GpuCommandBufferStubBase::SurfaceState&
+    GpuCommandBufferStub::surface_state() {
+  DCHECK(has_surface_state());
+  return *surface_state_.get();

[jonathan.backer, 2012/02/02 14:03:53]

Header?

[mmocny, 2012/02/02 14:58:10]

Done.

+}
+
+void GpuCommandBufferStub::SendMemoryAllocationToProxy(
+    const GpuMemoryAllocation& allocation) {
+  // TODO(mmocny): Send callback once gl extensions are added.
+}
+
 #endif  // defined(ENABLE_GPU)