Merge lp:~raof/mir/client-side-buffer-age into lp:~mir-team/mir/trunk

[Chris Halse Rogers]

Add an “age” field to MirBufferPackage

This is the number of frames submitted by the client since the buffer
has been rendered to; 0 indicates that the client has not rendered to it before.

This allows clients which have only small changes between frames to do
significantly less rendering, by only rendering the difference.

See also: http://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_buffer_age.txt

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:459
http://jenkins.qa.ubuntu.com/job/mir-ci/1143/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1143/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/1143//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:460
http://jenkins.qa.ubuntu.com/job/mir-ci/1146/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1146/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/1146//rebuild/?

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

On a quick scan the only thing I'd argue with is "set_age()" (because I hate set_xxx() member functions).

Rather than set_age(1)/set_age(age()+1) I'd prefer to see reset_age(), increment_age()

(Just commenting pending a closer review.)

[Alexandros Frantzis (afrantzis)]

Some notes on the loop starting at line 193:

The mir coding convention is to put curly braces on their on own line, and also, if one clause of an if statement uses braces, then so should all the others.

193 + for (auto buffer_pair :buffers) {

Is there a reason to copy the buffer_pair instead of taking a reference (auto&)?
A space is missing after ':'.

199 + buffers.erase(buffer_pair.first);

I am not sure if it is safe to delete an element while iterating with a range-based for loop. Unless we have a guarantee from the standard that this works, we should do something different (e.g. use iterators manually, save the keys to a vector for deletion after the loop)

204 + if (!buffers.empty())
205 + buffers[current_buffer]->set_age(1);

Is this needed? The loop should have reset the current buffer's age.

206 + auto find_it = buffers.find(id);
207 + if (find_it == buffers.end())

Since we don't need find_it later, we might as well use the more compact if (buffers.find(id) == buffers.end()).
(this is really minor, not a blocker).

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

[ RUN ] MirGBMBufferDepositoryTest.depository_forgets_old_buffers
==21586== Invalid read of size 4
==21586== at 0x4365E04: std::_Rb_tree_increment(std::_Rb_tree_node_base*) (in /usr/lib/i386-linux-gnu/libstdc++.so.6.0.17)
==21586== by 0x4058FBA: mir::client::gbm::GBMClientBufferDepository::deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage>&&, int, mir::geometry::Size, mir::geometry::PixelFormat) (stl_tree.h:188)
==21586== by 0x8276E2D: MirGBMBufferDepositoryTest_depository_forgets_old_buffers_Test::TestBody() (test_gbm_client_depository.cpp:226)
==21586== by 0x850407B: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2090)
==21586== by 0x84FB651: testing::Test::Run() (gtest.cc:2162)
==21586== by 0x84FB737: testing::TestInfo::Run() (gtest.cc:2338)
==21586== by 0x84FB886: testing::TestCase::Run() (gtest.cc:2445)
==21586== by 0x84FBB84: testing::internal::UnitTestImpl::RunAllTests() (gtest.cc:4243)
==21586== by 0x8503C5B: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (gtest.cc:2090)
==21586== by 0x84FABFB: testing::UnitTest::Run() (gtest.cc:3880)
==21586== by 0x4467934: (below main) (libc-start.c:260)
==21586== Address 0x49a209c is 12 bytes inside a block of size 28 free'd
==21586== at 0x402ACFC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==21586== by 0x405999F: std::_Rb_tree<unsigned int, std::pair<unsigned int const, std::shared_ptr<mir::client::gbm::GBMClientBuffer> >, std::_Select1st<std::pair<unsigned int const, std::shared_ptr<mir::client::gbm::GBMClientBuffer> > >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::shared_ptr<mir::client::gbm::GBMClientBuffer> > > >::_M_erase_aux(std::_Rb_tree_const_iterator<std::pair<unsigned int const, std::shared_ptr<mir::client::gbm::GBMClientBuffer> > >, std::_Rb_tree_const_iterator<std::pair<unsigned int const, std::shared_ptr<mir::client::gbm::GBMClientBuffer> > >) (new_allocator.h:98)
==21586==
==21586== Invalid read of size 4
==21586== at 0x4365E1B: std::_Rb_tree_increment(std::_Rb_tree_node_base*) (in /usr/lib/i386-linux-gnu/libstdc++.so.6.0.17)
==21586== by 0x4058FBA: mir::client::gbm::GBMClientBufferDepository::deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage>&&, int, mir::geometry::Size, mir::geometry::PixelFormat) (stl_tree.h:188)
==21586== by 0x8276E2D: MirGBMBufferDepositoryTest_depository_forgets_old_buffers_Test::TestBody() (test_gbm_client_depository.cpp:226)
==21586== by 0x850407B: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2090)
==21586== by 0x84FB651: testing::Test::Run() (gtest.cc:2162)
==21586== by 0x84FB737: testing::TestInfo::Run() (gtest.cc:2338)
==21586== by 0x84FB886: testing::TestCase::Run() (gtest.cc:2445)
==21586== by 0x84FBB84: testing::internal::UnitTestImpl::RunAllTests() (gtest....

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:463
http://jenkins.qa.ubuntu.com/job/mir-ci/5/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/5//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/5//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:464
http://jenkins.qa.ubuntu.com/job/mir-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/6//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/6//rebuild/?

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

It isn't obvious locally why:

200 + auto buffer_it = buffers.begin();
201 + while (buffer_it != buffers.end())
202 + {
203 + auto tmp_it = buffer_it;
204 + ++buffer_it;

isn't written as

    for (auto tmp_it : buffers)
    {

only when you read to:

213 + // C++ guarantees that buffers.erase() will only invalidate the iterator
214 + // we erase.
215 + buffers.erase(tmp_it);

Does it become apparent.

If we can't think of a way to write the code more clearly I think the comment should move to before

204 + ++buffer_it;

And I think "tmp_it" could be better named - e.g. "current"

~~~~

72 + std::shared_ptr<ClientBuffer> access_current_buffer(void);
144 +void mclg::GBMClientBuffer::increment_age(void)
150 +void mclg::GBMClientBuffer::mark_as_submitted(void)
169 + void increment_age(void);
170 + void mark_as_submitted(void);
...

s/void//

~~~~
591 +TEST_F(MirGBMBufferDepositoryTest, depository_forgets_old_buffers)
...
604 + // We've submitted 4 buffers now; we should have forgotten the first one.

This is just a naming issue, but I don't think the test proves anything about old buffers being "forgotten". (I think we'd need to take a weak pointer to the "old buffer" and test that it becomes null to prove "forgets".)

What it checks is that buffers that were seen four generations ago have age 0. (Which I accept is implemented by "forgetting" - but forgetting isn't the behaviour tested for.)

~~~~

246 void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> && package, int, geometry::Size size, geometry::PixelFormat pf);

I guess this MP didn't create this (so I wouldn't block on it). But WHY a rvalue-reference to a shared pointer?! A const ref would be idiomatic and work better. It would be nice to clean this up while changing this code.

review: Needs Fixing

[Chris Halse Rogers (raof)]

246 void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> && package, int, geometry::Size size, geometry::PixelFormat pf);

I guess this MP didn't create this (so I wouldn't block on it). But WHY a rvalue-reference to a shared pointer?! A const ref would be idiomatic and work better. It would be nice to clean this up while changing this code.
~~~~~~

Every time I look at that code I'm tempted to make deposit_package take a mir::protobuf::Buffer. The MirSurface does nothing with the m::p::Buffer but copy the values to a MirBufferPackage which it promptly hands off to the depository and forgets.

That's probably more controversial, though.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/mir-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/19//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/19//rebuild/?

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

116 + virtual std::shared_ptr<ClientBuffer> access_current_buffer() = 0;

This could might as well be named 'current_buffer()'.

[Alan Griffiths (alan-griffiths)]

593 +TEST_F(MirGBMBufferDepositoryTest, depository_forgets_old_buffers)
etc.

Hmm, I guess I shouldn't have mentioned this trick for probing the implementation. :(

Tests must check the required behaviour not the implementation. The previous version did test the right thing (it just had a bad name).

~~~~

PS I agree with Alexandros about s/access_current_buffer/current_buffer/ (the member variable can have another name).

PPS I had a go at writing a cleaner version of GBMClientBufferDepository::deposit_package - you can see if you like it here:

    https://code.launchpad.net/~alan-griffiths/mir/client-side-buffer-age/+merge/152392

review: Needs Fixing

[Chris Halse Rogers (raof)]

593 +TEST_F(MirGBMBufferDepositoryTest, depository_forgets_old_buffers)

This now tests exactly what I wanted it to test in the first place - that the depository does not keep old buffers around, and destroys them.

GBMClientBuffer is about to grow a non-trivial destructor, and I need to test that we're cleaning up old buffers - the GBM platform could easily send thousands of unique buffers to a client, each of which will have an associated fd, so without destroying old buffers we'll rapidly exhaust the client's fd space.

Although it's not API behaviour, “doesn't consume resources without limit” seems like testable behaviour to me - perhaps the test should be renamed to depository_destroys_old_buffers?

You've got more TTD experience than me; where do you believe this test should reside?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:468
http://jenkins.qa.ubuntu.com/job/mir-ci/32/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/32//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/32//rebuild/?

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> 593 +TEST_F(MirGBMBufferDepositoryTest, depository_forgets_old_buffers)
>
> This now tests exactly what I wanted it to test in the first place - that the
> depository does not keep old buffers around, and destroys them.
>
> GBMClientBuffer is about to grow a non-trivial destructor, and I need to test
> that we're cleaning up old buffers - the GBM platform could easily send
> thousands of unique buffers to a client, each of which will have an associated
> fd, so without destroying old buffers we'll rapidly exhaust the client's fd
> space.
>
> Although it's not API behaviour, “doesn't consume resources without limit”
> seems like testable behaviour to me - perhaps the test should be renamed to
> depository_destroys_old_buffers?
>
> You've got more TTD experience than me; where do you believe this test should
> reside?

OK, I understand that concern.

I'm not sure of the "right answer" here so I'll dump some stream of consciousness (this is where conversation would have saved days of reviews)...

TDD orthodoxy says that if you can't test what you want through the API then you can (and should) improve the design to make that requirement explicit.

I guess the problem arises because GBMClientBufferDepository creates the GBMClientBuffer objects directly - so there's no "seam" here to inject a mock with an instrumented destructor. Supplying the constructor with a GBMClientBufferFactory (that could be a std::function<>) is one option that adds a seam for this - another is to template GBMClientBufferDepository on the buffer type.

I don't know GBM or Android well enough to be sure, but, having thought through the above, I've a feeling that ClientBufferDepository *ought* to be pretty similar between GBM and Android (and just the buffer creation depend on the platform) - so maybe that is the right answer.

[Alan Griffiths (alan-griffiths)]

Re performance comments in the other MP:

My experience suggests that we'd get a bigger performance improvement by using an array (and liner search) instead of a std::map rather than by avoiding multiple loops. But measurement is the only true way to tell.

[Chris Halse Rogers (raof)]

Ok. This is untested on Android, as the build fails outside my changes for me.

I suspect that the age()/increment_age()/mark_as_submitted() might want to head up to the ClientBuffer, but haven't done that for this round.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:482
http://jenkins.qa.ubuntu.com/job/mir-ci/109/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/110/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/110/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/109/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

> Ok. This is untested on Android, as the build fails outside my changes for
> me.
>
> I suspect that the age()/increment_age()/mark_as_submitted() might want to
> head up to the ClientBuffer, but haven't done that for this round.

yes, looks like the android compile for this branch needs some dusting up. lp:mir (515) looks ok to me. I'm still trying to make the android cross compile easier to use with better scripts (lp:~mir-team/mir/consolidate-crossbuild-scripts), I can help set up your system (although i have limited bandwidth)

review: Needs Fixing

[Robert Carr (robertcarr)]

Overall the implementation appears to be semantically correct. However android build is definitely broken, at the least it looks like there is truncation at l236

review: Needs Fixing

[Robert Carr (robertcarr)]

What is going on around line 1347? Why the usage of this "_rv" function rather than mock create buffer directly?

review: Needs Information

[Robert Carr (robertcarr)]

Why the separate implementations of buffer age for android and GBM? Notably GBM refuses to increment the age if age == 0 while android happily increments?

review: Needs Information

[Robert Carr (robertcarr)]

Numerous whitespace errors
l284, l38, l1685, l1799, l1815 (examples). Might be good to use make style_check and grep for files modified in diff.

review: Needs Fixing

[Robert Carr (robertcarr)]

Some random musings:

* The tests around 1464 feel really verbose but I am not sure of a more concise set of tests. Perhaps it is best!

* BufferDepository seems to make sense for the interface the receiver (or rather the surface) uses to deposit buffers, but doesn't tell you much that it have this age tracking role. Perhaps the implementation should be renamed, BufferCollection, BufferBundle?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:484
http://jenkins.qa.ubuntu.com/job/mir-ci/140/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/52/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/141/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/141/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/140/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Enough nits to add up to a "Needs Fixing"

398 + std::shared_ptr<ClientBufferFactory> create_buffer_factory ();

Nit: s/y /y/

~~~~

859 +#include <stdexcept>
860 +#include <memory>

Not needed. (One not used, the other has to be included by header for signatures there.)

~~~~

905 +#include <stdexcept>
906 +#include <map>

Not needed.

~~~~

993 + for (auto it = release_wait_handles.begin(); it != release_wait_handles.end(); it++)
994 delete *it;

for (auto handle : release_wait_handles) delete handle;

~~~~

1651 + MockBuffer *first_buffer = reinterpret_cast<MockBuffer *>(depository.current_buffer().get());

Doesn't need "reinterpret_cast" (which should only be used as a last, non-portable, resort). Use static_cast - or, probably better, access the MockBuffer via MockClientBufferFactory member.

~~~~

2055 +#include <stdexcept>
2056 +

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

Re: <RAOF> What do you think of pulling the age() related bits into ClientBuffer; the implementation shouldn't be different between AndroidClientBuffer and GBMClientBuffer.

I think ClientBuffer should remain an interface (it is easier to mock).

But there is no reason that the age related bits couldn't have a common implementation inherited by AndroidClientBuffer and GBMClientBuffer.

There are several options, the simplest is:

class AgingClientBuffer : public ClientBuffer { ...age stuff... };
class AndroidClientBuffer : public AgingClientBuffer { ...non-age stuff... };
class GBMClientBuffer : public AgingClientBuffer { ...non-age stuff... };

But it is also possible to split out an AgingBuffer interface.

[Alexandros Frantzis (afrantzis)]

Now that ClientBufferDepository is a concrete class, injecting it into MirSurface is much less useful. I'd rather we injected the BufferFactory directly, and make the ClientBufferDepository an implementation detail. This direction is also supported by the MirSurface tests, where we currently need to mock an indirect dependency of the class under test. This approach presumes that we don't need to share depositories between surfaces, which seems to be the case currently.

Concerning making ClientBufferDepository concrete in the first place, I think it's a valid option as long as we are still able to perform the tests we want. The MirSurface unit-tests need some more tests (e.g. MirSurface.get_buffer_returns_last_received_buffer), so perhaps we are not yet at a position to answer this.

[Chris Halse Rogers (raof)]

I think this addresses all the review concerns. Perhaps we can defer further refactoring to a later MP? :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:492
http://jenkins.qa.ubuntu.com/job/mir-ci/154/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/69
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/155
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/155/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/154/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

628 + std::shared_ptr<ClientBufferFactory> factory;
          std::shared_ptr<ClientBufferFactory> const factory;

(dependencies should be const)

~~~~

AgingBuffer - the implicit constructor default initializes buffer_age - which for a POD is an unspecified value. There should be a constructor that initializes buffer_age to 0.

~~~~

struct MockAgingBuffer : public mcl::AgingBuffer

This doesn't mock AgingBuffer.

~~~~

1209 + auto mock_buffer = std::make_shared<NiceMock<MockAgingBuffer>>();
1210 +
1211 + EXPECT_EQ(0u, mock_buffer->age());

This reads really strangely because it appears you're testing the mock age() function (c.f. previous comment).

Rewriting the test as:

TEST(MirClientAgingBufferTest, buffer_age_starts_at_zero)
{
    using namespace testing;

    struct MyAgingBuffer : mcl::AgingBuffer
    {
        std::shared_ptr<mcl::MemoryRegion> secure_for_cpu_write() { exit(1); }
        geom::Size size() const { exit(1); }
        geom::Stride stride() const { exit(1); }
        geom::PixelFormat pixel_format() const { exit(1); }
        std::shared_ptr<mir_toolkit::MirBufferPackage> get_buffer_package() const { exit(1); }
        MirNativeBuffer get_native_handle() { exit(1); }
    } test_buffer;

    EXPECT_EQ(0u, test_buffer.age());
}

is less misleading and also exposes the failure to initialize buffer_age (unfortunately make_shared seems to hide it).

PS it almost seems worth either defining an "AgingBuffer interface" (or providing stub implementations in AgingBuffer) to avoid the need to stub the unimplemented methods in the tests.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:494
http://jenkins.qa.ubuntu.com/job/mir-ci/167/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/87
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/168
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/168/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/167/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Style nitpick: there are a lot of instances of "const &" instead of our preferred "const&". I don't want to block the branch for this (unless something else comes up, too), but we should fix these in style cleanup branch.

review: Approve

[Alexandros Frantzis (afrantzis)]

Note that I still stand by my previous comment "Now that ClientBufferDepository is a concrete class, injecting it into MirSurface is much less useful", but let's discuss further and amend in a future MP if needed.

[Alan Griffiths (alan-griffiths)]

> Note that I still stand by my previous comment "Now that
> ClientBufferDepository is a concrete class, injecting it into MirSurface is
> much less useful", but let's discuss further and amend in a future MP if
> needed.

Agreed.

Another nit:

1016 + std::shared_ptr<ClientBufferFactory> create_buffer_factory ()

s/y /y/

review: Approve

[Kevin DuBois (kdub)]

android clients work... algorithm seems good to me from the android perspective. overall, seems good

src/client/client_buffer_factory.h
virtual destructor, probably can disallow copy/assign

1205 (MyAgingBuffer)
although this code is not hit, exit(-1) will exit the test executable, we should assert in the test so we can continue the rest of the tests

also see some "//" for multiline, we've been preferring

Anyways, I think that this is good enough to go in, so I'll switch to 'abstain' so as to unblock this for the Australian monday work day

review: Abstain

[Chris Halse Rogers (raof)]

Marking as approved as per comments.

I'll post a follow-up merge proposal addressing the remaining niggles.

=== modified file 'include/shared/mir_toolkit/c_types.h'
--- include/shared/mir_toolkit/c_types.h	2013-03-21 17:00:06 +0000
+++ include/shared/mir_toolkit/c_types.h	2013-03-24 23:57:21 +0000
@@ -123,6 +123,9 @@
     int fd[mir_buffer_package_max];
 
     int stride;
+    int age; /**< Number of frames submitted by the client since the client has rendered to this buffer. */
+             /**< This has the same semantics as the EGL_EXT_buffer_age extension */
+             /**< \see http://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_buffer_age.txt */
 } MirBufferPackage;
 
 /**

=== modified file 'src/client/CMakeLists.txt'
--- src/client/CMakeLists.txt	2013-03-22 10:34:16 +0000
+++ src/client/CMakeLists.txt	2013-03-24 23:57:21 +0000
@@ -27,6 +27,8 @@
 set(
   CLIENT_SOURCES
 
+  aging_buffer.cpp
+  client_buffer_depository.cpp
   mir_client_library.cpp
   mir_connection.cpp
   mir_wait_handle.cpp
@@ -54,7 +56,7 @@
   CLIENT_SOURCES
 
   android/android_client_buffer.cpp
-  android/android_client_buffer_depository.cpp
+  android/android_client_buffer_factory.cpp
   android/android_registrar.cpp
   android/android_client_platform.cpp
   android/mir_native_window.cpp
@@ -70,7 +72,7 @@
   CLIENT_SOURCES
 
   gbm/gbm_client_platform.cpp
-  gbm/gbm_client_buffer_depository.cpp
+  gbm/gbm_client_buffer_factory.cpp
   gbm/gbm_client_buffer.cpp
   gbm/mesa_native_display_container.cpp
   ${CLIENT_SOURCES}

=== added file 'src/client/aging_buffer.cpp'
--- src/client/aging_buffer.cpp	1970-01-01 00:00:00 +0000
+++ src/client/aging_buffer.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,42 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "aging_buffer.h"
+
+namespace mcl = mir::client;
+
+mcl::AgingBuffer::AgingBuffer()
+    : buffer_age(0)
+{
+}
+
+uint32_t mcl::AgingBuffer::age() const
+{
+    return buffer_age;
+}
+
+void mcl::AgingBuffer::increment_age()
+{
+    if (buffer_age != 0)
+        ++buffer_age;
+}
+
+void mcl::AgingBuffer::mark_as_submitted()
+{
+    buffer_age = 1;
+}

=== added file 'src/client/aging_buffer.h'
--- src/client/aging_buffer.h	1970-01-01 00:00:00 +0000
+++ src/client/aging_buffer.h	2013-03-24 23:57:21 +0000
@@ -0,0 +1,45 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_AGING_BUFFER_H_
+#define MIR_CLIENT_AGING_BUFFER_H_
+
+#include "client_buffer.h"
+
+namespace mir
+{
+namespace client
+{
+
+class AgingBuffer : public ClientBuffer
+{
+public:
+    AgingBuffer();
+    virtual uint32_t age() const;
+    virtual void increment_age();
+    virtual void mark_as_submitted();
+
+private:
+    uint32_t buffer_age;
+};
+
+}
+}
+
+#endif /* MIR_CLIENT_AGING_BUFFER_H_ */
+

=== modified file 'src/client/android/android_client_buffer.cpp'
--- src/client/android/android_client_buffer.cpp	2013-03-22 10:34:16 +0000
+++ src/client/android/android_client_buffer.cpp	2013-03-24 23:57:21 +0000
@@ -64,19 +64,19 @@
 {
     int native_handle_header_size = sizeof(native_handle_t);
     int total = package->fd_items + package->data_items + native_handle_header_size;
-    native_handle_t* handle = (native_handle_t*) malloc(sizeof(int) * total );
+    native_handle_t* handle = (native_handle_t*) malloc(sizeof(int) * total);
 
     handle->version = native_handle_header_size;
     handle->numFds  = package->fd_items;
     handle->numInts = package->data_items;
 
-    for(auto i=0; i< handle->numFds; i++)
+    for (auto i=0; i< handle->numFds; i++)
     {
         handle->data[i] = package->fd[i];
     }
 
     int offset_i = handle->numFds;
-    for(auto i=0; i< handle->numInts; i++)
+    for (auto i=0; i< handle->numInts; i++)
     {
         handle->data[offset_i+i] = package->data[i];
     }

=== modified file 'src/client/android/android_client_buffer.h'
--- src/client/android/android_client_buffer.h	2013-03-22 10:34:16 +0000
+++ src/client/android/android_client_buffer.h	2013-03-24 23:57:21 +0000
@@ -21,7 +21,7 @@
 #define MIR_CLIENT_ANDROID_ANDROID_CLIENT_BUFFER_H_
 
 #include "mir_toolkit/mir_client_library.h"
-#include "../client_buffer.h"
+#include "../aging_buffer.h"
 #include "android_registrar.h"
 
 #include <system/window.h>
@@ -34,12 +34,12 @@
 namespace android
 {
 
-class AndroidClientBuffer : public ClientBuffer
+class AndroidClientBuffer : public AgingBuffer
 {
 public:
     AndroidClientBuffer(std::shared_ptr<AndroidRegistrar> const&,
                         std::shared_ptr<MirBufferPackage> const&,
-                        geometry::Size size, geometry::PixelFormat pf );
+                        geometry::Size size, geometry::PixelFormat pf);
     ~AndroidClientBuffer();
 
     std::shared_ptr<MemoryRegion> secure_for_cpu_write();

=== removed file 'src/client/android/android_client_buffer_depository.cpp'
--- src/client/android/android_client_buffer_depository.cpp	2013-03-13 04:54:15 +0000
+++ src/client/android/android_client_buffer_depository.cpp	1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by:
- *   Kevin DuBois <[e-mail]>
- */
-
-#include "android_client_buffer_depository.h"
-#include "android_client_buffer.h"
-
-#include <boost/throw_exception.hpp>
-
-namespace mcl=mir::client;
-namespace mcla=mir::client::android;
-namespace geom=mir::geometry;
-
-mcla::AndroidClientBufferDepository::AndroidClientBufferDepository(const std::shared_ptr<AndroidRegistrar>& buffer_registrar)
- : registrar(buffer_registrar)
-{
-}
-
-void mcla::AndroidClientBufferDepository::deposit_package(std::shared_ptr<MirBufferPackage>&& package, int id, geom::Size size, geom::PixelFormat pf)
-{
-    auto find_it = buffer_depository.find(id);
-    if (find_it == buffer_depository.end())
-    {
-        auto buffer = std::make_shared<mcla::AndroidClientBuffer>(registrar, std::move(package), size, pf);
-        buffer_depository[id] = buffer;
-    }
-}
-
-std::shared_ptr<mcl::ClientBuffer> mcla::AndroidClientBufferDepository::access_buffer(int id)
-{
-    auto find_it = buffer_depository.find(id);
-    if (find_it == buffer_depository.end())
-        BOOST_THROW_EXCEPTION(std::runtime_error("server told client to use buffer before"));
-
-    return buffer_depository[id];
-}

=== removed file 'src/client/android/android_client_buffer_depository.h'
--- src/client/android/android_client_buffer_depository.h	2013-03-13 04:54:15 +0000
+++ src/client/android/android_client_buffer_depository.h	1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by:
- *   Kevin DuBois <[e-mail]>
- */
-
-#ifndef MIR_CLIENT_ANDROID_ANDROID_CLIENT_BUFFER_DEPOSITORY_H_
-#define MIR_CLIENT_ANDROID_ANDROID_CLIENT_BUFFER_DEPOSITORY_H_
-
-#include "../client_buffer_depository.h"
-
-#include <stdexcept>
-#include <map>
-
-namespace mir
-{
-namespace client
-{
-class ClientBuffer;
-
-namespace android
-{
-class AndroidRegistrar;
-
-class AndroidClientBufferDepository : public ClientBufferDepository
-{
-public:
-    AndroidClientBufferDepository(const std::shared_ptr<AndroidRegistrar>&);
-
-    void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> && package, int, geometry::Size sz, geometry::PixelFormat pf);
-
-    std::shared_ptr<ClientBuffer> access_buffer(int id);
-private:
-    std::shared_ptr<AndroidRegistrar> registrar;
-
-    std::map<int, std::shared_ptr<mir::client::ClientBuffer>> buffer_depository;
-};
-
-}
-}
-}
-#endif /* MIR_CLIENT_ANDROID_ANDROID_CLIENT_BUFFER_DEPOSITORY_H_ */

=== added file 'src/client/android/android_client_buffer_factory.cpp'
--- src/client/android/android_client_buffer_factory.cpp	1970-01-01 00:00:00 +0000
+++ src/client/android/android_client_buffer_factory.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,35 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "android_client_buffer_factory.h"
+#include "android_client_buffer.h"
+
+namespace mcl=mir::client;
+namespace mcla=mir::client::android;
+namespace geom=mir::geometry;
+
+mcla::AndroidClientBufferFactory::AndroidClientBufferFactory(std::shared_ptr<AndroidRegistrar> const &buffer_registrar)
+ : registrar(buffer_registrar)
+{
+}
+
+std::shared_ptr<mcl::ClientBuffer> mcla::AndroidClientBufferFactory::create_buffer(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package, geom::Size size, geom::PixelFormat pf)
+{
+    return std::make_shared<mcla::AndroidClientBuffer>(registrar, package, size, pf);
+}

=== added file 'src/client/android/android_client_buffer_factory.h'
--- src/client/android/android_client_buffer_factory.h	1970-01-01 00:00:00 +0000
+++ src/client/android/android_client_buffer_factory.h	2013-03-24 23:57:21 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_ANDROID_ANDROID_BUFFER_FACTORY_H_
+#define MIR_CLIENT_ANDROID_ANDROID_BUFFER_FACTORY_H_
+
+#include <memory>
+
+#include "mir_toolkit/mir_client_library.h"
+#include "mir/geometry/pixel_format.h"
+#include "mir/geometry/size.h"
+
+#include "../client_buffer_factory.h"
+
+namespace mir
+{
+namespace client
+{
+class ClientBuffer;
+
+namespace android
+{
+class AndroidRegistrar;
+
+class AndroidClientBufferFactory : public ClientBufferFactory
+{
+public:
+    explicit AndroidClientBufferFactory(std::shared_ptr<AndroidRegistrar> const &);
+
+    virtual std::shared_ptr<ClientBuffer> create_buffer(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package,
+                                                        geometry::Size size, geometry::PixelFormat pf);
+private:
+    std::shared_ptr<AndroidRegistrar> registrar;
+};
+
+}
+}
+}
+#endif /* MIR_CLIENT_ANDROID_ANDROID_BUFFER_FACTORY_H_ */

=== modified file 'src/client/android/android_client_platform.cpp'
--- src/client/android/android_client_platform.cpp	2013-03-13 04:54:15 +0000
+++ src/client/android/android_client_platform.cpp	2013-03-24 23:57:21 +0000
@@ -19,7 +19,7 @@
 #include "mir_native_window.h"
 #include "android_client_platform.h"
 #include "android_registrar_gralloc.h"
-#include "android_client_buffer_depository.h"
+#include "android_client_buffer_factory.h"
 #include "client_surface_interpreter.h"
 #include "../mir_connection.h"
 #include "../native_client_platform_factory.h"
@@ -36,7 +36,7 @@
 
 struct EmptyDeleter
 {
-    void operator()(void* )
+    void operator()(void*)
     {
     }
 };
@@ -49,7 +49,7 @@
     return std::make_shared<mcla::AndroidClientPlatform>();
 }
 
-std::shared_ptr<mcl::ClientBufferDepository> mcla::AndroidClientPlatform::create_platform_depository()
+std::shared_ptr<mcl::ClientBufferFactory> mcla::AndroidClientPlatform::create_buffer_factory()
 {
     const hw_module_t *hw_module;
     int error = hw_get_module(GRALLOC_HARDWARE_MODULE_ID, &hw_module);
@@ -63,7 +63,7 @@
     EmptyDeleter empty_del;
     auto gralloc_dev = std::shared_ptr<gralloc_module_t>(gr_dev, empty_del);
     auto registrar = std::make_shared<mcla::AndroidRegistrarGralloc>(gralloc_dev);
-    return std::make_shared<mcla::AndroidClientBufferDepository>(registrar);
+    return std::make_shared<mcla::AndroidClientBufferFactory>(registrar);
 }
 
 namespace
@@ -73,7 +73,7 @@
     MirNativeWindowDeleter(mcla::MirNativeWindow* window)
      : window(window) {}
 
-    void operator()(EGLNativeWindowType* type )
+    void operator()(EGLNativeWindowType* type)
     {
         delete type;
         delete window;

=== modified file 'src/client/android/android_client_platform.h'
--- src/client/android/android_client_platform.h	2013-03-13 04:54:15 +0000
+++ src/client/android/android_client_platform.h	2013-03-24 23:57:21 +0000
@@ -32,7 +32,7 @@
 class AndroidClientPlatform : public ClientPlatform
 {
 public:
-    std::shared_ptr<ClientBufferDepository> create_platform_depository ();
+    std::shared_ptr<ClientBufferFactory> create_buffer_factory();
     std::shared_ptr<EGLNativeWindowType> create_egl_native_window(ClientSurface *surface);
     std::shared_ptr<EGLNativeDisplayType> create_egl_native_display();
 };

=== modified file 'src/client/client_buffer.h'
--- src/client/client_buffer.h	2013-03-21 03:32:59 +0000
+++ src/client/client_buffer.h	2013-03-24 23:57:21 +0000
@@ -53,6 +53,9 @@
     virtual geometry::Size size() const = 0;
     virtual geometry::Stride stride() const = 0;
     virtual geometry::PixelFormat pixel_format() const = 0;
+    virtual uint32_t age() const = 0;
+    virtual void increment_age() = 0;
+    virtual void mark_as_submitted() = 0;
 
     virtual MirNativeBuffer get_native_handle() = 0;
     virtual std::shared_ptr<mir_toolkit::MirBufferPackage> get_buffer_package() const = 0;

=== added file 'src/client/client_buffer_depository.cpp'
--- src/client/client_buffer_depository.cpp	1970-01-01 00:00:00 +0000
+++ src/client/client_buffer_depository.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,77 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "client_buffer.h"
+#include "client_buffer_factory.h"
+#include "client_buffer_depository.h"
+
+#include <stdexcept>
+#include <memory>
+#include <map>
+
+namespace mcl=mir::client;
+
+mcl::ClientBufferDepository::ClientBufferDepository(std::shared_ptr<ClientBufferFactory> const &factory, int max_buffers)
+    : factory(factory),
+      current_buffer_iter(buffers.end()),
+      max_age(max_buffers - 1)
+{
+}
+
+void mcl::ClientBufferDepository::deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package, int id, geometry::Size size, geometry::PixelFormat pf)
+{
+    for (auto next = buffers.begin(); next != buffers.end();)
+    {
+        // C++ guarantees that buffers.erase() will only invalidate the iterator we
+        // erase. Move to the next buffer before we potentially invalidate our iterator.
+        auto current = next;
+        next++;
+
+        if (current != current_buffer_iter &&
+            current->first != id &&
+            current->second->age() >= max_age)
+        {
+            buffers.erase(current);
+        }
+    }
+
+    for (auto& current : buffers)
+    {
+        current.second->increment_age();
+    }
+
+    if (current_buffer_iter != buffers.end())
+    {
+        current_buffer_iter->second->mark_as_submitted();
+    }
+
+    current_buffer_iter = buffers.find(id);
+
+    if (current_buffer_iter == buffers.end())
+    {
+        auto new_buffer = factory->create_buffer(package, size, pf);
+
+        current_buffer_iter = buffers.insert(current_buffer_iter, std::make_pair(id, new_buffer));
+    }
+}
+
+std::shared_ptr<mcl::ClientBuffer> mcl::ClientBufferDepository::current_buffer()
+{
+    return current_buffer_iter->second;
+}

=== modified file 'src/client/client_buffer_depository.h'
--- src/client/client_buffer_depository.h	2013-03-21 03:32:59 +0000
+++ src/client/client_buffer_depository.h	2013-03-24 23:57:21 +0000
@@ -1,9 +1,9 @@
 /*
  * Copyright © 2012 Canonical Ltd.
  *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License version 3,
- * as published by the Free Software Foundation.
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -20,7 +20,9 @@
 #ifndef MIR_CLIENT_PRIVATE_MIR_CLIENT_BUFFER_DEPOSITORY_H_
 #define MIR_CLIENT_PRIVATE_MIR_CLIENT_BUFFER_DEPOSITORY_H_
 
+#include <map>
 #include <memory>
+
 #include "mir/geometry/pixel_format.h"
 #include "mir/geometry/size.h"
 
@@ -28,19 +30,39 @@
 {
 struct MirBufferPackage;
 }
+
 namespace mir
 {
 
 namespace client
 {
 class ClientBuffer;
-
+class ClientBufferFactory;
+
+/// Responsible for taking the buffer data sent from the server and wrapping it in a ClientBuffer
+
+/// The ClientBufferDepository is responsible for taking the IPC buffer data and converting it into
+/// the more digestible form of a ClientBuffer. It maintains the client-side state necessary to
+/// construct a ClientBuffer.
 class ClientBufferDepository
 {
 public:
-    virtual void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> &&, int id,
-                                geometry::Size, geometry::PixelFormat) = 0;
-    virtual std::shared_ptr<ClientBuffer> access_buffer(int id) = 0;
+    ClientBufferDepository(std::shared_ptr<ClientBufferFactory> const &factory, int max_buffers);
+
+    /// Construct a ClientBuffer from the IPC data, and use it as the current buffer.
+
+    /// This also marks the previous current buffer (if any) as being submitted to the server.
+    /// \post current_buffer() will return a ClientBuffer constructed from this IPC data.
+    void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> const &, int id,
+                         geometry::Size, geometry::PixelFormat);
+    std::shared_ptr<ClientBuffer> current_buffer();
+
+private:
+    std::shared_ptr<ClientBufferFactory> const factory;
+    typedef std::map<int, std::shared_ptr<ClientBuffer>> BufferMap;
+    BufferMap buffers;
+    BufferMap::iterator current_buffer_iter;
+    const unsigned max_age;
 };
 }
 }

=== added file 'src/client/client_buffer_factory.h'
--- src/client/client_buffer_factory.h	1970-01-01 00:00:00 +0000
+++ src/client/client_buffer_factory.h	2013-03-24 23:57:21 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_CLIENT_BUFFER_FACTORY_H_
+#define MIR_CLIENT_CLIENT_BUFFER_FACTORY_H_
+
+#include <memory>
+
+#include "mir_toolkit/mir_client_library.h"
+#include "mir/geometry/pixel_format.h"
+#include "mir/geometry/size.h"
+
+namespace mir
+{
+namespace client
+{
+class ClientBuffer;
+
+class ClientBufferFactory
+{
+public:
+    virtual std::shared_ptr<ClientBuffer> create_buffer(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package,
+                                                        geometry::Size size, geometry::PixelFormat pf) = 0;
+};
+
+}
+}
+#endif /* MIR_CLIENT_CLIENT_BUFFER_FACTORY_H_ */

=== modified file 'src/client/client_platform.h'
--- src/client/client_platform.h	2013-03-13 04:54:15 +0000
+++ src/client/client_platform.h	2013-03-24 23:57:21 +0000
@@ -25,7 +25,7 @@
 {
 namespace client
 {
-class ClientBufferDepository;
+class ClientBufferFactory;
 class ClientSurface;
 class ClientContext;
 
@@ -36,7 +36,7 @@
     ClientPlatform(const ClientPlatform& p) = delete;
     ClientPlatform& operator=(const ClientPlatform& p) = delete;
 
-    virtual std::shared_ptr<ClientBufferDepository> create_platform_depository () = 0;
+    virtual std::shared_ptr<ClientBufferFactory> create_buffer_factory () = 0;
     virtual std::shared_ptr<EGLNativeWindowType> create_egl_native_window(ClientSurface *surface) = 0;
     virtual std::shared_ptr<EGLNativeDisplayType> create_egl_native_display() = 0;
 };

=== modified file 'src/client/gbm/gbm_client_buffer.cpp'
--- src/client/gbm/gbm_client_buffer.cpp	2013-03-22 10:34:16 +0000
+++ src/client/gbm/gbm_client_buffer.cpp	2013-03-24 23:57:21 +0000
@@ -128,11 +128,11 @@
 
 mclg::GBMClientBuffer::GBMClientBuffer(
         std::shared_ptr<mclg::DRMFDHandler> const& drm_fd_handler,
-        std::shared_ptr<mir_toolkit::MirBufferPackage> && package,
+        std::shared_ptr<mir_toolkit::MirBufferPackage> const& package,
         geom::Size size, geom::PixelFormat pf)
     : drm_fd_handler{drm_fd_handler},
       creation_package(std::move(package)),
-      rect({{geom::X(0),geom::Y(0)}, size}),
+      rect({{geom::X(0), geom::Y(0)}, size}),
       buffer_pf{pf}
 {
 }
@@ -165,6 +165,7 @@
 
 std::shared_ptr<mir_toolkit::MirBufferPackage> mclg::GBMClientBuffer::get_buffer_package() const
 {
+    creation_package->age = age();
     return creation_package;
 }
 

=== modified file 'src/client/gbm/gbm_client_buffer.h'
--- src/client/gbm/gbm_client_buffer.h	2013-03-22 10:34:16 +0000
+++ src/client/gbm/gbm_client_buffer.h	2013-03-24 23:57:21 +0000
@@ -20,7 +20,7 @@
 #ifndef MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_H_
 #define MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_H_
 
-#include "../client_buffer.h"
+#include "../aging_buffer.h"
 #include "mir_toolkit/mir_client_library.h"
 #include "mir/geometry/rectangle.h"
 
@@ -35,11 +35,11 @@
 
 class DRMFDHandler;
 
-class GBMClientBuffer : public ClientBuffer
+class GBMClientBuffer : public AgingBuffer
 {
 public:
     GBMClientBuffer(std::shared_ptr<DRMFDHandler> const& drm_fd_handler,
-                    std::shared_ptr<mir_toolkit::MirBufferPackage>&& buffer_package,
+                    std::shared_ptr<mir_toolkit::MirBufferPackage> const& buffer_package,
                     geometry::Size size,
                     geometry::PixelFormat pf);
 
@@ -52,7 +52,6 @@
 
     GBMClientBuffer(const GBMClientBuffer&) = delete;
     GBMClientBuffer& operator=(const GBMClientBuffer&) = delete;
-
 private:
     const std::shared_ptr<DRMFDHandler> drm_fd_handler;
     const std::shared_ptr<mir_toolkit::MirBufferPackage> creation_package;

=== removed file 'src/client/gbm/gbm_client_buffer_depository.cpp'
--- src/client/gbm/gbm_client_buffer_depository.cpp	2013-03-13 04:54:15 +0000
+++ src/client/gbm/gbm_client_buffer_depository.cpp	1970-01-01 00:00:00 +0000
@@ -1,44 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by:
- *   Kevin DuBois <[e-mail]>
- */
-
-#include "gbm_client_buffer_depository.h"
-#include "gbm_client_buffer.h"
-
-#include <stdexcept>
-#include <map>
-
-namespace mcl=mir::client;
-namespace mclg=mir::client::gbm;
-
-mclg::GBMClientBufferDepository::GBMClientBufferDepository(
-        std::shared_ptr<DRMFDHandler> const& drm_fd_handler)
-    : drm_fd_handler{drm_fd_handler}
-{
-}
-
-void mclg::GBMClientBufferDepository::deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage>&& package, int, geometry::Size size, geometry::PixelFormat pf)
-{
-    buffer = std::make_shared<mclg::GBMClientBuffer>(drm_fd_handler, std::move(package), size, pf);
-}
-
-std::shared_ptr<mcl::ClientBuffer> mclg::GBMClientBufferDepository::access_buffer(int)
-{
-    return buffer;
-}
-

=== removed file 'src/client/gbm/gbm_client_buffer_depository.h'
--- src/client/gbm/gbm_client_buffer_depository.h	2013-03-13 04:54:15 +0000
+++ src/client/gbm/gbm_client_buffer_depository.h	1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by:
- *   Kevin DuBois <[e-mail]>
- */
-
-#ifndef MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_DEPOSITORY_H_
-#define MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_DEPOSITORY_H_
-
-#include "../client_buffer_depository.h"
-
-#include <stdexcept>
-#include <map>
-
-namespace mir
-{
-namespace client
-{
-class ClientBuffer;
-
-namespace gbm
-{
-
-class DRMFDHandler;
-
-class GBMClientBufferDepository : public ClientBufferDepository
-{
-public:
-    GBMClientBufferDepository(std::shared_ptr<DRMFDHandler> const& drm_fd_handler);
-
-    void deposit_package(std::shared_ptr<mir_toolkit::MirBufferPackage> && package, int, geometry::Size size, geometry::PixelFormat pf);
-
-    std::shared_ptr<ClientBuffer> access_buffer(int id);
-private:
-    std::shared_ptr<ClientBuffer> buffer;
-    std::shared_ptr<DRMFDHandler> drm_fd_handler;
-};
-
-}
-}
-}
-#endif /* MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_DEPOSITORY_H_ */

=== added file 'src/client/gbm/gbm_client_buffer_factory.cpp'
--- src/client/gbm/gbm_client_buffer_factory.cpp	1970-01-01 00:00:00 +0000
+++ src/client/gbm/gbm_client_buffer_factory.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,35 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "gbm_client_buffer_factory.h"
+#include "gbm_client_buffer.h"
+
+namespace mcl=mir::client;
+namespace mclg=mir::client::gbm;
+
+mclg::GBMClientBufferFactory::GBMClientBufferFactory(
+        std::shared_ptr<DRMFDHandler> const& drm_fd_handler)
+    : drm_fd_handler{drm_fd_handler}
+{
+}
+
+std::shared_ptr<mcl::ClientBuffer> mclg::GBMClientBufferFactory::create_buffer(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package, geometry::Size size, geometry::PixelFormat pf)
+{
+    return std::make_shared<mclg::GBMClientBuffer>(drm_fd_handler, package, size, pf);
+}

=== added file 'src/client/gbm/gbm_client_buffer_factory.h'
--- src/client/gbm/gbm_client_buffer_factory.h	1970-01-01 00:00:00 +0000
+++ src/client/gbm/gbm_client_buffer_factory.h	2013-03-24 23:57:21 +0000
@@ -0,0 +1,51 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by:
+ *   Christopher James Halse Rogers <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_FACTORY_H_
+#define MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_FACTORY_H_
+
+#include "../client_buffer_factory.h"
+#include "gbm_client_buffer.h"
+
+namespace mir
+{
+namespace client
+{
+class ClientBuffer;
+
+namespace gbm
+{
+
+class DRMFDHandler;
+
+class GBMClientBufferFactory : public ClientBufferFactory
+{
+public:
+    explicit GBMClientBufferFactory(std::shared_ptr<DRMFDHandler> const& drm_fd_handler);
+
+    std::shared_ptr<ClientBuffer> create_buffer(std::shared_ptr<mir_toolkit::MirBufferPackage> const & package,
+                                                geometry::Size size, geometry::PixelFormat pf);
+private:
+    std::shared_ptr<DRMFDHandler> drm_fd_handler;
+};
+
+}
+}
+}
+#endif /* MIR_CLIENT_GBM_GBM_CLIENT_BUFFER_FACTORY_H_ */

=== modified file 'src/client/gbm/gbm_client_platform.cpp'
--- src/client/gbm/gbm_client_platform.cpp	2013-03-22 10:34:16 +0000
+++ src/client/gbm/gbm_client_platform.cpp	2013-03-24 23:57:21 +0000
@@ -18,10 +18,11 @@
 
 #include "mir_toolkit/mir_client_library.h"
 #include "gbm_client_platform.h"
-#include "gbm_client_buffer_depository.h"
+#include "gbm_client_buffer_factory.h"
 #include "mesa_native_display_container.h"
 #include "drm_fd_handler.h"
 #include "../mir_connection.h"
+#include "../client_buffer_factory.h"
 #include "../native_client_platform_factory.h"
 
 #include <xf86drm.h>
@@ -106,9 +107,9 @@
 {
 }
 
-std::shared_ptr<mcl::ClientBufferDepository> mclg::GBMClientPlatform::create_platform_depository()
+std::shared_ptr<mcl::ClientBufferFactory> mclg::GBMClientPlatform::create_buffer_factory()
 {
-    return std::make_shared<mclg::GBMClientBufferDepository>(drm_fd_handler);
+    return std::make_shared<mclg::GBMClientBufferFactory>(drm_fd_handler);
 }
 
 std::shared_ptr<EGLNativeWindowType> mclg::GBMClientPlatform::create_egl_native_window(ClientSurface* client_surface)

=== modified file 'src/client/gbm/gbm_client_platform.h'
--- src/client/gbm/gbm_client_platform.h	2013-03-22 10:34:16 +0000
+++ src/client/gbm/gbm_client_platform.h	2013-03-24 23:57:21 +0000
@@ -39,7 +39,7 @@
                       std::shared_ptr<DRMFDHandler> const& drm_fd_handler,
                       EGLNativeDisplayContainer& display_container);
 
-    std::shared_ptr<ClientBufferDepository> create_platform_depository ();
+    std::shared_ptr<ClientBufferFactory> create_buffer_factory ();
     std::shared_ptr<EGLNativeWindowType> create_egl_native_window(ClientSurface *surface);
     std::shared_ptr<EGLNativeDisplayType> create_egl_native_display();
 

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-03-13 04:54:15 +0000
+++ src/client/mir_connection.cpp	2013-03-24 23:57:21 +0000
@@ -66,7 +66,7 @@
     mir_surface_lifecycle_callback callback,
     void * context)
 {
-    auto depository = platform->create_platform_depository();
+    auto depository = std::make_shared<mir::client::ClientBufferDepository>(platform->create_buffer_factory(), 3);
     auto null_log = std::make_shared<mir::client::NullLogger>();
     auto surface = new MirSurface(this, server, null_log, depository, params, callback, context);
     return surface->get_create_wait_handle();
@@ -187,8 +187,8 @@
        is a kludge until we have a better story about the lifetime of MirWaitHandles */
     {
         std::lock_guard<std::mutex> lock(release_wait_handle_guard);
-        for(auto it = release_wait_handles.begin(); it != release_wait_handles.end(); it++)
-            delete *it;
+        for (auto handle : release_wait_handles)
+            delete handle;
     }
 
     disconnect_wait_handle.result_received();

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-03-22 10:34:16 +0000
+++ src/client/mir_surface.cpp	2013-03-24 23:57:21 +0000
@@ -38,7 +38,6 @@
     mir_surface_lifecycle_callback callback, void * context)
     : server(server),
       connection(allocating_connection),
-      last_buffer_id(-1),
       buffer_depository(depository),
       logger(logger)
 {
@@ -92,7 +91,7 @@
 
 void mir_toolkit::MirSurface::get_cpu_region(MirGraphicsRegion& region_out)
 {
-    auto buffer = buffer_depository->access_buffer(last_buffer_id);
+    auto buffer = buffer_depository->current_buffer();
 
     secured_region = buffer->secure_for_cpu_write();
     region_out.width = secured_region->width.as_uint32_t();
@@ -129,9 +128,9 @@
 
 /* todo: all these conversion functions are a bit of a kludge, probably
          better to have a more developed geometry::PixelFormat that can handle this */
-geom::PixelFormat mir_toolkit::MirSurface::convert_ipc_pf_to_geometry(gp::int32 pf )
+geom::PixelFormat mir_toolkit::MirSurface::convert_ipc_pf_to_geometry(gp::int32 pf)
 {
-    if ( pf == mir_pixel_format_abgr_8888 )
+    if (pf == mir_pixel_format_abgr_8888)
         return geom::PixelFormat::abgr_8888;
     return geom::PixelFormat::invalid;
 }
@@ -139,7 +138,6 @@
 void mir_toolkit::MirSurface::process_incoming_buffer()
 {
     auto const& buffer = surface.buffer();
-    last_buffer_id = buffer.buffer_id();
 
     auto surface_width = geom::Width(surface.width());
     auto surface_height = geom::Height(surface.height());
@@ -152,7 +150,7 @@
     try
     {
         buffer_depository->deposit_package(std::move(ipc_package),
-                                last_buffer_id,
+                                buffer.buffer_id(),
                                 surface_size, surface_pf);
     } catch (const std::runtime_error& err)
     {
@@ -188,13 +186,13 @@
 
 std::shared_ptr<mir_toolkit::MirBufferPackage> mir_toolkit::MirSurface::get_current_buffer_package()
 {
-    auto buffer = buffer_depository->access_buffer(last_buffer_id);
+    auto buffer = buffer_depository->current_buffer();
     return buffer->get_buffer_package();
 }
 
 std::shared_ptr<mcl::ClientBuffer> mir_toolkit::MirSurface::get_current_buffer()
 {
-    return buffer_depository->access_buffer(last_buffer_id);
+    return buffer_depository->current_buffer();
 }
 
 void mir_toolkit::MirSurface::populate(MirBufferPackage& buffer_package)

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2013-03-22 10:34:16 +0000
+++ src/client/mir_surface.h	2013-03-24 23:57:21 +0000
@@ -82,7 +82,7 @@
     void populate(MirBufferPackage& buffer_package);
     void created(mir_surface_lifecycle_callback callback, void * context);
     void new_buffer(mir_surface_lifecycle_callback callback, void * context);
-    mir::geometry::PixelFormat convert_ipc_pf_to_geometry(google::protobuf::int32 pf );
+    mir::geometry::PixelFormat convert_ipc_pf_to_geometry(google::protobuf::int32 pf);
 
     /* todo: race condition. protobuf does not guarantee that callbacks will be synchronized. potential
              race in surface, last_buffer_id */
@@ -95,8 +95,6 @@
     MirWaitHandle next_buffer_wait_handle;
     MirWaitHandle configure_wait_handle;
 
-    int last_buffer_id;
-
     std::shared_ptr<mir::client::MemoryRegion> secured_region;
     std::shared_ptr<mir::client::ClientBufferDepository> buffer_depository;
 

=== modified file 'tests/unit-tests/client/CMakeLists.txt'
--- tests/unit-tests/client/CMakeLists.txt	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/CMakeLists.txt	2013-03-24 23:57:21 +0000
@@ -1,5 +1,7 @@
 list(APPEND UNIT_TEST_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_aging_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_client_buffer_depository.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_platform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_mir_surface.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_connection.cpp

=== modified file 'tests/unit-tests/client/android/CMakeLists.txt'
--- tests/unit-tests/client/android/CMakeLists.txt	2013-03-14 18:35:31 +0000
+++ tests/unit-tests/client/android/CMakeLists.txt	2013-03-24 23:57:21 +0000
@@ -19,7 +19,6 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_android_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_android_registrar.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_android_native_window.cpp
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_depository.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_platform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_surface_interpreter.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_android_syncfence.cpp

=== removed file 'tests/unit-tests/client/android/test_android_client_depository.cpp'
--- tests/unit-tests/client/android/test_android_client_depository.cpp	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/android/test_android_client_depository.cpp	1970-01-01 00:00:00 +0000
@@ -1,169 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by: Kevin DuBois <[e-mail]>
- */
-
-#include "mir_toolkit/mir_client_library.h"
-#include "src/client/android/android_client_buffer_depository.h"
-#include "src/client/android/android_client_buffer.h"
-#include "mir_test_doubles/mock_android_registrar.h"
-
-#include <gtest/gtest.h>
-
-namespace geom = mir::geometry;
-namespace mcla = mir::client::android;
-namespace mt = mir::test;
-namespace mtd = mir::test::doubles;
-
-
-struct MirBufferDepositoryTest : public testing::Test
-{
-    void SetUp()
-    {
-        width = geom::Width(12);
-        height =geom::Height(14);
-        size = geom::Size{width, height};
-        pf = geom::PixelFormat::abgr_8888;
-
-        mock_registrar = std::make_shared<mtd::MockAndroidRegistrar>();
-        package1 = std::make_shared<MirBufferPackage>();
-        package2 = std::make_shared<MirBufferPackage>();
-
-    }
-    geom::Width width;
-    geom::Height height;
-    geom::PixelFormat pf;
-    geom::Size size;
-
-    std::shared_ptr<mtd::MockAndroidRegistrar> mock_registrar;
-
-    std::shared_ptr<MirBufferPackage> package1;
-    std::shared_ptr<MirBufferPackage> package2;
-
-};
-
-TEST_F(MirBufferDepositoryTest, depository_sets_width_and_height)
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(1);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(1);
-
-    depository.deposit_package(std::move(package1), 8, size, pf);
-    auto buffer = depository.access_buffer(8);
-
-    EXPECT_EQ(buffer->size().height, height);
-    EXPECT_EQ(buffer->size().width, width);
-    EXPECT_EQ(buffer->pixel_format(), pf);
-}
-
-TEST_F(MirBufferDepositoryTest, depository_does_not_create_a_buffer_its_seen_before )
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(1);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(1);
-
-    /* repeated id */
-    depository.deposit_package(std::move(package1), 8, size, pf);
-    depository.deposit_package(std::move(package2), 8, size, pf);
-}
-
-TEST_F(MirBufferDepositoryTest, depository_creates_two_buffers_with_distinct_id )
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(2);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(2);
-
-    /* repeated id */
-    depository.deposit_package(std::move(package1), 8, size, pf);
-    depository.deposit_package(std::move(package2), 9, size, pf);
-}
-
-TEST_F(MirBufferDepositoryTest, depository_returns_same_accessed_buffer_for_same_id )
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(2);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(2);
-
-    /* repeated id */
-    depository.deposit_package(std::move(package1), 8, size, pf);
-    depository.deposit_package(std::move(package2), 9, size, pf);
-
-    auto buffer1 = depository.access_buffer(8);
-    auto buffer2 = depository.access_buffer(8);
-
-    EXPECT_EQ(buffer1, buffer2);
-}
-
-TEST_F(MirBufferDepositoryTest, depository_returns_different_accessed_buffer_for_unique_id )
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(2);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(2);
-
-    /* repeated id */
-    depository.deposit_package(std::move(package1), 8, size, pf);
-    depository.deposit_package(std::move(package2), 9, size, pf);
-
-    auto buffer1 = depository.access_buffer(8);
-    auto buffer2 = depository.access_buffer(9);
-
-    EXPECT_NE(buffer1, buffer2);
-}
-
-TEST_F(MirBufferDepositoryTest, depository_throws_for_uncreated_id )
-{
-    using namespace testing;
-
-    mcla::AndroidClientBufferDepository depository(mock_registrar);
-
-    EXPECT_CALL(*mock_registrar, register_buffer(_))
-        .Times(1);
-    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
-        .Times(1);
-
-    /* repeated id */
-    depository.deposit_package(std::move(package1), 8, size, pf);
-
-    EXPECT_THROW({
-    auto buffer2 = depository.access_buffer(9);
-    }, std::runtime_error);
-
-}

=== modified file 'tests/unit-tests/client/android/test_client_surface_interpreter.cpp'
--- tests/unit-tests/client/android/test_client_surface_interpreter.cpp	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/android/test_client_surface_interpreter.cpp	2013-03-24 23:57:21 +0000
@@ -42,6 +42,10 @@
     MOCK_CONST_METHOD0(stride, geom::Stride());
     MOCK_CONST_METHOD0(pixel_format, geom::PixelFormat());
 
+    MOCK_CONST_METHOD0(age, uint32_t());
+    MOCK_METHOD0(mark_as_submitted, void());
+    MOCK_METHOD0(increment_age, void());
+
     MOCK_CONST_METHOD0(get_buffer_package, std::shared_ptr<MirBufferPackage>());
     MOCK_METHOD0(get_native_handle, ANativeWindowBuffer*());
 

=== modified file 'tests/unit-tests/client/gbm/CMakeLists.txt'
--- tests/unit-tests/client/gbm/CMakeLists.txt	2013-03-13 01:01:01 +0000
+++ tests/unit-tests/client/gbm/CMakeLists.txt	2013-03-24 23:57:21 +0000
@@ -16,7 +16,6 @@
 #              Alan Griffiths <[e-mail]>
 
 list(APPEND UNIT_TEST_SOURCES
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_gbm_client_depository.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gbm_client_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gbm_client_platform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_mesa_native_display_container.cpp

=== modified file 'tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp'
--- tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp	2013-03-24 23:57:21 +0000
@@ -17,7 +17,6 @@
  */
 
 #include "mir_toolkit/mir_client_library.h"
-#include "src/client/gbm/gbm_client_buffer_depository.h"
 #include "src/client/gbm/gbm_client_buffer.h"
 #include "mock_drm_fd_handler.h"
 
@@ -25,6 +24,8 @@
 #include <sys/mman.h>
 #include <gtest/gtest.h>
 
+#include <stdexcept>
+
 namespace geom=mir::geometry;
 namespace mclg=mir::client::gbm;
 
@@ -85,9 +86,9 @@
     EXPECT_EQ(package_return->data_items, package_copy->data_items);
     EXPECT_EQ(package_return->fd_items, package_copy->fd_items);
     EXPECT_EQ(package_return->stride, package_copy->stride);
-    for(auto i=0; i<mir_buffer_package_max; i++)
+    for (auto i=0; i<mir_buffer_package_max; i++)
         EXPECT_EQ(package_return->data[i], package_copy->data[i]);
-    for(auto i=0; i<mir_buffer_package_max; i++)
+    for (auto i=0; i<mir_buffer_package_max; i++)
         EXPECT_EQ(package_return->fd[i], package_copy->fd[i]);
 }
 
@@ -135,7 +136,7 @@
     mclg::GBMClientBuffer buffer(drm_fd_handler, std::move(package), size, pf);
 
     EXPECT_THROW({
-        auto mem_region = buffer.secure_for_cpu_write();
+        buffer.secure_for_cpu_write();
     }, std::runtime_error);
 }
 

=== removed file 'tests/unit-tests/client/gbm/test_gbm_client_depository.cpp'
--- tests/unit-tests/client/gbm/test_gbm_client_depository.cpp	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/gbm/test_gbm_client_depository.cpp	1970-01-01 00:00:00 +0000
@@ -1,79 +0,0 @@
-/*
- * Copyright © 2012 Canonical Ltd.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authored by: Kevin DuBois <[e-mail]>
- */
-
-#include "mir_toolkit/mir_client_library.h"
-#include "src/client/gbm/gbm_client_buffer_depository.h"
-#include "src/client/gbm/gbm_client_buffer.h"
-#include "mock_drm_fd_handler.h"
-
-#include <gtest/gtest.h>
-
-namespace geom=mir::geometry;
-namespace mclg=mir::client::gbm;
-
-struct MirGBMBufferDepositoryTest : public testing::Test
-{
-    void SetUp()
-    {
-        width = geom::Width(12);
-        height =geom::Height(14);
-        pf = geom::PixelFormat::abgr_8888;
-        size = geom::Size{width, height};
-
-        drm_fd_handler = std::make_shared<testing::NiceMock<mclg::MockDRMFDHandler>>();
-        package = std::make_shared<MirBufferPackage>();
-    }
-    geom::Width width;
-    geom::Height height;
-    geom::PixelFormat pf;
-    geom::Size size;
-
-    std::shared_ptr<testing::NiceMock<mclg::MockDRMFDHandler>> drm_fd_handler;
-    std::shared_ptr<MirBufferPackage> package;
-
-};
-
-TEST_F(MirGBMBufferDepositoryTest, depository_sets_width_and_height)
-{
-    using namespace testing;
-
-    mclg::GBMClientBufferDepository depository{drm_fd_handler};
-
-    depository.deposit_package(std::move(package), 8, size, pf);
-    auto buffer = depository.access_buffer(8);
-
-    EXPECT_EQ(buffer->size().height, height);
-    EXPECT_EQ(buffer->size().width, width);
-    EXPECT_EQ(buffer->pixel_format(), pf);
-}
-
-TEST_F(MirGBMBufferDepositoryTest, depository_new_deposit_changes_buffer )
-{
-    using namespace testing;
-
-    mclg::GBMClientBufferDepository depository{drm_fd_handler};
-
-    depository.deposit_package(std::move(package), 8, size, pf);
-    auto buffer1 = depository.access_buffer(8);
-
-    depository.deposit_package(std::move(package), 8, size, pf);
-    auto buffer2 = depository.access_buffer(8);
-
-    EXPECT_NE(buffer1, buffer2);
-}
-

=== added file 'tests/unit-tests/client/test_aging_buffer.cpp'
--- tests/unit-tests/client/test_aging_buffer.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/client/test_aging_buffer.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,114 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Christopher James Halse Rogers <[e-mail]>
+ */
+
+#include "src/client/aging_buffer.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace mcl = mir::client;
+namespace geom = mir::geometry;
+
+namespace mir_toolkit
+{
+    struct MirBufferPackage;
+}
+
+namespace mir
+{
+namespace test
+{
+
+struct MyAgingBuffer : public mcl::AgingBuffer
+{
+    std::shared_ptr<mcl::MemoryRegion> secure_for_cpu_write()
+    {
+        exit(1);
+    }
+
+    geom::Size size() const
+    {
+        exit(1);
+    }
+
+    geom::Stride stride() const
+    {
+        exit(1);
+    }
+
+    geom::PixelFormat pixel_format() const
+    {
+        exit(1);
+    }
+
+    std::shared_ptr<mir_toolkit::MirBufferPackage> get_buffer_package() const
+    {
+        exit(1);
+    }
+
+    MirNativeBuffer get_native_handle()
+    {
+        exit(1);
+    }
+};
+
+TEST(MirClientAgingBufferTest, buffer_age_starts_at_zero)
+{
+    using namespace testing;
+
+    MyAgingBuffer buffer;
+
+    EXPECT_EQ(0u, buffer.age());
+}
+
+TEST(MirClientAgingBufferTest, buffer_age_set_to_one_on_submit)
+{
+    using namespace testing;
+
+    MyAgingBuffer buffer;
+    buffer.mark_as_submitted();
+
+    EXPECT_EQ(1u, buffer.age());    
+}
+
+TEST(MirClientAgingBufferTest, buffer_age_increases_on_increment)
+{
+    using namespace testing;
+
+    MyAgingBuffer buffer;
+    buffer.mark_as_submitted();
+
+    for (uint32_t age = 2; age < 10; ++age)
+    {
+        buffer.increment_age();
+        EXPECT_EQ(age, buffer.age());
+    }
+}
+
+TEST(MirClientAgingBufferTest, incrementing_age_has_no_effect_for_unsubmitted_buffer)
+{
+    using namespace testing;
+
+    MyAgingBuffer buffer;
+    buffer.increment_age();
+
+    EXPECT_EQ(0u, buffer.age());
+}
+
+}
+}

=== added file 'tests/unit-tests/client/test_android_client_buffer_factory.cpp'
--- tests/unit-tests/client/test_android_client_buffer_factory.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/client/test_android_client_buffer_factory.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,74 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+#include "src/client/android/android_client_buffer_factory.h"
+#include "src/client/android/android_client_buffer.h"
+#include "mir_test_doubles/mock_android_registrar.h"
+
+#include <gtest/gtest.h>
+
+namespace geom = mir::geometry;
+namespace mcla = mir::client::android;
+namespace mt = mir::test;
+namespace mtd = mir::test::doubles;
+
+
+struct MirBufferFactoryTest : public testing::Test
+{
+    void SetUp()
+    {
+        width = geom::Width(12);
+        height =geom::Height(14);
+        size = geom::Size{width, height};
+        pf = geom::PixelFormat::abgr_8888;
+
+        mock_registrar = std::make_shared<mtd::MockAndroidRegistrar>();
+        package1 = std::make_shared<MirBufferPackage>();
+        package2 = std::make_shared<MirBufferPackage>();
+
+    }
+    geom::Width width;
+    geom::Height height;
+    geom::PixelFormat pf;
+    geom::Size size;
+
+    std::shared_ptr<mtd::MockAndroidRegistrar> mock_registrar;
+
+    std::shared_ptr<MirBufferPackage> package1;
+    std::shared_ptr<MirBufferPackage> package2;
+
+};
+
+TEST_F(MirBufferFactoryTest, factory_sets_width_and_height)
+{
+    using namespace testing;
+
+    mcla::AndroidClientBufferFactory buffer_factory(mock_registrar);
+
+    EXPECT_CALL(*mock_registrar, register_buffer(_))
+        .Times(1);
+    EXPECT_CALL(*mock_registrar, unregister_buffer(_))
+        .Times(1);
+
+    auto buffer = buffer_factory.create_buffer(package1, size, pf);
+
+    EXPECT_EQ(buffer->size().height, height);
+    EXPECT_EQ(buffer->size().width, width);
+    EXPECT_EQ(buffer->pixel_format(), pf);
+}

=== added file 'tests/unit-tests/client/test_client_buffer_depository.cpp'
--- tests/unit-tests/client/test_client_buffer_depository.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/client/test_client_buffer_depository.cpp	2013-03-24 23:57:21 +0000
@@ -0,0 +1,364 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+#include "src/client/client_buffer_depository.h"
+#include "src/client/client_buffer_factory.h"
+#include "src/client/aging_buffer.h"
+#include "mir/geometry/pixel_format.h"
+#include "mir/geometry/size.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace geom=mir::geometry;
+namespace mcl=mir::client;
+
+struct MockBuffer : public mcl::AgingBuffer
+{
+    MockBuffer()
+    {
+        using namespace testing;
+        ON_CALL(*this, mark_as_submitted())
+            .WillByDefault(Invoke([this](){this->AgingBuffer::mark_as_submitted();}));
+    }
+
+    MOCK_METHOD0(Destroy, void());
+    virtual ~MockBuffer()
+    {
+        Destroy();
+    }
+
+    MOCK_METHOD0(mark_as_submitted, void());
+    MOCK_METHOD0(secure_for_cpu_write, std::shared_ptr<mcl::MemoryRegion>());
+    MOCK_CONST_METHOD0(size, geom::Size());
+    MOCK_CONST_METHOD0(stride, geom::Stride());
+    MOCK_CONST_METHOD0(pixel_format, geom::PixelFormat());
+    MOCK_CONST_METHOD0(get_buffer_package, std::shared_ptr<MirBufferPackage>());
+    MOCK_METHOD0(get_native_handle, MirNativeBuffer());
+};
+
+struct MockClientBufferFactory : public mcl::ClientBufferFactory
+{
+    MockClientBufferFactory()
+    {
+        using namespace testing;
+
+        // Some tests, quite reasonably, rely on create_buffer returning a different buffer each time
+        // Handle this by first updating our "buffer" temporary, then returning-by-pointee.
+        ON_CALL(*this, create_buffer(_,_,_))
+            .WillByDefault(DoAll(InvokeWithoutArgs([this]() {this->buffer = std::make_shared<NiceMock<MockBuffer>>();}),
+                                 ReturnPointee(&buffer)));
+    }
+
+    MOCK_METHOD3(create_buffer,
+                 std::shared_ptr<mcl::ClientBuffer>(std::shared_ptr<MirBufferPackage> const &,
+                                                    geom::Size, geom::PixelFormat));
+
+    std::shared_ptr<mcl::ClientBuffer> buffer;
+};
+
+struct MirBufferDepositoryTest : public testing::Test
+{
+    void SetUp()
+    {
+        width = geom::Width(12);
+        height =geom::Height(14);
+        pf = geom::PixelFormat::abgr_8888;
+        size = geom::Size{width, height};
+
+        package = std::make_shared<MirBufferPackage>();
+        mock_factory = std::make_shared<testing::NiceMock<MockClientBufferFactory>>();
+    }
+    geom::Width width;
+    geom::Height height;
+    geom::PixelFormat pf;
+    geom::Size size;
+
+    std::shared_ptr<MirBufferPackage> package;
+    std::shared_ptr<MockClientBufferFactory> mock_factory;
+};
+
+MATCHER_P(SizeMatches, value, "")
+{
+    return value == arg;
+}
+
+TEST_F(MirBufferDepositoryTest, depository_sets_width_and_height)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    EXPECT_CALL(*mock_factory, create_buffer(_,size,pf))
+        .Times(1);
+    depository.deposit_package(package, 8, size, pf);
+}
+
+TEST_F(MirBufferDepositoryTest, depository_new_deposit_changes_current_buffer)
+{
+    using namespace testing;
+
+    auto package2 = std::make_shared<MirBufferPackage>();
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    depository.deposit_package(package, 8, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    depository.deposit_package(package2, 9, size, pf);
+    auto buffer2 = depository.current_buffer();
+
+    EXPECT_NE(buffer1, buffer2);
+}
+
+TEST_F(MirBufferDepositoryTest, depository_sets_buffer_age_to_zero_for_new_buffer)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    depository.deposit_package(package, 1, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    EXPECT_EQ(0u, buffer1->age());
+}
+
+TEST_F(MirBufferDepositoryTest, just_sumbitted_buffer_has_age_1)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+    auto package2 = std::make_shared<MirBufferPackage>();
+
+    depository.deposit_package(package, 1, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    ASSERT_EQ(0u, buffer1->age());
+
+    // Deposit new package, implicitly marking previous buffer as submitted
+    depository.deposit_package(package2, 2, size, pf);
+
+    EXPECT_EQ(1u, buffer1->age());
+}
+
+TEST_F(MirBufferDepositoryTest, submitting_buffer_ages_other_buffers)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    depository.deposit_package(package, 1, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    EXPECT_EQ(0u, buffer1->age());
+
+    auto package2 = std::make_shared<MirBufferPackage>();
+    depository.deposit_package(package2, 2, size, pf);
+    auto buffer2 = depository.current_buffer();
+
+    EXPECT_EQ(1u, buffer1->age());
+    EXPECT_EQ(0u, buffer2->age());
+
+    auto package3 = std::make_shared<MirBufferPackage>();
+    depository.deposit_package(package3, 3, size, pf);
+    auto buffer3 = depository.current_buffer();
+
+    EXPECT_EQ(2u, buffer1->age());
+    EXPECT_EQ(1u, buffer2->age());
+    EXPECT_EQ(0u, buffer3->age());
+}
+
+TEST_F(MirBufferDepositoryTest, double_buffering_reaches_steady_state_age)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    depository.deposit_package(package, 1, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    EXPECT_EQ(0u, buffer1->age());
+
+    auto package2 = std::make_shared<MirBufferPackage>();
+    depository.deposit_package(package2, 2, size, pf);
+    auto buffer2 = depository.current_buffer();
+
+    EXPECT_EQ(1u, buffer1->age());
+    EXPECT_EQ(0u, buffer2->age());
+
+    depository.deposit_package(package, 1, size, pf);
+
+    EXPECT_EQ(2u, buffer1->age());
+    EXPECT_EQ(1u, buffer2->age());
+
+    depository.deposit_package(package2, 2, size, pf);
+
+    EXPECT_EQ(1u, buffer1->age());
+    EXPECT_EQ(2u, buffer2->age());
+}
+
+TEST_F(MirBufferDepositoryTest, triple_buffering_reaches_steady_state_age)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    depository.deposit_package(package, 1, size, pf);
+    auto buffer1 = depository.current_buffer();
+
+    auto package2 = std::make_shared<MirBufferPackage>();
+    depository.deposit_package(package2, 2, size, pf);
+    auto buffer2 = depository.current_buffer();
+
+    auto package3 = std::make_shared<MirBufferPackage>();
+    depository.deposit_package(package3, 3, size, pf);
+    auto buffer3 = depository.current_buffer();
+
+    EXPECT_EQ(2u, buffer1->age());
+    EXPECT_EQ(1u, buffer2->age());
+    EXPECT_EQ(0u, buffer3->age());
+
+    depository.deposit_package(package, 1, size, pf);
+
+    EXPECT_EQ(3u, buffer1->age());
+    EXPECT_EQ(2u, buffer2->age());
+    EXPECT_EQ(1u, buffer3->age());
+
+    depository.deposit_package(package2, 2, size, pf);
+
+    EXPECT_EQ(1u, buffer1->age());
+    EXPECT_EQ(3u, buffer2->age());
+    EXPECT_EQ(2u, buffer3->age());
+
+    depository.deposit_package(package3, 3, size, pf);
+
+    EXPECT_EQ(2u, buffer1->age());
+    EXPECT_EQ(1u, buffer2->age());
+    EXPECT_EQ(3u, buffer3->age());
+}
+
+TEST_F(MirBufferDepositoryTest, depository_destroys_old_buffers)
+{
+    using namespace testing;
+    const int num_buffers = 3;
+
+    mcl::ClientBufferDepository depository{mock_factory, num_buffers};
+
+    const int num_packages = 4;
+    std::shared_ptr<MirBufferPackage> packages[num_packages];
+
+    depository.deposit_package(packages[0], 1, size, pf);
+    // Raw pointer so we don't influence the buffer's life-cycle
+    MockBuffer *first_buffer = static_cast<MockBuffer *>(depository.current_buffer().get());
+
+    depository.deposit_package(packages[1], 2, size, pf);
+    depository.deposit_package(packages[2], 3, size, pf);
+
+    // We've deposited three different buffers now; the fourth should trigger the destruction
+    // of the first buffer.
+    EXPECT_CALL(*first_buffer, Destroy());
+
+    depository.deposit_package(packages[3], 4, size, pf);
+
+    // Explicitly verify that the buffer has been destroyed here, before the depository goes out of scope
+    // and its destructor cleans everything up.
+    Mock::VerifyAndClearExpectations(first_buffer);
+}
+
+TEST_F(MirBufferDepositoryTest, depositing_packages_implicitly_submits_current_buffer)
+{
+    using namespace testing;
+    const int num_buffers = 3;
+
+    mcl::ClientBufferDepository depository{mock_factory, num_buffers};
+
+    auto package1 = std::make_shared<MirBufferPackage>();
+    auto package2 = std::make_shared<MirBufferPackage>();
+
+    depository.deposit_package(package1, 1, size, pf);
+    EXPECT_CALL(*static_cast<MockBuffer *>(depository.current_buffer().get()), mark_as_submitted());
+    depository.deposit_package(package2, 2, size, pf);
+}
+
+TEST_F(MirBufferDepositoryTest, depository_respects_max_buffer_parameter)
+{
+    using namespace testing;
+    std::shared_ptr<mcl::ClientBufferDepository> depository;
+    std::shared_ptr<MirBufferPackage> packages[10];
+    MockBuffer *buffers[10];
+
+    for (int num_buffers = 2; num_buffers < 10; ++num_buffers)
+    {
+        depository = std::make_shared<mcl::ClientBufferDepository>(mock_factory, num_buffers);
+        
+        depository->deposit_package(packages[0], 1, size, pf);
+        // Raw pointer so we don't influence the buffer's life-cycle
+        MockBuffer* first_buffer = static_cast<MockBuffer *>(depository->current_buffer().get());
+
+        int i;
+        for (i = 1; i < num_buffers ; ++i)
+        {
+            depository->deposit_package(packages[i], i + 1, size, pf);
+            buffers[i] = static_cast<MockBuffer *>(depository->current_buffer().get());
+            // None of these buffers should be destroyed
+            EXPECT_CALL(*buffers[i], Destroy()).Times(0);
+        }
+
+        // Next deposit should destroy first buffer
+        EXPECT_CALL(*first_buffer, Destroy()).Times(1);
+        depository->deposit_package(packages[i], i+1, size, pf);
+        EXPECT_TRUE(Mock::VerifyAndClearExpectations(first_buffer));
+
+        // Verify none of the other buffers have been destroyed
+        for (i = 1; i < num_buffers; ++i)
+            EXPECT_TRUE(Mock::VerifyAndClearExpectations(buffers[i]));
+    }
+}
+
+TEST_F(MirBufferDepositoryTest, depository_caches_recently_seen_buffer)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    auto package1 = std::make_shared<MirBufferPackage>();
+    auto package2 = std::make_shared<MirBufferPackage>();
+
+    EXPECT_CALL(*mock_factory, create_buffer(_,_,_))
+        .Times(1);
+
+    depository.deposit_package(package1, 8, size, pf);
+    depository.deposit_package(package1, 8, size, pf);
+    depository.deposit_package(package1, 8, size, pf);
+}
+
+TEST_F(MirBufferDepositoryTest, depository_creates_new_buffer_for_different_id)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    auto package1 = std::make_shared<MirBufferPackage>();
+    auto package2 = std::make_shared<MirBufferPackage>();
+
+    EXPECT_CALL(*mock_factory, create_buffer(_,_,_))
+        .Times(2);
+
+    depository.deposit_package(package1, 8, size, pf);
+    depository.deposit_package(package2, 9, size, pf);
+}

=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2013-03-22 10:34:16 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2013-03-24 23:57:21 +0000
@@ -21,6 +21,7 @@
 #include "src/client/mir_logger.h"
 #include "src/client/client_buffer.h"
 #include "src/client/client_buffer_depository.h"
+#include "src/client/client_buffer_factory.h"
 #include "src/client/client_platform.h"
 #include "src/client/client_platform_factory.h"
 #include "src/client/mir_surface.h"
@@ -55,12 +56,12 @@
         pf_sent = mir_pixel_format_abgr_8888;
     }
 
-    void create_surface(google::protobuf::RpcController* ,
+    void create_surface(google::protobuf::RpcController*,
                  const mir::protobuf::SurfaceParameters* request,
                  mir::protobuf::Surface* response,
                  google::protobuf::Closure* done)
     {
-        create_surface_response( response );
+        create_surface_response(response);
         surface_name = request->surface_name();
         done->Run();
     }
@@ -71,7 +72,7 @@
         ::mir::protobuf::Buffer* response,
         ::google::protobuf::Closure* done)
     {
-        create_buffer_response( response );
+        create_buffer_response(response);
         done->Run();
     }
 
@@ -117,6 +118,8 @@
         }
 
         response->set_stride(server_package.stride);
+
+        generate_unique_buffer();
     }
 
     void create_surface_response(mir::protobuf::Surface* response)
@@ -140,31 +143,27 @@
     MOCK_CONST_METHOD0(size, geom::Size());
     MOCK_CONST_METHOD0(stride, geom::Stride());
     MOCK_CONST_METHOD0(pixel_format, geom::PixelFormat());
+    MOCK_CONST_METHOD0(age, uint32_t());
+    MOCK_METHOD0(increment_age, void());
+    MOCK_METHOD0(mark_as_submitted, void());
     MOCK_CONST_METHOD0(get_buffer_package, std::shared_ptr<MirBufferPackage>());
     MOCK_METHOD0(get_native_handle, MirNativeBuffer());
 };
 
-struct MockClientDepository : public mcl::ClientBufferDepository
+struct MockClientBufferFactory : public mcl::ClientBufferFactory
 {
-    MockClientDepository()
+    MockClientBufferFactory()
     {
         using namespace testing;
 
-        emptybuffer=std::make_shared<MockBuffer>();
-        ON_CALL(*this, access_buffer(_))
+        emptybuffer=std::make_shared<NiceMock<MockBuffer>>();
+        ON_CALL(*this, create_buffer(_,_,_))
             .WillByDefault(Return(emptybuffer));
     }
 
-    void deposit_package(std::shared_ptr<MirBufferPackage> && p, int id,
-                         geometry::Size size, geometry::PixelFormat pf)
-    {
-        deposit_package_rv( p, id, size, pf);
-    }
-
-    MOCK_METHOD4(deposit_package_rv,
-                     void(std::shared_ptr<MirBufferPackage>, int,
-                            geom::Size, geom::PixelFormat));
-    MOCK_METHOD1(access_buffer, std::shared_ptr<mcl::ClientBuffer>(int));
+    MOCK_METHOD3(create_buffer,
+                 std::shared_ptr<mcl::ClientBuffer>(std::shared_ptr<MirBufferPackage> const &,
+                                                    geom::Size, geom::PixelFormat));
 
     std::shared_ptr<mcl::ClientBuffer> emptybuffer;
 };
@@ -172,9 +171,9 @@
 
 struct StubClientPlatform : public mcl::ClientPlatform
 {
-    std::shared_ptr<mcl::ClientBufferDepository> create_platform_depository()
+    std::shared_ptr<mcl::ClientBufferFactory> create_buffer_factory()
     {
-        return std::shared_ptr<mcl::ClientBufferDepository>();
+        return std::shared_ptr<MockClientBufferFactory>();
     }
 
     std::shared_ptr<EGLNativeWindowType> create_egl_native_window(mcl::ClientSurface* /*surface*/)
@@ -219,7 +218,8 @@
 
         test_server->comm->start();
 
-        mock_depository = std::make_shared<mt::MockClientDepository>();
+        mock_buffer_factory = std::make_shared<mt::MockClientBufferFactory>();
+        depository = std::make_shared<mcl::ClientBufferDepository>(mock_buffer_factory, 3);
 
         params = MirSurfaceParameters{"test", 33, 45, mir_pixel_format_abgr_8888,
                                       mir_buffer_usage_hardware};
@@ -249,7 +249,8 @@
     std::shared_ptr<MirConnection> connection;
 
     MirSurfaceParameters params;
-    std::shared_ptr<mt::MockClientDepository> mock_depository;
+    std::shared_ptr<mt::MockClientBufferFactory> mock_buffer_factory;
+    std::shared_ptr<mcl::ClientBufferDepository> depository;
 
     mir::protobuf::Connection response;
     mir::protobuf::ConnectParameters connect_parameters;
@@ -264,14 +265,14 @@
 };
 
 void empty_callback(MirSurface*, void*) { }
-TEST_F(MirClientSurfaceTest, client_buffer_created_on_surface_creation )
+TEST_F(MirClientSurfaceTest, client_buffer_created_on_surface_creation)
 {
     using namespace testing;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1);
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
 
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
@@ -282,35 +283,35 @@
 void empty_surface_callback(MirSurface*, void*) {}
 }
 
-TEST_F(MirClientSurfaceTest, client_buffer_created_on_next_buffer )
+TEST_F(MirClientSurfaceTest, client_buffer_created_on_next_buffer)
 {
     using namespace testing;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1);
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
 
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1);
     auto buffer_wait_handle = surface->next_buffer(&empty_surface_callback, (void*) NULL);
     buffer_wait_handle->wait_for_result();
 }
 
-TEST_F(MirClientSurfaceTest, client_buffer_uses_ipc_message_from_server_on_create )
+TEST_F(MirClientSurfaceTest, client_buffer_uses_ipc_message_from_server_on_create)
 {
     using namespace testing;
 
     std::shared_ptr<MirBufferPackage> submitted_package;
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1)
-        .WillOnce(
-            SaveArg<0>(&submitted_package));
+        .WillOnce(DoAll(SaveArg<0>(&submitted_package),
+                        Return(mock_buffer_factory->emptybuffer)));
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
 
@@ -318,7 +319,7 @@
     ASSERT_EQ(submitted_package->data_items, mock_server_tool->server_package.data_items);
     ASSERT_EQ(submitted_package->fd_items,   mock_server_tool->server_package.fd_items);
     ASSERT_EQ(submitted_package->stride,   mock_server_tool->server_package.stride);
-    for(auto i=0; i< submitted_package->data_items; i++)
+    for (auto i=0; i< submitted_package->data_items; i++)
         EXPECT_EQ(submitted_package->data[i], mock_server_tool->server_package.data[i]);
 }
 
@@ -329,11 +330,12 @@
     geom::Size sz;
     std::shared_ptr<MirBufferPackage> submitted_package;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1)
-        .WillOnce(SaveArg<2>(&sz));
+        .WillOnce(DoAll(SaveArg<1>(&sz),
+                        Return(mock_buffer_factory->emptybuffer)));
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
 
@@ -347,11 +349,12 @@
     geom::Size sz;
     std::shared_ptr<MirBufferPackage> submitted_package;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1)
-        .WillOnce(SaveArg<2>(&sz));
+        .WillOnce(DoAll(SaveArg<1>(&sz),
+                        Return(mock_buffer_factory->emptybuffer)));
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
 
@@ -365,11 +368,12 @@
     geom::PixelFormat pf;
     std::shared_ptr<MirBufferPackage> submitted_package;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1)
-        .WillOnce(SaveArg<3>(&pf));
+        .WillOnce(DoAll(SaveArg<2>(&pf),
+                        Return(mock_buffer_factory->emptybuffer)));
 
-    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, mock_depository, params, &empty_callback, (void*) NULL);
+    auto surface = std::make_shared<MirSurface> (connection.get(), *client_comm_channel, logger, depository, params, &empty_callback, (void*) NULL);
     auto wait_handle = surface->get_create_wait_handle();
     wait_handle->wait_for_result();
 
@@ -381,13 +385,13 @@
     using namespace testing;
     using namespace mir::protobuf;
 
-    EXPECT_CALL(*mock_depository, deposit_package_rv(_,_,_,_))
+    EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_))
         .Times(1);
 
     auto surface = std::make_shared<MirSurface> (connection.get(),
                                                  *client_comm_channel,
                                                  logger,
-                                                 mock_depository,
+                                                 depository,
                                                  params,
                                                  &empty_callback,
                                                  (void*) NULL);

=== modified file 'tests/unit-tests/client/test_client_platform.cpp'
--- tests/unit-tests/client/test_client_platform.cpp	2013-03-18 22:18:23 +0000
+++ tests/unit-tests/client/test_client_platform.cpp	2013-03-24 23:57:21 +0000
@@ -45,8 +45,8 @@
     mtd::MockClientContext context;
     mcl::NativeClientPlatformFactory factory;
     auto platform = factory.create_client_platform(&context);
-    auto depository = platform->create_platform_depository();
-    EXPECT_NE( depository.get(), (mcl::ClientBufferDepository*) NULL);
+    auto buffer_factory = platform->create_buffer_factory();
+    EXPECT_NE(buffer_factory.get(), (mcl::ClientBufferFactory*) NULL);
 }
 
 TEST_F(ClientPlatformTest, platform_creates_native_window)
@@ -56,7 +56,7 @@
     auto platform = factory.create_client_platform(&context);
     auto mock_client_surface = std::make_shared<mtd::MockClientSurface>();
     auto native_window = platform->create_egl_native_window(mock_client_surface.get());
-    EXPECT_NE( *native_window, (EGLNativeWindowType) NULL);
+    EXPECT_NE(*native_window, (EGLNativeWindowType) NULL);
 }
 
 TEST_F(ClientPlatformTest, platform_creates_egl_native_display)

=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2013-01-02 15:57:13 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2013-03-24 23:57:21 +0000
@@ -78,7 +78,7 @@
             .WillByDefault(Return(native_display));
     }
 
-    MOCK_METHOD0(create_platform_depository, std::shared_ptr<mcl::ClientBufferDepository>());
+    MOCK_METHOD0(create_buffer_factory, std::shared_ptr<mcl::ClientBufferFactory>());
     MOCK_METHOD1(create_egl_native_window, std::shared_ptr<EGLNativeWindowType>(mcl::ClientSurface*));
     MOCK_METHOD0(create_egl_native_display, std::shared_ptr<EGLNativeDisplayType>());
 };