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

content/common/gpu/gpu_memory_manager_unittest.cc

+// Copyright (c) 2011 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.
+
+#include "content/common/gpu/gpu_memory_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"

[nduca, 2012/01/31 06:53:47]

i think we put a space between the "class header file" (gpu_memory_manager) and
the other includes

[mmocny, 2012/01/31 18:54:57]

Done.

+
+////////////////////////////////////////////////////////////////////////////////

[nduca, 2012/01/31 06:53:47]

I dont see the /// pattern being used in the gpu codebase... its in other bits
of the browser, though. When in doubt, go with the style in your local
directory. Or if you feel strongly, ask @kbr or @apatrick

[mmocny, 2012/01/31 18:54:57]

I saw usage all over content/common..
I didn't realize we had micro standards in each subdirectory.
Someone has to introduce the trend, no?  @backer is also OWNER, so I'll ask his
opinion.

On 2012/01/31 06:53:47, nduca wrote:

+// Fake Overrides for testing

[nduca, 2012/01/31 06:53:47]

not sure this comment adds value

[mmocny, 2012/01/31 18:54:57]

Done.

+
+struct FakeCommandBufferStub : GpuMemoryManageableCommandBufferStub {

[nduca, 2012/01/31 06:53:47]

why struct?

[mmocny, 2012/01/31 18:54:57]

Done.

+  virtual const GpuSurfaceState& surface_state() {
+    return surface_state_;
+  }
+  virtual const std::vector<int32>& affected_surface_ids() {
+    return affected_surface_ids_;
+  }
+  virtual void SendMemoryAllocation(const GpuMemoryAllocation& alloc) {
+    allocation_ = alloc;
+  }
+
+  GpuSurfaceState surface_state_;
+  std::vector<int32> affected_surface_ids_;
+  GpuMemoryAllocation allocation_;
+};
+
+struct FakeClient : GpuMemoryManagerClient {

[nduca, 2012/01/31 06:53:47]

class

[mmocny, 2012/01/31 18:54:57]

Done.

+  virtual void AppendAllCommandBufferStubs(
+      std::vector<GpuMemoryManageableCommandBufferStub*>& stubs_with_surface,
+      std::vector<GpuMemoryManageableCommandBufferStub*>& stubs_without_surface)
+  {
+    stubs_with_surface.insert(stubs_with_surface.end(),
+                              stubs_with_surface_.begin(),
+                              stubs_with_surface_.end());
+    stubs_with_surface.insert(stubs_without_surface.end(),
+                              stubs_without_surface_.begin(),
+                              stubs_without_surface_.end());
+  }
+
+  std::vector<GpuMemoryManageableCommandBufferStub*> stubs_with_surface_;
+  std::vector<GpuMemoryManageableCommandBufferStub*> stubs_without_surface_;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Helpers
+
+FakeCommandBufferStub CreateFakeCommandBufferStubWithSurface() {
+  static int32 surface_id_ = 1;
+  FakeCommandBufferStub ret;

[nduca, 2012/01/31 06:53:47]

ret -> stub

[mmocny, 2012/01/31 18:54:57]

Done.

+  ret.surface_state_.surface_id = surface_id_++;
+  ret.surface_state_.visible = true;
+  ret.surface_state_.last_used_time = base::TimeTicks::Now();
+  return ret;
+}
+
+FakeCommandBufferStub CreateFakeCommandBufferStubWithoutSurface() {

[nduca, 2012/01/31 06:53:47]

you know, I'm thinking you might be better served by not having this helper
funciton.

Instead, just create the surface and set all its fields explicitly every time.

I find it easier to read test code this way... you'll do a little bit of typing
to get it written out but youll be able to read the test itself very very
clearly at that point.

[mmocny, 2012/01/31 18:54:57]

Done.

+  FakeCommandBufferStub ret;
+  ret.surface_state_.surface_id = 0;
+  ret.surface_state_.visible = true;
+  ret.surface_state_.last_used_time = base::TimeTicks::Now();
+  return ret;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+TEST(GpuMemoryManagerTest, BasicFunctionality) {
+  FakeClient fake_client;
+
+  FakeCommandBufferStub fake_stub_with_surface1 =

[nduca, 2012/01/31 06:53:47]

i think you're killing yourself with these names.

fake_client -> client
fake_stub_with_surface1 -> stub1, stub2, etc --- dont bother giving the names to
say whether they're with or without surface. If your setup is easy to read
[described earlier], you wont have to look hard to figure out what the different
stub numbers do.

Just to help the newcomer in reading a test, I usually put a comment above the
TEST(xxx) macro saying the objective of a test. E.g.
// Verifies that the Send method is called on all stubs.

You're gonna need more tests than this, I hope you know.

At minimum:

1. At least 3 or 4 tests on the comparator. In each individual test, set up
pairs of stubs and verify that the comparator orders them correcty. E.g. 
     FakeCommandBufferStub stub1();
     stub1.visible = true
     ...
     FakeCommndBufferStub stub2();
     stub2.visible = false;
     GpuMemoryManager::StubComparator cmp;
     EXPECT_TRUE(cmp(stub1,stub2) < 0);

[mmocny, 2012/01/31 18:54:57]

Working on it :)
On 2012/01/31 06:53:47, nduca wrote:

+      CreateFakeCommandBufferStubWithSurface();
+  FakeCommandBufferStub fake_stub_with_surface2 =
+      CreateFakeCommandBufferStubWithSurface();
+  fake_stub_with_surface1.surface_state_.visible = true;
+  fake_stub_with_surface2.surface_state_.visible = false;
+  fake_stub_with_surface2.surface_state_.last_used_time +=
+      base::TimeDelta::FromSeconds(1);
+
+  FakeCommandBufferStub fake_stub_without_surface1 =
+      CreateFakeCommandBufferStubWithoutSurface();
+  FakeCommandBufferStub fake_stub_without_surface2 =
+      CreateFakeCommandBufferStubWithoutSurface();
+  fake_stub_without_surface1.surface_state_.visible = false;
+  fake_stub_without_surface2.surface_state_.visible = false;
+  fake_stub_without_surface2.surface_state_.last_used_time -=
+      base::TimeDelta::FromSeconds(1);
+  fake_stub_without_surface1.affected_surface_ids_.push_back(
+      fake_stub_with_surface1.surface_state_.surface_id);
+  fake_stub_without_surface2.affected_surface_ids_.push_back(
+      fake_stub_with_surface1.surface_state_.surface_id);
+  fake_stub_without_surface2.affected_surface_ids_.push_back(
+      fake_stub_with_surface2.surface_state_.surface_id);
+
+  EXPECT_EQ(IsMoreImportant(&fake_stub_with_surface1,
+                            &fake_stub_with_surface2), true);
+  EXPECT_EQ(IsMoreImportant(&fake_stub_without_surface1,
+                            &fake_stub_without_surface2), true);
+
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface2);
+  fake_client.stubs_without_surface_.push_back(&fake_stub_without_surface1);
+  fake_client.stubs_without_surface_.push_back(&fake_stub_without_surface2);
+
+#if 0
+  GpuMemoryManager memory_manager(&fake_client);
+  memory_manager.Manage();
+
+  EXPECT_EQ(fake_stub_with_surface1.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_with_surface1.allocation_.hasBackbuffer, true);
+  EXPECT_EQ(fake_stub_with_surface2.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_with_surface2.allocation_.hasBackbuffer, false);
+  EXPECT_EQ(fake_stub_without_surface1.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_without_surface1.allocation_.hasBackbuffer, false);
+  EXPECT_EQ(fake_stub_without_surface2.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_without_surface2.allocation_.hasBackbuffer, true);
+
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+  fake_client.stubs_with_surface_.push_back(&fake_stub_with_surface1);
+
+  memory_manager.Manage();
+  EXPECT_EQ(fake_stub_with_surface1.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_with_surface1.allocation_.hasBackbuffer, true);
+  EXPECT_EQ(fake_stub_with_surface2.allocation_.hasFrontbuffer, false);
+  EXPECT_EQ(fake_stub_with_surface2.allocation_.hasBackbuffer, false);
+  EXPECT_EQ(fake_stub_without_surface1.allocation_.hasFrontbuffer, false);
+  EXPECT_EQ(fake_stub_without_surface1.allocation_.hasBackbuffer, false);
+  EXPECT_EQ(fake_stub_without_surface2.allocation_.hasFrontbuffer, true);
+  EXPECT_EQ(fake_stub_without_surface2.allocation_.hasBackbuffer, true);
+#endif
+}