Merge lp:~raof/mir/use-dma-buf into lp:~mir-team/mir/trunk

[Chris Halse Rogers]

Change from sending flink'd gem names over IPC to sending dma-buf fds.

This means that clients can no longer snoop on other client's buffers
by guessing (easily guessable) flink names.

Fixes LP: #1124631

Change from sending flink'd gem names over IPC to sending dma-buf fds.

This means that clients can no longer snoop on other client's buffers
by guessing (easily guessable) flink names.

Fixes LP: #1124631

[Chris Halse Rogers (raof)]

Note that this should be not be merged until we're ready to roll out updated the XMir and Mesa code; otherwise it breaks them.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Changed to Work In Progress due to raof's comment above.

[Chris Halse Rogers (raof)]

Should now be good to go. The mesa changes are ready for when this lands.

[Chris Halse Rogers (raof)]

I'm Chris Halse Rogers, and I approve this MP

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:488
http://jenkins.qa.ubuntu.com/job/mir-ci/273/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/222
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/105
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/274
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/274/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/82

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

It's worth noting that this breaks existing accelerated GBM clients, like Mesa. A working mesa is available in the mir-ppa-dma-buf of my github tree - https://github.com/RAOF/mesa

[Alexandros Frantzis (afrantzis)]

> I'm Chris Halse Rogers, and I approve this MP

That's cheating ;)

[Alexandros Frantzis (afrantzis)]

src/server/frontend/client_buffer_tracker.cpp
src/server/frontend/client_buffer_tracker.h
... perhaps in other files too

Tabs instead of 4 spaces.

190 + // TODO: Do we need to GEM_CLOSE here? Do we need to close the fd?

Any update on this? I'd rather we ensure we don't leak handles and fds, before getting this in.

356 +class ClientBufferTracker

Delete copy constructor and assignment operator if not needed.

...and some nits:

139 + const unsigned int max_buffers;

unsigned int const ...

711 + mc::BufferID id{5};
... and others in tests

Could be const.

review: Needs Fixing

[Robert Carr (robertcarr)]

Cool!

Lots of tabs. i.e. 284.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:497
http://jenkins.qa.ubuntu.com/job/mir-ci/297/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/254
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/302
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/302/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

479 + std::list<uint32_t>::iterator find_buffer(compositor::BufferID const& id);
480 +
481 + std::list<uint32_t> ids;

Why not use std::list<BufferID> (BufferID has a comparison operator, and should be as efficient as using an uint32_t directly) and std::find() instead?

745 + // We don't map the buffer, so we don't need to take a GEM reference...
746 + EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
747 + .Times(0);
748 + // We haven't taken a GEM reference, so we shouldn't close it.
749 + EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
750 + .Times(0);

Do these add any value to the prime_fd_closed_on_buffer_destruction test? They are already checked for in the buffer_does_not_take_a_gem_reference_when_not_mapping test.

Also, as a general comment, expectations should be set before creating and using any objects that depend on the mock objects (unless there is a strong reason to do otherwise).

1004 + mediator.create_surface(nullptr, &surface_request, &surface_response, null_callback.get());
1005 +
1006 + mediator.next_buffer(nullptr, &buffer_request, &buffer_response[0], null_callback.get());

It's also worth testing that the mediator sends the expected buffer data the first time around.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

22 === added file 'include/shared/mir/frontend/client_constants.h
...
52 +/// mir::client::ClientBufferDepository and mir::frontend::ClientBufferTracker need to use the same value
53 +unsigned int const client_buffer_cache_size = 3;

There's obviously conflicting forces to resolve in deciding where to put this, but I don't think namespace mir is the right answer. Especially when the file suggests namespace mir::frontend.

At the moment I'm struggling to understand why the server needs to be *compiled* with the size of a client-side cache - but maybe I'll grok that later. In any case, something here needs fixing.

review: Needs Fixing

[Chris Halse Rogers (raof)]

--- Alexandros ---

> 479 + std::list<uint32_t>::iterator find_buffer(compositor::BufferID const& id);
> 480 +
> 481 + std::list<uint32_t> ids;

> Why not use std::list<BufferID> (BufferID has a comparison operator, and should be as efficient as using an uint32_t directly) and std::find() instead?

Hm, yeah, that works better. Thanks!

> 745 + // We don't map the buffer, so we don't need to take a GEM reference...
> 746 + EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
> 747 + .Times(0);
> 748 + // We haven't taken a GEM reference, so we shouldn't close it.
> 749 + EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
> 750 + .Times(0);

> Do these add any value to the prime_fd_closed_on_buffer_destruction test? They are already checked for in the buffer_does_not_take_a_gem_reference_when_not_mapping test.

Oh, no. I forgot to delete that bit when I decided to make buffer_does_not_take_a_gem_reference_when_not_mapping a separate test.

> 1004 + mediator.create_surface(nullptr, &surface_request, &surface_response, null_callback.get());
> 1005 +
> 1006 + mediator.next_buffer(nullptr, &buffer_request, &buffer_response[0], null_callback.get());

> It's also worth testing that the mediator sends the expected buffer data the first time around.

Done.

--- Alan ---
> At the moment I'm struggling to understand why the server needs to be *compiled* with the size of a client-side cache - but maybe I'll grok that later. In any case, something here needs fixing.

The server and the client need to negotiate the size of the cache, because the server will not send data for buffers it knows the client has cached.

The easiest way to do this is to compile in a fixed value; if need be we can move to explicit IPC negotiation later.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:501
http://jenkins.qa.ubuntu.com/job/mir-ci/307/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/268
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/312
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/312/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/119

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Alan Griffiths (alan-griffiths)]

106 + for (auto pair = buffers.begin(); pair != buffers.end(); ++pair)

you can write:

for (auto& pair : buffers)

(With a bit of s/->/./)

[Alan Griffiths (alan-griffiths)]

> 106 + for (auto pair = buffers.begin(); pair != buffers.end(); ++pair)
>
> you can write:
>
> for (auto& pair : buffers)
>
> (With a bit of s/->/./)

Forget that - I just realized you store the iterator.

review: Approve

[Kevin DuBois (kdub)]

looks good to me too. Side point: I recently rooted the ID out of the buffer swapper, i think we're getting pretty close to being able to isolate the ID to just the frontend... (a good thing)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:501
http://jenkins.qa.ubuntu.com/job/mir-ci/311/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/278/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/161/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/316/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:502
http://jenkins.qa.ubuntu.com/job/mir-ci/316/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/285
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/168
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/321
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/321/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/129

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

review: Approve (continuous-integration)

[Robert Carr (robertcarr)]

LGTM

review: Approve

[Alexandros Frantzis (afrantzis)]

Please don't merge this yet, as the Mesa side for radeon and nouveau is not ready yet. Marking as "needs fixing" in order to block this until the mesa side is ready.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:503
http://jenkins.qa.ubuntu.com/job/mir-ci/345/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/330
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/213
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/350
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/350/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/150

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

Marked as approved, as the mesa changes for radeon and nouveau are now available.

=== modified file 'include/server/mir/frontend/session_mediator.h'
--- include/server/mir/frontend/session_mediator.h	2013-03-21 03:32:59 +0000
+++ include/server/mir/frontend/session_mediator.h	2013-04-10 08:47:23 +0000
@@ -47,6 +47,7 @@
 class Session;
 class ResourceCache;
 class SessionMediatorReport;
+class ClientBufferTracker;
 
 // SessionMediator relays requests from the client process into the server.
 class SessionMediator : public mir::protobuf::DisplayServer
@@ -116,6 +117,7 @@
 
     std::shared_ptr<SessionMediatorReport> const report;
     std::shared_ptr<ResourceCache> const resource_cache;
+    std::shared_ptr<ClientBufferTracker> const client_tracker;
 
     std::shared_ptr<Session> session;
 };

=== added directory 'include/shared/mir/frontend'
=== added file 'include/shared/mir/frontend/client_constants.h'
--- include/shared/mir/frontend/client_constants.h	1970-01-01 00:00:00 +0000
+++ include/shared/mir/frontend/client_constants.h	2013-04-10 08:47:23 +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]>
+ */
+
+#ifndef MIR_FRONTEND_CLIENT_CONSTANTS_H_
+#define MIR_FRONTEND_CLIENT_CONSTANTS_H_
+
+namespace mir
+{
+
+namespace frontend
+{
+/// Number of buffers the client library will keep.
+
+/// mir::client::ClientBufferDepository and mir::frontend::ClientBufferTracker need to use the same value
+unsigned int const client_buffer_cache_size = 3;
+
+}
+}
+
+#endif

=== modified file 'src/client/client_buffer_depository.cpp'
--- src/client/client_buffer_depository.cpp	2013-03-29 11:27:00 +0000
+++ src/client/client_buffer_depository.cpp	2013-04-10 08:47:23 +0000
@@ -29,49 +29,39 @@
 
 mcl::ClientBufferDepository::ClientBufferDepository(std::shared_ptr<ClientBufferFactory> const& factory, int max_buffers)
     : factory(factory),
-      current_buffer_iter(buffers.end()),
-      max_age(max_buffers - 1)
+      max_buffers(max_buffers)
 {
 }
 
 void mcl::ClientBufferDepository::deposit_package(std::shared_ptr<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 existing_buffer_id_pair = buffers.end();
+    for (auto pair = buffers.begin(); pair != buffers.end(); ++pair)
+    {
+        pair->second->increment_age();
+        if (pair->first == id)
+            existing_buffer_id_pair = pair;
+    }
+
+    if (buffers.size() > 0)
+        buffers.front().second->mark_as_submitted();
+
+    if (existing_buffer_id_pair == buffers.end())
     {
         auto new_buffer = factory->create_buffer(package, size, pf);
+        buffers.push_front(std::make_pair(id, new_buffer));
+    }
+    else
+    {
+        buffers.push_front(*existing_buffer_id_pair);
+        buffers.erase(existing_buffer_id_pair);
+    }
 
-        current_buffer_iter = buffers.insert(current_buffer_iter, std::make_pair(id, new_buffer));
-    }
+    if (buffers.size() > max_buffers)
+        buffers.pop_back();
 }
 
 std::shared_ptr<mcl::ClientBuffer> mcl::ClientBufferDepository::current_buffer()
 {
-    return current_buffer_iter->second;
+    return buffers.front().second;
 }

=== modified file 'src/client/client_buffer_depository.h'
--- src/client/client_buffer_depository.h	2013-03-29 17:42:12 +0000
+++ src/client/client_buffer_depository.h	2013-04-10 08:47:23 +0000
@@ -20,7 +20,7 @@
 #ifndef MIR_CLIENT_PRIVATE_MIR_CLIENT_BUFFER_DEPOSITORY_H_
 #define MIR_CLIENT_PRIVATE_MIR_CLIENT_BUFFER_DEPOSITORY_H_
 
-#include <map>
+#include <list>
 #include <memory>
 
 #include "mir/geometry/pixel_format.h"
@@ -39,9 +39,19 @@
 /// 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.
+/// \sa mir::frontend::ClientBufferTracker for the server-side analogue.
+/// \note Changes to the tracking algorithm will need associated server-side changes to mir::frontend::ClientBufferTracker
 class ClientBufferDepository
 {
 public:
+    /// Constructor
+
+    /// \param factory is the ClientBufferFactory that will be used to convert IPC data to ClientBuffers
+    /// \param max_buffers is the number of buffers the depository will cache. After the client has received
+    /// its initial buffers the ClientBufferDepository will always have the last \ref max_buffers buffers
+    /// cached.
+    /// \note The server relies on the depository caching \ref max_buffers buffers to optimise away buffer
+    /// passing. As such, this number needs to be shared between the server and client libraries.
     ClientBufferDepository(std::shared_ptr<ClientBufferFactory> const& factory, int max_buffers);
 
     /// Construct a ClientBuffer from the IPC data, and use it as the current buffer.
@@ -54,10 +64,8 @@
 
 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;
+    std::list<std::pair<int, std::shared_ptr<ClientBuffer>>> buffers;
+    unsigned int const max_buffers;
 };
 }
 }

=== modified file 'src/client/gbm/drm_fd_handler.h'
--- src/client/gbm/drm_fd_handler.h	2013-03-13 04:54:15 +0000
+++ src/client/gbm/drm_fd_handler.h	2013-04-10 08:47:23 +0000
@@ -34,6 +34,8 @@
     virtual ~DRMFDHandler() {}
 
     virtual int ioctl(unsigned long request, void* arg) = 0;
+    virtual int primeFDToHandle(int prime_fd, uint32_t *handle) = 0;
+    virtual int close(int fd) = 0;
     virtual void* map(size_t size, off_t offset) = 0;
     virtual void unmap(void* addr, size_t size) = 0;
 

=== modified file 'src/client/gbm/gbm_client_buffer.cpp'
--- src/client/gbm/gbm_client_buffer.cpp	2013-03-29 11:27:00 +0000
+++ src/client/gbm/gbm_client_buffer.cpp	2013-04-10 08:47:23 +0000
@@ -40,30 +40,26 @@
 struct GEMHandle
 {
     GEMHandle(std::shared_ptr<mclg::DRMFDHandler> const& drm_fd_handler,
-              uint32_t gem_name)
+              int prime_fd)
         : drm_fd_handler{drm_fd_handler}
     {
-        struct drm_gem_open gem_open;
-        memset(&gem_open, 0, sizeof(gem_open));
-        gem_open.name = gem_name;
-
-        int ret = drm_fd_handler->ioctl(DRM_IOCTL_GEM_OPEN, &gem_open);
+        int ret = drm_fd_handler->primeFDToHandle(prime_fd, &handle);
         if (ret)
         {
-            std::string msg("Failed to open GEM handle for DRM buffer");
+            std::string msg("Failed to import PRIME fd for DRM buffer");
             BOOST_THROW_EXCEPTION(
                 boost::enable_error_info(
                     std::runtime_error(msg)) << boost::errinfo_errno(errno));
         }
-
-        handle = gem_open.handle;
     }
 
     ~GEMHandle()
     {
-        static const uint32_t pad{0};
-        struct drm_gem_close gem_close = {handle, pad};
-        drm_fd_handler->ioctl(DRM_IOCTL_GEM_CLOSE, &gem_close);
+        struct drm_gem_close arg;
+        arg.handle = handle;
+        // TODO (@raof): Error reporting? I do not believe it should be possible for this to fail,
+        //               so if it does we should probably flag it.
+        drm_fd_handler->ioctl(DRM_IOCTL_GEM_CLOSE, &arg);
     }
 
     std::shared_ptr<mclg::DRMFDHandler> const drm_fd_handler;
@@ -78,10 +74,10 @@
 struct GBMMemoryRegion : mcl::MemoryRegion
 {
     GBMMemoryRegion(std::shared_ptr<mclg::DRMFDHandler> const& drm_fd_handler,
-                    uint32_t gem_name, geom::Size const& size_param,
+                    int prime_fd, geom::Size const& size_param,
                     geom::Stride stride_param, geom::PixelFormat format_param)
         : drm_fd_handler{drm_fd_handler},
-          gem_handle{drm_fd_handler, gem_name},
+          gem_handle{drm_fd_handler, prime_fd},
           size_in_bytes{size_param.height.as_uint32_t() * stride_param.as_uint32_t()}
     {
         width = size_param.width;
@@ -137,12 +133,19 @@
 {
 }
 
+mclg::GBMClientBuffer::~GBMClientBuffer()
+{
+    // TODO (@raof): Error reporting? It should not be possible for this to fail; if it does,
+    //               something's seriously wrong.
+    drm_fd_handler->close(creation_package->fd[0]);
+}
+
 std::shared_ptr<mcl::MemoryRegion> mclg::GBMClientBuffer::secure_for_cpu_write()
 {
-    const uint32_t gem_name = creation_package->data[0];
+    const int prime_fd = creation_package->fd[0];
 
     return std::make_shared<GBMMemoryRegion>(drm_fd_handler,
-                                             gem_name,
+                                             prime_fd,
                                              size(),
                                              stride(),
                                              pixel_format());

=== modified file 'src/client/gbm/gbm_client_buffer.h'
--- src/client/gbm/gbm_client_buffer.h	2013-03-29 11:27:00 +0000
+++ src/client/gbm/gbm_client_buffer.h	2013-04-10 08:47:23 +0000
@@ -43,6 +43,8 @@
                     geometry::Size size,
                     geometry::PixelFormat pf);
 
+    virtual ~GBMClientBuffer();
+
     std::shared_ptr<MemoryRegion> secure_for_cpu_write();
     geometry::Size size() const;
     geometry::Stride stride() const;

=== modified file 'src/client/gbm/gbm_client_platform.cpp'
--- src/client/gbm/gbm_client_platform.cpp	2013-03-29 11:27:00 +0000
+++ src/client/gbm/gbm_client_platform.cpp	2013-04-10 08:47:23 +0000
@@ -27,6 +27,7 @@
 
 #include <xf86drm.h>
 #include <sys/mman.h>
+#include <unistd.h>
 
 namespace mcl=mir::client;
 namespace mclg=mir::client::gbm;
@@ -47,6 +48,22 @@
         return drmIoctl(drm_fd, request, arg);
     }
 
+    int primeFDToHandle(int prime_fd, uint32_t *handle)
+    {
+        return drmPrimeFDToHandle(drm_fd, prime_fd, handle);
+    }
+
+    int close(int fd)
+    {
+        while (::close(fd) == -1)
+        {
+            // Retry on EINTR, return error on anything else
+            if (errno != EINTR)
+                return errno;
+        }
+        return 0;
+    }
+
     void* map(size_t size, off_t offset)
     {
         return mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED,

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-04-08 05:28:44 +0000
+++ src/client/mir_surface.cpp	2013-04-10 08:47:23 +0000
@@ -17,6 +17,7 @@
  */
 
 #include "mir_toolkit/mir_client_library.h"
+#include "mir/frontend/client_constants.h"
 #include "mir_logger.h"
 #include "client_buffer.h"
 #include "mir_surface.h"
@@ -42,7 +43,7 @@
     mir_surface_lifecycle_callback callback, void * context)
     : server(server),
       connection(allocating_connection),
-      buffer_depository(std::make_shared<mcl::ClientBufferDepository>(factory, 3)),
+      buffer_depository(std::make_shared<mcl::ClientBufferDepository>(factory, mir::frontend::client_buffer_cache_size)),
       input_platform(input_platform),
       logger(logger)
 {

=== modified file 'src/server/frontend/CMakeLists.txt'
--- src/server/frontend/CMakeLists.txt	2013-03-28 09:50:09 +0000
+++ src/server/frontend/CMakeLists.txt	2013-04-10 08:47:23 +0000
@@ -1,6 +1,7 @@
 set(
   FRONTEND_SOURCES
 
+  client_buffer_tracker.cpp
   session_mediator.cpp
   null_session_mediator_report.cpp
   null_message_processor_report.cpp

=== added file 'src/server/frontend/client_buffer_tracker.cpp'
--- src/server/frontend/client_buffer_tracker.cpp	1970-01-01 00:00:00 +0000
+++ src/server/frontend/client_buffer_tracker.cpp	2013-04-10 08:47:23 +0000
@@ -0,0 +1,53 @@
+/*
+ * 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 <algorithm>
+
+#include "client_buffer_tracker.h"
+#include "mir/compositor/buffer_id.h"
+
+namespace mf = mir::frontend;
+namespace mc = mir::compositor;
+
+mf::ClientBufferTracker::ClientBufferTracker(unsigned int client_cache_size)
+    : ids(),
+      cache_size{client_cache_size}
+{
+}
+
+void mf::ClientBufferTracker::add(mc::BufferID const& id)
+{
+    auto existing_id = std::find(ids.begin(), ids.end(), id);
+
+    if (existing_id != ids.end())
+    {
+        ids.push_front(*existing_id);
+        ids.erase(existing_id);
+    }
+    else
+    {
+        ids.push_front(id);
+    }
+    if (ids.size() > cache_size)
+        ids.pop_back();
+}
+
+bool mf::ClientBufferTracker::client_has(mc::BufferID const& id) const
+{
+    return std::find(ids.begin(), ids.end(), id) != ids.end();
+}

=== added file 'src/server/frontend/client_buffer_tracker.h'
--- src/server/frontend/client_buffer_tracker.h	1970-01-01 00:00:00 +0000
+++ src/server/frontend/client_buffer_tracker.h	2013-04-10 08:47:23 +0000
@@ -0,0 +1,62 @@
+/*
+ * 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_FRONTEND_CLIENT_BUFFER_TRACKER_H_
+ #define MIR_FRONTEND_CLIENT_BUFFER_TRACKER_H_
+
+#include <stdint.h>
+#include <list>
+
+namespace mir
+{
+
+namespace compositor
+{
+class BufferID;
+}
+
+namespace frontend
+{
+
+/// Responsible for tracking what buffers the client library knows about for a surface.
+
+/// \sa mir::client::ClientBufferDepository for the client-side of this tracking
+/// \note Changes to the tracking algorithm of mir::client::ClientBufferDepository will need to be mirrored here
+class ClientBufferTracker
+{
+public:
+    ClientBufferTracker(unsigned int client_cache_size);
+
+    ClientBufferTracker(ClientBufferTracker const&) = delete;
+    ClientBufferTracker& operator=(ClientBufferTracker const&) = delete;
+
+    /// Add a BufferID to the list of buffers known by the client.
+    ///
+    /// Typically this should be done just prior to or just after sending the buffer information
+    void add(compositor::BufferID const& id);
+    bool client_has(compositor::BufferID const& id) const;
+private:
+
+    std::list<compositor::BufferID> ids;
+    unsigned int const cache_size;
+};
+
+}
+}
+
+ #endif // MIR_FRONTEND_CLIENT_BUFFER_TRACKER_H_

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2013-03-21 03:32:59 +0000
+++ src/server/frontend/session_mediator.cpp	2013-04-10 08:47:23 +0000
@@ -33,6 +33,8 @@
 #include "mir/graphics/platform.h"
 #include "mir/graphics/viewable_area.h"
 #include "mir/graphics/platform_ipc_package.h"
+#include "mir/frontend/client_constants.h"
+#include "client_buffer_tracker.h"
 
 #include <boost/throw_exception.hpp>
 
@@ -48,7 +50,8 @@
     viewable_area(viewable_area),
     buffer_allocator(buffer_allocator),
     report(report),
-    resource_cache(resource_cache)
+    resource_cache(resource_cache),
+    client_tracker(std::make_shared<ClientBufferTracker>(frontend::client_buffer_cache_size))
 {
 }
 
@@ -122,20 +125,25 @@
         auto const& buffer_resource = surface->client_buffer();
 
         auto const& id = buffer_resource->id();
-        auto ipc_package = buffer_resource->get_ipc_package();
+
         auto buffer = response->mutable_buffer();
-
         buffer->set_buffer_id(id.as_uint32_t());
 
-        for (auto& data : ipc_package->ipc_data)
-            buffer->add_data(data);
-
-        for (auto& ipc_fds : ipc_package->ipc_fds)
-            buffer->add_fd(ipc_fds);
-
-        buffer->set_stride(ipc_package->stride);
-
-        resource_cache->save_resource(response, ipc_package);
+        if (!client_tracker->client_has(id))
+        {
+            auto ipc_package = buffer_resource->get_ipc_package();
+
+            for (auto& data : ipc_package->ipc_data)
+                buffer->add_data(data);
+
+            for (auto& ipc_fds : ipc_package->ipc_fds)
+                buffer->add_fd(ipc_fds);
+
+            buffer->set_stride(ipc_package->stride);
+
+            resource_cache->save_resource(response, ipc_package);
+        }
+        client_tracker->add(id);
     }
 
     done->Run();
@@ -157,18 +165,23 @@
     surface->advance_client_buffer();
     auto const& buffer_resource = surface->client_buffer();
     auto const& id = buffer_resource->id();
-    auto ipc_package = buffer_resource->get_ipc_package();
-
     response->set_buffer_id(id.as_uint32_t());
-    for (auto& data : ipc_package->ipc_data)
-        response->add_data(data);
-
-    for (auto& ipc_fds : ipc_package->ipc_fds)
-        response->add_fd(ipc_fds);
-
-    response->set_stride(ipc_package->stride);
-
-    resource_cache->save_resource(response, ipc_package);
+
+    if (!client_tracker->client_has(id))
+    {
+        auto ipc_package = buffer_resource->get_ipc_package();
+
+        for (auto& data : ipc_package->ipc_data)
+            response->add_data(data);
+
+        for (auto& ipc_fds : ipc_package->ipc_fds)
+            response->add_fd(ipc_fds);
+
+        response->set_stride(ipc_package->stride);
+
+        resource_cache->save_resource(response, ipc_package);
+    }
+    client_tracker->add(id);
     done->Run();
 }
 

=== modified file 'src/server/graphics/gbm/gbm_buffer.cpp'
--- src/server/graphics/gbm/gbm_buffer.cpp	2013-03-13 04:54:15 +0000
+++ src/server/graphics/gbm/gbm_buffer.cpp	2013-04-10 08:47:23 +0000
@@ -22,6 +22,7 @@
 #include "buffer_texture_binder.h"
 #include "mir/compositor/buffer_ipc_package.h"
 
+#include <fcntl.h>
 #include <xf86drm.h>
 
 #include <boost/exception/errinfo_errno.hpp>
@@ -84,19 +85,16 @@
     auto device = gbm_bo_get_device(gbm_handle.get());
     auto gem_handle = gbm_bo_get_handle(gbm_handle.get()).u32;
     auto drm_fd = gbm_device_get_fd(device);
-    struct drm_gem_flink flink;
-    flink.handle = gem_handle;
-
-    auto ret = drmIoctl(drm_fd, DRM_IOCTL_GEM_FLINK, &flink);
+
+    auto ret = drmPrimeHandleToFD(drm_fd, gem_handle, DRM_CLOEXEC, &prime_fd);    
+
     if (ret)
     {
-        std::string const msg("Failed to get GEM flink name from gbm bo");
+        std::string const msg("Failed to get PRIME fd from gbm bo");
         BOOST_THROW_EXCEPTION(
             boost::enable_error_info(
                 std::runtime_error(msg)) << boost::errinfo_errno(errno));
     }
-
-    gem_flink_name = flink.name;
 }
 
 geom::Size mgg::GBMBuffer::size() const
@@ -119,7 +117,7 @@
 {
     auto temp = std::make_shared<mc::BufferIPCPackage>();
 
-    temp->ipc_data.push_back(gem_flink_name);
+    temp->ipc_fds.push_back(prime_fd);
     temp->stride = stride().as_uint32_t();
 
     return temp;

=== modified file 'src/server/graphics/gbm/gbm_buffer.h'
--- src/server/graphics/gbm/gbm_buffer.h	2013-03-13 04:54:15 +0000
+++ src/server/graphics/gbm/gbm_buffer.h	2013-04-10 08:47:23 +0000
@@ -60,7 +60,7 @@
 private:
     std::shared_ptr<gbm_bo> const gbm_handle;
     std::unique_ptr<BufferTextureBinder> const texture_binder;
-    uint32_t gem_flink_name;
+    int prime_fd;
 };
 
 }

=== modified file 'tests/unit-tests/client/gbm/mock_drm_fd_handler.h'
--- tests/unit-tests/client/gbm/mock_drm_fd_handler.h	2013-02-24 19:16:46 +0000
+++ tests/unit-tests/client/gbm/mock_drm_fd_handler.h	2013-04-10 08:47:23 +0000
@@ -34,6 +34,8 @@
 {
 public:
     MOCK_METHOD2(ioctl, int(unsigned long request, void* arg));
+    MOCK_METHOD2(primeFDToHandle, int(int prime_fd, uint32_t *handle));
+    MOCK_METHOD1(close, int(int fd));
     MOCK_METHOD2(map, void*(size_t size, off_t offset));
     MOCK_METHOD2(unmap, void(void* addr, size_t size));
 };

=== modified file 'tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp'
--- tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp	2013-03-24 23:33:49 +0000
+++ tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp	2013-04-10 08:47:23 +0000
@@ -97,7 +97,7 @@
     using namespace testing;
     void *map_addr{reinterpret_cast<void*>(0xabcdef)};
 
-    EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_OPEN,_))
+    EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
         .WillOnce(Return(0));
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_MODE_MAP_DUMB,_))
         .WillOnce(Return(0));
@@ -106,7 +106,7 @@
     EXPECT_CALL(*drm_fd_handler, unmap(_,_))
         .Times(1);
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
-        .WillOnce(Return(0));
+        .Times(1);
 
     mclg::GBMClientBuffer buffer(drm_fd_handler, std::move(package), size, pf);
 
@@ -118,12 +118,12 @@
     ASSERT_EQ(pf, mem_region->format);
 }
 
-TEST_F(MirGBMBufferTest, secure_for_cpu_write_throws_on_gem_open_failure)
+TEST_F(MirGBMBufferTest, secure_for_cpu_write_throws_on_prime_handle_failure)
 {
     using namespace testing;
 
-    EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_OPEN,_))
-        .WillOnce(Return(1));
+    EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
+        .WillOnce(Return(-1));
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_MODE_MAP_DUMB,_))
         .Times(0);
     EXPECT_CALL(*drm_fd_handler, map(_,_))
@@ -144,7 +144,7 @@
 {
     using namespace testing;
 
-    EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_OPEN,_))
+    EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
         .WillOnce(Return(0));
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_MODE_MAP_DUMB,_))
         .WillOnce(Return(1));
@@ -153,7 +153,7 @@
     EXPECT_CALL(*drm_fd_handler, unmap(_,_))
         .Times(0);
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
-        .WillOnce(Return(0));
+        .Times(1);
 
     mclg::GBMClientBuffer buffer(drm_fd_handler, std::move(package), size, pf);
 
@@ -166,7 +166,7 @@
 {
     using namespace testing;
 
-    EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_OPEN,_))
+    EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
         .WillOnce(Return(0));
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_MODE_MAP_DUMB,_))
         .WillOnce(Return(0));
@@ -175,7 +175,7 @@
     EXPECT_CALL(*drm_fd_handler, unmap(_,_))
         .Times(0);
     EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
-        .WillOnce(Return(0));
+        .Times(1);
 
     mclg::GBMClientBuffer buffer(drm_fd_handler, std::move(package), size, pf);
 
@@ -183,3 +183,32 @@
         auto mem_region = buffer.secure_for_cpu_write();
     }, std::runtime_error);
 }
+
+TEST_F(MirGBMBufferTest, prime_fd_closed_on_buffer_destruction)
+{
+    using namespace testing;
+
+    int const prime_fd{42};
+
+    package->fd[0] = prime_fd;
+    package->fd_items = 1;
+
+    EXPECT_CALL(*drm_fd_handler, close(prime_fd))
+        .Times(1);
+
+    mclg::GBMClientBuffer buffer(drm_fd_handler, package, size, pf);
+}
+
+TEST_F(MirGBMBufferTest, buffer_does_not_take_a_gem_reference_when_not_mapping)
+{
+    using namespace testing;
+
+    // We don't map the buffer, so we don't need to take a GEM reference...
+    EXPECT_CALL(*drm_fd_handler, primeFDToHandle(_,_))
+        .Times(0);
+    // We haven't taken a GEM reference, so we shouldn't close it.
+    EXPECT_CALL(*drm_fd_handler, ioctl(DRM_IOCTL_GEM_CLOSE,_))
+        .Times(0);
+
+    mclg::GBMClientBuffer buffer(drm_fd_handler, package, size, pf);
+}

=== modified file 'tests/unit-tests/client/test_client_buffer_depository.cpp'
--- tests/unit-tests/client/test_client_buffer_depository.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/client/test_client_buffer_depository.cpp	2013-04-10 08:47:23 +0000
@@ -369,3 +369,36 @@
     depository.deposit_package(package1, 8, size, pf);
     depository.deposit_package(package2, 9, size, pf);
 }
+
+TEST_F(MirBufferDepositoryTest, depository_keeps_last_2_buffers_regardless_of_age)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 2};
+
+    EXPECT_CALL(*mock_factory, create_buffer(_,_,_)).Times(2);
+
+    depository.deposit_package(package, 8, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 8, size, pf);
+}
+
+TEST_F(MirBufferDepositoryTest, depository_keeps_last_3_buffers_regardless_of_age)
+{
+    using namespace testing;
+
+    mcl::ClientBufferDepository depository{mock_factory, 3};
+
+    EXPECT_CALL(*mock_factory, create_buffer(_,_,_)).Times(3);
+
+    depository.deposit_package(package, 8, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 10, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 10, size, pf);
+    depository.deposit_package(package, 9, size, pf);
+    depository.deposit_package(package, 10, size, pf);
+    depository.deposit_package(package, 8, size, pf);
+}

=== modified file 'tests/unit-tests/frontend/CMakeLists.txt'
--- tests/unit-tests/frontend/CMakeLists.txt	2013-04-09 11:33:19 +0000
+++ tests/unit-tests/frontend/CMakeLists.txt	2013-04-10 08:47:23 +0000
@@ -1,4 +1,5 @@
 list(APPEND UNIT_TEST_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_client_buffer_tracker.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/stress_protobuf_communicator.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_communicator.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_surface_apis.cpp

=== added file 'tests/unit-tests/frontend/test_client_buffer_tracker.cpp'
--- tests/unit-tests/frontend/test_client_buffer_tracker.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/frontend/test_client_buffer_tracker.cpp	2013-04-10 08:47:23 +0000
@@ -0,0 +1,108 @@
+/*
+ * 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/server/frontend/client_buffer_tracker.h"
+#include "mir/compositor/buffer_id.h"
+
+#include <gtest/gtest.h>
+
+namespace mf = mir::frontend;
+namespace mc = mir::compositor;
+
+TEST(ClientBufferTracker, just_added_buffer_is_known_by_client)
+{
+    mf::ClientBufferTracker tracker(3);
+    mc::BufferID const id{5};
+
+    tracker.add(id);
+    EXPECT_TRUE(tracker.client_has(id));
+}
+
+TEST(ClientBufferTracker, unadded_buffer_is_unknown_by_client)
+{
+    mf::ClientBufferTracker tracker(3);
+
+    tracker.add(mc::BufferID{5});
+    EXPECT_FALSE(tracker.client_has(mc::BufferID{6}));
+}
+
+TEST(ClientBufferTracker, tracks_sequence_of_buffers)
+{
+    mf::ClientBufferTracker tracker(3);
+    mc::BufferID const one{1};
+    mc::BufferID const two{2};
+    mc::BufferID const three{3};
+    mc::BufferID const four{4};
+
+    tracker.add(one);
+    tracker.add(two);
+    tracker.add(three);
+
+    EXPECT_TRUE(tracker.client_has(one));
+    EXPECT_TRUE(tracker.client_has(two));
+    EXPECT_TRUE(tracker.client_has(three));
+    EXPECT_FALSE(tracker.client_has(four));
+}
+
+TEST(ClientBufferTracker, old_buffers_expire_from_tracker)
+{
+    mf::ClientBufferTracker tracker(3);
+
+    mc::BufferID const one{1};
+    mc::BufferID const two{2};
+    mc::BufferID const three{3};
+    mc::BufferID const four{4};
+
+    tracker.add(one);
+    tracker.add(two);
+    tracker.add(three);
+
+    ASSERT_TRUE(tracker.client_has(one));
+    ASSERT_TRUE(tracker.client_has(two));
+    ASSERT_TRUE(tracker.client_has(three));
+
+    tracker.add(two);
+    tracker.add(three);
+    tracker.add(four);
+
+    EXPECT_FALSE(tracker.client_has(one));
+    EXPECT_TRUE(tracker.client_has(two));
+    EXPECT_TRUE(tracker.client_has(three));
+    EXPECT_TRUE(tracker.client_has(four));
+}
+
+TEST(ClientBufferTracker, tracks_correct_number_of_buffers)
+{
+    mc::BufferID ids[10];
+    for (unsigned int i = 0; i < 10; ++i)
+        ids[i] = mc::BufferID{i};
+
+    for (unsigned int tracker_size = 2; tracker_size < 10; ++tracker_size)
+    {
+        mf::ClientBufferTracker tracker{tracker_size};
+
+        for (unsigned int i = 0; i < tracker_size; ++i)
+            tracker.add(ids[i]);
+
+        tracker.add(ids[tracker_size]);
+
+        EXPECT_FALSE(tracker.client_has(ids[0]));
+        for (unsigned int i = 1; i <= tracker_size; ++i)
+            EXPECT_TRUE(tracker.client_has(ids[i]));
+    }
+}

=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp	2013-04-10 08:47:23 +0000
@@ -30,7 +30,7 @@
 #include "mir_test_doubles/null_display.h"
 #include "mir_test_doubles/mock_shell.h"
 #include "mir_test_doubles/mock_surface.h"
-#include "mir_test_doubles/stub_buffer.h"
+#include "mir_test_doubles/mock_buffer.h"
 #include "mir_test_doubles/stub_session.h"
 #include "mir_test_doubles/stub_surface_builder.h"
 #include "mir_test/fake_shared.h"
@@ -63,10 +63,11 @@
         using namespace ::testing;
 
         mock_surface = std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder));
+        mock_buffer = std::make_shared<NiceMock<mtd::MockBuffer>>(geom::Size(), geom::Stride(), geom::PixelFormat());
 
         EXPECT_CALL(*mock_surface, size()).Times(AnyNumber()).WillRepeatedly(Return(geom::Size()));
         EXPECT_CALL(*mock_surface, pixel_format()).Times(AnyNumber()).WillRepeatedly(Return(geom::PixelFormat()));
-        EXPECT_CALL(*mock_surface, client_buffer()).Times(AnyNumber()).WillRepeatedly(Return(mt::fake_shared(stub_buffer)));
+        EXPECT_CALL(*mock_surface, client_buffer()).Times(AnyNumber()).WillRepeatedly(Return(mock_buffer));
         EXPECT_CALL(*mock_surface, advance_client_buffer()).Times(AnyNumber());
 
         EXPECT_CALL(*mock_surface, supports_input()).Times(AnyNumber()).WillRepeatedly(Return(true));
@@ -80,7 +81,7 @@
 
     mtd::StubSurfaceBuilder surface_builder;
     std::shared_ptr<mtd::MockSurface> mock_surface;
-    mtd::StubBuffer stub_buffer;
+    std::shared_ptr<mtd::MockBuffer> mock_buffer;
     static int const testing_client_input_fd;
 };
 
@@ -354,3 +355,68 @@
 
     mediator.disconnect(nullptr, nullptr, nullptr, null_callback.get());
 }
+
+TEST_F(SessionMediatorTest, session_only_sends_needed_buffers)
+{
+    using namespace testing;
+
+    mp::ConnectParameters connect_parameters;
+    mp::Connection connection;
+
+    mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
+
+    {
+        auto package = std::make_shared<mc::BufferIPCPackage>();
+        package->ipc_data = std::vector<int32_t>{5, 23, 1};
+        package->ipc_fds = std::vector<int32_t>{2, 1};
+
+        ON_CALL(*stubbed_session->mock_buffer, get_ipc_package())
+            .WillByDefault(Return(package));
+
+        EXPECT_CALL(*stubbed_session->mock_buffer, id())
+            .WillOnce(Return(mc::BufferID{4}))
+            .WillOnce(Return(mc::BufferID{5}))
+            .WillOnce(Return(mc::BufferID{4}))
+            .WillOnce(Return(mc::BufferID{5}));
+
+        EXPECT_CALL(*stubbed_session->mock_buffer, get_ipc_package())
+            .Times(2);
+
+        mp::SurfaceParameters surface_request;
+        mp::Surface surface_response;
+        mp::SurfaceId buffer_request;
+        mp::Buffer buffer_response[3];
+
+        mediator.create_surface(nullptr, &surface_request, &surface_response, null_callback.get());
+        ASSERT_EQ(mc::BufferID{4}.as_uint32_t(), static_cast<uint32_t>(surface_response.buffer().buffer_id()));
+        EXPECT_EQ(2, surface_response.buffer().fd_size());
+        EXPECT_EQ(3, surface_response.buffer().data_size());
+        for (int i = 0; i < surface_response.buffer().fd_size(); ++i)
+            EXPECT_EQ(package->ipc_fds[i], surface_response.buffer().fd(i));
+        for (int i = 0; i < surface_response.buffer().data_size(); ++i)
+            EXPECT_EQ(package->ipc_data[i], surface_response.buffer().data(i));
+
+
+        mediator.next_buffer(nullptr, &buffer_request, &buffer_response[0], null_callback.get());
+        ASSERT_EQ(mc::BufferID{5}.as_uint32_t(), static_cast<uint32_t>(buffer_response[0].buffer_id()));
+        EXPECT_EQ(2, buffer_response[0].fd_size());
+        EXPECT_EQ(3, buffer_response[0].data_size());
+        for (int i = 0; i < buffer_response[0].fd_size(); ++i)
+            EXPECT_EQ(package->ipc_fds[i], buffer_response[0].fd(i));
+        for (int i = 0; i < buffer_response[0].data_size(); ++i)
+            EXPECT_EQ(package->ipc_data[i], buffer_response[0].data(i));
+
+        mediator.next_buffer(nullptr, &buffer_request, &buffer_response[1], null_callback.get());
+        ASSERT_EQ(mc::BufferID{4}.as_uint32_t(), static_cast<uint32_t>(buffer_response[1].buffer_id()));
+        EXPECT_EQ(0, buffer_response[1].fd_size());
+        EXPECT_EQ(0, buffer_response[1].data_size());
+
+        mediator.next_buffer(nullptr, &buffer_request, &buffer_response[2], null_callback.get());
+
+        ASSERT_EQ(mc::BufferID{5}.as_uint32_t(), static_cast<uint32_t>(buffer_response[2].buffer_id()));
+        EXPECT_EQ(0, buffer_response[2].fd_size());
+        EXPECT_EQ(0, buffer_response[2].data_size());
+    }
+
+    mediator.disconnect(nullptr, nullptr, nullptr, null_callback.get());
+}

=== modified file 'tests/unit-tests/graphics/gbm/mock_drm.cpp'
--- tests/unit-tests/graphics/gbm/mock_drm.cpp	2013-02-05 16:18:10 +0000
+++ tests/unit-tests/graphics/gbm/mock_drm.cpp	2013-04-10 08:47:23 +0000
@@ -341,3 +341,13 @@
 {
     return global_mock->drmAuthMagic(fd, magic);
 }
+
+int drmPrimeHandleToFD(int fd, uint32_t handle, uint32_t flags, int *prime_fd)
+{
+    return global_mock->drmPrimeHandleToFD(fd, handle, flags, prime_fd);
+}
+
+int drmPrimeFDToHandle(int fd, int prime_fd, uint32_t *handle)
+{
+    return global_mock->drmPrimeFDToHandle(fd, prime_fd, handle);
+}

=== modified file 'tests/unit-tests/graphics/gbm/mock_drm.h'
--- tests/unit-tests/graphics/gbm/mock_drm.h	2013-02-05 16:18:10 +0000
+++ tests/unit-tests/graphics/gbm/mock_drm.h	2013-04-10 08:47:23 +0000
@@ -114,6 +114,9 @@
     MOCK_METHOD2(drmGetMagic, int(int fd, drm_magic_t *magic));
     MOCK_METHOD2(drmAuthMagic, int(int fd, drm_magic_t magic));
 
+    MOCK_METHOD4(drmPrimeHandleToFD, int(int fd, uint32_t handle, uint32_t flags, int *prime_fd));
+    MOCK_METHOD3(drmPrimeFDToHandle, int(int fd, int prime_fd, uint32_t *handle));
+
     FakeDRMResources fake_drm;
 };
 

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_buffer.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_buffer.cpp	2013-03-13 08:09:52 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_buffer.cpp	2013-04-10 08:47:23 +0000
@@ -142,8 +142,8 @@
 
     auto buffer = allocator->alloc_buffer(buffer_properties);
     auto ipc_package = buffer->get_ipc_package();
-    ASSERT_TRUE(ipc_package->ipc_fds.empty());
-    ASSERT_EQ(size_t(1), ipc_package->ipc_data.size());
+    ASSERT_EQ(size_t(1), ipc_package->ipc_fds.size());
+    ASSERT_TRUE(ipc_package->ipc_data.empty());
 }
 
 MATCHER_P(GEMFlinkHandleIs, value, "")
@@ -162,7 +162,7 @@
 {
     using namespace testing;
 
-    uint32_t gem_flink_name{0x77};
+    uint32_t prime_fd{0x77};
     gbm_bo_handle mock_handle;
     mock_handle.u32 = 0xdeadbeef;
 
@@ -170,23 +170,23 @@
             .Times(Exactly(1))
             .WillOnce(Return(mock_handle));
 
-    EXPECT_CALL(mock_drm, drmIoctl(_,DRM_IOCTL_GEM_FLINK, GEMFlinkHandleIs(mock_handle.u32)))
+    EXPECT_CALL(mock_drm, drmPrimeHandleToFD(_,mock_handle.u32,_,_))
             .Times(Exactly(1))
-            .WillOnce(DoAll(SetGEMFlinkName(gem_flink_name), Return(0)));
+            .WillOnce(DoAll(SetArgPointee<3>(prime_fd), Return(0)));
 
     EXPECT_NO_THROW({
         auto buffer = allocator->alloc_buffer(buffer_properties);
         auto ipc_package = buffer->get_ipc_package();
-        ASSERT_EQ(gem_flink_name, static_cast<uint32_t>(ipc_package->ipc_data[0]));
+        ASSERT_EQ(prime_fd, static_cast<uint32_t>(ipc_package->ipc_fds[0]));
         ASSERT_EQ(stride.as_uint32_t(), static_cast<uint32_t>(ipc_package->stride));
     });
 }
 
-TEST_F(GBMGraphicBufferBasic, buffer_ipc_package_throws_on_gem_flink_failure)
+TEST_F(GBMGraphicBufferBasic, buffer_ipc_package_throws_on_prime_fd_failure)
 {
     using namespace testing;
 
-    EXPECT_CALL(mock_drm, drmIoctl(_,DRM_IOCTL_GEM_FLINK,_))
+    EXPECT_CALL(mock_drm, drmPrimeHandleToFD(_,_,_,_))
             .Times(Exactly(1))
             .WillOnce(Return(-1));