Merge lp:~cemil-azizoglu/mir/improve-raii-take2 into lp:mir

[Cemil Azizoglu]

Improve RAII take 2

Unmerged revisions

2354. By Cemil Azizoglu on 2015-03-25

Merge trunk

2353. By Cemil Azizoglu on 2015-03-03

Whitespace.

2352. By Cemil Azizoglu on 2015-03-03

Don't include buffer_bundle.h.

2351. By Cemil Azizoglu on 2015-03-03

Do away with compositor/snapshot handle subclasses.

2350. By Cemil Azizoglu on 2015-03-03

Merge trunk.

2349. By Cemil Azizoglu on 2015-03-03

Use release_callback.

2348. By Cemil Azizoglu on 2015-03-02

Address review comments (more to do).

2347. By Cemil Azizoglu on 2015-03-02

Remove auto var.

2346. By Cemil Azizoglu on 2015-03-02

const &

2345. By Cemil Azizoglu on 2015-03-02

Fix spacing.

=== modified file 'src/server/compositor/CMakeLists.txt'
--- src/server/compositor/CMakeLists.txt	2015-01-21 07:34:50 +0000
+++ src/server/compositor/CMakeLists.txt	2015-03-25 14:30:39 +0000
@@ -3,7 +3,6 @@
 
   default_display_buffer_compositor.cpp
   default_display_buffer_compositor_factory.cpp
-  temporary_buffers.cpp
   buffer_stream_factory.cpp
   buffer_stream_surfaces.cpp
   gl_renderer.cpp
@@ -16,6 +15,7 @@
   compositing_screencast.cpp
   timeout_frame_dropping_policy_factory.cpp
   buffer_queue.cpp
+  buffer_handle.cpp
   recently_used_cache.cpp
 )
 

=== modified file 'src/server/compositor/buffer_bundle.h'
--- src/server/compositor/buffer_bundle.h	2015-01-21 08:53:28 +0000
+++ src/server/compositor/buffer_bundle.h	2015-03-25 14:30:39 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013, 2015 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,
@@ -29,6 +29,7 @@
 
 namespace compositor
 {
+class BufferHandle;
 
 class BufferBundle
 {
@@ -48,11 +49,8 @@
      *                     collisions, all callers should determine user_id
      *                     in the same way (e.g. always use "this" pointer).
      */
-    virtual std::shared_ptr<graphics::Buffer>
-        compositor_acquire(void const* user_id) = 0;
-    virtual void compositor_release(std::shared_ptr<graphics::Buffer> const&) = 0;
-    virtual std::shared_ptr<graphics::Buffer> snapshot_acquire() = 0;
-    virtual void snapshot_release(std::shared_ptr<graphics::Buffer> const&) = 0;
+    virtual std::shared_ptr<BufferHandle> compositor_acquire(void const* user_id) = 0;
+    virtual std::shared_ptr<BufferHandle> snapshot_acquire() = 0;
 
     virtual graphics::BufferProperties properties() const = 0;
     virtual void allow_framedropping(bool dropping_allowed) = 0;

=== added file 'src/server/compositor/buffer_handle.cpp'
--- src/server/compositor/buffer_handle.cpp	1970-01-01 00:00:00 +0000
+++ src/server/compositor/buffer_handle.cpp	2015-03-25 14:30:39 +0000
@@ -0,0 +1,42 @@
+/*
+ * Copyright © 2015 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: Cemil Azizoglu <[e-mail]>
+ */
+
+#include "buffer_handle.h"
+
+namespace mc = mir::compositor;
+namespace mg = mir::graphics;
+
+mc::BufferHandle::BufferHandle(BufferBundle* bundle,
+                               std::shared_ptr<mg::Buffer> const& buffer,
+                               release_callback const& release)
+                               : buffer_bundle(bundle),
+                                 wrapped(buffer),
+                                 release_fn(release)
+{
+}
+
+mc::BufferHandle::~BufferHandle()
+{
+    if (release_fn)
+    	release_fn(wrapped.get());
+}
+
+std::shared_ptr<mg::Buffer> mc::BufferHandle::buffer()
+{
+    return wrapped;
+}

=== added file 'src/server/compositor/buffer_handle.h'
--- src/server/compositor/buffer_handle.h	1970-01-01 00:00:00 +0000
+++ src/server/compositor/buffer_handle.h	2015-03-25 14:30:39 +0000
@@ -0,0 +1,62 @@
+/*
+ * Copyright © 2015 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:
+ * Cemil Azizoglu <[e-mail]>
+ */
+
+#ifndef MIR_BUFFER_HANDLE_H_
+#define MIR_BUFFER_HANDLE_H_
+
+#include <memory>
+
+namespace mir
+{
+namespace graphics { class Buffer; }
+
+namespace compositor
+{
+
+class BufferBundle;
+
+typedef std::function<void(graphics::Buffer* buffer)> release_callback;
+
+class BufferHandle
+{
+public:
+    explicit BufferHandle(BufferBundle* bundle,
+        std::shared_ptr<graphics::Buffer> const& buffer,
+        release_callback const& release);
+
+    virtual ~BufferHandle();
+
+    std::shared_ptr<graphics::Buffer> buffer();
+
+    BufferHandle(BufferHandle&& other);
+    BufferHandle& operator=(BufferHandle&& other);
+
+private:
+    BufferHandle(BufferHandle const&) = delete;
+    BufferHandle& operator=(BufferHandle const&) = delete;
+
+    BufferBundle* buffer_bundle;
+    std::shared_ptr<graphics::Buffer> wrapped;
+    release_callback const release_fn;
+};
+
+}
+}
+
+#endif /* MIR_BUFFER_HANDLE_H_*/

=== modified file 'src/server/compositor/buffer_queue.cpp'
--- src/server/compositor/buffer_queue.cpp	2015-03-24 16:02:48 +0000
+++ src/server/compositor/buffer_queue.cpp	2015-03-25 14:30:39 +0000
@@ -19,6 +19,7 @@
 
 #include "mir/graphics/graphic_buffer_allocator.h"
 #include "mir/graphics/buffer_id.h"
+#include "buffer_handle.h"
 #include "mir/lockable_callback.h"
 
 #include <boost/throw_exception.hpp>
@@ -239,7 +240,7 @@
     ready_to_composite_queue.push_back(buffer);
 }
 
-std::shared_ptr<mg::Buffer>
+std::shared_ptr<mc::BufferHandle>
 mc::BufferQueue::compositor_acquire(void const* user_id)
 {
     std::unique_lock<decltype(guard)> lock(guard);
@@ -285,8 +286,27 @@
 
     buffers_sent_to_compositor.push_back(current_compositor_buffer);
 
-    std::shared_ptr<mg::Buffer> const acquired_buffer =
-        buffer_for(current_compositor_buffer, buffers);
+    std::shared_ptr<mc::BufferHandle> const acquired_buffer =
+        std::make_shared<mc::BufferHandle>(
+           this,
+           buffer_for(current_compositor_buffer, buffers),
+           [this](mg::Buffer* b)
+           {
+               std::unique_lock<decltype(guard)> lock(guard);
+
+               remove(b, buffers_sent_to_compositor);
+
+               /* Not ready to release it yet, other compositors still reference this buffer */
+               if (contains(b, buffers_sent_to_compositor))
+                   return;
+
+               if (nbuffers <= 1)
+                   return;
+
+               if (current_compositor_buffer != b)
+                   release(b, std::move(lock));
+           }
+        );
 
     if (buffer_to_release)
         release(buffer_to_release, std::move(lock));
@@ -294,44 +314,23 @@
     return acquired_buffer;
 }
 
-void mc::BufferQueue::compositor_release(std::shared_ptr<graphics::Buffer> const& buffer)
-{
-    std::unique_lock<decltype(guard)> lock(guard);
-
-    if (!remove(buffer.get(), buffers_sent_to_compositor))
-    {
-        BOOST_THROW_EXCEPTION(
-            std::logic_error("unexpected release: buffer was not given to compositor"));
-    }
-
-    /* Not ready to release it yet, other compositors still reference this buffer */
-    if (contains(buffer.get(), buffers_sent_to_compositor))
-        return;
-
-    if (nbuffers <= 1)
-        return;
-
-    if (current_compositor_buffer != buffer.get())
-        release(buffer.get(), std::move(lock));
-}
-
-std::shared_ptr<mg::Buffer> mc::BufferQueue::snapshot_acquire()
+std::shared_ptr<mc::BufferHandle> mc::BufferQueue::snapshot_acquire()
 {
     std::unique_lock<decltype(guard)> lock(guard);
     pending_snapshots.push_back(current_compositor_buffer);
-    return buffer_for(current_compositor_buffer, buffers);
-}
-
-void mc::BufferQueue::snapshot_release(std::shared_ptr<graphics::Buffer> const& buffer)
-{
-    std::unique_lock<std::mutex> lock(guard);
-    if (!remove(buffer.get(), pending_snapshots))
-    {
-        BOOST_THROW_EXCEPTION(
-            std::logic_error("unexpected release: no buffers were given to snapshotter"));
-    }
-
-    snapshot_released.notify_all();
+
+    return std::make_shared<mc::BufferHandle>(
+               this,
+               buffer_for(current_compositor_buffer, buffers),
+               [this](mg::Buffer* b)
+               {
+                   std::unique_lock<std::mutex> lock(guard);
+
+                   remove(b, pending_snapshots);
+
+                   snapshot_released.notify_all();
+               }
+           );
 }
 
 mg::BufferProperties mc::BufferQueue::properties() const

=== modified file 'src/server/compositor/buffer_queue.h'
--- src/server/compositor/buffer_queue.h	2015-03-24 16:02:48 +0000
+++ src/server/compositor/buffer_queue.h	2015-03-25 14:30:39 +0000
@@ -37,6 +37,8 @@
 namespace compositor
 {
 
+class BufferHandle;
+
 class BufferQueue : public BufferBundle
 {
 public:
@@ -49,10 +51,8 @@
 
     void client_acquire(Callback complete) override;
     void client_release(graphics::Buffer* buffer) override;
-    std::shared_ptr<graphics::Buffer> compositor_acquire(void const* user_id) override;
-    void compositor_release(std::shared_ptr<graphics::Buffer> const& buffer) override;
-    std::shared_ptr<graphics::Buffer> snapshot_acquire() override;
-    void snapshot_release(std::shared_ptr<graphics::Buffer> const& buffer) override;
+    std::shared_ptr<BufferHandle> compositor_acquire(void const* user_id) override;
+    std::shared_ptr<BufferHandle> snapshot_acquire() override;
 
     graphics::BufferProperties properties() const override;
     void allow_framedropping(bool dropping_allowed) override;

=== modified file 'src/server/compositor/buffer_stream_surfaces.cpp'
--- src/server/compositor/buffer_stream_surfaces.cpp	2015-01-21 08:53:28 +0000
+++ src/server/compositor/buffer_stream_surfaces.cpp	2015-03-25 14:30:39 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2012 Canonical Ltd.
+ * Copyright © 2012, 2015 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,
@@ -19,10 +19,9 @@
 
 #include "buffer_stream_surfaces.h"
 #include "buffer_bundle.h"
+#include "buffer_handle.h"
 #include "mir/graphics/buffer_properties.h"
 
-#include "temporary_buffers.h"
-
 namespace mc = mir::compositor;
 namespace mg = mir::graphics;
 namespace geom = mir::geometry;
@@ -40,13 +39,12 @@
 std::shared_ptr<mg::Buffer> mc::BufferStreamSurfaces::lock_compositor_buffer(
     void const* user_id)
 {
-    return std::make_shared<mc::TemporaryCompositorBuffer>(
-        buffer_bundle, user_id);
+    return buffer_bundle->compositor_acquire(user_id)->buffer();
 }
 
 std::shared_ptr<mg::Buffer> mc::BufferStreamSurfaces::lock_snapshot_buffer()
 {
-    return std::make_shared<mc::TemporarySnapshotBuffer>(buffer_bundle);
+    return buffer_bundle->snapshot_acquire()->buffer();
 }
 
 void mc::BufferStreamSurfaces::acquire_client_buffer(

=== removed file 'src/server/compositor/temporary_buffers.cpp'
--- src/server/compositor/temporary_buffers.cpp	2015-03-24 16:02:48 +0000
+++ src/server/compositor/temporary_buffers.cpp	1970-01-01 00:00:00 +0000
@@ -1,92 +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 "buffer_bundle.h"
-#include "temporary_buffers.h"
-
-#include <boost/throw_exception.hpp>
-#include <stdexcept>
-
-namespace mc=mir::compositor;
-namespace mg=mir::graphics;
-namespace geom=mir::geometry;
-
-mc::TemporaryBuffer::TemporaryBuffer(std::shared_ptr<mg::Buffer> const& real_buffer)
-    : buffer(real_buffer)
-{
-}
-
-mc::TemporaryCompositorBuffer::TemporaryCompositorBuffer(
-    std::shared_ptr<BufferBundle> const& bun, void const* user_id)
-    : TemporaryBuffer(bun->compositor_acquire(user_id)),
-      bundle(bun)
-{
-}
-
-mc::TemporaryCompositorBuffer::~TemporaryCompositorBuffer()
-{
-    bundle->compositor_release(buffer);
-}
-
-mc::TemporarySnapshotBuffer::TemporarySnapshotBuffer(
-    std::shared_ptr<BufferBundle> const& bun)
-    : TemporaryBuffer(bun->snapshot_acquire()),
-      bundle(bun)
-{
-}
-
-mc::TemporarySnapshotBuffer::~TemporarySnapshotBuffer()
-{
-    bundle->snapshot_release(buffer);
-}
-
-geom::Size mc::TemporaryBuffer::size() const
-{
-    return buffer->size();
-}
-
-geom::Stride mc::TemporaryBuffer::stride() const
-{
-    return buffer->stride();
-}
-
-MirPixelFormat mc::TemporaryBuffer::pixel_format() const
-{
-    return buffer->pixel_format();
-}
-
-mg::BufferID mc::TemporaryBuffer::id() const
-{
-    return buffer->id();
-}
-
-void mc::TemporaryBuffer::gl_bind_to_texture()
-{
-    buffer->gl_bind_to_texture();
-}
-
-std::shared_ptr<mg::NativeBuffer> mc::TemporaryBuffer::native_buffer_handle() const
-{
-    return buffer->native_buffer_handle();
-}
-
-void mc::TemporaryBuffer::write(unsigned char const*, size_t)
-{
-    BOOST_THROW_EXCEPTION(
-        std::runtime_error("Write to temporary buffer snapshot is ill advised and indicates programmer error"));
-}

=== removed file 'src/server/compositor/temporary_buffers.h'
--- src/server/compositor/temporary_buffers.h	2015-03-24 16:02:48 +0000
+++ src/server/compositor/temporary_buffers.h	1970-01-01 00:00:00 +0000
@@ -1,76 +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]>
- */
-
-#ifndef MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_
-#define MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_
-
-#include "mir/graphics/buffer.h"
-#include "mir/graphics/buffer_id.h"
-
-namespace mg = mir::graphics;
-
-namespace mir
-{
-namespace compositor
-{
-
-class BufferBundle;
-class BackBufferStrategy;
-
-class TemporaryBuffer : public mg::Buffer
-{
-public:
-    geometry::Size size() const override;
-    geometry::Stride stride() const override;
-    MirPixelFormat pixel_format() const override;
-    mg::BufferID id() const override;
-    void gl_bind_to_texture() override;
-    std::shared_ptr<mg::NativeBuffer> native_buffer_handle() const override;
-    void write (unsigned char const* data, size_t size) override;
-
-protected:
-    explicit TemporaryBuffer(std::shared_ptr<mg::Buffer> const& real_buffer);
-    std::shared_ptr<mg::Buffer> const buffer;
-};
-
-class TemporaryCompositorBuffer : public TemporaryBuffer
-{
-public:
-    explicit TemporaryCompositorBuffer(
-        std::shared_ptr<BufferBundle> const& bun, void const* user_id);
-    ~TemporaryCompositorBuffer();
-
-private:
-    std::shared_ptr<BufferBundle> const bundle;
-};
-
-class TemporarySnapshotBuffer : public TemporaryBuffer
-{
-public:
-    explicit TemporarySnapshotBuffer(
-        std::shared_ptr<BufferBundle> const& bun);
-    ~TemporarySnapshotBuffer();
-
-private:
-    std::shared_ptr<BufferBundle> const bundle;
-};
-
-}
-}
-
-#endif /* MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_ */

=== modified file 'tests/include/mir_test_doubles/mock_buffer_bundle.h'
--- tests/include/mir_test_doubles/mock_buffer_bundle.h	2015-01-21 08:53:28 +0000
+++ tests/include/mir_test_doubles/mock_buffer_bundle.h	2015-03-25 14:30:39 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013, 2015 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,
@@ -39,10 +39,10 @@
 
     MOCK_METHOD1(client_acquire,     void(std::function<void(graphics::Buffer*)>));
     MOCK_METHOD1(client_release,     void(graphics::Buffer*));
-    MOCK_METHOD1(compositor_acquire, std::shared_ptr<graphics::Buffer>(void const*));
-    MOCK_METHOD1(compositor_release, void(std::shared_ptr<graphics::Buffer> const&));
-    MOCK_METHOD0(snapshot_acquire, std::shared_ptr<graphics::Buffer>());
-    MOCK_METHOD1(snapshot_release, void(std::shared_ptr<graphics::Buffer> const&));
+    MOCK_METHOD1(compositor_acquire, std::shared_ptr<compositor::BufferHandle>(void const*));
+    MOCK_METHOD1(compositor_release, void(graphics::Buffer* const));
+    MOCK_METHOD0(snapshot_acquire, std::shared_ptr<compositor::BufferHandle>());
+    MOCK_METHOD1(snapshot_release, void(graphics::Buffer* const));
     MOCK_METHOD1(allow_framedropping, void(bool));
     MOCK_CONST_METHOD0(properties, graphics::BufferProperties());
     MOCK_METHOD0(force_client_abort, void());

=== modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
--- tests/integration-tests/compositor/test_swapping_swappers.cpp	2015-01-21 07:34:50 +0000
+++ tests/integration-tests/compositor/test_swapping_swappers.cpp	2015-03-25 14:30:39 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013, 2015 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
@@ -105,7 +105,6 @@
                     {
                         auto b = switching_bundle->compositor_acquire(0);
                         std::this_thread::yield();
-                        switching_bundle->compositor_release(b);
                     }
                 });
 
@@ -149,7 +148,6 @@
                     {
                         auto b = switching_bundle->compositor_acquire(0);
                         std::this_thread::yield();
-                        switching_bundle->compositor_release(b);
                     }
                 });
 

=== modified file 'tests/unit-tests/compositor/CMakeLists.txt'
--- tests/unit-tests/compositor/CMakeLists.txt	2015-03-24 16:02:48 +0000
+++ tests/unit-tests/compositor/CMakeLists.txt	2015-03-25 14:30:39 +0000
@@ -1,7 +1,7 @@
 list(APPEND UNIT_TEST_SOURCES
   ${CMAKE_CURRENT_SOURCE_DIR}/test_default_display_buffer_compositor.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer_stream.cpp
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_temporary_buffers.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer_handle.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_multi_threaded_compositor.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer_queue.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gl_renderer.cpp

=== added file 'tests/unit-tests/compositor/test_buffer_handle.cpp'
--- tests/unit-tests/compositor/test_buffer_handle.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/compositor/test_buffer_handle.cpp	2015-03-25 14:30:39 +0000
@@ -0,0 +1,65 @@
+/*
+ * Copyright © 2015 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: Cemil Azizoglu <[e-mail]>
+ */
+
+#include "mir_test_doubles/stub_buffer.h"
+#include "mir_test_doubles/mock_buffer_bundle.h"
+#include "src/server/compositor/buffer_handle.h"
+
+#include <gtest/gtest.h>
+
+namespace mc = mir::compositor;
+namespace mtd = mir::test::doubles;
+
+class BufferHandleTest : public ::testing::Test
+{
+protected:
+    BufferHandleTest()
+    {
+        mock_buffer = std::make_shared<mtd::StubBuffer>();
+        mock_bundle = std::make_shared<mtd::MockBufferBundle>();
+    }
+
+    std::shared_ptr<mtd::StubBuffer> mock_buffer;
+    std::shared_ptr<mtd::MockBufferBundle> mock_bundle;
+};
+
+TEST_F(BufferHandleTest, can_contain_handout_and_release_compositor_buffer)
+{
+    using namespace testing;
+
+    std::shared_ptr<mc::BufferHandle> buffer_handle =
+        std::make_shared<mc::CompositorBufferHandle>(mock_bundle.get(), mock_buffer);
+
+    EXPECT_CALL(*mock_bundle, compositor_release(_))
+        .Times(1);
+
+    ASSERT_EQ(buffer_handle->buffer(), mock_buffer);
+}
+
+TEST_F(BufferHandleTest, can_contain_handout_and_release_snapshot_buffer)
+{
+    using namespace testing;
+
+    std::shared_ptr<mc::BufferHandle> buffer_handle =
+        std::make_shared<mc::SnapshotBufferHandle>(mock_bundle.get(), mock_buffer);
+
+    EXPECT_CALL(*mock_bundle, snapshot_release(_))
+        .Times(1);
+
+    ASSERT_EQ(buffer_handle->buffer(), mock_buffer);
+}

=== modified file 'tests/unit-tests/compositor/test_buffer_queue.cpp'
--- tests/unit-tests/compositor/test_buffer_queue.cpp	2015-03-24 16:02:48 +0000
+++ tests/unit-tests/compositor/test_buffer_queue.cpp	2015-03-25 14:30:39 +0000
@@ -18,6 +18,7 @@
  */
 
 #include "src/server/compositor/buffer_queue.h"
+#include "src/server/compositor/buffer_handle.h"
 #include "mir_test_doubles/stub_buffer_allocator.h"
 #include "mir_test_doubles/stub_buffer.h"
 #include "mir_test_doubles/stub_frame_dropping_policy_factory.h"
@@ -155,7 +156,7 @@
 {
    while (!done)
    {
-       bundle.compositor_release(bundle.compositor_acquire(nullptr));
+       bundle.compositor_acquire(nullptr);
        std::this_thread::yield();
    }
 }
@@ -165,7 +166,7 @@
 {
    while (!done)
    {
-       bundle.snapshot_release(bundle.snapshot_acquire());
+       bundle.snapshot_acquire();
        std::this_thread::yield();
    }
 }
@@ -212,18 +213,18 @@
     auto next_request = client_acquire_async(*q);
     EXPECT_THAT(next_request->has_acquired_buffer(), Eq(false));
 
-    auto comp_buffer = q->compositor_acquire(this);
-    auto client_id = handle->id();
-
-    /* Client and compositor always share the same buffer */
-    EXPECT_THAT(client_id, Eq(comp_buffer->id()));
-
-    EXPECT_NO_THROW(handle->release_buffer());
-    EXPECT_NO_THROW(q->compositor_release(comp_buffer));
+    {
+        auto comp_buffer_handle = q->compositor_acquire(this);
+        auto client_id = handle->id();
+
+        /* Client and compositor always share the same buffer */
+        EXPECT_THAT(client_id, Eq(comp_buffer_handle->buffer()->id()));
+
+        EXPECT_NO_THROW(handle->release_buffer());
+    }
 
     /* Simulate a composite pass */
-    comp_buffer = q->compositor_acquire(this);
-    q->compositor_release(comp_buffer);
+    q->compositor_acquire(this);
 
     /* The request should now be fullfilled after compositor
      * released the buffer
@@ -244,17 +245,17 @@
     auto buffer = handle->buffer();
     ASSERT_THAT(buffer->size(), Eq(expect_size));
 
-    /* Client and compositor share the same buffer so
-     * expect the new size
-     */
-    std::shared_ptr<mg::Buffer> comp_buffer;
-    ASSERT_NO_THROW(comp_buffer = q.compositor_acquire(this));
+    {
+        /* Client and compositor share the same buffer so
+         * expect the new size
+         */
+        ASSERT_NO_THROW(q.compositor_acquire(this)->buffer());
 
-    EXPECT_THAT(buffer->size(), Eq(expect_size));
-    EXPECT_NO_THROW(q.compositor_release(comp_buffer));
+        EXPECT_THAT(buffer->size(), Eq(expect_size));
+    }
 
     EXPECT_NO_THROW(handle->release_buffer());
-    EXPECT_NO_THROW(q.compositor_release(q.compositor_acquire(this)));
+    EXPECT_NO_THROW(q.compositor_acquire(this));
 }
 
 TEST_F(BufferQueueTest, framedropping_is_disabled_by_default)
@@ -320,7 +321,7 @@
     ASSERT_THAT(handle1->has_acquired_buffer(), Eq(false));
 
     for (int i = 0; i < nbuffers + 1; ++i)
-        q.compositor_release(q.compositor_acquire(this));
+        q.compositor_acquire(this);
 
     EXPECT_THAT(handle1->has_acquired_buffer(), Eq(true));
     EXPECT_THAT(handle2->has_acquired_buffer(), Eq(true));
@@ -346,15 +347,18 @@
             auto client_id = handle->id();
             handle->release_buffer();
 
-            auto comp_buffer = q.compositor_acquire(main_compositor);
-            auto composited_id = comp_buffer->id();
-            q.compositor_release(comp_buffer);
+            mg::BufferID composited_id;
+            {
+                auto comp_buffer_handle = q.compositor_acquire(main_compositor);
+                composited_id = comp_buffer_handle->buffer()->id();
+            }
 
             EXPECT_THAT(composited_id, Eq(client_id));
 
-            comp_buffer = q.compositor_acquire(second_compositor);
-            EXPECT_THAT(composited_id, Eq(comp_buffer->id()));
-            q.compositor_release(comp_buffer);
+            {
+                auto comp_buffer_handle = q.compositor_acquire(second_compositor);
+                EXPECT_THAT(composited_id, Eq(comp_buffer_handle->buffer()->id()));
+            }
         }
     }
 }
@@ -389,9 +393,8 @@
     ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
     handle->release_buffer();
 
-    auto comp_buffer = q.compositor_acquire(this);
-    EXPECT_THAT(client_id, Eq(comp_buffer->id()));
-    q.compositor_release(comp_buffer);
+    auto comp_buffer_handle = q.compositor_acquire(this);
+    EXPECT_THAT(client_id, Eq(comp_buffer_handle->buffer()->id()));
 }
 
 TEST_F(BufferQueueTest, throws_on_out_of_order_client_release)
@@ -461,9 +464,8 @@
         auto client_id = handle->id();
         ASSERT_NO_THROW(handle->release_buffer());
 
-        auto comp_buffer = q.compositor_acquire(this);
-        EXPECT_THAT(client_id, Eq(comp_buffer->id()));
-        EXPECT_NO_THROW(q.compositor_release(comp_buffer));
+        auto comp_buffer_handle = q.compositor_acquire(this);
+        EXPECT_THAT(client_id, Eq(comp_buffer_handle->buffer()->id()));
     }
 }
 
@@ -482,9 +484,8 @@
         for (int monitor = 0; monitor < 10; monitor++)
         {
             void const* user_id = reinterpret_cast<void const*>(monitor);
-            auto comp_buffer = q.compositor_acquire(user_id);
-            EXPECT_THAT(client_id, Eq(comp_buffer->id()));
-            q.compositor_release(comp_buffer);
+            auto comp_buffer_handle = q.compositor_acquire(user_id);
+            EXPECT_THAT(client_id, Eq(comp_buffer_handle->buffer()->id()));
         }
     }
 }
@@ -511,18 +512,16 @@
         for (int monitor = 0; monitor < 10; monitor++)
         {
             void const* user_id = reinterpret_cast<void const*>(monitor);
-            auto comp_buffer = q.compositor_acquire(user_id);
-            ASSERT_EQ(client_id1, comp_buffer->id());
-            q.compositor_release(comp_buffer);
+            auto comp_buffer_handle = q.compositor_acquire(user_id);
+            ASSERT_EQ(client_id1, comp_buffer_handle->buffer()->id());
         }
 
         // Still many monitors... verify they all get the second frame.
         for (int monitor = 0; monitor < 10; monitor++)
         {
             void const* user_id = reinterpret_cast<void const*>(monitor);
-            auto comp_buffer = q.compositor_acquire(user_id);
-            ASSERT_EQ(client_id2, comp_buffer->id());
-            q.compositor_release(comp_buffer);
+            auto comp_buffer_handle = q.compositor_acquire(user_id);
+            ASSERT_EQ(client_id2, comp_buffer_handle->buffer()->id());
         }
     }
 }
@@ -553,9 +552,8 @@
 
             for (auto const& client_id : client_release_sequence)
             {
-                auto comp_buffer = q.compositor_acquire(this);
-                EXPECT_THAT(client_id, Eq(comp_buffer->id()));
-                q.compositor_release(comp_buffer);
+                auto comp_buffer_handle = q.compositor_acquire(this);
+                EXPECT_THAT(client_id, Eq(comp_buffer_handle->buffer()->id()));
             }
         }
     }
@@ -568,10 +566,7 @@
         mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
 
         for (int i = 0; i < 100; i++)
-        {
-            auto buffer = q.compositor_acquire(this);
-            q.compositor_release(buffer);
-        }
+            q.compositor_acquire(this);
     }
 }
 
@@ -600,10 +595,11 @@
         {
             while (!done)
             {
-                std::shared_ptr<mg::Buffer> buffer;
-                EXPECT_NO_THROW(buffer = q.compositor_acquire(this));
-                EXPECT_THAT(buffer, Ne(nullptr));
-                EXPECT_NO_THROW(q.compositor_release(buffer));
+                {
+                    std::shared_ptr<mg::Buffer> buffer;
+                    EXPECT_NO_THROW(buffer = q.compositor_acquire(this)->buffer());
+                    EXPECT_THAT(buffer, Ne(nullptr));
+                }
                 std::this_thread::yield();
             }
         });
@@ -634,30 +630,13 @@
             for (int monitor_id = 0; monitor_id < 10; monitor_id++)
             {
                 void const* user_id = reinterpret_cast<void const*>(monitor_id);
-                auto buffer = q.compositor_acquire(user_id);
-                ASSERT_THAT(buffer.get(), Eq(last_client_acquired_buffer));
-                q.compositor_release(buffer);
+                auto buffer_handle = q.compositor_acquire(user_id);
+                ASSERT_THAT(buffer_handle->buffer().get(), Eq(last_client_acquired_buffer));
             }
         }
     }
 }
 
-TEST_F(BufferQueueTest, compositor_release_verifies_parameter)
-{
-    for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
-    {
-        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
-
-        auto handle = client_acquire_async(q);
-        ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
-        handle->release_buffer();
-
-        auto comp_buffer = q.compositor_acquire(this);
-        q.compositor_release(comp_buffer);
-        EXPECT_THROW(q.compositor_release(comp_buffer), std::logic_error);
-    }
-}
-
 /* Regression test for LP#1270964 */
 TEST_F(BufferQueueTest, compositor_client_interleaved)
 {
@@ -673,12 +652,11 @@
     ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
 
     // in the original bug, compositor would be given the wrong buffer here
-    auto compositor_buffer = q.compositor_acquire(this);
+    auto compositor_buffer_handle = q.compositor_acquire(this);
 
-    EXPECT_THAT(compositor_buffer->id(), Eq(first_ready_buffer_id));
+    EXPECT_THAT(compositor_buffer_handle->buffer()->id(), Eq(first_ready_buffer_id));
 
     handle->release_buffer();
-    q.compositor_release(compositor_buffer);
 }
 
 TEST_F(BufferQueueTest, overlapping_compositors_get_different_frames)
@@ -688,7 +666,7 @@
     {
         mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
 
-        std::shared_ptr<mg::Buffer> compositor[2];
+        std::shared_ptr<mc::BufferHandle> compositor[2];
 
         auto handle = client_acquire_async(q);
         ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -703,19 +681,16 @@
         for (int i = 0; i < 20; i++)
         {
             // Two compositors acquired, and they're always different...
-            ASSERT_THAT(compositor[0]->id(), Ne(compositor[1]->id()));
+            ASSERT_THAT(compositor[0]->buffer()->id(), Ne(compositor[1]->buffer()->id()));
 
             // One of the compositors (the oldest one) gets a new buffer...
             int oldest = i & 1;
-            q.compositor_release(compositor[oldest]);
+            compositor[oldest].reset();
             auto handle = client_acquire_async(q);
             ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
             handle->release_buffer();
             compositor[oldest] = q.compositor_acquire(this);
         }
-
-        q.compositor_release(compositor[0]);
-        q.compositor_release(compositor[1]);
     }
 }
 
@@ -725,11 +700,10 @@
     {
         mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
 
-        auto comp_buffer = q.compositor_acquire(this);
-        auto snapshot = q.snapshot_acquire();
-        EXPECT_THAT(snapshot->id(), Eq(comp_buffer->id()));
-        q.compositor_release(comp_buffer);
-        q.snapshot_release(snapshot);
+        auto comp_buffer_handle = q.compositor_acquire(this);
+        auto snapshot_handle = q.snapshot_acquire();
+        EXPECT_THAT(snapshot_handle->buffer()->id(),
+                    Eq(comp_buffer_handle->buffer()->id()));
     }
 }
 
@@ -746,9 +720,10 @@
 {
     mc::BufferQueue q(1, allocator, basic_properties, policy_factory);
 
-    auto snapshot = q.snapshot_acquire();
-    q.drop_client_requests();
-    q.snapshot_release(snapshot);
+    {
+        auto snapshot = q.snapshot_acquire();
+        q.drop_client_requests();
+    }
 
     auto client = client_acquire_async(q);
     ASSERT_FALSE(client->has_acquired_buffer());
@@ -761,36 +736,9 @@
         mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
         int const num_snapshots = 100;
 
-        std::shared_ptr<mg::Buffer> buf[num_snapshots];
+        std::shared_ptr<mc::BufferHandle> buf[num_snapshots];
         for (int i = 0; i < num_snapshots; i++)
             buf[i] = q.snapshot_acquire();
-
-        for (int i = 0; i < num_snapshots; i++)
-            q.snapshot_release(buf[i]);
-    }
-}
-
-TEST_F(BufferQueueTest, snapshot_release_verifies_parameter)
-{
-    for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
-    {
-        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
-
-        auto handle = client_acquire_async(q);
-        ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
-        handle->release_buffer();
-
-        auto comp_buffer = q.compositor_acquire(this);
-        EXPECT_THROW(q.snapshot_release(comp_buffer), std::logic_error);
-
-        handle = client_acquire_async(q);
-        ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
-        auto snapshot = q.snapshot_acquire();
-
-        EXPECT_THAT(snapshot->id(), Eq(comp_buffer->id()));
-        EXPECT_THAT(snapshot->id(), Ne(handle->id()));
-        EXPECT_NO_THROW(q.snapshot_release(snapshot));
-        EXPECT_THROW(q.snapshot_release(snapshot), std::logic_error);
     }
 }
 
@@ -892,8 +840,7 @@
             {
                 std::this_thread::sleep_for(std::chrono::milliseconds(10));
 
-                auto buf = q.compositor_acquire(this);
-                q.compositor_release(buf);
+                q.compositor_acquire(this);
 
                 if (frame == compose_frames)
                 {
@@ -950,8 +897,7 @@
             {
                 std::this_thread::sleep_for(frame_time);
                 sync.lock();
-                auto buf = q.compositor_acquire(this);
-                q.compositor_release(buf);
+                q.compositor_acquire(this);
                 sync.unlock();
 
                 if (frame == compose_frames)
@@ -1007,9 +953,8 @@
                 ASSERT_THAT(expect_size, Eq(buffer->size()));
                 handle->release_buffer();
 
-                auto comp_buffer = q.compositor_acquire(this);
-                ASSERT_THAT(expect_size, Eq(comp_buffer->size()));
-                q.compositor_release(comp_buffer);
+                auto comp_buffer_handle = q.compositor_acquire(this);
+                ASSERT_THAT(expect_size, Eq(comp_buffer_handle->buffer()->size()));
             }
         }
     }
@@ -1065,25 +1010,22 @@
             width += dx;
             height += dy;
 
-            auto buffer = q.compositor_acquire(this);
+            auto buffer_handle = q.compositor_acquire(this);
 
             // Verify the compositor gets resized buffers, eventually
-            ASSERT_THAT(buffer->size(), Eq(expect_size));
+            ASSERT_THAT(buffer_handle->buffer()->size(), Eq(expect_size));
 
             // Verify the compositor gets buffers with *contents*, ie. that
             // they have not been resized prematurely and are empty.
-            ASSERT_THAT(history[consume], Eq(buffer->id()));
-
-            q.compositor_release(buffer);
+            ASSERT_THAT(history[consume], Eq(buffer_handle->buffer()->id()));
         }
 
         // Verify the final buffer size sticks
         const geom::Size final_size{width - dx, height - dy};
         for (int unchanging = 0; unchanging < 100; ++unchanging)
         {
-            auto buffer = q.compositor_acquire(this);
-            ASSERT_THAT(buffer->size(), Eq(final_size));
-            q.compositor_release(buffer);
+            auto buffer_handle = q.compositor_acquire(this);
+            ASSERT_THAT(buffer_handle->buffer()->size(), Eq(final_size));
         }
     }
 }
@@ -1103,7 +1045,7 @@
 
         // Start rendering one (don't finish)
         auto d = q.compositor_acquire(nullptr);
-        ASSERT_EQ(first, d.get());
+        ASSERT_EQ(first, d->buffer().get());
 
         auto second = client_acquire_sync(q);
         q.client_release(second);
@@ -1121,8 +1063,6 @@
         // will produce another frame.
         if (end->has_acquired_buffer())
             ASSERT_NE(second, end->buffer());
-
-        q.compositor_release(d);
     }
 }
 
@@ -1151,8 +1091,8 @@
         for (int n = 0; n < nbuffers-1; ++n)
         {
             auto c = q.compositor_acquire(nullptr);
-            compositing.push_back(c);
-            ASSERT_EQ(order[n], c.get());
+            compositing.push_back(c->buffer());
+            ASSERT_EQ(order[n], c->buffer().get());
         }
 
         // Ensure it's not the newest frame that gets dropped to satisfy the
@@ -1249,12 +1189,12 @@
     auto const handle = client_acquire_async(q);
     EXPECT_THAT(handle->has_acquired_buffer(), Eq(false));
 
-    auto buf = q.compositor_acquire(this);
-    q.compositor_release(buf);
+    q.compositor_acquire(this);
 
-    buf = q.compositor_acquire(this);
-    EXPECT_THAT(buf->size(), Eq(new_size));
-    q.compositor_release(buf);
+    {
+        auto buf_handle = q.compositor_acquire(this);
+        EXPECT_THAT(buf_handle->buffer()->size(), Eq(new_size));
+    }
 }
 
 TEST_F(BufferQueueTest, double_buffered_client_is_not_blocked_prematurely)
@@ -1268,19 +1208,19 @@
     q.client_release(client_acquire_sync(q));
     auto b = q.compositor_acquire(this);
 
-    ASSERT_NE(a.get(), b.get());
+    ASSERT_NE(a->buffer(), b->buffer());
 
-    q.compositor_release(a);
+    a.reset();
     q.client_release(client_acquire_sync(q));
 
-    q.compositor_release(b);
+    b.reset();
 
     /*
      * Update to the original test case; This additional compositor acquire
      * represents the fixing of LP: #1395581 in the compositor logic.
      */
     if (q.buffers_ready_for_compositor(this))
-        q.compositor_release(q.compositor_acquire(this));
+        q.compositor_acquire(this);
 
     auto handle = client_acquire_async(q);
     // With the fix, a buffer will be available instantaneously:
@@ -1310,27 +1250,27 @@
     
         ASSERT_NE(a.get(), b.get());
     
-        q.compositor_release(a);
+        a.reset();
 
         auto w = client_acquire_async(q);
         ASSERT_TRUE(w->has_acquired_buffer());
         w->release_buffer();
     
-        q.compositor_release(b);
+        b.reset();
 
         /*
          * Update to the original test case; This additional compositor acquire
          * represents the fixing of LP: #1395581 in the compositor logic.
          */
         if (q.buffers_ready_for_compositor(this))
-            q.compositor_release(q.compositor_acquire(this));
+            q.compositor_acquire(this);
 
         auto z = client_acquire_async(q);
         ASSERT_TRUE(z->has_acquired_buffer());
         z->release_buffer();
 
-        q.compositor_release(q.compositor_acquire(this));
-        q.compositor_release(q.compositor_acquire(this));
+        q.compositor_acquire(this);
+        q.compositor_acquire(this);
     }
 }
 
@@ -1353,11 +1293,11 @@
         auto b = q.compositor_acquire(this);
     
         // Release frame 1
-        q.compositor_release(a);
+        a.reset();
         // Produce frame 3
         q.client_release(client_acquire_sync(q));
         // Release frame 2
-        q.compositor_release(b);
+        b.reset();
     
         // Verify frame 3 is ready for the first compositor
         ASSERT_THAT(q.buffers_ready_for_compositor(this), Ge(1));
@@ -1366,8 +1306,6 @@
         // Verify frame 3 is ready for a second compositor
         int const that = 0;
         ASSERT_THAT(q.buffers_ready_for_compositor(&that), Ge(1));
-
-        q.compositor_release(c);
     }
 }
 
@@ -1394,7 +1332,7 @@
         for (int m = 0; m < nmonitors; ++m)
         {
             ASSERT_EQ(1, q.buffers_ready_for_compositor(&monitor[m]));
-            q.compositor_release(q.compositor_acquire(&monitor[m]));
+            q.compositor_acquire(&monitor[m]);
             ASSERT_EQ(0, q.buffers_ready_for_compositor(&monitor[m]));
         }
     }
@@ -1410,11 +1348,11 @@
     mc::BufferQueue q{nbuffers, allocator, basic_properties, policy_factory};
     q.allow_framedropping(true);
 
-    std::vector<std::shared_ptr<mg::Buffer>> buffers;
+    std::vector<std::shared_ptr<mc::BufferHandle>> buffer_handles;
 
     /* The client can never own this acquired buffer */
-    auto comp_buffer = q.compositor_acquire(this);
-    buffers.push_back(comp_buffer);
+    auto comp_buffer_handle = q.compositor_acquire(this);
+    buffer_handles.push_back(comp_buffer_handle);
 
     /* Let client release all possible buffers so they go into
      * the ready queue
@@ -1424,14 +1362,14 @@
         auto handle = client_acquire_async(q);
         ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
         /* Check the client never got the compositor buffer acquired above */
-        ASSERT_THAT(handle->id(), Ne(comp_buffer->id()));
+        ASSERT_THAT(handle->id(), Ne(comp_buffer_handle->buffer()->id()));
         handle->release_buffer();
     }
 
     /* Let the compositor acquire all ready buffers */
     for (int i = 0; i < nbuffers; ++i)
     {
-        buffers.push_back(q.compositor_acquire(this));
+    	buffer_handles.push_back(q.compositor_acquire(this));
     }
 
     /* At this point the queue has 0 free buffers and 0 ready buffers
@@ -1444,10 +1382,9 @@
     if (!handle->has_acquired_buffer())
     {
         /* Release compositor buffers so that the client can get one */
-        for (auto const& buffer : buffers)
-        {
-            q.compositor_release(buffer);
-        }
+        for (auto& buf_handle : buffer_handles)
+        	buf_handle.reset();
+
         EXPECT_THAT(handle->has_acquired_buffer(), Eq(true));
     }
 }
@@ -1470,7 +1407,7 @@
         {
             while (!done)
             {
-                auto buffer = q.compositor_acquire(this);
+                auto buffer_handle = q.compositor_acquire(this);
 
                 {
                     std::unique_lock<std::mutex> lock(client_buffer_guard);
@@ -1480,12 +1417,11 @@
                         time_for_client_to_acquire,
                         [&]()->bool{ return client_buffer; }))
                     {
-                        ASSERT_THAT(buffer->id(), Ne(client_buffer->id()));
+                        ASSERT_THAT(buffer_handle->buffer()->id(), Ne(client_buffer->id()));
                     }
                 }
 
                 std::this_thread::yield();
-                q.compositor_release(buffer);
             }
         });
 
@@ -1521,23 +1457,24 @@
             ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
 
             auto client_id = handle->id();
-            std::vector<std::shared_ptr<mg::Buffer>> buffers;
+            std::vector<std::shared_ptr<mc::BufferHandle>> buffer_handles;
             for (int j = 0; j < nbuffers; j++)
             {
-                auto buffer = q.compositor_acquire(this);
-                ASSERT_THAT(client_id, Ne(buffer->id()));
-                buffers.push_back(buffer);
+                auto buffer_handle = q.compositor_acquire(this);
+                ASSERT_THAT(client_id, Ne(buffer_handle->buffer()->id()));
+                buffer_handles.push_back(buffer_handle);
             }
 
-            for (auto const& buffer: buffers)
-                q.compositor_release(buffer);
+            for (auto& buf_handle: buffer_handles)
+                buf_handle.reset();
 
             handle->release_buffer();
 
-            /* Flush out one ready buffer */
-            auto buffer = q.compositor_acquire(this);
-            ASSERT_THAT(client_id, Eq(buffer->id()));
-            q.compositor_release(buffer);
+            {
+                /* Flush out one ready buffer */
+                auto buffer_handle = q.compositor_acquire(this);
+                ASSERT_THAT(client_id, Eq(buffer_handle->buffer()->id()));
+            }
         }
     }
 }
@@ -1569,11 +1506,9 @@
 
         /* Have a second compositor advance the current compositor buffer at least twice */
         for (int acquires = 0; acquires < nbuffers; ++acquires)
-        {
-            auto comp_buffer = q.compositor_acquire(second_compositor);
-            q.compositor_release(comp_buffer);
-        }
-        q.compositor_release(comp_buffer1);
+            q.compositor_acquire(second_compositor);
+
+        comp_buffer1.reset();
 
         /* An async client should still be able to cycle through all the available buffers */
         std::atomic<bool> done(false);
@@ -1666,8 +1601,7 @@
             q.client_release(client_acquire_sync(q));
         }};
 
-    auto b = q.compositor_acquire(this);
-    q.compositor_release(b);
+    q.compositor_acquire(this);
 }
 
 TEST_F(BufferQueueTest, first_user_is_recorded)
@@ -1676,9 +1610,10 @@
     {
         mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
 
-        auto comp = q.compositor_acquire(this);
-        EXPECT_TRUE(q.is_a_current_buffer_user(this));
-        q.compositor_release(comp);
+        {
+            auto comp = q.compositor_acquire(this);
+            EXPECT_TRUE(q.is_a_current_buffer_user(this));
+        }
     }
 }
 
@@ -1708,12 +1643,15 @@
 
     q.drop_old_buffers();
 
-    auto comp = q.compositor_acquire(this);
-    ASSERT_THAT(comp->id(), Eq(handle2->id()));
-    q.compositor_release(comp);
+    {
+        auto buffer_handle = q.compositor_acquire(this);
+        ASSERT_THAT(buffer_handle->buffer()->id(), Eq(handle2->id()));
+    }
 
-    comp = q.compositor_acquire(this);
-    ASSERT_THAT(comp->id(), Eq(handle2->id()));
+    {
+        auto buffer_handle = q.compositor_acquire(this);
+        ASSERT_THAT(buffer_handle->buffer()->id(), Eq(handle2->id()));
+    }
 }
 
 TEST_F(BufferQueueTest, gives_new_compositor_the_newest_buffer_after_dropping_old_buffers)
@@ -1727,9 +1665,10 @@
     ASSERT_THAT(handle1->has_acquired_buffer(), Eq(true));
     handle1->release_buffer();
 
-    auto comp = q.compositor_acquire(this);
-    ASSERT_THAT(comp->id(), Eq(handle1->id()));
-    q.compositor_release(comp);
+    {
+        auto buffer_handle = q.compositor_acquire(this);
+        ASSERT_THAT(buffer_handle->buffer()->id(), Eq(handle1->id()));
+    }
 
     auto handle2 = client_acquire_async(q);
     ASSERT_THAT(handle2->has_acquired_buffer(), Eq(true));
@@ -1737,6 +1676,6 @@
 
     q.drop_old_buffers();
 
-    auto comp2 = q.compositor_acquire(new_compositor_id);
-    ASSERT_THAT(comp2->id(), Eq(handle2->id()));
+    auto buffer_handle2 = q.compositor_acquire(new_compositor_id);
+    ASSERT_THAT(buffer_handle2->buffer()->id(), Eq(handle2->id()));
 }

=== modified file 'tests/unit-tests/compositor/test_buffer_stream.cpp'
--- tests/unit-tests/compositor/test_buffer_stream.cpp	2015-01-21 07:34:50 +0000
+++ tests/unit-tests/compositor/test_buffer_stream.cpp	2015-03-25 14:30:39 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2012 Canonical Ltd.
+ * Copyright © 2012, 2015 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
@@ -21,6 +21,7 @@
 #include "mir_test_doubles/stub_buffer.h"
 #include "mir_test_doubles/mock_buffer_bundle.h"
 #include "mir_test/gmock_fixes.h"
+#include "src/server/compositor/buffer_handle.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -92,28 +93,17 @@
     mc::BufferStreamSurfaces buffer_stream(mock_bundle);
 }
 
-TEST_F(BufferStreamTest, get_buffer_for_compositor_handles_resources)
-{
-    using namespace testing;
-
-    EXPECT_CALL(*mock_bundle, compositor_acquire(_))
-        .Times(1)
-        .WillOnce(Return(mock_buffer));
-    EXPECT_CALL(*mock_bundle, compositor_release(_))
-        .Times(1);
-
-    mc::BufferStreamSurfaces buffer_stream(mock_bundle);
-
-    buffer_stream.lock_compositor_buffer(0);
-}
-
 TEST_F(BufferStreamTest, get_buffer_for_compositor_can_lock)
 {
     using namespace testing;
 
+    std::shared_ptr<mc::BufferHandle> stub_handle =
+        std::make_shared<mc::CompositorBufferHandle>(mock_bundle.get(), mock_buffer);
+
     EXPECT_CALL(*mock_bundle, compositor_acquire(_))
         .Times(1)
-        .WillOnce(Return(mock_buffer));
+        .WillOnce(ReturnPointee(&stub_handle));
+
     EXPECT_CALL(*mock_bundle, compositor_release(_))
         .Times(1);
 

=== removed file 'tests/unit-tests/compositor/test_temporary_buffers.cpp'
--- tests/unit-tests/compositor/test_temporary_buffers.cpp	2015-01-21 07:34:50 +0000
+++ tests/unit-tests/compositor/test_temporary_buffers.cpp	1970-01-01 00:00:00 +0000
@@ -1,149 +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 "src/server/compositor/temporary_buffers.h"
-#include "mir_test_doubles/mock_buffer.h"
-#include "mir_test_doubles/stub_buffer.h"
-#include "mir_test_doubles/mock_buffer_bundle.h"
-#include <gtest/gtest.h>
-#include <stdexcept>
-
-namespace mtd=mir::test::doubles;
-namespace mg = mir::graphics;
-namespace mc=mir::compositor;
-namespace geom=mir::geometry;
-
-namespace
-{
-class TemporaryTestBuffer : public mc::TemporaryBuffer
-{
-public:
-    TemporaryTestBuffer(const std::shared_ptr<mg::Buffer>& buf)
-        : TemporaryBuffer(buf)
-    {
-    }
-};
-
-class TemporaryBuffersTest : public ::testing::Test
-{
-public:
-    TemporaryBuffersTest()
-        : buffer_size{1024, 768},
-          buffer_stride{1024},
-          buffer_pixel_format{mir_pixel_format_abgr_8888},
-          mock_buffer{std::make_shared<testing::NiceMock<mtd::MockBuffer>>(
-                          buffer_size, buffer_stride, buffer_pixel_format)},
-          mock_bundle{std::make_shared<testing::NiceMock<mtd::MockBufferBundle>>()}
-    {
-        using namespace testing;
-
-        ON_CALL(*mock_bundle, client_acquire(_))
-            .WillByDefault(InvokeArgument<0>(mock_buffer.get()));
-        ON_CALL(*mock_bundle, compositor_acquire(_))
-            .WillByDefault(Return(mock_buffer));
-    }
-
-    geom::Size const buffer_size;
-    geom::Stride const buffer_stride;
-    MirPixelFormat const buffer_pixel_format;
-    std::shared_ptr<mtd::MockBuffer> const mock_buffer;
-    std::shared_ptr<mtd::MockBufferBundle> mock_bundle;
-};
-}
-
-TEST_F(TemporaryBuffersTest, compositor_buffer_acquires_and_releases)
-{
-    using namespace testing;
-    EXPECT_CALL(*mock_bundle, compositor_acquire(_))
-        .WillOnce(Return(mock_buffer));
-    EXPECT_CALL(*mock_bundle, compositor_release(_))
-        .Times(1);
-
-    mc::TemporaryCompositorBuffer proxy_buffer(mock_bundle, 0);
-}
-
-TEST_F(TemporaryBuffersTest, snapshot_buffer_acquires_and_releases)
-{
-    using namespace testing;
-    EXPECT_CALL(*mock_bundle, snapshot_acquire())
-        .WillOnce(Return(mock_buffer));
-    EXPECT_CALL(*mock_bundle, snapshot_release(_))
-        .Times(1);
-
-    mc::TemporarySnapshotBuffer proxy_buffer(mock_bundle);
-}
-
-TEST_F(TemporaryBuffersTest, base_test_size)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, size())
-        .Times(1);
-
-    geom::Size size;
-    size = proxy_buffer.size();
-    EXPECT_EQ(buffer_size, size);
-}
-
-TEST_F(TemporaryBuffersTest, base_test_stride)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, stride())
-        .Times(1);
-
-    geom::Stride stride;
-    stride = proxy_buffer.stride();
-    EXPECT_EQ(buffer_stride, stride);
-}
-
-TEST_F(TemporaryBuffersTest, base_test_pixel_format)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, pixel_format())
-        .Times(1);
-
-    MirPixelFormat pixel_format;
-    pixel_format = proxy_buffer.pixel_format();
-    EXPECT_EQ(buffer_pixel_format, pixel_format);
-}
-
-TEST_F(TemporaryBuffersTest, base_gl_bind_to_texture)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, gl_bind_to_texture())
-        .Times(1);
-
-    proxy_buffer.gl_bind_to_texture();
-}
-
-TEST_F(TemporaryBuffersTest, base_test_id)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, id())
-        .Times(1);
-
-    proxy_buffer.id();
-}
-
-TEST_F(TemporaryBuffersTest, base_test_native_buffer_handle)
-{
-    TemporaryTestBuffer proxy_buffer(mock_buffer);
-    EXPECT_CALL(*mock_buffer, native_buffer_handle())
-        .Times(1);
-
-    proxy_buffer.native_buffer_handle();
-}