Merge lp:~mir-team/mir/bstream-constructor-cleanup into lp:mir

[Kevin DuBois]

client: unify the constructors for mcl::BufferStream. Could be scrubbed again to reduce the number of parameters.

client: unify the constructors for mcl::BufferStream. Could be scrubbed again to reduce the number of parameters.

[Kevin DuBois (kdub)]

the few conflicts are in base, probably not too hard to review while conflicts get resolved.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3807
https://mir-jenkins.ubuntu.com/job/mir-ci/2245/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2904/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2969
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2962
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2962
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2962
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2934/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2934/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2934/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2934
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2934/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2934
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2934/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2934/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2245/rebuild

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

> the few conflicts are in base, probably not too hard to review while conflicts
> get resolved.

The prereq has been updated to remove the conflict.

[Alan Griffiths (alan-griffiths)]

Conflicts still

review: Needs Fixing

[lp:~mir-team/mir/bstream-constructor-cleanup]

3808. By Kevin DuBois on 2016-12-02

merge, fix up teh conflicts

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3808
https://mir-jenkins.ubuntu.com/job/mir-ci/2320/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3018/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3083
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3075
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3075
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3075
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3047/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3047/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3047/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3047/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3047/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3047
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3047/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3047/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2320/rebuild

review: Needs Fixing (continuous-integration)

[Chris Halse Rogers (raof)]

LGTM.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[Daniel van Vugt (vanvugt)]

^^^
Jenkins failures are crashing tests. Seems like this branch needs some fixing.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

review: Abstain

[Cemil Azizoglu (cemil-azizoglu)]

Needs an update.

Needs to remove other instances of render_surface(), e.g..

review: Needs Fixing

Unmerged revisions

3808. By Kevin DuBois on 2016-12-02

merge, fix up teh conflicts

3807. By Kevin DuBois on 2016-11-22

better handling of default steram

3806. By Kevin DuBois on 2016-11-22

merge in base

3805. By Kevin DuBois on 2016-11-22

cleanup another function, corrects int/acc tests

3804. By Kevin DuBois on 2016-11-22

port unit tests to new constructor

3803. By Kevin DuBois on 2016-11-21

fix up commented out coed

3802. By Kevin DuBois on 2016-11-21

merge in base

3801. By Kevin DuBois on 2016-11-21

need to restack on branch that removes one of constructors

3800. By Kevin DuBois on 2016-11-18

merge in mir

3799. By Kevin DuBois on 2016-11-17

merge in mir

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2016-11-30 08:13:49 +0000
+++ src/client/buffer_stream.cpp	2016-12-02 14:27:14 +0000
@@ -56,7 +56,7 @@
 class Requests : public mcl::ServerBufferRequests
 {
 public:
-    Requests(mclr::DisplayServer& server, int stream_id) :
+    Requests(mclr::DisplayServer& server, mf::BufferStreamId stream_id) :
         server(server),
         stream_id(stream_id)
     {
@@ -65,7 +65,7 @@
     void allocate_buffer(geom::Size size, MirPixelFormat format, int usage) override
     {
         mp::BufferAllocation request;
-        request.mutable_id()->set_value(stream_id);
+        request.mutable_id()->set_value(stream_id.as_value());
         auto buf_params = request.add_buffer_requests();
         buf_params->set_width(size.width.as_int());
         buf_params->set_height(size.height.as_int());
@@ -80,7 +80,7 @@
     void free_buffer(int buffer_id) override
     {
         mp::BufferRelease request;
-        request.mutable_id()->set_value(stream_id);
+        request.mutable_id()->set_value(stream_id.as_value());
         request.add_buffers()->set_buffer_id(buffer_id);
 
         auto protobuf_void = std::make_shared<mp::Void>();
@@ -91,7 +91,7 @@
     void submit_buffer(mcl::MirBuffer& buffer) override
     {
         mp::BufferRequest request;
-        request.mutable_id()->set_value(stream_id);
+        request.mutable_id()->set_value(stream_id.as_value());
         request.mutable_buffer()->set_buffer_id(buffer.rpc_id());
 
         auto protobuf_void = std::make_shared<mp::Void>();
@@ -105,7 +105,7 @@
 
 private:
     mclr::DisplayServer& server;
-    int stream_id;
+    mf::BufferStreamId const stream_id;
 };
 
 mir::optional_value<int> parse_env_for_swap_interval()
@@ -238,80 +238,48 @@
 
 mcl::BufferStream::BufferStream(
     MirConnection* connection,
-    MirRenderSurface* render_surface,
+    mf::BufferStreamId id,
     std::shared_ptr<MirWaitHandle> creation_wait_handle,
     mclr::DisplayServer& server,
     std::shared_ptr<mcl::ClientPlatform> const& client_platform,
     std::weak_ptr<mcl::SurfaceMap> const& map,
     std::shared_ptr<mcl::AsyncBufferFactory> const& factory,
-    mp::BufferStream const& a_protobuf_bs,
     std::shared_ptr<mcl::PerfReport> const& perf_report,
     std::string const& surface_name,
     geom::Size ideal_size,
+    MirPixelFormat format,
+    MirBufferUsage usage,
     size_t nbuffers)
     : connection_(connection),
       client_platform(client_platform),
-      protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>(a_protobuf_bs)},
       user_swap_interval(parse_env_for_swap_interval()),
-      interval_config{server, frontend::BufferStreamId{a_protobuf_bs.id().value()}},
+      interval_config{server, id},
       scale_(1.0f),
       perf_report(perf_report),
       protobuf_void{mcl::make_protobuf_object<mir::protobuf::Void>()},
+      id(id),
       ideal_buffer_size(ideal_size),
+      format(format),
+      usage(usage),
       nbuffers(nbuffers),
       creation_wait_handle(creation_wait_handle),
       map(map),
-      factory(factory),
-      render_surface_(render_surface)
+      factory(factory)
 {
-    if (!protobuf_bs->has_id())
-    {
-        if (!protobuf_bs->has_error())
-            protobuf_bs->set_error("Error processing buffer stream create response, no ID (disconnected?)");
-    }
-
-    if (protobuf_bs->has_error())
-    {
-        if (protobuf_bs->has_buffer())
-        {
-            for (int i = 0; i < protobuf_bs->buffer().fd_size(); i++)
-                ::close(protobuf_bs->buffer().fd(i));
-        }
-
-        BOOST_THROW_EXCEPTION(std::runtime_error("Can not create buffer stream: " + std::string(protobuf_bs->error())));
-    }
-
-    try
-    {
-        buffer_depository = std::make_unique<BufferDepository>(
-            client_platform->create_buffer_factory(), factory,
-            std::make_shared<Requests>(server, protobuf_bs->id().value()), map,
-            ideal_buffer_size, static_cast<MirPixelFormat>(protobuf_bs->pixel_format()), 
-            protobuf_bs->buffer_usage(), nbuffers);
-
-        egl_native_window_ = client_platform->create_egl_native_window(this);
-
-        // This might seem like something to provide during creation but
-        // knowing the swap interval is not a precondition to creation. It's
-        // only a precondition to your second and subsequent swaps, so don't
-        // bother the creation parameters with this stuff...
-        if (user_swap_interval.is_set())
-            set_swap_interval(user_swap_interval.value());
-    }
-    catch (std::exception const& error)
-    {
-        protobuf_bs->set_error(std::string{"Error processing buffer stream creating response:"} +
-                               boost::diagnostic_information(error));
-
-        if (!buffer_depository)
-        {
-            for (int i = 0; i < protobuf_bs->buffer().fd_size(); i++)
-                ::close(protobuf_bs->buffer().fd(i));
-        }
-    }
-
-    if (!valid())
-        BOOST_THROW_EXCEPTION(std::runtime_error("Can not create buffer stream: " + std::string(protobuf_bs->error())));
+    buffer_depository = std::make_unique<BufferDepository>(
+        client_platform->create_buffer_factory(), factory,
+        std::make_shared<Requests>(server, id), map,
+        ideal_buffer_size, format, usage, nbuffers);
+
+    egl_native_window_ = client_platform->create_egl_native_window(this);
+
+    // This might seem like something to provide during creation but
+    // knowing the swap interval is not a precondition to creation. It's
+    // only a precondition to your second and subsequent swaps, so don't
+    // bother the creation parameters with this stuff...
+    if (user_swap_interval.is_set())
+        set_swap_interval(user_swap_interval.value());
+
     perf_report->name_surface(surface_name.c_str());
 }
 
@@ -338,9 +306,8 @@
 
     try
     {
-        auto pixel_format = static_cast<MirPixelFormat>(protobuf_bs->pixel_format());
         lk.unlock();
-        buffer_depository->deposit(buffer, size, pixel_format);
+        buffer_depository->deposit(buffer, size, format);
     }
     catch (const std::runtime_error& err)
     {
@@ -359,8 +326,7 @@
     // TODO: We can fix the strange "ID casting" used below in the second phase
     // of buffer stream which generalizes and clarifies the server side logic.
     lock.unlock();
-    return buffer_depository->submit(done,
-        static_cast<MirPixelFormat>(protobuf_bs->pixel_format()), protobuf_bs->id().value());
+    return buffer_depository->submit(done, format, id);
 }
 
 std::shared_ptr<mcl::ClientBuffer> mcl::BufferStream::get_current_buffer()
@@ -401,8 +367,8 @@
         "",
         size.width.as_int(),
         size.height.as_int(),
-        static_cast<MirPixelFormat>(protobuf_bs->pixel_format()),
-        static_cast<MirBufferUsage>(protobuf_bs->buffer_usage()),
+        format,
+        usage,
         mir_display_output_id_invalid};
 }
 
@@ -454,15 +420,12 @@
 
 mf::BufferStreamId mcl::BufferStream::rpc_id() const
 {
-    std::unique_lock<decltype(mutex)> lock(mutex);
-
-    return mf::BufferStreamId(protobuf_bs->id().value());
+    return id;
 }
 
 bool mcl::BufferStream::valid() const
 {
-    std::unique_lock<decltype(mutex)> lock(mutex);
-    return protobuf_bs->has_id() && !protobuf_bs->has_error();
+    return id.as_value() >= 0;
 }
 
 void mcl::BufferStream::set_buffer_cache_size(unsigned int)
@@ -493,27 +456,15 @@
 
 MirWaitHandle* mcl::BufferStream::set_scale(float scale)
 {
-    return buffer_depository->set_scale(scale, mf::BufferStreamId(protobuf_bs->id().value()));
+    return buffer_depository->set_scale(scale, id);
 }
 
 char const * mcl::BufferStream::get_error_message() const
 {
-    std::lock_guard<decltype(mutex)> lock(mutex);
-
-    if (protobuf_bs->has_error())
-    {
-        return protobuf_bs->error().c_str();
-    }
-
-    return error_message.c_str();
+    return "";
 }
 
 MirConnection* mcl::BufferStream::connection() const
 {
     return connection_;
 }
-
-MirRenderSurface* mcl::BufferStream::render_surface() const
-{
-    return render_surface_;
-}

=== modified file 'src/client/buffer_stream.h'
--- src/client/buffer_stream.h	2016-12-02 05:05:06 +0000
+++ src/client/buffer_stream.h	2016-12-02 14:27:14 +0000
@@ -73,22 +73,20 @@
 {
 public:
     BufferStream(
-        mir::client::rpc::DisplayServer& server,
-        std::weak_ptr<SurfaceMap> const& map);
-    BufferStream(
         MirConnection* connection,
-        MirRenderSurface* render_surface,
+        frontend::BufferStreamId id,
         std::shared_ptr<MirWaitHandle> creation_wait_handle,
         mir::client::rpc::DisplayServer& server,
         std::shared_ptr<ClientPlatform> const& native_window_factory,
         std::weak_ptr<SurfaceMap> const& map,
         std::shared_ptr<AsyncBufferFactory> const& factory,
-        mir::protobuf::BufferStream const& protobuf_bs,
         std::shared_ptr<PerfReport> const& perf_report,
         std::string const& surface_name,
-        geometry::Size ideal_size, size_t nbuffers);
-
-    virtual ~BufferStream();
+        geometry::Size ideal_size,
+        MirPixelFormat format,
+        MirBufferUsage usage,
+        size_t nbuffers);
+    ~BufferStream();
 
     MirWaitHandle* swap_buffers(std::function<void()> const& done) override;
     std::shared_ptr<mir::client::ClientBuffer> get_current_buffer() override;
@@ -122,7 +120,6 @@
     MirWaitHandle* set_scale(float scale) override;
     char const* get_error_message() const override;
     MirConnection* connection() const override;
-    MirRenderSurface* render_surface() const override;
 
 protected:
     BufferStream(BufferStream const&) = delete;
@@ -140,7 +137,6 @@
 
     MirConnection* connection_;
     std::shared_ptr<ClientPlatform> const client_platform;
-    std::unique_ptr<mir::protobuf::BufferStream> protobuf_bs;
 
     optional_value<int> user_swap_interval;
     BufferStreamConfiguration interval_config;
@@ -154,13 +150,15 @@
     std::shared_ptr<MemoryRegion> secured_region;
 
     std::unique_ptr<BufferDepository> buffer_depository;
+    frontend::BufferStreamId const id;
     geometry::Size ideal_buffer_size;
+    MirPixelFormat const format;
+    MirBufferUsage const usage;
     size_t const nbuffers;
     std::string error_message;
     std::shared_ptr<MirWaitHandle> creation_wait_handle;
     std::weak_ptr<SurfaceMap> const map;
     std::shared_ptr<AsyncBufferFactory> const factory;
-    MirRenderSurface* render_surface_;
 };
 
 }

=== modified file 'src/client/mir_buffer_stream_api.cpp'
--- src/client/mir_buffer_stream_api.cpp	2016-12-02 05:05:06 +0000
+++ src/client/mir_buffer_stream_api.cpp	2016-12-02 14:27:14 +0000
@@ -58,7 +58,7 @@
 try
 {
     return connection->create_client_buffer_stream(
-        width, height, format, buffer_usage, nullptr, callback, context);
+        width, height, format, buffer_usage, callback, context);
 }
 catch (std::exception const& ex)
 {

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2016-12-02 05:05:06 +0000
+++ src/client/mir_connection.cpp	2016-12-02 14:27:14 +0000
@@ -242,6 +242,24 @@
     }
 }
 
+bool validate_buffer_stream(mp::BufferStream& msg)
+{
+    if (!msg.has_id())
+    {
+        if (!msg.has_error())
+            msg.set_error("no ID in response (disconnected?)");
+    }
+
+    if (msg.has_error())
+    {
+        for (int i = 0; i < msg.buffer().fd_size(); i++)
+            ::close(msg.buffer().fd(i));
+        return false;
+    }
+    return true;
+}
+
+
 std::mutex connection_guard;
 MirConnection* valid_connections{nullptr};
 }
@@ -385,16 +403,24 @@
     auto const& spec = request->spec;
     std::string name{spec.surface_name.is_set() ?
                      spec.surface_name.value() : ""};
+    auto pf = static_cast<MirPixelFormat>(surface_proto->pixel_format());
+    auto usage = static_cast<MirBufferUsage>(surface_proto->buffer_usage());
+    auto size = mir::geometry::Size{surface_proto->width(), surface_proto->height()};
 
     std::shared_ptr<MirBufferStream> default_stream {nullptr};
-    if (surface_proto->buffer_stream().has_id())
+
+    if (surface_proto->has_buffer_stream() && surface_proto->buffer_stream().has_id())
     {
         try
         {
+            if (!validate_buffer_stream(*surface_proto->mutable_buffer_stream()))
+                throw std::invalid_argument(surface_proto->buffer_stream().error());
+
             default_stream = std::make_shared<mcl::BufferStream>(
-                this, nullptr, request->wh, server, platform, surface_map, buffer_factory,
-                surface_proto->buffer_stream(), make_perf_report(logger), name,
-                mir::geometry::Size{surface_proto->width(), surface_proto->height()}, nbuffers);
+                this, mf::BufferStreamId{surface_proto->buffer_stream().id().value()},
+                request->wh, server, platform, surface_map, buffer_factory,
+                make_perf_report(logger), name,
+                size, pf, usage, nbuffers);
             surface_map->insert(default_stream->rpc_id(), default_stream);
         }
         catch (std::exception const& error)
@@ -405,6 +431,10 @@
             // surface_proto->buffer_stream()
             for (auto i = 0; i < surface_proto->fd_size(); i++)
                 ::close(surface_proto->fd(i));
+
+            mf::BufferStreamId id(next_error_id(lock).as_value());
+            default_stream = std::make_shared<mcl::ErrorStream>(
+                std::string{error.what()}, this, id, nullptr);
         }
     }
 
@@ -812,30 +842,24 @@
         request = *stream_it;
         stream_requests.erase(stream_it);
     }
+
     auto& protobuf_bs = request->response;
-
-    if (!protobuf_bs->has_id())
-    {
-        if (!protobuf_bs->has_error())
-            protobuf_bs->set_error("no ID in response (disconnected?)");
-    }
-
-    if (protobuf_bs->has_error())
-    {
-        for (int i = 0; i < protobuf_bs->buffer().fd_size(); i++)
-            ::close(protobuf_bs->buffer().fd(i));
-        stream_error(
-            std::string{"Error processing buffer stream response: "} + protobuf_bs->error(),
-            request);
+    if (!validate_buffer_stream(*protobuf_bs))
+    {
+        stream_error(std::string{"Error processing buffer stream response: "} + protobuf_bs->error(), request);
         return;
     }
 
     try
     {
+        auto pf = static_cast<MirPixelFormat>(request->parameters.pixel_format());
+        auto usage = static_cast<MirBufferUsage>(request->parameters.buffer_usage());
+        auto size = mir::geometry::Size{request->parameters.width(), request->parameters.height()};
+        auto id = mir::frontend::BufferStreamId{protobuf_bs->id().value()};
         auto stream = std::make_shared<mcl::BufferStream>(
-            this, request->rs, request->wh, server, platform, surface_map, buffer_factory,
-            *protobuf_bs, make_perf_report(logger), std::string{},
-            mir::geometry::Size{request->parameters.width(), request->parameters.height()}, nbuffers);
+            this, id, request->wh, server, platform, surface_map, buffer_factory,
+            make_perf_report(logger), std::string{},
+            size, pf, usage, nbuffers);
         surface_map->insert(mf::BufferStreamId(protobuf_bs->id().value()), stream);
 
         if (request->mbs_callback)
@@ -846,8 +870,7 @@
     catch (std::exception const& error)
     {
         stream_error(
-            std::string{"Error processing buffer stream creating response:"} + boost::diagnostic_information(error),
-            request);
+            std::string{"Error processing buffer stream creating response:"} + boost::diagnostic_information(error), request);
     }
 }
 
@@ -855,7 +878,6 @@
     int width, int height,
     MirPixelFormat format,
     MirBufferUsage buffer_usage,
-    MirRenderSurface* render_surface,
     mir_buffer_stream_callback mbs_callback,
     void *context)
 {
@@ -865,10 +887,7 @@
     params.set_pixel_format(format);
     params.set_buffer_usage(buffer_usage);
 
-    auto request = std::make_shared<StreamCreationRequest>(render_surface,
-                                                           mbs_callback,
-                                                           context,
-                                                           params);
+    auto request = std::make_shared<StreamCreationRequest>(mbs_callback, context, params);
     request->wh->expect_result();
 
     {
@@ -890,16 +909,13 @@
 }
 
 std::shared_ptr<mir::client::BufferStream> MirConnection::create_client_buffer_stream_with_id(
-    int width, int height,
-    MirRenderSurface* render_surface,
-    mir::protobuf::BufferStream const& a_protobuf_bs)
+    mir::geometry::Size size, MirPixelFormat format, MirBufferUsage usage,
+    MirRenderSurface* render_surface)
 {
-    auto stream = std::make_shared<mcl::BufferStream>(
-        this, render_surface, nullptr, server, platform, surface_map, buffer_factory,
-        a_protobuf_bs, make_perf_report(logger), std::string{},
-        mir::geometry::Size{width, height}, nbuffers);
-    surface_map->insert(render_surface->stream_id(), stream);
-    return stream;
+    return std::make_shared<mcl::BufferStream>(
+        this, render_surface->stream_id(), nullptr, server, platform, surface_map, buffer_factory,
+        make_perf_report(logger), std::string{},
+        size, format, usage, nbuffers);
 }
 
 void MirConnection::render_surface_error(std::string const& error_msg, std::shared_ptr<RenderSurfaceCreationRequest> const& request)

=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2016-12-02 05:05:06 +0000
+++ src/client/mir_connection.h	2016-12-02 14:27:14 +0000
@@ -154,13 +154,13 @@
         int width, int height,
         MirPixelFormat format,
         MirBufferUsage buffer_usage,
-        MirRenderSurface* render_surface,
         mir_buffer_stream_callback mbs_callback,
         void *context);
+
     std::shared_ptr<mir::client::BufferStream> create_client_buffer_stream_with_id(
-        int width, int height,
-        MirRenderSurface* render_surface,
-        mir::protobuf::BufferStream const& a_protobuf_bs);
+        mir::geometry::Size size, MirPixelFormat format, MirBufferUsage usage,
+        MirRenderSurface* render_surface);
+
     MirWaitHandle* release_buffer_stream(
         MirBufferStream*,
         mir_buffer_stream_callback callback,
@@ -246,19 +246,16 @@
     struct StreamCreationRequest
     {
         StreamCreationRequest(
-            MirRenderSurface* rs,
             mir_buffer_stream_callback mbs_cb,
             void* context,
             mir::protobuf::BufferStreamParameters const& params)
-            : rs(rs),
-              mbs_callback(mbs_cb),
+            : mbs_callback(mbs_cb),
               context(context),
               parameters(params),
               response(std::make_shared<mir::protobuf::BufferStream>()),
               wh(std::make_shared<MirWaitHandle>())
         {
         }
-        MirRenderSurface* rs;
         mir_buffer_stream_callback mbs_callback;
         void* context;
         mir::protobuf::BufferStreamParameters const parameters;

=== modified file 'src/client/render_surface.cpp'
--- src/client/render_surface.cpp	2016-12-02 05:05:06 +0000
+++ src/client/render_surface.cpp	2016-12-02 14:27:14 +0000
@@ -65,10 +65,9 @@
     {
         protobuf_bs->set_pixel_format(format);
         protobuf_bs->set_buffer_usage(buffer_usage);
-        stream_from_id = connection_->create_client_buffer_stream_with_id(width,
-                                                                          height,
-                                                                          this,
-                                                                          *protobuf_bs);
+        stream_from_id = connection_->create_client_buffer_stream_with_id(
+            geom::Size{width, height}, format, buffer_usage, this);
+
         if (buffer_usage == mir_buffer_usage_hardware)
         {
             platform->use_egl_native_window(

=== modified file 'src/client/screencast_stream.cpp'
--- src/client/screencast_stream.cpp	2016-11-30 08:13:49 +0000
+++ src/client/screencast_stream.cpp	2016-12-02 14:27:14 +0000
@@ -244,11 +244,6 @@
     return connection_;
 }
 
-MirRenderSurface* mcl::ScreencastStream::render_surface() const
-{
-    return nullptr;
-}
-
 void mcl::ScreencastStream::set_buffer_cache_size(unsigned int)
 {
     BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set cache size on screencast is invalid"));

=== modified file 'src/client/screencast_stream.h'
--- src/client/screencast_stream.h	2016-12-02 05:05:06 +0000
+++ src/client/screencast_stream.h	2016-12-02 14:27:14 +0000
@@ -98,7 +98,6 @@
     MirWaitHandle* set_scale(float scale) override;
     char const* get_error_message() const override;
     MirConnection* connection() const override;
-    MirRenderSurface* render_surface() const override;
 
 private:
     void process_buffer(protobuf::Buffer const& buffer);

=== modified file 'src/include/client/mir/mir_buffer_stream.h'
--- src/include/client/mir/mir_buffer_stream.h	2016-12-02 05:05:06 +0000
+++ src/include/client/mir/mir_buffer_stream.h	2016-12-02 14:27:14 +0000
@@ -99,7 +99,6 @@
     virtual MirWaitHandle* set_scale(float) = 0;
     virtual char const* get_error_message() const = 0;
     virtual MirConnection* connection() const = 0;
-    virtual MirRenderSurface* render_surface() const = 0;
 
     virtual void buffer_available(mir::protobuf::Buffer const& buffer) = 0;
     virtual void buffer_unavailable() = 0;

=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
--- tests/unit-tests/client/test_client_buffer_stream.cpp	2016-11-30 08:13:49 +0000
+++ tests/unit-tests/client/test_client_buffer_stream.cpp	2016-12-02 14:27:14 +0000
@@ -212,15 +212,16 @@
 
     testing::NiceMock<mtd::MockProtobufServer> mock_protobuf_server;
 
-    MirPixelFormat const default_pixel_format = mir_pixel_format_argb_8888;
-    MirBufferUsage const default_buffer_usage = mir_buffer_usage_hardware;
+    MirPixelFormat const format = mir_pixel_format_argb_8888;
+    MirBufferUsage const usage = mir_buffer_usage_hardware;
 
     std::shared_ptr<mcl::PerfReport> const perf_report = std::make_shared<mcl::NullPerfReport>();
 
     MirBufferPackage buffer_package = a_buffer_package();
     geom::Size size{buffer_package.width, buffer_package.height};
     mp::BufferStream response = a_protobuf_buffer_stream(
-        default_pixel_format, default_buffer_usage);
+        format, usage);
+    mir::frontend::BufferStreamId id { 4 };
     size_t nbuffers{3};
 };
 
@@ -255,55 +256,14 @@
 
 }
 
-TEST_F(ClientBufferStream, protobuf_requirements)
-{
-    auto valid_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage);
-    EXPECT_NO_THROW({
-        mcl::BufferStream bs(
-            nullptr, nullptr, wait_handle, mock_protobuf_server,
-            std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-            map, factory,
-            valid_bs, perf_report, "", size, nbuffers);
-    });
-    valid_bs.clear_buffer();
-    EXPECT_NO_THROW({
-        mcl::BufferStream bs(
-            nullptr, nullptr, wait_handle, mock_protobuf_server,
-            std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-            map, factory,
-            valid_bs, perf_report, "", size, nbuffers);
-    });
-
-    auto error_bs = valid_bs;
-    error_bs.set_error("An error");
-    EXPECT_THROW({
-        mcl::BufferStream bs(
-            nullptr, nullptr, wait_handle, mock_protobuf_server,
-            std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-            map, factory,
-            error_bs, perf_report, "", size, nbuffers);
-    }, std::runtime_error);
-    
-    auto no_id_bs = valid_bs;
-    no_id_bs.clear_id();
-    EXPECT_THROW({
-        mcl::BufferStream bs(
-            nullptr, nullptr, wait_handle, mock_protobuf_server,
-            std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-            map, factory,
-            no_id_bs, perf_report, "", size, nbuffers);
-    }, std::runtime_error);
-}
-
 TEST_F(ClientBufferStream, uses_buffer_message_from_server)
 {
     EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
         .WillOnce(Return(std::make_shared<mtd::NullClientBuffer>()));
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
 }
 
@@ -311,11 +271,10 @@
 {
     EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_))
         .WillOnce(mtd::RunProtobufClosure());
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     bs.swap_buffers([]{});
@@ -324,11 +283,10 @@
 TEST_F(ClientBufferStream, invokes_callback_on_swap_buffers)
 {
     mp::Buffer buffer;
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
     ON_CALL(mock_protobuf_server, submit_buffer(_,_,_))
         .WillByDefault(DoAll(
@@ -347,17 +305,16 @@
     size = { width, height };
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     auto params = bs.get_parameters();
 
     EXPECT_STREQ("", params.name);
     EXPECT_EQ(width, params.width);
     EXPECT_EQ(height, params.height);
-    EXPECT_EQ(default_pixel_format, params.pixel_format);
-    EXPECT_EQ(default_buffer_usage, params.buffer_usage);
+    EXPECT_EQ(format, params.pixel_format);
+    EXPECT_EQ(usage, params.buffer_usage);
 }
 
 TEST_F(ClientBufferStream, returns_current_client_buffer)
@@ -371,7 +328,7 @@
     mp::Buffer protobuf_buffer_2;
     fill_protobuf_buffer_from_package(&protobuf_buffer_1, buffer_package_1);
     fill_protobuf_buffer_from_package(&protobuf_buffer_2, buffer_package_2);
-    auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage);
+    auto protobuf_bs = a_protobuf_buffer_stream(format, usage);
     
     Sequence seq;
     EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package_1),_,_))
@@ -382,10 +339,9 @@
         .WillOnce(Return(client_buffer_2));
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        protobuf_bs, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
     EXPECT_EQ(client_buffer_1, bs.get_current_buffer());
 
@@ -400,7 +356,7 @@
     auto const client_buffer_2 = std::make_shared<mtd::NullClientBuffer>(size);
     auto buffer_package_1 = a_buffer_package();
     auto buffer_package_2 = a_buffer_package();
-    auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage);
+    auto protobuf_bs = a_protobuf_buffer_stream(format, usage);
     mp::Buffer protobuf_buffer_2;
     fill_protobuf_buffer_from_package(&protobuf_buffer_2, buffer_package_2);
     auto expected_size = geom::Size{buffer_package_1.width, buffer_package_1.height};
@@ -414,10 +370,9 @@
         .WillOnce(Return(client_buffer_2));
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        protobuf_bs, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
     EXPECT_EQ(client_buffer_1, bs.get_current_buffer());
     async_buffer_arrives(protobuf_buffer_2);
@@ -427,11 +382,10 @@
 
 TEST_F(ClientBufferStream, gets_egl_native_window)
 {
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     EXPECT_EQ(StubClientPlatform::egl_native_window, bs.egl_native_window());
 }
 
@@ -444,10 +398,9 @@
         .WillOnce(Return(mt::fake_shared(mock_client_buffer)));
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
 
     mcl::MemoryRegion expected_memory_region;
@@ -465,10 +418,9 @@
     ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
         .WillByDefault(Return(mt::fake_shared(mock_client_buffer)));
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(2);
 
     mcl::MemoryRegion first_expected_memory_region;
@@ -493,10 +445,9 @@
     std::string const name = "a_unique_surface_name";
     EXPECT_CALL(mock_perf_report, name_surface(StrEq(name)));
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, mt::fake_shared(mock_perf_report), name, size, nbuffers);
+        map, factory, mt::fake_shared(mock_perf_report), name, size, format, usage, nbuffers);
 }
 
 TEST_F(ClientBufferStream, perf_report_starts_frame_at_get_current_buffer)
@@ -506,10 +457,9 @@
     EXPECT_CALL(mock_perf_report, end_frame(_)).Times(0);
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, mt::fake_shared(mock_perf_report), "Foo", size, nbuffers);
+        map, factory, mt::fake_shared(mock_perf_report), "Foo", size, format, usage, nbuffers);
 
     service_requests_for(1);
 
@@ -523,10 +473,9 @@
     EXPECT_CALL(mock_perf_report, end_frame(_)).Times(0);
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, mt::fake_shared(mock_perf_report), "Foo", size, nbuffers);
+        map, factory, mt::fake_shared(mock_perf_report), "Foo", size, format, usage, nbuffers);
 
     service_requests_for(1);
 
@@ -540,10 +489,9 @@
     EXPECT_CALL(mock_perf_report, end_frame(_)).Times(1);
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, mt::fake_shared(mock_perf_report), "Foo", size, nbuffers);
+        map, factory, mt::fake_shared(mock_perf_report), "Foo", size, format, usage, nbuffers);
 
     service_requests_for(2);
 
@@ -564,10 +512,9 @@
         .WillOnce(Return(mt::fake_shared(mock_client_buffer)));
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, ClientBufferStream::id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
 
     mir::protobuf::Buffer another_buffer_package;
@@ -601,10 +548,9 @@
     bool started{false};
 
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     auto never_serviced_request = std::async(std::launch::async,[&] {
@@ -633,11 +579,10 @@
     MirWaitHandle* wh{nullptr};
     bool wait_handle_has_result_in_callback = false;
 
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     wh = bs.swap_buffers(
@@ -655,11 +600,10 @@
     MirWaitHandle* wh{nullptr};
     bool wait_handle_has_result_in_callback = false;
 
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     wh = bs.swap_buffers(
@@ -675,11 +619,10 @@
 
 TEST_F(ClientBufferStream, configures_swap_interval)
 {
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     EXPECT_CALL(mock_protobuf_server, configure_buffer_stream(_,_,_));
@@ -688,11 +631,10 @@
 
 TEST_F(ClientBufferStream, sets_swap_interval_requested)
 {
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     bs.set_swap_interval(1);
@@ -705,11 +647,10 @@
 TEST_F(ClientBufferStream, environment_overrides_requested_swap_interval)
 {
     setenv("MIR_CLIENT_FORCE_SWAP_INTERVAL", "0", 1);
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     bs.set_swap_interval(1);
@@ -736,10 +677,9 @@
     EXPECT_CALL(mock_factory, create_buffer(_,_,format))
         .WillRepeatedly(Return(std::make_shared<mtd::NullClientBuffer>()));
     mcl::BufferStream bs(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(1);
     auto params = bs.get_parameters();
     EXPECT_THAT(params.pixel_format, Eq(format));
@@ -750,10 +690,9 @@
     ON_CALL(mock_factory, create_buffer(_,_,_))
         .WillByDefault(Return(std::make_shared<mtd::NullClientBuffer>(size)));
     mcl::BufferStream stream(
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers);
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     for(auto i = 0u; i < 2; i++)
     {
         mp::Buffer buffer;
@@ -772,11 +711,10 @@
 {
     ON_CALL(mock_protobuf_server, submit_buffer(_,_,_))
         .WillByDefault(mtd::RunProtobufClosure());
-    mcl::BufferStream bs{
-        nullptr, nullptr, wait_handle, mock_protobuf_server,
+    mcl::BufferStream bs(
+        nullptr, id, wait_handle, mock_protobuf_server,
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
-        map, factory,
-        response, perf_report, "", size, nbuffers};
+        map, factory, perf_report, "", size, format, usage, nbuffers);
     service_requests_for(mock_protobuf_server.alloc_count);
 
     auto count = 0;

=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2016-12-02 05:05:06 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2016-12-02 14:27:14 +0000
@@ -651,7 +651,7 @@
     EXPECT_CALL(*mock_channel, on_buffer_stream_create(_,_))
         .WillOnce(Invoke([](mp::BufferStream& bs, google::protobuf::Closure*){ bs.set_error("danger will robertson"); }));
     EXPECT_FALSE(connection->create_client_buffer_stream(
-        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware, nullptr, nullptr, nullptr)->is_pending());
+        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware, nullptr, nullptr)->is_pending());
 }
 
 TEST_F(MirConnectionTest, wait_handle_is_signalled_during_creation_exception)
@@ -661,7 +661,7 @@
             Invoke([](mp::BufferStream&, google::protobuf::Closure* c){ c->Run(); }),
             Throw(std::runtime_error("pay no attention to the man behind the curtain"))));
     auto wh = connection->create_client_buffer_stream(
-        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware, nullptr, nullptr, nullptr);
+        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware, nullptr, nullptr);
     ASSERT_THAT(wh, Ne(nullptr));
     EXPECT_FALSE(wh->is_pending()); 
 }
@@ -678,7 +678,7 @@
 
     connection->create_client_buffer_stream(
         2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware,
-        nullptr, &BufferStreamCallback::created, &callback);
+        &BufferStreamCallback::created, &callback);
     EXPECT_TRUE(callback.invoked);
     ASSERT_TRUE(callback.resulting_stream);
     EXPECT_THAT(mir_buffer_stream_get_error_message(callback.resulting_stream),
@@ -694,9 +694,29 @@
             Invoke([](mp::BufferStream&, google::protobuf::Closure* c){ c->Run(); }),
             Throw(std::runtime_error("pay no attention to the man behind the curtain"))));
     connection->create_client_buffer_stream(
-        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware, nullptr,
-        &BufferStreamCallback::created, &callback);
-
+        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware,
+        &BufferStreamCallback::created, &callback);
+
+    EXPECT_TRUE(callback.invoked);
+    ASSERT_TRUE(callback.resulting_stream);
+    EXPECT_THAT(mir_buffer_stream_get_error_message(callback.resulting_stream),
+        StrEq("Error processing buffer stream response: no ID in response (disconnected?)"));
+}
+
+TEST_F(MirConnectionTest, creates_error_stream_when_not_given_stream_id)
+{
+    using namespace testing;
+    BufferStreamCallback callback;
+    EXPECT_CALL(*mock_channel, on_buffer_stream_create(_,_))
+        .WillOnce(Invoke([&](mp::BufferStream& bs, google::protobuf::Closure*)
+        {
+            bs.clear_id();
+            bs.clear_error();
+        }));
+
+    connection->create_client_buffer_stream(
+        2, 2, mir_pixel_format_abgr_8888, mir_buffer_usage_hardware,
+        &BufferStreamCallback::created, &callback);
     EXPECT_TRUE(callback.invoked);
     ASSERT_TRUE(callback.resulting_stream);
     EXPECT_THAT(mir_buffer_stream_get_error_message(callback.resulting_stream),