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..1bf83224d7e3bc8083c8673183aa345b4b438fab 100644
--- a/content/common/gpu/gpu_command_buffer_stub.cc
+++ b/content/common/gpu/gpu_command_buffer_stub.cc
@@ -9,11 +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_messages.h"
+#include "content/common/gpu/gpu_memory_manager.h"
 #include "content/common/gpu/gpu_watchdog.h"
 #include "content/common/gpu/image_transport_surface.h"
 #include "gpu/command_buffer/common/constants.h"
@@ -43,6 +45,8 @@ GpuCommandBufferStub::GpuCommandBufferStub(
       route_id_(route_id),
       software_(software),
       last_flush_count_(0),
+      visible_(true),
+      last_used_time_(kUnknownLastUsedTime),
       surface_id_(surface_id),
       parent_stub_for_initialization_(),
       parent_texture_for_initialization_(0),
@@ -153,6 +157,8 @@ void GpuCommandBufferStub::Destroy() {
 
   context_ = NULL;
   surface_ = NULL;
+
+  channel_->gpu_channel_manager()->GetGpuMemoryManager()->Manage();
 }
 
 void GpuCommandBufferStub::OnInitializeFailed(IPC::Message* reply_message) {
@@ -274,6 +280,8 @@ void GpuCommandBufferStub::OnInitialize(
 
   GpuCommandBufferMsg_Initialize::WriteReplyParams(reply_message, true);
   Send(reply_message);
+
+  channel_->gpu_channel_manager()->GetGpuMemoryManager()->Manage();
 }
 
 void GpuCommandBufferStub::OnSetGetBuffer(
@@ -364,6 +372,7 @@ void GpuCommandBufferStub::OnAsyncFlush(int32 put_offset,
   }
 
   ReportState();
+  // TODO(mmocny): Call GpuMemoryManager::Manage here?
 }
 
 void GpuCommandBufferStub::OnRescheduled() {
@@ -496,6 +505,11 @@ void GpuCommandBufferStub::OnDestroyVideoDecoder(int decoder_route_id) {
 
 void GpuCommandBufferStub::OnSetSurfaceVisible(bool visible) {
   surface_->SetVisible(visible);
+  visible_ = visible;
+  base::Time now = base::Time::Now();
+  last_used_time_ = now.ToInternalValue();
+
+  channel_->gpu_channel_manager()->GetGpuMemoryManager()->Manage();

[nduca, 2012/01/27 10:10:13]

Not liking this. I would rather you modify the upper level plumbing to hit
manage() after every PutChanged, for instance. Or, add a method on
GpuMemoryManager saying ScheduleManage() that itself contains the logic about
when to manage.

[mmocny, 2012/01/27 19:51:33]

Added ScheduleManage.
On 2012/01/27 10:10:13, nduca wrote:

 }
 
 void GpuCommandBufferStub::SendConsoleMessage(
@@ -510,4 +524,24 @@ void GpuCommandBufferStub::SendConsoleMessage(
   Send(msg);
 }
 
+std::vector<int> GpuCommandBufferStub::render_widget_ids() const {
+  // TODO(mmocny): Hack alert, treat route_id as renderer_widget_id until
+  //               we actually feed real render_widget_ids through
+  return std::vector<int>(1,int(route_id_));

[nduca, 2012/01/27 10:10:13]

woah, what? You can wire this up already if you think about it. When you create
a stub, you set whether it is rendering to a render_view_ --- I think that if
thats nonzero, it should be immediately added to the affected_render_widget_ids_
vector. I made a similar comment about this in the chrome-side extensions patch.

[mmocny, 2012/01/27 19:51:33]

I cannot find any reference to render_view during stub creation.
This return value was used as a temporary placeholder during testing, I will
change it to return a reference to an empty vector.
On 2012/01/27 10:10:13, nduca wrote:

+}

[nduca, 2012/01/27 10:10:13]

Shouldn't you be using render_view_id, not route_id_? Is this why your merge
stuff is wonky?

+
+void GpuCommandBufferStub::SetMemoryAllocation(
+    GpuMemoryAllocation const& allocation) {
+  // TODO(mmocny): 1. Send callback once gl extensions are added
+  //               2. Move drop backbuffer hook from SetSurfaceVisibile to here

[nduca, 2012/01/27 10:10:13]

Woah, this will have side effects other than just sending? I think you should
blow this up into a Send which you should do right away.

2-4 should be a different function call on this class. its fine if the memory
manager makes two calls to this class. Its a huge win to make your functions do
clear and well defined things rather than have 4 side effects.

You'll remove these printfs right?

[mmocny, 2012/01/27 19:51:33]

You are correct on all points, this was leftover debugging stuff that should
have been removed
On 2012/01/27 10:10:13, nduca wrote:

+  //               3. Implement drop frontbuffer with hook here
+  //               4. Implement browser notification on drop frontbuffer
+#if 0
+  printf("   [%p] Allocation: %i, front: %s, back: %s\n", this,
+      allocation.gpuResourceSizeInBytes,
+      allocation.hasFrontbuffer ? "true" : "false",
+      allocation.hasBackbuffer ? "true" : "false");
+#endif
+}
+
 #endif  // defined(ENABLE_GPU)