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

content/common/gpu/gpu_command_buffer_stub.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_COMMON_GPU_GPU_COMMAND_BUFFER_STUB_H_
 #define CONTENT_COMMON_GPU_GPU_COMMAND_BUFFER_STUB_H_
 #pragma once
 
 #if defined(ENABLE_GPU)
 
 #include <string>
 #include <vector>
 
 #include "base/id_map.h"
 #include "base/memory/weak_ptr.h"
+#include "content/common/content_export.h"
 #include "content/common/gpu/media/gpu_video_decode_accelerator.h"
 #include "gpu/command_buffer/common/constants.h"
 #include "gpu/command_buffer/service/command_buffer_service.h"
 #include "gpu/command_buffer/service/context_group.h"
 #include "gpu/command_buffer/service/gpu_scheduler.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_message.h"
 #include "ui/gfx/gl/gl_context.h"
 #include "ui/gfx/gl/gl_surface.h"
 #include "ui/gfx/gl/gpu_preference.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/size.h"
 #include "ui/gfx/surface/transport_dib.h"
 
 #if defined(OS_MACOSX)
 #include "ui/gfx/surface/accelerated_surface_mac.h"
 #endif
 
 class GpuChannel;
+class GpuMemoryAllocation;
 class GpuWatchdog;
 
+// This Base class is used to expose methods of GpuCommandBufferStub used for
+// testability.
+class CONTENT_EXPORT GpuCommandBufferStubBase {
+ public:
+  struct CONTENT_EXPORT SurfaceState {
+    int32 surface_id;
+    bool visible;
+    base::TimeTicks last_used_time;
+
+    SurfaceState(int32 surface_id,
+                 bool visible,
+                 base::TimeTicks last_used_time);
+  };
+
+ public:
+  virtual ~GpuCommandBufferStubBase() {}
+
+  // Null if this is an offscreen commandbuffer.
+  virtual SurfaceState* surface_state() = 0;

[jonathan.backer, 2012/02/01 19:43:17]

Can the surface state be modified? maybe return a pointer to a const
SurfaceState?

[mmocny, 2012/02/01 20:44:19]

It is modified: last_used_time and visibility are changed.
I am not sure if it would be better to make getters and setters..
On 2012/02/01 19:43:17, jonathan.backer wrote:

[jonathan.backer, 2012/02/01 21:39:14]

Inside the Stub, we can use surface_state_ to modify. Is it modified outside the
Stub?

[mmocny, 2012/02/01 22:31:08]

Done.

+
+  // This stubs' surface_id should *not* be in the affected_surface_ids list.
+  virtual const std::vector<int32>& affected_surface_ids() = 0;
+
+  virtual void SendMemoryAllocationToProxy(
+      const GpuMemoryAllocation& allocation) = 0;

[jonathan.backer, 2012/02/01 19:43:17]

Not used in this CL. Nix?

[mmocny, 2012/02/01 20:44:19]

It is used by the Manage function.  Even though it is a no-op in the Stub at the
moment, it is not a no-op in the tests, and it is the only way to test the
Manage function.
On 2012/02/01 19:43:17, jonathan.backer wrote:

+};
+
 class GpuCommandBufferStub
     : public IPC::Channel::Listener,
       public IPC::Message::Sender,
+      public GpuCommandBufferStubBase,
       public base::SupportsWeakPtr<GpuCommandBufferStub> {
  public:
   GpuCommandBufferStub(
       GpuChannel* channel,
       GpuCommandBufferStub* share_group,
       gfx::PluginWindowHandle handle,
       const gfx::Size& size,
       const gpu::gles2::DisallowedFeatures& disallowed_features,
       const std::string& allowed_extensions,
       const std::vector<int32>& attribs,
       gfx::GpuPreference gpu_preference,
       int32 route_id,
       int32 surface_id,
       GpuWatchdog* watchdog,
       bool software);
 
   virtual ~GpuCommandBufferStub();
 
   // IPC::Channel::Listener implementation:
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // IPC::Message::Sender implementation:
   virtual bool Send(IPC::Message* msg) OVERRIDE;
 
+  // GpuCommandBufferStubBase implementation:
+  virtual GpuCommandBufferStubBase::SurfaceState* surface_state() OVERRIDE;
+
+  // The affected_surface_ids field refers to others stubs' surfaces.  This
+  // stubs' surface_id is found via surface state.
+  virtual const std::vector<int32>& affected_surface_ids() OVERRIDE;

[jonathan.backer, 2012/02/01 19:43:17]

Chatted with mmocny offline. It appears that this isn't necessary right now. So
he'll move it to a follow-on CL.

+
+  // Sends memory allocation limits to render process.
+  virtual void SendMemoryAllocationToProxy(
+      const GpuMemoryAllocation& allocation) OVERRIDE;
+
   // Whether this command buffer can currently handle IPC messages.
   bool IsScheduled();
 
   // Whether this command buffer needs to be polled again in the future.
   bool HasMoreWork();
 
   // Set the swap interval according to the command line.
   void SetSwapInterval();
 
   gpu::gles2::GLES2Decoder* decoder() const { return decoder_.get(); }
   gpu::GpuScheduler* scheduler() const { return scheduler_.get(); }
 
   // Identifies the target surface.
-  int32 surface_id() const { return surface_id_; }
+  int32 surface_id() const;

[jonathan.backer, 2012/02/01 19:43:17]

Why? Doesn't this prevent it from being inlined?

[mmocny, 2012/02/01 20:44:19]

It may prevent it from being inlined (linkers can do function inlining, and you
can add the inline keyword to a declaration to hint to the compiler).
Also, defining a function inside a class in a header strongly suggests inlining,
but it isn't guaranteed.
C++ details aside, I think the convention is to have only trivial
getters/setters defined in the header, and the current getter is slightly less
than trivial, imho.
On 2012/02/01 19:43:17, jonathan.backer wrote:

[jonathan.backer, 2012/02/01 21:39:14]

It's very trivial, IMHO.

[mmocny, 2012/02/01 22:31:08]

Done.

 
   // Identifies the various GpuCommandBufferStubs in the GPU process belonging
   // to the same renderer process.
   int32 route_id() const { return route_id_; }
 
   gfx::GpuPreference gpu_preference() { return gpu_preference_; }
 
   // Sends a message to the console.
   void SendConsoleMessage(int32 id, const std::string& message);
 
@@ skipping 46 matching lines @@
 
   gfx::PluginWindowHandle handle_;
   gfx::Size initial_size_;
   gpu::gles2::DisallowedFeatures disallowed_features_;
   std::string allowed_extensions_;
   std::vector<int32> requested_attribs_;
   gfx::GpuPreference gpu_preference_;
   int32 route_id_;
   bool software_;
   uint32 last_flush_count_;
-
-  // Identifies the window for the rendering results on the browser side.
-  int32 surface_id_;
+  scoped_ptr<GpuCommandBufferStubBase::SurfaceState> surface_state_;
+  std::vector<int32> affected_surface_ids_;
 
   scoped_ptr<gpu::CommandBufferService> command_buffer_;
   scoped_ptr<gpu::gles2::GLES2Decoder> decoder_;
   scoped_ptr<gpu::GpuScheduler> scheduler_;
   scoped_refptr<gfx::GLContext> context_;
   scoped_refptr<gfx::GLSurface> surface_;
 
   // SetParent may be called before Initialize, in which case we need to keep
   // around the parent stub, so that Initialize can set the parent correctly.
   base::WeakPtr<GpuCommandBufferStub> parent_stub_for_initialization_;
   uint32 parent_texture_for_initialization_;
 
   GpuWatchdog* watchdog_;
 
   // Zero or more video decoders owned by this stub, keyed by their
   // decoder_route_id.
   IDMap<GpuVideoDecodeAccelerator, IDMapOwnPointer> video_decoders_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuCommandBufferStub);
 };
 
 #endif  // defined(ENABLE_GPU)
 
 #endif  // CONTENT_COMMON_GPU_GPU_COMMAND_BUFFER_STUB_H_