Merge lp:~albaguirre/mir/back-to-stack-alloc-mir-protobuf into lp:mir

[Alberto Aguirre]

client: Use stack allocated mir::protobuf objects when possible

client: Use stack allocated mir::protobuf objects when possible

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2789
http://jenkins.qa.ubuntu.com/job/mir-ci/4439/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3347
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/200
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/872/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3295
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/588
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/588/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3295
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3295/artifact/work/output/*zip*/output.zip
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6088/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2789
http://jenkins.qa.ubuntu.com/job/mir-ci/4438/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3346/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/199/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/871/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3294/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/587/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3294/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2790
http://jenkins.qa.ubuntu.com/job/mir-ci/4440/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3348
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/201
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/873
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/589
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/589/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3296
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6089
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22198

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I know I asked for this. Although now I realize it's actually less efficient than using Arenas. Because on the stack your object is indeed allocated super fast (adjusting the stack pointer has always taken one clock tick or less). But stack allocation doesn't solve the problem for the subobjects (variable length lists, strings) that are still allocated on the heap. Arenas however solve both problems with the speed of a stack:

   https://developers.google.com/protocol-buffers/docs/reference/arenas?hl=en

So I've got nothing against this proposal right now. But if it makes the syntax less like the Arena syntax then it might be harder for us to get that optimal performance with Arenas in the long run.

[Alan Griffiths (alan-griffiths)]

281 class SharedLibrary;
282 -
283 +namespace protobuf
284 +{

I've a mild preference for a blank line before the namespace block.

[Kevin DuBois (kdub)]

+1

review: Approve

[Alberto Aguirre (albaguirre)]

> I know I asked for this. Although now I realize it's actually less efficient
> than using Arenas. Because on the stack your object is indeed allocated super
> fast (adjusting the stack pointer has always taken one clock tick or less).
> But stack allocation doesn't solve the problem for the subobjects (variable
> length lists, strings) that are still allocated on the heap. Arenas however
> solve both problems with the speed of a stack:
>
> https://developers.google.com/protocol-buffers/docs/reference/arenas?hl=en
>
> So I've got nothing against this proposal right now. But if it makes the
> syntax less like the Arena syntax then it might be harder for us to get that
> optimal performance with Arenas in the long run.

Ok, it shouldn't be too difficult to change to Arena based allocation even if this lands, but I need to investigate the code generated when "option cc_enable_arenas = true;" is used.

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1726/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3368
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/221
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/893
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3316
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/279/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3316
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3316/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6107
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22253

review: Needs Fixing (continuous-integration)

[Alberto Aguirre (albaguirre)]

^--Failure seems to be lp:1480422, lp:1480420

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1727/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3373
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/898
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3321/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/280
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/280/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3321
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3321/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6112/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22262

review: Needs Fixing (continuous-integration)

[Alberto Aguirre (albaguirre)]

"E: Failed to fetch http://ports.ubuntu.com/ubuntu-ports/pool/universe/g/glmark2/glmark2-data_2014.03+git20150611.fa71af2d-0ubuntu2_all.deb Temporary failure resolving 'ports.ubuntu.com'"

Arrghh....Re-trying.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2015-07-29 17:33:10 +0000
+++ src/client/buffer_stream.cpp	2015-07-29 17:33:10 +0000
@@ -24,6 +24,7 @@
 #include "perf_report.h"
 #include "logging/perf_report.h"
 #include "rpc/mir_display_server.h"
+#include "mir_protobuf.pb.h"
 
 #include "mir/log.h"
 #include "mir/client_platform.h"
@@ -201,12 +202,12 @@
 MirWaitHandle* mcl::BufferStream::submit(std::function<void()> const& done, std::unique_lock<std::mutex> lock)
 {
     //always submit what we have, whether we have a buffer, or will have to wait for an async reply
-    auto request = mcl::make_protobuf_object<mp::BufferRequest>();
-    request->mutable_id()->set_value(protobuf_bs->id().value());
-    request->mutable_buffer()->set_buffer_id(buffer_depository.current_buffer_id());
+    mp::BufferRequest request;
+    request.mutable_id()->set_value(protobuf_bs->id().value());
+    request.mutable_buffer()->set_buffer_id(buffer_depository.current_buffer_id());
     lock.unlock();
 
-    display_server.submit_buffer(request.get(), protobuf_void.get(),
+    display_server.submit_buffer(&request, protobuf_void.get(),
         google::protobuf::NewCallback(google::protobuf::DoNothing));
 
     lock.lock();
@@ -241,14 +242,14 @@
     }
     else
     {
-        auto screencast_id = mcl::make_protobuf_object<mp::ScreencastId>();
-        screencast_id->set_value(protobuf_bs->id().value());
+        mp::ScreencastId screencast_id;
+        screencast_id.set_value(protobuf_bs->id().value());
 
         lock.unlock();
         next_buffer_wait_handle.expect_result();
 
         display_server.screencast_buffer(
-            screencast_id.get(),
+            &screencast_id,
             protobuf_bs->mutable_buffer(),
             google::protobuf::NewCallback(
             this, &mcl::BufferStream::next_buffer_received,
@@ -337,21 +338,21 @@
         BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set swap interval on screencast is invalid"));
     }
 
-    auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
-    auto result = mcl::make_protobuf_object<mp::SurfaceSetting>();
-    setting->mutable_surfaceid()->set_value(protobuf_bs->id().value());
-    setting->set_attrib(attrib);
-    setting->set_ivalue(value);
+    mp::SurfaceSetting setting;
+    mp::SurfaceSetting result;
+    setting.mutable_surfaceid()->set_value(protobuf_bs->id().value());
+    setting.set_attrib(attrib);
+    setting.set_ivalue(value);
     lock.unlock();
 
     configure_wait_handle.expect_result();
-    display_server.configure_surface(setting.get(), result.get(),
+    display_server.configure_surface(&setting, &result,
         google::protobuf::NewCallback(this, &mcl::BufferStream::on_configured));
 
     configure_wait_handle.wait_for_all();
 
     lock.lock();
-    swap_interval_ = result->ivalue();
+    swap_interval_ = result.ivalue();
 }
 
 uint32_t mcl::BufferStream::get_current_buffer_id()
 
=== modified file 'src/client/buffer_stream.h'
--- src/client/buffer_stream.h	2015-07-29 17:33:10 +0000
+++ src/client/buffer_stream.h	2015-07-29 17:33:10 +0000
@@ -19,8 +19,6 @@
 #ifndef MIR_CLIENT_BUFFER_STREAM_H
 #define MIR_CLIENT_BUFFER_STREAM_H
 
-#include "mir_protobuf.pb.h"
-
 #include "mir_wait_handle.h"
 #include "mir/egl_native_surface.h"
 #include "mir/client_buffer.h"
@@ -41,6 +39,12 @@
 {
 class Logger;
 }
+namespace protobuf
+{
+class BufferStream;
+class BufferStreamParameters;
+class Void;
+}
 namespace client
 {
 namespace rpc
 
=== modified file 'src/client/client_buffer_stream.h'
--- src/client/client_buffer_stream.h	2015-07-16 07:03:19 +0000
+++ src/client/client_buffer_stream.h	2015-07-29 17:33:10 +0000
@@ -32,6 +32,10 @@
 
 namespace mir
 {
+namespace protobuf
+{
+class Buffer;
+}
 namespace client
 {
 class ClientBuffer;
 
=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2015-07-29 17:33:10 +0000
+++ src/client/mir_connection.cpp	2015-07-29 17:33:10 +0000
@@ -19,6 +19,7 @@
 #include "mir_connection.h"
 #include "mir_surface.h"
 #include "mir_prompt_session.h"
+#include "mir_protobuf.pb.h"
 #include "default_client_buffer_stream_factory.h"
 #include "make_protobuf_object.h"
 #include "mir_toolkit/mir_platform_message.h"
@@ -48,6 +49,7 @@
 namespace mev = mir::events;
 namespace gp = google::protobuf;
 namespace mf = mir::frontend;
+namespace mp = mir::protobuf;
 
 namespace
 {
@@ -229,8 +231,8 @@
 
     SurfaceRelease surf_release{surface, new_wait_handle, callback, context};
 
-    auto message = mcl::make_protobuf_object<mir::protobuf::SurfaceId>();
-    message->set_value(surface->id());
+    mp::SurfaceId message;
+    message.set_value(surface->id());
 
     {
         std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
@@ -238,7 +240,7 @@
     }
 
     new_wait_handle->expect_result();
-    server.release_surface(message.get(), void_response.get(),
+    server.release_surface(&message, void_response.get(),
                            gp::NewCallback(this, &MirConnection::released, surf_release));
 
 
@@ -391,19 +393,19 @@
         return nullptr;
     }
 
-    auto protobuf_request = mcl::make_protobuf_object<mir::protobuf::PlatformOperationMessage>();
+    mp::PlatformOperationMessage protobuf_request;
 
-    protobuf_request->set_opcode(mir_platform_message_get_opcode(request));
+    protobuf_request.set_opcode(mir_platform_message_get_opcode(request));
     auto const request_data = mir_platform_message_get_data(request);
     auto const request_fds = mir_platform_message_get_fds(request);
 
-    protobuf_request->set_data(request_data.data, request_data.size);
+    protobuf_request.set_data(request_data.data, request_data.size);
     for (size_t i = 0; i != request_fds.num_fds; ++i)
-        protobuf_request->add_fd(request_fds.fds[i]);
+        protobuf_request.add_fd(request_fds.fds[i]);
 
     platform_operation_wait_handle.expect_result();
     server.platform_operation(
-        protobuf_request.get(),
+        &protobuf_request,
         platform_operation_reply.get(),
         google::protobuf::NewCallback(this, &MirConnection::done_platform_operation,
                                       callback, context));
@@ -499,17 +501,17 @@
     mir_buffer_stream_callback callback,
     void *context)
 {
-    auto params = mcl::make_protobuf_object<mir::protobuf::BufferStreamParameters>();
-    params->set_width(width);
-    params->set_height(height);
-    params->set_pixel_format(format);
-    params->set_buffer_usage(buffer_usage);
+    mp::BufferStreamParameters params;
+    params.set_width(width);
+    params.set_height(height);
+    params.set_pixel_format(format);
+    params.set_buffer_usage(buffer_usage);
 
-    return buffer_stream_factory->make_producer_stream(this, server, *params, callback, context);
+    return buffer_stream_factory->make_producer_stream(this, server, params, callback, context);
 }
 
 std::shared_ptr<mir::client::ClientBufferStream> MirConnection::make_consumer_stream(
-   mir::protobuf::BufferStream const& protobuf_bs, std::string const& surface_name)
+   mp::BufferStream const& protobuf_bs, std::string const& surface_name)
 {
     return buffer_stream_factory->make_consumer_stream(this, server, protobuf_bs, surface_name);
 }
@@ -549,9 +551,9 @@
 
 void MirConnection::pong(int32_t serial)
 {
-    auto pong = mcl::make_protobuf_object<mir::protobuf::PingEvent>();
-    pong->set_serial(serial);
-    server.pong(pong.get(), void_response.get(), pong_callback.get());
+    mp::PingEvent pong;
+    pong.set_serial(serial);
+    server.pong(&pong, void_response.get(), pong_callback.get());
 }
 
 void MirConnection::register_display_change_callback(mir_display_config_callback callback, void* context)
@@ -603,14 +605,14 @@
         return NULL;
     }
 
-    auto request = mcl::make_protobuf_object<mir::protobuf::DisplayConfiguration>();
+    mp::DisplayConfiguration request;
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
 
         for (auto i=0u; i < config->num_outputs; i++)
         {
             auto output = config->outputs[i];
-            auto display_request = request->add_display_output();
+            auto display_request = request.add_display_output();
             display_request->set_output_id(output.output_id);
             display_request->set_used(output.used);
             display_request->set_current_mode(output.current_mode);
@@ -623,7 +625,7 @@
     }
 
     configure_display_wait_handle.expect_result();
-    server.configure_display(request.get(), display_configuration_response.get(),
+    server.configure_display(&request, display_configuration_response.get(),
         google::protobuf::NewCallback(this, &MirConnection::done_display_configure));
 
     return &configure_display_wait_handle;
@@ -648,8 +650,8 @@
 
     StreamRelease stream_release{stream, new_wait_handle, callback, context};
 
-    auto buffer_stream_id = mcl::make_protobuf_object<mir::protobuf::BufferStreamId>();
-    buffer_stream_id->set_value(stream->rpc_id().as_value());
+    mp::BufferStreamId buffer_stream_id;
+    buffer_stream_id.set_value(stream->rpc_id().as_value());
 
     {
         std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
@@ -658,7 +660,7 @@
 
     new_wait_handle->expect_result();
     server.release_buffer_stream(
-        buffer_stream_id.get(), void_response.get(),
+        &buffer_stream_id, void_response.get(),
         google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
     return new_wait_handle;
 }
 
=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2015-07-29 17:33:10 +0000
+++ src/client/mir_connection.h	2015-07-29 17:33:10 +0000
@@ -18,7 +18,6 @@
 #ifndef MIR_CLIENT_MIR_CONNECTION_H_
 #define MIR_CLIENT_MIR_CONNECTION_H_
 
-#include "mir_protobuf.pb.h"
 #include "mir_wait_handle.h"
 #include "lifecycle_control.h"
 #include "ping_handler.h"
@@ -39,7 +38,14 @@
 namespace mir
 {
 class SharedLibrary;
-
+namespace protobuf
+{
+class BufferStream;
+class Connection;
+class ConnectParameters;
+class PlatformOperationMessage;
+class DisplayConfiguration;
+}
 /// The client-side library implementation namespace
 namespace client
 {
 
=== modified file 'src/client/mir_prompt_session.cpp'
--- src/client/mir_prompt_session.cpp	2015-07-29 17:33:10 +0000
+++ src/client/mir_prompt_session.cpp	2015-07-29 17:33:10 +0000
@@ -20,6 +20,7 @@
 #include "event_handler_register.h"
 #include "make_protobuf_object.h"
 #include "rpc/mir_display_server.h"
+#include "mir_protobuf.pb.h"
 
 namespace mp = mir::protobuf;
 namespace mcl = mir::client;
@@ -146,13 +147,13 @@
     mir_client_fd_callback callback,
     void * context)
 {
-    auto request = mcl::make_protobuf_object<mir::protobuf::SocketFDRequest>();
-    request->set_number(no_of_fds);
+    mp::SocketFDRequest request;
+    request.set_number(no_of_fds);
 
     fds_for_prompt_providers_wait_handle.expect_result();
 
     server.new_fds_for_prompt_providers(
-        request.get(),
+        &request,
         socket_fd_response.get(),
         google::protobuf::NewCallback(this, &MirPromptSession::done_fds_for_prompt_providers,
                                               callback, context));
 
=== modified file 'src/client/mir_prompt_session.h'
--- src/client/mir_prompt_session.h	2015-07-29 17:33:10 +0000
+++ src/client/mir_prompt_session.h	2015-07-29 17:33:10 +0000
@@ -21,7 +21,6 @@
 
 #include "mir_toolkit/mir_client_library.h"
 
-#include "mir_protobuf.pb.h"
 #include "mir_wait_handle.h"
 
 #include <mutex>
@@ -30,6 +29,12 @@
 
 namespace mir
 {
+namespace protobuf
+{
+class PromptSessionParameters;
+class SocketFD;
+class Void;
+}
 /// The client-side library implementation namespace
 namespace client
 {
 
=== modified file 'src/client/mir_screencast.cpp'
--- src/client/mir_screencast.cpp	2015-07-29 17:33:10 +0000
+++ src/client/mir_screencast.cpp	2015-07-29 17:33:10 +0000
@@ -18,6 +18,7 @@
 
 #include "mir_screencast.h"
 #include "mir_connection.h"
+#include "mir_protobuf.pb.h"
 #include "make_protobuf_object.h"
 #include "client_buffer_stream.h"
 #include "mir/frontend/client_constants.h"
@@ -53,19 +54,19 @@
     }
     protobuf_screencast->set_error("Not initialized");
 
-    auto parameters = mcl::make_protobuf_object<mir::protobuf::ScreencastParameters>();
+    mp::ScreencastParameters parameters;
 
-    parameters->mutable_region()->set_left(region.top_left.x.as_int());
-    parameters->mutable_region()->set_top(region.top_left.y.as_int());
-    parameters->mutable_region()->set_width(region.size.width.as_uint32_t());
-    parameters->mutable_region()->set_height(region.size.height.as_uint32_t());
-    parameters->set_width(output_size.width.as_uint32_t());
-    parameters->set_height(output_size.height.as_uint32_t());
-    parameters->set_pixel_format(pixel_format);
+    parameters.mutable_region()->set_left(region.top_left.x.as_int());
+    parameters.mutable_region()->set_top(region.top_left.y.as_int());
+    parameters.mutable_region()->set_width(region.size.width.as_uint32_t());
+    parameters.mutable_region()->set_height(region.size.height.as_uint32_t());
+    parameters.set_width(output_size.width.as_uint32_t());
+    parameters.set_height(output_size.height.as_uint32_t());
+    parameters.set_pixel_format(pixel_format);
 
     create_screencast_wait_handle.expect_result();
     server.create_screencast(
-        parameters.get(),
+        &parameters,
         protobuf_screencast.get(),
         google::protobuf::NewCallback(
             this, &MirScreencast::screencast_created,
@@ -85,12 +86,12 @@
 MirWaitHandle* MirScreencast::release(
         mir_screencast_callback callback, void* context)
 {
-    auto screencast_id = mcl::make_protobuf_object<mir::protobuf::ScreencastId>();
-    screencast_id->set_value(protobuf_screencast->screencast_id().value());
+    mp::ScreencastId screencast_id;
+    screencast_id.set_value(protobuf_screencast->screencast_id().value());
     
     release_wait_handle.expect_result();
     server.release_screencast(
-        screencast_id.get(),
+        &screencast_id,
         protobuf_void.get(),
         google::protobuf::NewCallback(
             this, &MirScreencast::released, callback, context));
 
=== modified file 'src/client/mir_screencast.h'
--- src/client/mir_screencast.h	2015-07-29 17:33:10 +0000
+++ src/client/mir_screencast.h	2015-07-29 17:33:10 +0000
@@ -21,7 +21,6 @@
 
 #include "mir_wait_handle.h"
 #include "mir_toolkit/client_types.h"
-#include "mir_protobuf.pb.h"
 #include "mir/geometry/size.h"
 #include "mir/geometry/rectangle.h"
 
@@ -31,6 +30,11 @@
 
 namespace mir
 {
+namespace protobuf
+{
+class Screencast;
+class Void;
+}
 namespace client
 {
 namespace rpc
 
=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2015-07-29 17:33:10 +0000
+++ src/client/mir_surface.cpp	2015-07-29 17:33:10 +0000
@@ -22,6 +22,7 @@
 #include "client_buffer_stream.h"
 #include "client_buffer_stream_factory.h"
 #include "make_protobuf_object.h"
+#include "mir_protobuf.pb.h"
 
 #include "mir_toolkit/mir_client_library.h"
 #include "mir/frontend/client_constants.h"
@@ -43,14 +44,81 @@
 namespace gp = google::protobuf;
 namespace md = mir::dispatch;
 
-#define SERIALIZE_OPTION_IF_SET(option, message) \
-    if (option.is_set()) \
-        message->set_##option(option.value());
-
 namespace
 {
 std::mutex handle_mutex;
 std::unordered_set<MirSurface*> valid_surfaces;
+
+mir::protobuf::SurfaceParameters serialize_spec(MirSurfaceSpec const& spec)
+{
+    mp::SurfaceParameters message;
+
+#define SERIALIZE_OPTION_IF_SET(option) \
+    if (spec.option.is_set()) \
+        message.set_##option(spec.option.value());
+
+    SERIALIZE_OPTION_IF_SET(width);
+    SERIALIZE_OPTION_IF_SET(height);
+    SERIALIZE_OPTION_IF_SET(pixel_format);
+    SERIALIZE_OPTION_IF_SET(buffer_usage);
+    SERIALIZE_OPTION_IF_SET(surface_name);
+    SERIALIZE_OPTION_IF_SET(output_id);
+    SERIALIZE_OPTION_IF_SET(type);
+    SERIALIZE_OPTION_IF_SET(state);
+    SERIALIZE_OPTION_IF_SET(pref_orientation);
+    SERIALIZE_OPTION_IF_SET(edge_attachment);
+    SERIALIZE_OPTION_IF_SET(min_width);
+    SERIALIZE_OPTION_IF_SET(min_height);
+    SERIALIZE_OPTION_IF_SET(max_width);
+    SERIALIZE_OPTION_IF_SET(max_height);
+    SERIALIZE_OPTION_IF_SET(width_inc);
+    SERIALIZE_OPTION_IF_SET(height_inc);
+    // min_aspect is a special case (below)
+    // max_aspect is a special case (below)
+
+    if (spec.parent.is_set() && spec.parent.value() != nullptr)
+        message.set_parent_id(spec.parent.value()->id());
+
+    if (spec.parent_id)
+    {
+        auto id = message.mutable_parent_persistent_id();
+        id->set_value(spec.parent_id->as_string());
+    }
+
+    if (spec.aux_rect.is_set())
+    {
+        message.mutable_aux_rect()->set_left(spec.aux_rect.value().left);
+        message.mutable_aux_rect()->set_top(spec.aux_rect.value().top);
+        message.mutable_aux_rect()->set_width(spec.aux_rect.value().width);
+        message.mutable_aux_rect()->set_height(spec.aux_rect.value().height);
+    }
+
+    if (spec.min_aspect.is_set())
+    {
+        message.mutable_min_aspect()->set_width(spec.min_aspect.value().width);
+        message.mutable_min_aspect()->set_height(spec.min_aspect.value().height);
+    }
+
+    if (spec.max_aspect.is_set())
+    {
+        message.mutable_max_aspect()->set_width(spec.max_aspect.value().width);
+        message.mutable_max_aspect()->set_height(spec.max_aspect.value().height);
+    }
+
+    if (spec.input_shape.is_set())
+    {
+        for (auto const& rect : spec.input_shape.value())
+        {
+            auto const new_shape = message.add_input_shape();
+            new_shape->set_left(rect.left);
+            new_shape->set_top(rect.top);
+            new_shape->set_width(rect.width);
+            new_shape->set_height(rect.height);
+        }
+    }
+
+    return message;
+}
 }
 
 MirSurfaceSpec::MirSurfaceSpec(
@@ -78,77 +146,7 @@
     }
 }
 
-MirSurfaceSpec::MirSurfaceSpec()
-{
-}
-
-std::unique_ptr<mir::protobuf::SurfaceParameters> MirSurfaceSpec::serialize() const
-{
-    //std::unique_ptr<mp::SurfaceParameters> message{mp::SurfaceParameters::default_instance().New()};
-    auto message = mcl::make_protobuf_object<mp::SurfaceParameters>();
-
-    SERIALIZE_OPTION_IF_SET(width, message);
-    SERIALIZE_OPTION_IF_SET(height, message);
-    SERIALIZE_OPTION_IF_SET(pixel_format, message);
-    SERIALIZE_OPTION_IF_SET(buffer_usage, message);
-    SERIALIZE_OPTION_IF_SET(surface_name, message);
-    SERIALIZE_OPTION_IF_SET(output_id, message);
-    SERIALIZE_OPTION_IF_SET(type, message);
-    SERIALIZE_OPTION_IF_SET(state, message);
-    SERIALIZE_OPTION_IF_SET(pref_orientation, message);
-    SERIALIZE_OPTION_IF_SET(edge_attachment, message);
-    SERIALIZE_OPTION_IF_SET(min_width, message);
-    SERIALIZE_OPTION_IF_SET(min_height, message);
-    SERIALIZE_OPTION_IF_SET(max_width, message);
-    SERIALIZE_OPTION_IF_SET(max_height, message);
-    SERIALIZE_OPTION_IF_SET(width_inc, message);
-    SERIALIZE_OPTION_IF_SET(height_inc, message);
-    // min_aspect is a special case (below)
-    // max_aspect is a special case (below)
-
-    if (parent.is_set() && parent.value() != nullptr)
-        message->set_parent_id(parent.value()->id());
-
-    if (parent_id)
-    {
-        auto id = message->mutable_parent_persistent_id();
-        id->set_value(parent_id->as_string());
-    }
-
-    if (aux_rect.is_set())
-    {
-        message->mutable_aux_rect()->set_left(aux_rect.value().left);
-        message->mutable_aux_rect()->set_top(aux_rect.value().top);
-        message->mutable_aux_rect()->set_width(aux_rect.value().width);
-        message->mutable_aux_rect()->set_height(aux_rect.value().height);
-    }
-
-    if (min_aspect.is_set())
-    {
-        message->mutable_min_aspect()->set_width(min_aspect.value().width);
-        message->mutable_min_aspect()->set_height(min_aspect.value().height);
-    }
-
-    if (max_aspect.is_set())
-    {
-        message->mutable_max_aspect()->set_width(max_aspect.value().width);
-        message->mutable_max_aspect()->set_height(max_aspect.value().height);
-    }
-
-    if (input_shape.is_set())
-    {
-        for (auto const& rect : input_shape.value())
-        {
-            auto const new_shape = message->add_input_shape();
-            new_shape->set_left(rect.left);
-            new_shape->set_top(rect.top);
-            new_shape->set_width(rect.width);
-            new_shape->set_height(rect.height);
-        }
-    }
-
-    return message;
-}
+MirSurfaceSpec::MirSurfaceSpec() = default;
 
 MirPersistentId::MirPersistentId(std::string const& string_id)
     : string_id{string_id}
@@ -193,11 +191,11 @@
     for (int i = 0; i < mir_surface_attribs; i++)
         attrib_cache[i] = -1;
 
-    auto const message = spec.serialize();
+    auto const message = serialize_spec(spec);
     create_wait_handle.expect_result();
     try 
     {
-        server->create_surface(message.get(), surface.get(), gp::NewCallback(this, &MirSurface::created, callback, context));
+        server->create_surface(&message, surface.get(), gp::NewCallback(this, &MirSurface::created, callback, context));
     }
     catch (std::exception const& ex)
     {
@@ -383,28 +381,28 @@
 
 MirWaitHandle* MirSurface::configure_cursor(MirCursorConfiguration const* cursor)
 {
-    auto setting = mcl::make_protobuf_object<mp::CursorSetting>();
+    mp::CursorSetting setting;
 
     {
         std::unique_lock<decltype(mutex)> lock(mutex);
-        setting->mutable_surfaceid()->CopyFrom(surface->id());
+        setting.mutable_surfaceid()->CopyFrom(surface->id());
         if (cursor)
         {
             if (cursor->stream != nullptr)
             {
-                setting->mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
-                setting->set_hotspot_x(cursor->hotspot_x);
-                setting->set_hotspot_y(cursor->hotspot_y);
+                setting.mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
+                setting.set_hotspot_x(cursor->hotspot_x);
+                setting.set_hotspot_y(cursor->hotspot_y);
             }
             else if (cursor->name != mir_disabled_cursor_name)
             {
-                setting->set_name(cursor->name.c_str());
+                setting.set_name(cursor->name.c_str());
             }
         }
     }
     
     configure_cursor_wait_handle.expect_result();
-    server->configure_cursor(setting.get(), void_response.get(),
+    server->configure_cursor(&setting, void_response.get(),
         google::protobuf::NewCallback(this, &MirSurface::on_cursor_configured));
     
     return &configure_cursor_wait_handle;
@@ -424,14 +422,14 @@
 
     std::unique_lock<decltype(mutex)> lock(mutex);
 
-    auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
-    setting->mutable_surfaceid()->CopyFrom(surface->id());
-    setting->set_attrib(at);
-    setting->set_ivalue(value);
+    mp::SurfaceSetting setting;
+    setting.mutable_surfaceid()->CopyFrom(surface->id());
+    setting.set_attrib(at);
+    setting.set_ivalue(value);
     lock.unlock();
 
     configure_wait_handle.expect_result();
-    server->configure_surface(setting.get(), configure_result.get(),
+    server->configure_surface(&setting, configure_result.get(),
               google::protobuf::NewCallback(this, &MirSurface::on_configured));
 
     return &configure_wait_handle;
@@ -453,12 +451,12 @@
         return false;
     }
 
-    auto request = mcl::make_protobuf_object<mp::CoordinateTranslationRequest>();
+    mp::CoordinateTranslationRequest request;
 
-    request->set_x(x);
-    request->set_y(y);
-    *request->mutable_surfaceid() = surface->id();
-    auto response = mcl::make_protobuf_object<mp::CoordinateTranslationResponse>();
+    request.set_x(x);
+    request.set_y(y);
+    *request.mutable_surfaceid() = surface->id();
+    mp::CoordinateTranslationResponse response;
 
     MirWaitHandle signal;
     signal.expect_result();
@@ -467,16 +465,16 @@
         std::lock_guard<decltype(mutex)> lock(mutex);
 
         debug->translate_surface_to_screen(
-            request.get(),
-            response.get(),
+            &request,
+            &response,
             google::protobuf::NewCallback(&signal_response_received, &signal));
     }
 
     signal.wait_for_one();
 
-    *screen_x = response->x();
-    *screen_y = response->y();
-    return !response->has_error();
+    *screen_x = response.x();
+    *screen_y = response.y();
+    return !response.has_error();
 }
 
 void MirSurface::on_configured()
@@ -628,14 +626,14 @@
 
 MirWaitHandle* MirSurface::modify(MirSurfaceSpec const& spec)
 {
-    auto mods = mcl::make_protobuf_object<mp::SurfaceModifications>();
+    mp::SurfaceModifications mods;
 
     {
         std::unique_lock<decltype(mutex)> lock(mutex);
-        mods->mutable_surface_id()->set_value(surface->id().value());
+        mods.mutable_surface_id()->set_value(surface->id().value());
     }
 
-    auto const surface_specification = mods->mutable_surface_specification();
+    auto const surface_specification = mods.mutable_surface_specification();
 
     #define COPY_IF_SET(field)\
         if (spec.field.is_set())\
@@ -727,7 +725,7 @@
     }
 
     modify_wait_handle.expect_result();
-    server->modify_surface(mods.get(), modify_result.get(),
+    server->modify_surface(&mods, modify_result.get(),
               google::protobuf::NewCallback(this, &MirSurface::on_modified));
 
     return &modify_wait_handle;
 
=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2015-07-29 17:33:10 +0000
+++ src/client/mir_surface.h	2015-07-29 17:33:10 +0000
@@ -18,7 +18,6 @@
 #ifndef MIR_CLIENT_MIR_SURFACE_H_
 #define MIR_CLIENT_MIR_SURFACE_H_
 
-#include "mir_protobuf.pb.h"
 #include "client_buffer_depository.h"
 #include "client_buffer_stream.h"
 #include "mir_wait_handle.h"
@@ -51,6 +50,14 @@
 class XKBMapper;
 }
 }
+namespace protobuf
+{
+class PersistentSurfaceId;
+class Surface;
+class SurfaceParameters;
+class SurfaceSetting;
+class Void;
+}
 namespace client
 {
 namespace rpc
@@ -73,8 +80,6 @@
     MirSurfaceSpec(MirConnection* connection, int width, int height, MirPixelFormat format);
     MirSurfaceSpec(MirConnection* connection, MirSurfaceParameters const& params);
 
-    std::unique_ptr<mir::protobuf::SurfaceParameters> serialize() const;
-
     struct AspectRatio { unsigned width; unsigned height; };
 
     // Required parameters
 
=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
--- src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-07-29 17:33:10 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-07-29 17:33:10 +0000
@@ -40,6 +40,7 @@
 namespace mcl = mir::client;
 namespace mclr = mir::client::rpc;
 namespace md = mir::dispatch;
+namespace mp = mir::protobuf;
 
 namespace
 {
@@ -241,48 +242,48 @@
 
 void mclr::MirProtobufRpcChannel::process_event_sequence(std::string const& event)
 {
-    auto seq = mcl::make_protobuf_object<mir::protobuf::EventSequence>();
-
-    seq->ParseFromString(event);
-
-    if (seq->has_display_configuration())
-    {
-        display_configuration->update_configuration(seq->display_configuration());
-    }
-
-    if (seq->has_lifecycle_event())
-    {
-        (*lifecycle_control)(static_cast<MirLifecycleState>(seq->lifecycle_event().new_state()));
-    }
-
-    if (seq->has_ping_event())
-    {
-        (*ping_handler)(seq->ping_event().serial());
-    }
-
-    if (seq->has_buffer_request())
+    mp::EventSequence seq;
+
+    seq.ParseFromString(event);
+
+    if (seq.has_display_configuration())
+    {
+        display_configuration->update_configuration(seq.display_configuration());
+    }
+
+    if (seq.has_lifecycle_event())
+    {
+        (*lifecycle_control)(static_cast<MirLifecycleState>(seq.lifecycle_event().new_state()));
+    }
+
+    if (seq.has_ping_event())
+    {
+        (*ping_handler)(seq.ping_event().serial());
+    }
+
+    if (seq.has_buffer_request())
     {
         std::array<char, 1> dummy;
-        auto const num_fds = seq->mutable_buffer_request()->mutable_buffer()->fds_on_side_channel();
+        auto const num_fds = seq.mutable_buffer_request()->mutable_buffer()->fds_on_side_channel();
         std::vector<mir::Fd> fds(num_fds);
         if (num_fds > 0)
         {
             transport->receive_data(dummy.data(), dummy.size(), fds);
-            seq->mutable_buffer_request()->mutable_buffer()->clear_fd();
+            seq.mutable_buffer_request()->mutable_buffer()->clear_fd();
             for(auto& fd : fds)
-                seq->mutable_buffer_request()->mutable_buffer()->add_fd(fd);
+                seq.mutable_buffer_request()->mutable_buffer()->add_fd(fd);
         }
 
-        surface_map->with_stream_do(mf::BufferStreamId(seq->buffer_request().id().value()),
+        surface_map->with_stream_do(mf::BufferStreamId(seq.buffer_request().id().value()),
         [&] (mcl::ClientBufferStream* stream) {
-            stream->buffer_available(seq->buffer_request().buffer());
+            stream->buffer_available(seq.buffer_request().buffer());
         });
     }
 
-    int const nevents = seq->event_size();
+    int const nevents = seq.event_size();
     for (int i = 0; i != nevents; ++i)
     {
-        mir::protobuf::Event const& event = seq->event(i);
+        mp::Event const& event = seq.event(i);
         if (event.has_raw())
         {
             std::string const& raw_event = event.raw();
@@ -349,7 +350,7 @@
      */
     std::lock_guard<decltype(read_mutex)> lock(read_mutex);
 
-    auto result = mcl::make_protobuf_object<mir::protobuf::wire::Result>();
+    auto result = mcl::make_protobuf_object<mp::wire::Result>();
     try
     {
         uint16_t message_size;
@@ -393,7 +394,7 @@
                 {
                     // It's too difficult to convince C++ to move this lambda everywhere, so
                     // just give up and let it pretend its a shared_ptr.
-                    std::shared_ptr<mir::protobuf::wire::Result> appeaser{std::move(result)};
+                    std::shared_ptr<mp::wire::Result> appeaser{std::move(result)};
                     delayed_processor->enqueue([delayed_result = std::move(appeaser), this]() mutable
                     {
                         pending_calls.complete_response(*delayed_result);
 
=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.h'
--- src/client/rpc/mir_protobuf_rpc_channel.h	2015-07-29 17:33:10 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.h	2015-07-29 17:33:10 +0000
@@ -28,8 +28,6 @@
 #include "../lifecycle_control.h"
 #include "../ping_handler.h"
 
-#include "mir_protobuf_wire.pb.h"
-
 #include <thread>
 #include <atomic>
 #include <experimental/optional>