Merge lp:~alan-griffiths/mir/surface-states-simplification into lp:~mir-team/mir/trunk

[Alan Griffiths]

frontend, shell, tests: surface-states updated to avoid supplying dependencies through public member functions.

frontend, shell, tests: surface-states updated to avoid supplying dependencies through public member functions.

Where possible, dependencies should be supplied as constructor arguments and held as const members.

(This required updates to a few factory interfaces to supply the dependencies.)

[Daniel van Vugt (vanvugt)]

Firstly, needs fixing:
Text conflict in tests/unit-tests/shell/test_application_session.cpp
1 conflicts encountered.

Secondly, I disagree with the approach. There are no strong "dependencies" here. The information in question is all optional to the class receiving it, so should not be passed in constructors. The classes can function without event sinks or surface IDs. They will work as they already do. Only event emission would not happen. So it's optional relative to those classes, and therefore not something to enforce in the constructor.

Aside from anything else, the degree of coupling introduced here looks very high. It's always better to minimize coupling. You can't call it a simplification if coupling is increased and the diff is "+205/-148".

review: Disapprove

[Daniel van Vugt (vanvugt)]

Also, here's an example use-case for changing the event sink of a surface:

Windows (like the one Flash video uses in a web browser) are implemented using reparenting. If Mir was to support such a thing in future then it would be handy to be able to re-assign a surface to a different session. Hence you'd also need to be able to change a surface's event target (its session).

[Alexandros Frantzis (afrantzis)]

> Secondly, I disagree with the approach. There are no strong "dependencies" here.

As mentioned in an older comment [1], they are effectively dependencies because of how Mir uses the class. That is, in all cases in production code we need to set both the id and the sink for our features to work properly.

> If Mir was to support such a thing in future then it would be handy to be able to re-assign
> a surface to a different session

The key words are "if ... in the future ...". If we need this in the future, we are free to change the code to suit our needs. Right now the values are meant to be set only once at creation time, and accepting them in the constructor enforces this restriction (and, as mentioned above, also enforces that we always get a fully working object).

[1] https://code.launchpad.net/~vanvugt/mir/surface-states/+merge/158289/comments/348068

[Daniel van Vugt (vanvugt)]

There is a difference between a local dependency and a system dependency. On the local scale, the classes do not depend on EventSink to work. So on a local scale, it should not affect the constructor.

A surface can emit events. A surface can have an ID. However those are not required attributes for the surface to be created and work as a surface.

No amount of "modern C++ tradition" supersedes the basic programming ideal of low coupling.

[Alan Griffiths (alan-griffiths)]

> Firstly, needs fixing:
> Text conflict in tests/unit-tests/shell/test_application_session.cpp
> 1 conflicts encountered.

Resolved.

> Secondly, I disagree with the approach. There are no strong "dependencies"
> here. The information in question is all optional to the class receiving it,
> so should not be passed in constructors. The classes can function without
> event sinks or surface IDs. They will work as they already do. Only event
> emission would not happen. So it's optional relative to those classes, and
> therefore not something to enforce in the constructor.

Arguing that a subset of the functionality works without these attributes is an argument that the subset belongs in a class providing just that subset. While we could factor that subset out, I don't think we need to that now.

> Aside from anything else, the degree of coupling introduced here looks very
> high. It's always better to minimize coupling.

I agree that coupling should be reduced - I disagree that this MP increases coupling. For example, the interface used by frontend should not need Session::set_event_sink().

> You can't call it a
> simplification if coupling is increased and the diff is "+205/-148".

You can't measure coupling (or other attributes of the resulting code) by diff size.

[Daniel van Vugt (vanvugt)]

I have stated my opinions strongly. I disagree with this proposal as it significantly decreases readability and maintainability. Those are the traits of high coupling, and things I would like to avoid.

[Kevin DuBois (kdub)]

> > Secondly, I disagree with the approach. There are no strong "dependencies"
> > here. The information in question is all optional to the class receiving it,
> > so should not be passed in constructors. The classes can function without
> > event sinks or surface IDs. They will work as they already do. Only event
> > emission would not happen. So it's optional relative to those classes, and
> > therefore not something to enforce in the constructor.
>
> Arguing that a subset of the functionality works without these attributes is
> an argument that the subset belongs in a class providing just that subset.
> While we could factor that subset out, I don't think we need to that now.

I think we're using the term 'coupling' too broadly to make much progress arguing :) I'll propose re-framing the argument, and then give my 2 cents

I think we can agree on this though:
1) Its better if a function call doesn't have any hidden prerequisites, like "you must call function A before you call function B". This is sort of 'coupling of calling requirements".
2) A class with a lot of constructor parameters has a lot of dependencies and might be trying to do too much. This is sort of 'coupling of object dependencies'

my 2cents...
so we have to find a way to balance.

I think that, with regards to notify_change/set_id, set_id() is a dependency that has to be called before notify_change() is called, so the id may as well be in the constructor. Another possibility (if we really don't want any more construction parameters for Surface) is to have another object with expanded functionality that takes

We've tried to mitigate the problem of #2 by having a lot of mocks and stubs and nulls that are easy to plug in and make the object easy to manipulate in test. Since its easier to read a constructor than it is to trace what the call order is, I'd rather plug in mocks/stubs to the constructor than have to trace a call order to figure out if we've fulfilled our usage requirements.

[Daniel van Vugt (vanvugt)]

You are free to propose and land this directly to lp:mir.

I just don't want code like this landing in a branch with my name on it.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:669
http://jenkins.qa.ubuntu.com/job/mir-ci/480/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/516/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/399
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/485
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/485/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/234

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:670
http://jenkins.qa.ubuntu.com/job/mir-ci/482/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/518
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/401
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/487
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/487/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

might be the first time there's been 2 rival reviews going on ;-)

again, seems the biggest point of contention is about 2 phase initialization and what has more or less coupling. maybe a new day will let me rephrase a bit better. :) Seems to me that (for example) in msh::Surface msh::Surface's notify_change function is coupled to having the ID, so whether you put it in the constructor or require a set_ function to be called, both depend on having an id to use notify_change() in a way. I'd rather come down on the side of putting it all dependencies in the constructor, because it allows us to see pretty quickly what a class needs, and gives us an easy-to-read signal about if a class is doing too much and needs to refactor. So, msh::Shell, might just be on the point of needing a refactor. (if its constructor is getting "too bloated" of course)

I know we had a debate similar to this one at an earlier point in the project, I hope we have another one next week :)

[Chris Halse Rogers (raof)]

This doesn't immediately *look* like a simplification to me.

My understanding is that this is a simplification for clients of this code - ie: consumers of this class no longer have to check if the EventSink or SurfaceID is set.

My question is then - do consumers of this class need to check if the EventSink or SurfaceID is set currently? It looks like consumers of this class generally won't care - if they configure() a surface which doesn't have an EventSink set then nothing will get the event but everything else will work as expected?

review: Needs Information

[Daniel van Vugt (vanvugt)]

From a high-level perspective I can see two problems here:

1. It's 1000 lines bigger than my proposal. Even if you agree that's not a simplification, but for other purposes, then I think it's still a bit much to review all in one go.

2. This does not solve the problem of code I disagree with ending up in a commit with my name on it. Because once merged it would just show one commit with both our names as authors.

To keep code reviews smaller and simpler, please propose your changes separately (after) surface-states to lp:mir. I won't agree with them, but I won't block them there either.

review: Needs Resubmitting

[Daniel van Vugt (vanvugt)]

Just further to Kevin's comment:

"We've tried to mitigate the problem of #2 by having a lot of mocks and stubs and nulls that are easy to plug in and make the object easy to manipulate in test. Since its easier to read a constructor than it is to trace what the call order is, I'd rather plug in mocks/stubs to the constructor than have to trace a call order to figure out if we've fulfilled our usage requirements."

This is false: "lot of mocks and stubs and nulls that are easy to plug in and make the object easy to manipulate in test". You define "easy" by how much effort it requires for someone like you guys (who have been at it for over a year) to modify the code. I define "easy" by how much effort a newcomer requires (with no prior knowledge of the code).

This is false: "Since its easier to read a constructor than it is to trace what the call order is". Constructors do not explicitly state what they're doing, unlike "set_id" or "set_event_sink". And if you did make a mistake somewhere and dereference a null shared_ptr, then the trace would be a call stack. Automatically generated and easier to read.

[Daniel van Vugt (vanvugt)]

If you resubmit and prerequisite the surface-states branch, I will close my eyes and happily Abstain.

review: Needs Resubmitting

[Alan Griffiths (alan-griffiths)]

> My question is then - do consumers of this class need to check if the
> EventSink or SurfaceID is set currently? It looks like consumers of this class
> generally won't care - if they configure() a surface which doesn't have an
> EventSink set then nothing will get the event but everything else will work as
> expected?

On this point is that consumers shouldn't even know about them - they shouldn't see these functions in the interface they use (i.e. they don't belong in frontend::Surface). Unnecessarily exposing these functions *is* increasing coupling.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:671
http://jenkins.qa.ubuntu.com/job/mir-ci/491/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/533
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/416
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/496
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/496/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/242

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

"So, msh::Shell, might just be on the point of needing a refactor. (if its constructor is getting "too bloated" of course"

I assume that means msh::Surface?

I too have questioned in the past whether some of the attributes currently being added to shell::Surface are best modeled as member variables. (As opposed to say, their being an event filter that has an associative map from surface to event sink.)

This proposal doesn't address this question.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:671
http://jenkins.qa.ubuntu.com/job/mir-ci/492/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/534
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/417
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/497
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/497/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/243

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:674
http://jenkins.qa.ubuntu.com/job/mir-ci/493/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/536
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/419
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/498
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/498/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

looks good to me, although a good future improvement might be to consolidate ApplicationSession's constructor

review: Approve

[Robert Ancell (robert-ancell)]

review: Approve

[Chris Halse Rogers (raof)]

It's still not clear to me that this is a simplification, but there's nothing objectionable in here.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'examples/demo-inprocess-egl/inprocess_egl_client.cpp'
--- examples/demo-inprocess-egl/inprocess_egl_client.cpp	2013-04-24 05:22:20 +0000
+++ examples/demo-inprocess-egl/inprocess_egl_client.cpp	2013-04-25 09:50:50 +0000
@@ -60,7 +60,7 @@
         .of_size(surface_size)
         .of_buffer_usage(mc::BufferUsage::hardware)
         .of_pixel_format(geom::PixelFormat::argb_8888);
-    auto surface = surface_factory->create_surface(params);
+    auto surface = surface_factory->create_surface(params, mf::SurfaceId(), std::shared_ptr<events::EventSink>());
 
     surface->advance_client_buffer(); // TODO: What a wart!
 
 
=== modified file 'include/server/mir/frontend/protobuf_ipc_factory.h'
--- include/server/mir/frontend/protobuf_ipc_factory.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/protobuf_ipc_factory.h	2013-04-25 09:50:50 +0000
@@ -23,8 +23,10 @@
 
 namespace mir
 {
+namespace events
+{
 class EventSink;
-
+}
 namespace protobuf
 {
 class DisplayServer;
@@ -38,7 +40,7 @@
 {
 public:
     virtual std::shared_ptr<protobuf::DisplayServer> make_ipc_server(
-        std::shared_ptr<EventSink> const& sink) = 0;
+        std::shared_ptr<events::EventSink> const& sink) = 0;
     virtual std::shared_ptr<ResourceCache> resource_cache() = 0;
     virtual std::shared_ptr<MessageProcessorReport> report() = 0;
 
 
=== modified file 'include/server/mir/frontend/session.h'
--- include/server/mir/frontend/session.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/session.h	2013-04-25 09:50:50 +0000
@@ -29,7 +29,6 @@
 
 namespace mir
 {
-class EventSink;
 
 namespace frontend
 {
@@ -52,8 +51,6 @@
 
     virtual int configure_surface(SurfaceId id, MirSurfaceAttrib attrib, int value) = 0;
 
-    virtual void set_event_sink(std::shared_ptr<EventSink> const&) = 0;
-
 protected:
     Session() = default;
     Session(Session const&) = delete;
 
=== modified file 'include/server/mir/frontend/session_mediator.h'
--- include/server/mir/frontend/session_mediator.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/session_mediator.h	2013-04-25 09:50:50 +0000
@@ -27,8 +27,10 @@
 
 namespace mir
 {
+namespace events
+{
 class EventSink;
-
+}
 namespace graphics
 {
 class Platform;
@@ -62,7 +64,7 @@
         std::shared_ptr<graphics::ViewableArea> const& viewable_area,
         std::shared_ptr<compositor::GraphicBufferAllocator> const& buffer_allocator,
         std::shared_ptr<SessionMediatorReport> const& report,
-        std::shared_ptr<EventSink> const& event_sink,
+        std::shared_ptr<events::EventSink> const& event_sink,
         std::shared_ptr<ResourceCache> const& resource_cache);
 
     /* Platform independent requests */
@@ -119,7 +121,7 @@
     std::shared_ptr<compositor::GraphicBufferAllocator> const buffer_allocator;
 
     std::shared_ptr<SessionMediatorReport> const report;
-    std::shared_ptr<EventSink> const event_sink;
+    std::shared_ptr<events::EventSink> const event_sink;
     std::shared_ptr<ResourceCache> const resource_cache;
     std::shared_ptr<ClientBufferTracker> const client_tracker;
 
 
=== modified file 'include/server/mir/frontend/shell.h'
--- include/server/mir/frontend/shell.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/shell.h	2013-04-25 09:50:50 +0000
@@ -24,7 +24,10 @@
 
 namespace mir
 {
-
+namespace events
+{
+class EventSink;
+}
 namespace frontend
 {
 class Session;
@@ -35,7 +38,7 @@
 public:
     virtual ~Shell() {}
 
-    virtual std::shared_ptr<Session> open_session(std::string const& name) = 0;
+    virtual std::shared_ptr<Session> open_session(std::string const& name, std::shared_ptr<events::EventSink> const& sink) = 0;
     virtual void close_session(std::shared_ptr<Session> const& session)  = 0;
 
     virtual void tag_session_with_lightdm_id(std::shared_ptr<Session> const& session, int id) = 0;
 
=== modified file 'include/server/mir/shell/application_session.h'
--- include/server/mir/shell/application_session.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/shell/application_session.h	2013-04-25 09:50:50 +0000
@@ -25,8 +25,10 @@
 
 namespace mir
 {
+namespace events
+{
 class EventSink;
-
+}
 namespace shell
 {
 class SurfaceFactory;
@@ -38,6 +40,13 @@
 public:
     explicit ApplicationSession(std::shared_ptr<SurfaceFactory> const& surface_factory, 
         std::shared_ptr<InputTargetListener> const& input_target_listener, std::string const& session_name);
+
+    ApplicationSession(
+        std::shared_ptr<SurfaceFactory> const& surface_factory,
+        std::shared_ptr<InputTargetListener> const& input_target_listener,
+        std::string const& session_name,
+        std::shared_ptr<events::EventSink> const& sink);
+
     ~ApplicationSession();
 
     frontend::SurfaceId create_surface(frontend::SurfaceCreationParameters const& params);
@@ -55,8 +64,6 @@
 
     int configure_surface(frontend::SurfaceId id, MirSurfaceAttrib attrib, int value);
 
-    void set_event_sink(std::shared_ptr<mir::EventSink> const& sink);
-
 protected:
     ApplicationSession(ApplicationSession const&) = delete;
     ApplicationSession& operator=(ApplicationSession const&) = delete;
@@ -65,6 +72,7 @@
     std::shared_ptr<SurfaceFactory> const surface_factory;
     std::shared_ptr<InputTargetListener> const input_target_listener;
     std::string const session_name;
+    std::shared_ptr<events::EventSink> const event_sink;
 
     frontend::SurfaceId next_id();
 
@@ -74,8 +82,6 @@
     Surfaces::const_iterator checked_find(frontend::SurfaceId id) const;
     std::mutex mutable surfaces_mutex;
     Surfaces surfaces;
-
-    std::shared_ptr<EventSink> event_sink;
 };
 
 }
 
=== modified file 'include/server/mir/shell/organising_surface_factory.h'
--- include/server/mir/shell/organising_surface_factory.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/shell/organising_surface_factory.h	2013-04-25 09:50:50 +0000
@@ -36,7 +36,10 @@
                              std::shared_ptr<PlacementStrategy> const& placement_strategy);
     virtual ~OrganisingSurfaceFactory();
 
-    std::shared_ptr<Surface> create_surface(frontend::SurfaceCreationParameters const& params);
+    std::shared_ptr<Surface> create_surface(
+        frontend::SurfaceCreationParameters const& params,
+        frontend::SurfaceId id,
+        std::shared_ptr<events::EventSink> const& sink) override;
 
 protected:
     OrganisingSurfaceFactory(OrganisingSurfaceFactory const&) = delete;
 
=== modified file 'include/server/mir/shell/session_manager.h'
--- include/server/mir/shell/session_manager.h	2013-04-24 22:14:08 +0000
+++ include/server/mir/shell/session_manager.h	2013-04-25 09:50:50 +0000
@@ -54,7 +54,7 @@
                             std::shared_ptr<InputTargetListener> const& input_target_listener);
     virtual ~SessionManager();
 
-    virtual std::shared_ptr<frontend::Session> open_session(std::string const& name);
+    virtual std::shared_ptr<frontend::Session> open_session(std::string const& name, std::shared_ptr<events::EventSink> const& sink);
     virtual void close_session(std::shared_ptr<frontend::Session> const& session);
 
     virtual void tag_session_with_lightdm_id(std::shared_ptr<frontend::Session> const& session, int id);
 
=== modified file 'include/server/mir/shell/surface.h'
--- include/server/mir/shell/surface.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/shell/surface.h	2013-04-25 09:50:50 +0000
@@ -31,9 +31,10 @@
 
 namespace mir
 {
-
+namespace events
+{
 class EventSink;
-
+}
 namespace frontend
 {
 struct SurfaceCreationParameters;
@@ -54,6 +55,14 @@
         std::shared_ptr<SurfaceBuilder> const& builder,
         frontend::SurfaceCreationParameters const& params,
         std::shared_ptr<input::InputChannel> const& input_channel);
+
+    Surface(
+        std::shared_ptr<SurfaceBuilder> const& builder,
+        frontend::SurfaceCreationParameters const& params,
+        std::shared_ptr<input::InputChannel> const& input_channel,
+        frontend::SurfaceId id,
+        std::shared_ptr<events::EventSink> const& sink);
+
     ~Surface();
 
     virtual void hide();
@@ -82,9 +91,6 @@
     virtual MirSurfaceType type() const;
     virtual MirSurfaceState state() const;
 
-    void set_id(frontend::SurfaceId i);
-    void set_event_target(std::shared_ptr<EventSink> const& sink);
-
 private:
     bool set_type(MirSurfaceType t);  // Use configure() to make public changes
     bool set_state(MirSurfaceState s);
@@ -94,8 +100,8 @@
     std::shared_ptr<mir::input::InputChannel> const input_channel;
     std::weak_ptr<mir::surfaces::Surface> const surface;
 
-    std::shared_ptr<EventSink> event_sink;
-    frontend::SurfaceId id;
+    frontend::SurfaceId const id;
+    std::shared_ptr<events::EventSink> const event_sink;
 
     MirSurfaceType type_value;
     MirSurfaceState state_value;
 
=== modified file 'include/server/mir/shell/surface_factory.h'
--- include/server/mir/shell/surface_factory.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/shell/surface_factory.h	2013-04-25 09:50:50 +0000
@@ -19,10 +19,15 @@
 #ifndef MIR_SHELL_SURFACE_FACTORY_H_
 #define MIR_SHELL_SURFACE_FACTORY_H_
 
+#include "mir/frontend/surface_id.h"
 #include <memory>
 
 namespace mir
 {
+namespace events
+{
+class EventSink;
+}
 namespace frontend
 {
 struct SurfaceCreationParameters;
@@ -34,7 +39,10 @@
 class SurfaceFactory
 {
 public:
-    virtual std::shared_ptr<Surface> create_surface(const frontend::SurfaceCreationParameters& params) = 0;
+    virtual std::shared_ptr<Surface> create_surface(
+        frontend::SurfaceCreationParameters const& params,
+        frontend::SurfaceId id,
+        std::shared_ptr<events::EventSink> const& sink) = 0;
 
 protected:
     virtual ~SurfaceFactory() {}
 
=== modified file 'include/server/mir/shell/surface_source.h'
--- include/server/mir/shell/surface_source.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/shell/surface_source.h	2013-04-25 09:50:50 +0000
@@ -40,7 +40,10 @@
     explicit SurfaceSource(std::shared_ptr<SurfaceBuilder> const& surface_builder, std::shared_ptr<input::InputChannelFactory> const& input_factory);
     virtual ~SurfaceSource() {}
 
-    std::shared_ptr<Surface> create_surface(const frontend::SurfaceCreationParameters& params);
+    std::shared_ptr<Surface> create_surface(
+        frontend::SurfaceCreationParameters const& params,
+        frontend::SurfaceId id,
+        std::shared_ptr<events::EventSink> const& sink);
 
 protected:
     SurfaceSource(const SurfaceSource&) = delete;
 
=== added directory 'include/shared/mir/events'
=== renamed file 'include/shared/mir/event_sink.h' => 'include/shared/mir/events/event_sink.h'
--- include/shared/mir/event_sink.h	2013-04-19 04:33:51 +0000
+++ include/shared/mir/events/event_sink.h	2013-04-25 09:50:50 +0000
@@ -16,13 +16,15 @@
  * Authored by: Daniel van Vugt <[e-mail]>
  */
 
-#ifndef MIR_EVENT_SINK_H_
-#define MIR_EVENT_SINK_H_
+#ifndef MIR_EVENTS_EVENT_SINK_H_
+#define MIR_EVENTS_EVENT_SINK_H_
 
 #include "mir_toolkit/event.h"
 
 namespace mir
 {
+namespace events
+{
 
 class EventSink
 {
@@ -36,8 +38,7 @@
     EventSink(EventSink const&) = delete;
     EventSink& operator=(EventSink const&) = delete;
 };
-
+}
 } // namespace mir
 
-#endif
-
+#endif // MIR_EVENTS_EVENT_SINK_H_
 
=== modified file 'include/test/mir_test_doubles/mock_session.h'
--- include/test/mir_test_doubles/mock_session.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/mock_session.h	2013-04-25 09:50:50 +0000
@@ -43,8 +43,6 @@
     MOCK_METHOD0(show, void());
 
     MOCK_METHOD3(configure_surface, int(frontend::SurfaceId, MirSurfaceAttrib, int));
-
-    MOCK_METHOD1(set_event_sink, void(std::shared_ptr<mir::EventSink> const&));
 };
 
 }
 
=== modified file 'include/test/mir_test_doubles/mock_shell.h'
--- include/test/mir_test_doubles/mock_shell.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/mock_shell.h	2013-04-25 09:50:50 +0000
@@ -34,7 +34,7 @@
 
 struct MockShell : public frontend::Shell
 {
-    MOCK_METHOD1(open_session, std::shared_ptr<frontend::Session>(std::string const&));
+    MOCK_METHOD2(open_session, std::shared_ptr<frontend::Session>(std::string const&, std::shared_ptr<events::EventSink> const&));
     MOCK_METHOD1(close_session, void(std::shared_ptr<frontend::Session> const&));
 
     MOCK_METHOD2(tag_session_with_lightdm_id, void(std::shared_ptr<frontend::Session> const&, int));
 
=== modified file 'include/test/mir_test_doubles/mock_surface_factory.h'
--- include/test/mir_test_doubles/mock_surface_factory.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/mock_surface_factory.h	2013-04-25 09:50:50 +0000
@@ -33,7 +33,10 @@
 
 struct MockSurfaceFactory : public shell::SurfaceFactory
 {
-    MOCK_METHOD1(create_surface, std::shared_ptr<shell::Surface>(const frontend::SurfaceCreationParameters&));
+    MOCK_METHOD3(create_surface, std::shared_ptr<shell::Surface>(
+        const frontend::SurfaceCreationParameters&,
+        frontend::SurfaceId,
+        std::shared_ptr<events::EventSink> const&));
 };
 
 }
 
=== modified file 'include/test/mir_test_doubles/stub_ipc_factory.h'
--- include/test/mir_test_doubles/stub_ipc_factory.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/stub_ipc_factory.h	2013-04-25 09:50:50 +0000
@@ -42,7 +42,7 @@
     }
 
     std::shared_ptr<protobuf::DisplayServer> make_ipc_server(
-        std::shared_ptr<EventSink> const&)
+        std::shared_ptr<events::EventSink> const&)
     {
         return server;
     }
 
=== modified file 'include/test/mir_test_doubles/stub_session.h'
--- include/test/mir_test_doubles/stub_session.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/stub_session.h	2013-04-25 09:50:50 +0000
@@ -58,7 +58,7 @@
     {
         return 0;
     }
-    void set_event_sink(std::shared_ptr<EventSink> const&)
+    void set_event_sink(std::shared_ptr<events::EventSink> const&)
     {
     }
 };
 
=== modified file 'include/test/mir_test_doubles/stub_shell.h'
--- include/test/mir_test_doubles/stub_shell.h	2013-04-24 05:22:20 +0000
+++ include/test/mir_test_doubles/stub_shell.h	2013-04-25 09:50:50 +0000
@@ -31,7 +31,7 @@
 
 class StubShell : public frontend::Shell
 {
-    std::shared_ptr<frontend::Session> open_session(std::string const& /* name */)
+    std::shared_ptr<frontend::Session> open_session(std::string const& /* name */, std::shared_ptr<events::EventSink> const& /* sink */)
     {
         return std::make_shared<StubSession>();
     }
 
=== modified file 'src/client/make_rpc_channel.h'
--- src/client/make_rpc_channel.h	2013-04-24 05:22:20 +0000
+++ src/client/make_rpc_channel.h	2013-04-25 09:50:50 +0000
@@ -19,7 +19,7 @@
 #define MIR_CLIENT_MAKE_RPC_CHANNEL_H_
 
 #include <memory>
-#include "mir_protobuf.pb.h"
+#include "mir_basic_rpc_channel.h"
 
 namespace mir
 {
@@ -27,7 +27,7 @@
 {
 class Logger;
 
-std::shared_ptr<google::protobuf::RpcChannel>
+std::shared_ptr<MirBasicRpcChannel>
 make_rpc_channel(std::string const& name, std::shared_ptr<Logger> const& log);
 }
 }
 
=== modified file 'src/client/make_socket_rpc_channel.cpp'
--- src/client/make_socket_rpc_channel.cpp	2013-04-24 05:22:20 +0000
+++ src/client/make_socket_rpc_channel.cpp	2013-04-25 09:50:50 +0000
@@ -21,7 +21,7 @@
 
 namespace mcl = mir::client;
 
-std::shared_ptr<google::protobuf::RpcChannel>
+std::shared_ptr<mcl::MirBasicRpcChannel>
 mcl::make_rpc_channel(std::string const& name, std::shared_ptr<Logger> const& log)
 {
     return std::make_shared<MirSocketRpcChannel>(name, log);
 
=== modified file 'src/client/mir_basic_rpc_channel.cpp'
--- src/client/mir_basic_rpc_channel.cpp	2013-04-24 05:22:20 +0000
+++ src/client/mir_basic_rpc_channel.cpp	2013-04-25 09:50:50 +0000
@@ -100,7 +100,5 @@
 
 int mcl::MirBasicRpcChannel::next_id()
 {
-    int id = next_message_id.load();
-    while (!next_message_id.compare_exchange_weak(id, id + 1)) std::this_thread::yield();
-    return id;
+    return next_message_id.fetch_add(1);
 }
 
=== modified file 'src/client/mir_basic_rpc_channel.h'
--- src/client/mir_basic_rpc_channel.h	2013-04-24 05:22:20 +0000
+++ src/client/mir_basic_rpc_channel.h	2013-04-25 09:50:50 +0000
@@ -33,6 +33,10 @@
 
 namespace mir
 {
+namespace events
+{
+class EventSink;
+}
 namespace protobuf
 {
 namespace wire
@@ -92,6 +96,8 @@
     MirBasicRpcChannel();
     ~MirBasicRpcChannel();
 
+    virtual void set_event_handler(events::EventSink *sink) = 0;
+
 protected:
     mir::protobuf::wire::Invocation invocation_for(const google::protobuf::MethodDescriptor* method,
         const google::protobuf::Message* request);
 
=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-04-24 05:22:20 +0000
+++ src/client/mir_client_library.cpp	2013-04-25 09:50:50 +0000
@@ -26,7 +26,7 @@
 #include "native_client_platform_factory.h"
 #include "egl_native_display_container.h"
 #include "mir_logger.h"
-#include "mir_socket_rpc_channel.h"
+#include "make_rpc_channel.h"
 
 #include <set>
 #include <unordered_set>
@@ -60,12 +60,10 @@
         auto log = std::make_shared<mcl::ConsoleLogger>();
         auto client_platform_factory = std::make_shared<mcl::NativeClientPlatformFactory>();
 
-        auto rpc = std::make_shared<mcl::MirSocketRpcChannel>(sock, log);
-
-        MirConnection* connection = new MirConnection(rpc, log,
-                                                      client_platform_factory);
-        
-        rpc->set_event_handler(connection);
+        MirConnection* connection = new MirConnection(
+            mcl::make_rpc_channel(sock, log),
+            log,
+            client_platform_factory);
 
         return connection->connect(name, callback, context);
     }
@@ -258,12 +256,10 @@
     auto log = std::make_shared<mcl::ConsoleLogger>();
     auto client_platform_factory = std::make_shared<mcl::NativeClientPlatformFactory>();
 
-    auto rpc = std::make_shared<mcl::MirSocketRpcChannel>(server, log);
-
-    MirConnection* connection = new MirConnection(rpc, log,
-                                                  client_platform_factory);
-
-    rpc->set_event_handler(connection);
+    MirConnection* connection = new MirConnection(
+        mcl::make_rpc_channel(server, log),
+        log,
+        client_platform_factory);
 
     return connection->connect(lightdm_id, app_name, callback, client_context);
 }
 
=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-04-24 05:22:20 +0000
+++ src/client/mir_connection.cpp	2013-04-25 09:50:50 +0000
@@ -43,7 +43,7 @@
 }
 
 MirConnection::MirConnection(
-    std::shared_ptr<google::protobuf::RpcChannel> const& channel,
+    std::shared_ptr<mir::client::MirBasicRpcChannel> const& channel,
     std::shared_ptr<mcl::Logger> const & log,
     std::shared_ptr<mcl::ClientPlatformFactory> const& client_platform_factory) :
         channel(channel),
@@ -52,6 +52,7 @@
         client_platform_factory(client_platform_factory),
         input_platform(mcli::InputPlatform::create())
 {
+    channel->set_event_handler(this);
     {
         std::lock_guard<std::mutex> lock(connection_guard);
         valid_connections.insert(this);
 
=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2013-04-24 05:22:20 +0000
+++ src/client/mir_connection.h	2013-04-25 09:50:50 +0000
@@ -34,7 +34,7 @@
 #include "client_context.h"
 
 #include "mir_wait_handle.h"
-#include "mir/event_sink.h"
+#include "mir/events/event_sink.h"
 
 namespace mir
 {
@@ -44,6 +44,8 @@
 class Logger;
 class ClientBufferDepository;
 class ClientPlatformFactory;
+class MirBasicRpcChannel;
+
 namespace input
 {
 class InputPlatform;
@@ -51,13 +53,12 @@
 }
 }
 
-struct MirConnection : public mir::client::ClientContext,
-                       public mir::EventSink
+struct MirConnection : mir::client::ClientContext, private mir::events::EventSink
 {
 public:
     MirConnection();
 
-    MirConnection(std::shared_ptr<google::protobuf::RpcChannel> const& channel,
+    MirConnection(std::shared_ptr<mir::client::MirBasicRpcChannel> const& channel,
                   std::shared_ptr<mir::client::Logger> const & log,
                   std::shared_ptr<mir::client::ClientPlatformFactory> const& client_platform_factory);
     ~MirConnection() noexcept;
 
=== modified file 'src/client/mir_socket_rpc_channel.cpp'
--- src/client/mir_socket_rpc_channel.cpp	2013-04-24 05:22:20 +0000
+++ src/client/mir_socket_rpc_channel.cpp	2013-04-25 09:50:50 +0000
@@ -253,11 +253,12 @@
 
         log->debug() << __PRETTY_FUNCTION__ << " result.id():" << result.id() << std::endl;
 
-        if (!result.has_id())  // It's an event sequence
+        for (int i = 0; i != result.events_size(); ++i)
         {
-            process_event_sequence(result);
+            process_event_sequence(result.events(i));
         }
-        else
+
+        if (result.has_id())
         {
             pending_calls.complete_response(result);
         }
@@ -269,44 +270,42 @@
     }
 }
 
-void mcl::MirSocketRpcChannel::process_event_sequence(
-    mir::protobuf::wire::Result const& result)
+void mcl::MirSocketRpcChannel::process_event_sequence(std::string const& event)
 {
     if (!event_handler)
         return;
 
     mir::protobuf::EventSequence seq;
-    if (seq.ParseFromString(result.response()))
+
+    seq.ParseFromString(event);
+    int const nevents = seq.event_size();
+    for (int i = 0; i != nevents; ++i)
     {
-        int const nevents = seq.event_size();
-        for (int i = 0; i < nevents; i++)
+        mir::protobuf::Event const& event = seq.event(i);
+        if (event.has_raw())
         {
-            mir::protobuf::Event const& event = seq.event(i);
-            if (event.has_raw())
-            {
-                std::string const& raw_event = event.raw();
-
-                // In future, events might be compressed where possible.
-                // But that's a job for later...
-                if (raw_event.size() == sizeof(MirEvent))
-                {
-                    MirEvent e;
-
-                    // Make a copy to ensure integer fields get correct memory
-                    // alignment, which is critical on many non-x86
-                    // architectures.
-                    memcpy(&e, raw_event.data(), sizeof e);
-                    event_handler->handle_event(e);
-                }
-                else
-                {
-                    log->error() << __PRETTY_FUNCTION__
-                                 << " Received MirEvent of an unexpected size."
-                                 << std::endl;
-                }
+            std::string const& raw_event = event.raw();
+
+            // In future, events might be compressed where possible.
+            // But that's a job for later...
+            if (raw_event.size() == sizeof(MirEvent))
+            {
+                MirEvent e;
+
+                // Make a copy to ensure integer fields get correct memory
+                // alignment, which is critical on many non-x86
+                // architectures.
+                memcpy(&e, raw_event.data(), sizeof e);
+                event_handler->handle_event(e);
+            }
+            else
+            {
+                log->error() << __PRETTY_FUNCTION__
+                             << " Received MirEvent of an unexpected size."
+                             << std::endl;
             }
         }
-    } // else protobuf will log an error
+    }
 }
 
 size_t mcl::MirSocketRpcChannel::read_message_header()
@@ -331,7 +330,7 @@
     return result;
 }
 
-void mcl::MirSocketRpcChannel::set_event_handler(mir::EventSink *sink)
+void mcl::MirSocketRpcChannel::set_event_handler(events::EventSink *sink)
 {
     /*
      * Yes, these have to be regular pointers. Because ownership of the object
 
=== modified file 'src/client/mir_socket_rpc_channel.h'
--- src/client/mir_socket_rpc_channel.h	2013-04-24 05:22:20 +0000
+++ src/client/mir_socket_rpc_channel.h	2013-04-25 09:50:50 +0000
@@ -22,7 +22,7 @@
 
 #include "mir_basic_rpc_channel.h"
 #include "mir_logger.h"
-#include "mir/event_sink.h"
+#include "mir/events/event_sink.h"
 
 #include <boost/asio.hpp>
 
@@ -52,7 +52,7 @@
     MirSocketRpcChannel(const std::string& endpoint, const std::shared_ptr<Logger>& log);
     ~MirSocketRpcChannel();
 
-    void set_event_handler(EventSink *sink);
+    void set_event_handler(events::EventSink *sink);
 
 private:
     virtual void CallMethod(const google::protobuf::MethodDescriptor* method, google::protobuf::RpcController*,
@@ -75,13 +75,13 @@
     void on_header_read(const boost::system::error_code& error);
 
     void read_message();
-    void process_event_sequence(mir::protobuf::wire::Result const& result);
+    void process_event_sequence(std::string const& event);
 
     size_t read_message_header();
 
     mir::protobuf::wire::Result read_message_body(const size_t body_size);
 
-    EventSink *event_handler;
+    events::EventSink *event_handler;
 };
 
 }
 
=== modified file 'src/server/CMakeLists.txt'
--- src/server/CMakeLists.txt	2013-04-24 05:22:20 +0000
+++ src/server/CMakeLists.txt	2013-04-25 09:50:50 +0000
@@ -26,7 +26,6 @@
   display_server.cpp
   default_server_configuration.cpp
   asio_main_loop.cpp
-  event_queue.cpp
 )
 
 set(MIRSERVER_LINKAGE SHARED)
 
=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-04-24 22:14:08 +0000
+++ src/server/default_server_configuration.cpp	2013-04-25 09:50:50 +0000
@@ -63,6 +63,7 @@
 #include "mir/default_configuration.h"
 
 namespace mc = mir::compositor;
+namespace me = mir::events;
 namespace geom = mir::geometry;
 namespace mf = mir::frontend;
 namespace mg = mir::graphics;
@@ -109,7 +110,7 @@
     std::shared_ptr<mc::GraphicBufferAllocator> const buffer_allocator;
 
     virtual std::shared_ptr<mir::protobuf::DisplayServer> make_ipc_server(
-        std::shared_ptr<mir::EventSink> const& sink)
+        std::shared_ptr<me::EventSink> const& sink)
     {
         return std::make_shared<mf::SessionMediator>(
             shell,
 
=== modified file 'src/server/frontend/CMakeLists.txt'
--- src/server/frontend/CMakeLists.txt	2013-04-24 05:22:20 +0000
+++ src/server/frontend/CMakeLists.txt	2013-04-25 09:50:50 +0000
@@ -8,6 +8,7 @@
   protobuf_message_processor.cpp
   null_message_processor.cpp
   surface_creation_parameters.cpp
+  event_pipe.cpp
 
   resource_cache.cpp
   ${PROTO_HDRS}
 
=== renamed file 'src/server/event_queue.cpp' => 'src/server/frontend/event_pipe.cpp'
--- src/server/event_queue.cpp	2013-04-24 03:44:27 +0000
+++ src/server/frontend/event_pipe.cpp	2013-04-25 09:50:50 +0000
@@ -16,17 +16,16 @@
  * Authored by: Daniel van Vugt <[e-mail]>
  */
 
-#include "mir/event_queue.h"
-#include "mir/event_sink.h"
-
-using namespace mir;
-
-void EventQueue::set_target(std::weak_ptr<EventSink> const& s)
+#include "event_pipe.h"
+
+namespace mf = mir::frontend;
+
+void mf::EventPipe::set_target(std::weak_ptr<EventSink> const& s)
 {
     target = s;
 }
 
-void EventQueue::handle_event(MirEvent const& e)
+void mf::EventPipe::handle_event(MirEvent const& e)
 {
     // In future, we might put e on a queue and wait for some background
     // thread to push it through to target. But that's not required right now.
 
=== renamed file 'include/server/mir/event_queue.h' => 'src/server/frontend/event_pipe.h'
--- include/server/mir/event_queue.h	2013-04-16 05:46:28 +0000
+++ src/server/frontend/event_pipe.h	2013-04-25 09:50:50 +0000
@@ -16,25 +16,27 @@
  * Authored by: Daniel van Vugt <[e-mail]>
  */
 
-#ifndef MIR_EVENT_QUEUE_H_
-#define MIR_EVENT_QUEUE_H_
+#ifndef MIR_FRONTEND_EVENT_PIPE_H_
+#define MIR_FRONTEND_EVENT_PIPE_H_
 
 #include "mir_toolkit/event.h"
-#include "mir/event_sink.h"
+#include "mir/events/event_sink.h"
 #include <memory>
 
 namespace mir
 {
-class EventQueue : public EventSink
+namespace frontend
+{
+class EventPipe : public events::EventSink
 {
 public:
-    void set_target(std::weak_ptr<EventSink> const& s);
+    void set_target(std::weak_ptr<events::EventSink> const& s);
     void handle_event(MirEvent const& e) override;
 
 private:
-    std::weak_ptr<EventSink> target;
+    std::weak_ptr<events::EventSink> target;
 };
-
-} // namespace mir
-
-#endif
+}
+}
+
+#endif // MIR_FRONTEND_EVENT_PIPE_H_
 
=== modified file 'src/server/frontend/protobuf_message_processor.cpp'
--- src/server/frontend/protobuf_message_processor.cpp	2013-04-24 05:22:20 +0000
+++ src/server/frontend/protobuf_message_processor.cpp	2013-04-25 09:50:50 +0000
@@ -138,6 +138,8 @@
 
 void mfd::ProtobufMessageProcessor::send_event(MirEvent const& e)
 {
+    // In future we might send multiple events, or insert them into messages
+    // containing other responses, but for now we send them individually.
     mir::protobuf::EventSequence seq;
     mir::protobuf::Event *ev = seq.add_event();
     ev->set_raw(&e, sizeof(MirEvent));
@@ -146,7 +148,7 @@
     seq.SerializeToString(&buffer);
 
     mir::protobuf::wire::Result result;
-    result.set_response(buffer);
+    result.add_events(buffer);
 
     result.SerializeToString(&buffer);
 
 
=== modified file 'src/server/frontend/protobuf_message_processor.h'
--- src/server/frontend/protobuf_message_processor.h	2013-04-24 05:22:20 +0000
+++ src/server/frontend/protobuf_message_processor.h	2013-04-25 09:50:50 +0000
@@ -24,7 +24,7 @@
 
 #include "mir_protobuf.pb.h"
 #include "mir_protobuf_wire.pb.h"
-#include "mir/event_sink.h"
+#include "mir/events/event_sink.h"
 
 #include <vector>
 #include <memory>
@@ -44,7 +44,7 @@
 {
 
 struct ProtobufMessageProcessor : MessageProcessor,
-                                  public EventSink
+                                  public events::EventSink
 {
     ProtobufMessageProcessor(
         MessageSender* sender,
 
=== modified file 'src/server/frontend/protobuf_socket_communicator.cpp'
--- src/server/frontend/protobuf_socket_communicator.cpp	2013-04-24 05:22:20 +0000
+++ src/server/frontend/protobuf_socket_communicator.cpp	2013-04-25 09:50:50 +0000
@@ -22,8 +22,7 @@
 
 #include "mir/frontend/protobuf_ipc_factory.h"
 #include "mir/protobuf/google_protobuf_guard.h"
-#include "mir/event_sink.h"
-#include "mir/event_queue.h"
+#include "event_pipe.h"
 
 #include <boost/signals2.hpp>
 
@@ -51,7 +50,7 @@
 
 void mf::ProtobufSocketCommunicator::start_accept()
 {
-    std::shared_ptr<EventQueue> event_queue = std::make_shared<EventQueue>();
+    auto const& event_pipe = std::make_shared<EventPipe>();
 
     auto const& socket_session = std::make_shared<mfd::SocketSession>(
         io_service,
@@ -60,11 +59,11 @@
 
     auto session = std::make_shared<detail::ProtobufMessageProcessor>(
         socket_session.get(),
-        ipc_factory->make_ipc_server(event_queue),
+        ipc_factory->make_ipc_server(event_pipe),
         ipc_factory->resource_cache(),
         ipc_factory->report());
 
-    event_queue->set_target(session);
+    event_pipe->set_target(session);
     socket_session->set_processor(session);
 
     acceptor.async_accept(
 
=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2013-04-24 05:22:20 +0000
+++ src/server/frontend/session_mediator.cpp	2013-04-25 09:50:50 +0000
@@ -44,7 +44,7 @@
     std::shared_ptr<graphics::ViewableArea> const& viewable_area,
     std::shared_ptr<compositor::GraphicBufferAllocator> const& buffer_allocator,
     std::shared_ptr<SessionMediatorReport> const& report,
-    std::shared_ptr<EventSink> const& event_sink,
+    std::shared_ptr<events::EventSink> const& event_sink,
     std::shared_ptr<ResourceCache> const& resource_cache) :
     shell(shell),
     graphics_platform(graphics_platform),
@@ -65,8 +65,7 @@
 {
     report->session_connect_called(request->application_name());
 
-    session = shell->open_session(request->application_name());
-    session->set_event_sink(event_sink);
+    session = shell->open_session(request->application_name(), event_sink);
 
     auto ipc_package = graphics_platform->get_ipc_package();
     auto platform = response->mutable_platform();
 
=== modified file 'src/server/shell/application_session.cpp'
--- src/server/shell/application_session.cpp	2013-04-24 05:22:20 +0000
+++ src/server/shell/application_session.cpp	2013-04-25 09:50:50 +0000
@@ -28,21 +28,32 @@
 #include <cassert>
 #include <algorithm>
 
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 
 msh::ApplicationSession::ApplicationSession(
-    std::shared_ptr<msh::SurfaceFactory> const& surface_factory,
+    std::shared_ptr<SurfaceFactory> const& surface_factory,
     std::shared_ptr<msh::InputTargetListener> const& input_target_listener,
-    std::string const& session_name) :
+    std::string const& session_name,
+    std::shared_ptr<me::EventSink> const& sink) :
     surface_factory(surface_factory),
     input_target_listener(input_target_listener),
     session_name(session_name),
+    event_sink(sink),
     next_surface_id(0)
 {
     assert(surface_factory);
 }
 
+msh::ApplicationSession::ApplicationSession(
+    std::shared_ptr<SurfaceFactory> const& surface_factory,
+    std::shared_ptr<msh::InputTargetListener> const& input_target_listener,
+    std::string const& session_name) :
+    ApplicationSession(surface_factory, input_target_listener, session_name, std::shared_ptr<me::EventSink>())
+{
+}
+
 msh::ApplicationSession::~ApplicationSession()
 {
     std::unique_lock<std::mutex> lock(surfaces_mutex);
@@ -53,12 +64,6 @@
     }
 }
 
-void msh::ApplicationSession::set_event_sink(
-    std::shared_ptr<mir::EventSink> const& sink)
-{
-    event_sink = sink;
-}
-
 mf::SurfaceId msh::ApplicationSession::next_id()
 {
     return mf::SurfaceId(next_surface_id.fetch_add(1));
@@ -66,11 +71,8 @@
 
 mf::SurfaceId msh::ApplicationSession::create_surface(const mf::SurfaceCreationParameters& params)
 {
-    auto surf = surface_factory->create_surface(params);
     auto const id = next_id();
-
-    surf->set_id(id);
-    surf->set_event_target(event_sink);
+    auto surf = surface_factory->create_surface(params, id, event_sink);
 
     std::unique_lock<std::mutex> lock(surfaces_mutex);
     surfaces[id] = surf;
 
=== modified file 'src/server/shell/organising_surface_factory.cpp'
--- src/server/shell/organising_surface_factory.cpp	2013-04-24 05:22:20 +0000
+++ src/server/shell/organising_surface_factory.cpp	2013-04-25 09:50:50 +0000
@@ -34,10 +34,13 @@
 {
 }
 
-std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface(const mf::SurfaceCreationParameters& params)
+std::shared_ptr<msh::Surface> msh::OrganisingSurfaceFactory::create_surface(
+    frontend::SurfaceCreationParameters const& params,
+    frontend::SurfaceId id,
+    std::shared_ptr<events::EventSink> const& sink)
 {
     auto placed_params = placement_strategy->place(params);
 
-    return underlying_factory->create_surface(placed_params);
+    return underlying_factory->create_surface(placed_params, id, sink);
 }
 
 
=== modified file 'src/server/shell/session_manager.cpp'
--- src/server/shell/session_manager.cpp	2013-04-24 05:22:20 +0000
+++ src/server/shell/session_manager.cpp	2013-04-25 09:50:50 +0000
@@ -55,9 +55,11 @@
 {
 }
 
-std::shared_ptr<mf::Session> msh::SessionManager::open_session(std::string const& name)
+std::shared_ptr<mf::Session> msh::SessionManager::open_session(
+    std::string const& name,
+    std::shared_ptr<events::EventSink> const& sink)
 {
-    auto new_session = std::make_shared<msh::ApplicationSession>(surface_factory, input_target_listener, name);
+    auto new_session = std::make_shared<msh::ApplicationSession>(surface_factory, input_target_listener, name, sink);
 
     app_container->insert_session(new_session);
     
 
=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp	2013-04-24 05:22:20 +0000
+++ src/server/shell/surface.cpp	2013-04-25 09:50:50 +0000
@@ -20,7 +20,7 @@
 #include "mir/shell/surface_builder.h"
 #include "mir/input/input_channel.h"
 #include "mir_toolkit/event.h"
-#include "mir/event_sink.h"
+#include "mir/events/event_sink.h"
 
 #include <boost/throw_exception.hpp>
 
@@ -34,11 +34,28 @@
 msh::Surface::Surface(
     std::shared_ptr<SurfaceBuilder> const& builder,
     frontend::SurfaceCreationParameters const& params,
+    std::shared_ptr<input::InputChannel> const& input_channel,
+    frontend::SurfaceId id,
+    std::shared_ptr<events::EventSink> const& sink)
+  : builder(builder),
+    input_channel(input_channel),
+    surface(builder->create_surface(params)),
+    id(id),
+    event_sink(sink),
+    type_value(mir_surface_type_normal),
+    state_value(mir_surface_state_restored)
+{
+}
+
+msh::Surface::Surface(
+    std::shared_ptr<SurfaceBuilder> const& builder,
+    frontend::SurfaceCreationParameters const& params,
     std::shared_ptr<input::InputChannel> const& input_channel)
   : builder(builder),
     input_channel(input_channel),
     surface(builder->create_surface(params)),
-    id(0),
+    id(),
+    event_sink(),
     type_value(mir_surface_type_normal),
     state_value(mir_surface_state_restored)
 {
@@ -52,16 +69,6 @@
     }
 }
 
-void msh::Surface::set_id(mir::frontend::SurfaceId i)
-{
-    id = i;
-}
-
-void msh::Surface::set_event_target(std::shared_ptr<EventSink> const& sink)
-{
-    event_sink = sink;
-}
-
 void msh::Surface::hide()
 {
     if (auto const& s = surface.lock())
 
=== modified file 'src/server/shell/surface_source.cpp'
--- src/server/shell/surface_source.cpp	2013-04-24 05:22:20 +0000
+++ src/server/shell/surface_source.cpp	2013-04-25 09:50:50 +0000
@@ -39,11 +39,16 @@
     assert(surface_builder);
 }
 
-std::shared_ptr<msh::Surface> msh::SurfaceSource::create_surface(const mf::SurfaceCreationParameters& params)
+std::shared_ptr<msh::Surface> msh::SurfaceSource::create_surface(
+    frontend::SurfaceCreationParameters const& params,
+    frontend::SurfaceId id,
+    std::shared_ptr<events::EventSink> const& sink)
 {
     return std::make_shared<Surface>(
         surface_builder,
         params,
-        input_factory->make_input_channel());
+        input_factory->make_input_channel(),
+        id,
+        sink);
 }
 
 
=== modified file 'src/shared/protobuf/mir_protobuf_wire.proto'
--- src/shared/protobuf/mir_protobuf_wire.proto	2013-04-24 05:22:20 +0000
+++ src/shared/protobuf/mir_protobuf_wire.proto	2013-04-25 09:50:50 +0000
@@ -7,7 +7,9 @@
 }
 
 message Result {
-  // Invocation results have id and response. Events only have response.
+  // Invocation results have id and response.
   optional uint32 id = 1;
   optional bytes response = 2;
+  // Events are in events.
+  repeated bytes events = 3;
 }
 
=== modified file 'tests/acceptance-tests/test_focus_management_api.cpp'
--- tests/acceptance-tests/test_focus_management_api.cpp	2013-04-24 22:14:08 +0000
+++ tests/acceptance-tests/test_focus_management_api.cpp	2013-04-25 09:50:50 +0000
@@ -126,7 +126,7 @@
     {
         using namespace ::testing;
         using frontend::Shell;
-        ON_CALL(*this, open_session(_)).WillByDefault(Invoke(impl.get(), &Shell::open_session));
+        ON_CALL(*this, open_session(_, _)).WillByDefault(Invoke(impl.get(), &Shell::open_session));
         ON_CALL(*this, close_session(_)).WillByDefault(Invoke(impl.get(), &Shell::close_session));
 
         ON_CALL(*this, tag_session_with_lightdm_id(_, _)).WillByDefault(Invoke(impl.get(), &Shell::tag_session_with_lightdm_id));
@@ -135,7 +135,7 @@
         ON_CALL(*this, create_surface_for(_, _)).WillByDefault(Invoke(impl.get(), &Shell::create_surface_for));
     }
 
-    MOCK_METHOD1(open_session, std::shared_ptr<mf::Session> (std::string const& name));
+    MOCK_METHOD2(open_session, std::shared_ptr<mf::Session> (std::string const& name, std::shared_ptr<mir::events::EventSink> const&));
     MOCK_METHOD1(close_session, void (std::shared_ptr<mf::Session> const& session));
 
     MOCK_METHOD2(tag_session_with_lightdm_id, void (std::shared_ptr<mf::Session> const& session, int id));
@@ -167,13 +167,13 @@
 
                     Sequence s1, s2;
 
-                    EXPECT_CALL(*mock_shell, open_session(_))
+                    EXPECT_CALL(*mock_shell, open_session(_, _))
                         .InSequence(s1, s2);
 
                     EXPECT_CALL(*mock_shell, create_surface_for(_,_))
                         .InSequence(s1);
 
-                    EXPECT_CALL(*mock_shell, open_session(_))
+                    EXPECT_CALL(*mock_shell, open_session(_, _))
                         .InSequence(s2);
 
                     EXPECT_CALL(*mock_shell, close_session(_))
 
=== modified file 'tests/behavior-tests/session_management_context.cpp'
--- tests/behavior-tests/session_management_context.cpp	2013-04-24 05:22:20 +0000
+++ tests/behavior-tests/session_management_context.cpp	2013-04-25 09:50:50 +0000
@@ -66,12 +66,16 @@
     {
     }
 
-    std::shared_ptr<msh::Surface> create_surface(const mf::SurfaceCreationParameters& params)
+    std::shared_ptr<msh::Surface> create_surface(const mf::SurfaceCreationParameters& params,
+        frontend::SurfaceId id,
+        std::shared_ptr<events::EventSink> const& sink) override
     {
         return std::make_shared<msh::Surface>(
             mt::fake_shared(surface_builder),
             params,
-            std::shared_ptr<mir::input::InputChannel>());
+            std::shared_ptr<mir::input::InputChannel>(),
+            id,
+            sink);
     }
 
     mtd::StubSurfaceBuilder surface_builder;
@@ -152,7 +156,7 @@
 bool mtc::SessionManagementContext::open_window_consuming(std::string const& window_name)
 {
     auto const params = mf::a_surface().of_name(window_name);
-    auto session = shell->open_session(window_name);
+    auto session = shell->open_session(window_name, std::shared_ptr<mir::events::EventSink>());
     auto const surface_id = session->create_surface(params);
 
     open_windows[window_name] = std::make_tuple(session, surface_id);
@@ -164,7 +168,7 @@
                                                           geom::Size const& size)
 {
     auto const params = mf::a_surface().of_name(window_name).of_size(size);
-    auto session = shell->open_session(window_name);
+    auto session = shell->open_session(window_name, std::shared_ptr<mir::events::EventSink>());
     auto const surface_id = session->create_surface(params);
 
     open_windows[window_name] = std::make_tuple(session, surface_id);
 
=== modified file 'tests/integration-tests/cucumber/test_session_management_context.cpp'
--- tests/integration-tests/cucumber/test_session_management_context.cpp	2013-04-24 05:22:20 +0000
+++ tests/integration-tests/cucumber/test_session_management_context.cpp	2013-04-25 09:50:50 +0000
@@ -106,7 +106,7 @@
 
     mtd::MockSession session;
 
-    EXPECT_CALL(shell, open_session(test_window_name)).Times(1)
+    EXPECT_CALL(shell, open_session(test_window_name, _)).Times(1)
         .WillOnce(Return(mt::fake_shared<mf::Session>(session)));
 
     // As consuming mode is the default, omiting geometry is sufficient to request it.
@@ -122,7 +122,7 @@
 
     mtd::MockSession session;
 
-    EXPECT_CALL(shell, open_session(test_window_name)).Times(1)
+    EXPECT_CALL(shell, open_session(test_window_name, _)).Times(1)
         .WillOnce(Return(mt::fake_shared<mf::Session>(session)));
 
     EXPECT_CALL(session, create_surface(NamedWindowWithGeometry(test_window_name, test_window_size))).Times(1)
@@ -138,7 +138,7 @@
     mtd::MockSession session;
     mtd::MockSurface surface(std::make_shared<mtd::StubSurfaceBuilder>());
 
-    EXPECT_CALL(shell, open_session(test_window_name)).Times(1)
+    EXPECT_CALL(shell, open_session(test_window_name, _)).Times(1)
         .WillOnce(Return(mt::fake_shared<mf::Session>(session)));
 
     EXPECT_CALL(session, create_surface(NamedWindowWithGeometry(test_window_name, test_window_size))).Times(1)
 
=== modified file 'tests/integration-tests/shell/test_session_manager.cpp'
--- tests/integration-tests/shell/test_session_manager.cpp	2013-04-24 05:22:20 +0000
+++ tests/integration-tests/shell/test_session_manager.cpp	2013-04-25 09:50:50 +0000
@@ -36,6 +36,7 @@
 #include "mir_test_doubles/stub_input_target_listener.h"
 
 namespace mc = mir::compositor;
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
@@ -59,12 +60,12 @@
             mt::fake_shared(sequence),
             mt::fake_shared(focus_setter),
             mt::fake_shared(input_target_listener));
-    
+
     EXPECT_CALL(focus_setter, set_focus_to(_)).Times(3);
 
-    auto session1 = session_manager.open_session("Visual Basic Studio");
-    auto session2 = session_manager.open_session("Microsoft Access");
-    auto session3 = session_manager.open_session("WordPerfect");
+    auto session1 = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
+    auto session2 = session_manager.open_session("Microsoft Access", std::shared_ptr<me::EventSink>());
+    auto session3 = session_manager.open_session("WordPerfect", std::shared_ptr<me::EventSink>());
 
     {
       InSequence seq;
@@ -98,9 +99,9 @@
 
     EXPECT_CALL(focus_setter, set_focus_to(_)).Times(3);
 
-    auto session1 = session_manager.open_session("Visual Basic Studio");
-    auto session2 = session_manager.open_session("Microsoft Access");
-    auto session3 = session_manager.open_session("WordPerfect");
+    auto session1 = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
+    auto session2 = session_manager.open_session("Microsoft Access", std::shared_ptr<me::EventSink>());
+    auto session3 = session_manager.open_session("WordPerfect", std::shared_ptr<me::EventSink>());
 
     {
       InSequence seq;
 
=== modified file 'tests/unit-tests/CMakeLists.txt'
--- tests/unit-tests/CMakeLists.txt	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/CMakeLists.txt	2013-04-25 09:50:50 +0000
@@ -20,7 +20,6 @@
 add_subdirectory(android_input/)
 add_subdirectory(surfaces/)
 add_subdirectory(draw/)
-add_subdirectory(event/)
 
 add_executable(unit-tests ${UNIT_TEST_SOURCES})
 uses_android_input(unit-tests)
 
=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2013-04-25 09:50:50 +0000
@@ -298,7 +298,7 @@
         test_server.reset();
     }
 
-    std::shared_ptr<google::protobuf::RpcChannel> channel;
+    std::shared_ptr<mcl::MirBasicRpcChannel> channel;
     std::shared_ptr<mcl::Logger> logger;
     std::shared_ptr<mcl::ClientPlatformFactory> platform_factory;
     std::shared_ptr<MirConnection> connection;
 
=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2013-04-25 09:50:50 +0000
@@ -39,7 +39,7 @@
 namespace
 {
 
-struct MockRpcChannel : public google::protobuf::RpcChannel
+struct MockRpcChannel : public mir::client::MirBasicRpcChannel
 {
     void CallMethod(const google::protobuf::MethodDescriptor* method,
                     google::protobuf::RpcController*,
@@ -63,6 +63,8 @@
 
     MOCK_METHOD1(drm_auth_magic, void(const mp::DRMMagic*));
     MOCK_METHOD2(connect, void(mp::ConnectParameters const*,mp::Connection*));
+
+    void set_event_handler(mir::events::EventSink *) {}
 };
 
 struct MockClientPlatform : public mcl::ClientPlatform
 
=== removed directory 'tests/unit-tests/event'
=== removed file 'tests/unit-tests/event/CMakeLists.txt'
--- tests/unit-tests/event/CMakeLists.txt	2013-04-09 08:23:32 +0000
+++ tests/unit-tests/event/CMakeLists.txt	1970-01-01 00:00:00 +0000
@@ -1,5 +0,0 @@
-set (UNIT_TEST_SOURCES
-  ${UNIT_TEST_SOURCES}
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_event_queue.cpp
-  PARENT_SCOPE
-)
 
=== modified file 'tests/unit-tests/frontend/CMakeLists.txt'
--- tests/unit-tests/frontend/CMakeLists.txt	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/frontend/CMakeLists.txt	2013-04-25 09:50:50 +0000
@@ -6,6 +6,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_reports_errors.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_resource_cache.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_session_mediator.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_event_pipe.cpp
 )
 
 # TODO this test is a mess - something better is needed.
 
=== renamed file 'tests/unit-tests/event/test_event_queue.cpp' => 'tests/unit-tests/frontend/test_event_pipe.cpp'
--- tests/unit-tests/event/test_event_queue.cpp	2013-04-22 03:37:28 +0000
+++ tests/unit-tests/frontend/test_event_pipe.cpp	2013-04-25 09:50:50 +0000
@@ -16,14 +16,17 @@
  * Authored by: Daniel van Vugt <[e-mail]>
  */
 
+#include "src/server/frontend/event_pipe.h"
+#include <cstring>
+
 #include <gtest/gtest.h>
-#include "mir/event_queue.h"
-#include "mir/event_sink.h"
-#include <cstring>
-
-TEST(EventQueue, no_sink)
+
+using mir::frontend::EventPipe;
+
+
+TEST(EventPipe, no_sink)
 {
-    mir::EventQueue q;
+    EventPipe q;
     MirEvent e;
 
     e.type = mir_event_type_key;
@@ -38,7 +41,7 @@
 
 namespace
 {
-    class TestSink : public mir::EventSink
+    class TestSink : public mir::events::EventSink
     {
     public:
         TestSink()
@@ -61,9 +64,9 @@
     };
 }
 
-TEST(EventQueue, events_get_handled)
+TEST(EventPipe, events_get_handled)
 {
-    mir::EventQueue q;
+    EventPipe q;
     std::shared_ptr<TestSink> s(new TestSink);
 
     q.set_target(s);
@@ -84,9 +87,9 @@
     EXPECT_EQ(mir_event_type_surface, s->last_event_handled().type);
 }
 
-TEST(EventQueue, sink_is_changeable)
+TEST(EventPipe, sink_is_changeable)
 {
-    mir::EventQueue q;
+    EventPipe q;
     std::shared_ptr<TestSink> a(new TestSink);
     std::shared_ptr<TestSink> b(new TestSink);
 
 
=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp	2013-04-25 09:50:50 +0000
@@ -34,7 +34,7 @@
 #include "mir_test_doubles/stub_session.h"
 #include "mir_test_doubles/stub_surface_builder.h"
 #include "mir_test/fake_shared.h"
-#include "mir/event_queue.h"
+#include "mir/events/event_sink.h"
 #include "mir/shell/surface.h"
 
 #include <gtest/gtest.h>
@@ -129,7 +129,11 @@
     }
 };
 
-}
+class NullEventSink : public mir::events::EventSink
+{
+public:
+    void handle_event(MirEvent const& ) override {}
+};
 
 struct SessionMediatorTest : public ::testing::Test
 {
@@ -142,14 +146,14 @@
           resource_cache{std::make_shared<mf::ResourceCache>()},
           mediator{shell, graphics_platform, graphics_display,
                    buffer_allocator, report, 
-                   std::make_shared<mir::EventQueue>(),
+                   std::make_shared<NullEventSink>(),
                    resource_cache},
           stubbed_session{std::make_shared<StubbedSession>()},
           null_callback{google::protobuf::NewPermanentCallback(google::protobuf::DoNothing)}
     {
         using namespace ::testing;
 
-        ON_CALL(*shell, open_session(_)).WillByDefault(Return(stubbed_session));
+        ON_CALL(*shell, open_session(_, _)).WillByDefault(Return(stubbed_session));
         ON_CALL(*shell, create_surface_for(_, _)).WillByDefault(Return(mf::SurfaceId{1}));
     }
 
@@ -164,7 +168,7 @@
 
     std::unique_ptr<google::protobuf::Closure> null_callback;
 };
-
+}
 
 TEST_F(SessionMediatorTest, disconnect_releases_session)
 {
 
=== modified file 'tests/unit-tests/frontend/test_session_mediator_android.cpp'
--- tests/unit-tests/frontend/test_session_mediator_android.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/frontend/test_session_mediator_android.cpp	2013-04-25 09:50:50 +0000
@@ -31,7 +31,7 @@
 #include "mir_test_doubles/mock_session.h"
 #include "mir_test_doubles/stub_shell.h"
 
-#include "mir/event_queue.h"
+#include "mir/events/event_sink.h"
 
 #include <gtest/gtest.h>
 
@@ -87,7 +87,11 @@
     }
 };
 
-}
+class NullEventSink : public mir::events::EventSink
+{
+public:
+    void handle_event(MirEvent const& ) override {}
+};
 
 struct SessionMediatorAndroidTest : public ::testing::Test
 {
@@ -100,7 +104,7 @@
           resource_cache{std::make_shared<mf::ResourceCache>()},
           mediator{shell, graphics_platform, graphics_display,
                    buffer_allocator, report,
-                   std::make_shared<mir::EventQueue>(),
+                   std::make_shared<NullEventSink>(),
                    resource_cache},
           null_callback{google::protobuf::NewPermanentCallback(google::protobuf::DoNothing)}
     {
@@ -117,6 +121,8 @@
     std::unique_ptr<google::protobuf::Closure> null_callback;
 };
 
+}
+
 TEST_F(SessionMediatorAndroidTest, drm_auth_magic_throws)
 {
     mp::ConnectParameters connect_parameters;
 
=== modified file 'tests/unit-tests/frontend/test_session_mediator_gbm.cpp'
--- tests/unit-tests/frontend/test_session_mediator_gbm.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/frontend/test_session_mediator_gbm.cpp	2013-04-25 09:50:50 +0000
@@ -27,7 +27,7 @@
 #include "mir/graphics/drm_authenticator.h"
 #include "mir/graphics/platform.h"
 #include "mir/graphics/platform_ipc_package.h"
-#include "mir/event_queue.h"
+#include "mir/events/event_sink.h"
 
 #include <boost/exception/errinfo_errno.hpp>
 #include <boost/throw_exception.hpp>
@@ -91,7 +91,11 @@
     MOCK_METHOD1(drm_auth_magic, void(drm_magic_t));
 };
 
-}
+class NullEventSink : public mir::events::EventSink
+{
+public:
+    void handle_event(MirEvent const& ) override {}
+};
 
 struct SessionMediatorGBMTest : public ::testing::Test
 {
@@ -104,7 +108,7 @@
           resource_cache{std::make_shared<mf::ResourceCache>()},
           mediator{shell, mock_platform, graphics_display,
                    buffer_allocator, report,
-                   std::make_shared<mir::EventQueue>(),
+                   std::make_shared<NullEventSink>(),
                    resource_cache},
           null_callback{google::protobuf::NewPermanentCallback(google::protobuf::DoNothing)}
     {
@@ -121,6 +125,8 @@
     std::unique_ptr<google::protobuf::Closure> null_callback;
 };
 
+}
+
 TEST_F(SessionMediatorGBMTest, drm_auth_magic_uses_drm_authenticator)
 {
     mp::ConnectParameters connect_parameters;
 
=== modified file 'tests/unit-tests/shell/test_application_session.cpp'
--- tests/unit-tests/shell/test_application_session.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_application_session.cpp	2013-04-25 09:50:50 +0000
@@ -32,6 +32,7 @@
 #include <gtest/gtest.h>
 
 namespace mc = mir::compositor;
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
@@ -47,9 +48,9 @@
     auto const mock_surface = std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder));
 
     mtd::MockSurfaceFactory surface_factory;
-    ON_CALL(surface_factory, create_surface(_)).WillByDefault(Return(mock_surface));
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(Return(mock_surface));
 
-    EXPECT_CALL(surface_factory, create_surface(_));
+    EXPECT_CALL(surface_factory, create_surface(_, _, _));
     EXPECT_CALL(*mock_surface, destroy());
 
     mtd::StubInputTargetListener input_listener;
@@ -69,11 +70,11 @@
     mtd::StubSurfaceBuilder surface_builder;
     {
         InSequence seq;
-        EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
-            .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
-        EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
-            .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
-        EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
+            .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
+            .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1)
             .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
     }
 
@@ -106,12 +107,12 @@
     auto const mock_surface = std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder));
 
     mtd::MockSurfaceFactory surface_factory;
-    ON_CALL(surface_factory, create_surface(_)).WillByDefault(Return(mock_surface));
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(Return(mock_surface));
 
     mtd::StubInputTargetListener input_listener;
     msh::ApplicationSession app_session(mt::fake_shared(surface_factory), mt::fake_shared(input_listener), "Foo");
 
-    EXPECT_CALL(surface_factory, create_surface(_));
+    EXPECT_CALL(surface_factory, create_surface(_, _, _));
 
     {
         InSequence seq;
 
=== modified file 'tests/unit-tests/shell/test_organising_surface_factory.cpp'
--- tests/unit-tests/shell/test_organising_surface_factory.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_organising_surface_factory.cpp	2013-04-25 09:50:50 +0000
@@ -25,6 +25,7 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace geom = mir::geometry;
@@ -45,7 +46,7 @@
         using namespace ::testing;
 
         underlying_surface_factory = std::make_shared<mtd::MockSurfaceFactory>();
-        ON_CALL(*underlying_surface_factory, create_surface(_)).WillByDefault(Return(null_surface));
+        ON_CALL(*underlying_surface_factory, create_surface(_, _, _)).WillByDefault(Return(null_surface));
 
         placement_strategy = std::make_shared<MockPlacementStrategy>();
     }
@@ -62,13 +63,13 @@
 
     msh::OrganisingSurfaceFactory factory(underlying_surface_factory, placement_strategy);
 
-    EXPECT_CALL(*underlying_surface_factory, create_surface(_)).Times(1);
+    EXPECT_CALL(*underlying_surface_factory, create_surface(_, _, _)).Times(1);
 
     auto params = mf::a_surface();
     EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
         .WillOnce(Return(mf::a_surface()));
 
-    factory.create_surface(params);
+    factory.create_surface(params, mf::SurfaceId(), std::shared_ptr<me::EventSink>());
 }
 
 TEST_F(OrganisingSurfaceFactorySetup, forwards_create_surface_parameters_from_placement_strategy_to_underlying_factory)
@@ -83,8 +84,8 @@
 
     EXPECT_CALL(*placement_strategy, place(Ref(params))).Times(1)
         .WillOnce(Return(placed_params));
-    EXPECT_CALL(*underlying_surface_factory, create_surface(placed_params));
+    EXPECT_CALL(*underlying_surface_factory, create_surface(placed_params, mf::SurfaceId(), std::shared_ptr<me::EventSink>()));
 
-    factory.create_surface(params);
+    factory.create_surface(params, mf::SurfaceId(), std::shared_ptr<me::EventSink>());
 }
 
 
=== modified file 'tests/unit-tests/shell/test_registration_order_focus_sequence.cpp'
--- tests/unit-tests/shell/test_registration_order_focus_sequence.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_registration_order_focus_sequence.cpp	2013-04-25 09:50:50 +0000
@@ -33,6 +33,7 @@
 #include <string>
 
 namespace mc = mir::compositor;
+namespace me = mir::events;
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
 namespace mt = mir::test;
@@ -65,9 +66,9 @@
 {
     using namespace ::testing;
 
-    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1);
-    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2);
-    auto app3 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name3);
+    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1, std::shared_ptr<me::EventSink>());
+    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2, std::shared_ptr<me::EventSink>());
+    auto app3 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name3, std::shared_ptr<me::EventSink>());
 
     container->insert_session(app1);
     container->insert_session(app2);
@@ -83,9 +84,9 @@
 {
     using namespace ::testing;
 
-    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1);
-    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2);
-    auto app3 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name3);
+    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1, std::shared_ptr<me::EventSink>());
+    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2, std::shared_ptr<me::EventSink>());
+    auto app3 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name3, std::shared_ptr<me::EventSink>());
     container->insert_session(app1);
     container->insert_session(app2);
     container->insert_session(app3);
@@ -100,7 +101,7 @@
 {
     using namespace ::testing;
 
-    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1);
+    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1, std::shared_ptr<me::EventSink>());
     container->insert_session(app1);
 
     msh::RegistrationOrderFocusSequence focus_sequence(container);
@@ -112,8 +113,8 @@
 {
     using namespace ::testing;
 
-    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1);
-    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2);
+    auto app1 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1, std::shared_ptr<me::EventSink>());
+    auto app2 = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name2, std::shared_ptr<me::EventSink>());
     auto null_session = std::shared_ptr<msh::ApplicationSession>();
 
     msh::RegistrationOrderFocusSequence focus_sequence(container);
@@ -129,7 +130,7 @@
 {
     using namespace ::testing;
 
-    auto invalid_session = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1);
+    auto invalid_session = std::make_shared<msh::ApplicationSession>(factory, mt::fake_shared(input_listener), testing_app_name1, std::shared_ptr<me::EventSink>());
     auto null_session = std::shared_ptr<msh::ApplicationSession>();
 
     msh::RegistrationOrderFocusSequence focus_sequence(container);
 
=== modified file 'tests/unit-tests/shell/test_session_manager.cpp'
--- tests/unit-tests/shell/test_session_manager.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_session_manager.cpp	2013-04-25 09:50:50 +0000
@@ -41,6 +41,7 @@
 #include <gtest/gtest.h>
 
 namespace mc = mir::compositor;
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
@@ -101,7 +102,7 @@
 
     EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
 
-    auto session = session_manager.open_session("Visual Basic Studio");
+    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
     session_manager.close_session(session);
 }
 
@@ -109,10 +110,10 @@
 {
     using namespace ::testing;
 
-    EXPECT_CALL(surface_factory, create_surface(_)).Times(1);
+    EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);
 
     std::shared_ptr<mi::InputChannel> null_input_channel;
-    ON_CALL(surface_factory, create_surface(_)).WillByDefault(
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(
        Return(std::make_shared<msh::Surface>(
            mt::fake_shared(surface_builder),
            mf::a_surface(),
@@ -126,7 +127,7 @@
 
     EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
 
-    auto session = session_manager.open_session("Visual Basic Studio");
+    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
     session->create_surface(mf::a_surface().of_size(geom::Size{geom::Width{1024}, geom::Height{768}}));
 
     session_manager.close_session(session);
@@ -140,7 +141,7 @@
     EXPECT_CALL(container, insert_session(_)).Times(1);
     EXPECT_CALL(focus_setter, set_focus_to(_)).WillOnce(SaveArg<0>(&new_session));
 
-    auto session = session_manager.open_session("Visual Basic Studio");
+    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
     EXPECT_EQ(session, new_session);
 }
 
@@ -148,8 +149,8 @@
 {
     using namespace ::testing;
 
-    auto session1 = session_manager.open_session("Visual Basic Studio");
-    auto session2 = session_manager.open_session("IntelliJ IDEA");
+    auto session1 = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
+    auto session2 = session_manager.open_session("IntelliJ IDEA", std::shared_ptr<me::EventSink>());
 
     session_manager.tag_session_with_lightdm_id(session1, 1);
 
@@ -161,8 +162,8 @@
 {
     using namespace ::testing;
 
-    auto session1 = session_manager.open_session("Visual Basic Studio");
-    auto session2 = session_manager.open_session("IntelliJ IDEA");
+    auto session1 = session_manager.open_session("Visual Basic Studio", std::shared_ptr<me::EventSink>());
+    auto session2 = session_manager.open_session("IntelliJ IDEA", std::shared_ptr<me::EventSink>());
 
     auto shell_session1 = std::dynamic_pointer_cast<msh::Session>(session1);
     auto shell_session2 = std::dynamic_pointer_cast<msh::Session>(session2);
@@ -180,7 +181,7 @@
 {
     using namespace ::testing;
     std::shared_ptr<mi::InputChannel> null_input_channel;
-    ON_CALL(surface_factory, create_surface(_)).WillByDefault(
+    ON_CALL(surface_factory, create_surface(_, _, _)).WillByDefault(
         Return(std::make_shared<msh::Surface>(
             mt::fake_shared(surface_builder),
             mf::a_surface(),
@@ -191,11 +192,11 @@
         InSequence seq;
 
         EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation
-        EXPECT_CALL(surface_factory, create_surface(_)).Times(1);
+        EXPECT_CALL(surface_factory, create_surface(_, _, _)).Times(1);
         EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Post Surface creation
     }
 
-    auto session1 = session_manager.open_session("Weather Report");
+    auto session1 = session_manager.open_session("Weather Report", std::shared_ptr<me::EventSink>());
     session_manager.create_surface_for(session1, mf::a_surface());
 }
 
@@ -230,12 +231,12 @@
     using namespace ::testing;
 
     std::shared_ptr<mi::InputChannel> null_input_channel;
-    ON_CALL(surface_factory, create_surface(_)).WillByDefault(
+    ON_CALL(surface_factory, create_surface(_,_,_)).WillByDefault(
        Return(std::make_shared<msh::Surface>(
            mt::fake_shared(surface_builder),
            mf::a_surface(),
            null_input_channel)));
-    EXPECT_CALL(surface_factory, create_surface(_)).Times(1);
+    EXPECT_CALL(surface_factory, create_surface(_,_,_)).Times(1);
 
     EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
     {
@@ -252,7 +253,7 @@
     }
 
     {
-        auto session = session_manager.open_session("test");
+        auto session = session_manager.open_session("test", std::shared_ptr<me::EventSink>());
         auto surf = session_manager.create_surface_for(session, mf::a_surface());
         session->destroy_surface(surf);
         session_manager.close_session(session);
 
=== modified file 'tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp'
--- tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp	2013-04-25 09:50:50 +0000
@@ -57,7 +57,6 @@
     MOCK_METHOD0(show, void());
 
     MOCK_METHOD3(configure_surface, int(mf::SurfaceId, MirSurfaceAttrib, int));
-    MOCK_METHOD1(set_event_sink, void(std::shared_ptr<mir::EventSink> const&));
 };
 
 TEST(SingleVisibilityFocusMechanism, mechanism_sets_visibility)
 
=== modified file 'tests/unit-tests/shell/test_surface.cpp'
--- tests/unit-tests/shell/test_surface.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_surface.cpp	2013-04-25 09:50:50 +0000
@@ -33,6 +33,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+namespace me = mir::events;
 namespace ms = mir::surfaces;
 namespace msh = mir::shell;
 namespace mf = mir::frontend;
@@ -450,3 +451,4 @@
                              mir_surface_state_fullscreen));
     EXPECT_EQ(mir_surface_state_fullscreen, surf.state());
 }
+
 
=== modified file 'tests/unit-tests/shell/test_the_session_container_implementation.cpp'
--- tests/unit-tests/shell/test_the_session_container_implementation.cpp	2013-04-24 05:22:20 +0000
+++ tests/unit-tests/shell/test_the_session_container_implementation.cpp	2013-04-25 09:50:50 +0000
@@ -29,6 +29,7 @@
 #include <gtest/gtest.h>
 #include <string>
 
+namespace me = mir::events;
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace mtd = mir::test::doubles;
@@ -39,10 +40,8 @@
     auto factory = std::make_shared<mtd::MockSurfaceFactory>();
     msh::DefaultSessionContainer container;
 
-    container.insert_session(std::make_shared<msh::ApplicationSession>(factory, std::make_shared<mtd::StubInputTargetListener>(), 
-        "Visual Studio 7"));
-    container.insert_session(std::make_shared<msh::ApplicationSession>(factory, std::make_shared<mtd::StubInputTargetListener>(), 
-        "Visual Studio 8"));
+    container.insert_session(std::make_shared<msh::ApplicationSession>(factory, std::make_shared<mtd::StubInputTargetListener>(), "Visual Studio 7"));
+    container.insert_session(std::make_shared<msh::ApplicationSession>(factory, std::make_shared<mtd::StubInputTargetListener>(), "Visual Studio 8"));
 
     struct local
     {