Merge lp:~vanvugt/mir/surface-states into lp:~mir-team/mir/trunk

[Daniel van Vugt]

Add support for surface states.

Actually, most of this branch is about adding generic support for events. And
changes to the state of a surface are one good example of an event originating
on the server that the client needs to be told about. This is because most
state changes will come from the shell/user. Not the client. We use events
to notify the client of such changes.

[Daniel van Vugt (vanvugt)]

This will conflict slightly with the unique-id branch. I will fix that if and when it becomes a problem. But unique-id is not a prerequisite for this.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:622
http://jenkins.qa.ubuntu.com/job/mir-ci/357/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/346
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/362
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/362/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:626
http://jenkins.qa.ubuntu.com/job/mir-ci/371/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/362
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/245
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/376
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/376/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/163

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

I don't see the utility of EventQueue as thing stand currently. I think a msh::Surface only needs an EventSink (i.e. an event handler). If we need an EventQueue in the future, it should be derived from EventSink. The surface doesn't need to know anything about the details of how its events are handled.

Also, I would argue that since the id and the event handler are not really optional for the correct functioning of mir, nor there is any need to reset them, they should be taken at construction time:

msh::Surface(..., id, event_sink, ...)

msh::SurfaceFactory::create_surface(params, id/id_generator, EventSink)

908 + // A dynamic cast is not ideal. But the alternatives all seem to require
909 + // non-trivial architectural changes. Another day...

I think these changes should be discussed in this MP, or alternatively we can split out the communications part to a different MP, leaving this with only the implementation of the internal event propagation and handling mechanisms on the server and client side.

Marking "Needs Information" => "Needs discussion"

review: Needs Information

[Kevin DuBois (kdub)]

> I don't see the utility of EventQueue as thing stand currently. I think a
> msh::Surface only needs an EventSink (i.e. an event handler). If we need an
> EventQueue in the future, it should be derived from EventSink. The surface
> doesn't need to know anything about the details of how its events are handled.
>
> Also, I would argue that since the id and the event handler are not really
> optional for the correct functioning of mir, nor there is any need to reset
> them, they should be taken at construction time:
>
> msh::Surface(..., id, event_sink, ...)
>
> msh::SurfaceFactory::create_surface(params, id/id_generator, EventSink)
that makes sense to me too.

1399 +TEST(EventQueue, events_get_handled)
we test that the same thing (but with different arguments) happens 3 times. We could test that one event gets handled and cover the same bit of code. (same sort of thing with ShellSurface.states)

The acceptance test names don't really capture what they're doing.
Its good to see a new acceptance test though :) I've been meaning to complain that the #of unit tests are expanding a lot faster than our integration/acceptance.

[Daniel van Vugt (vanvugt)]

Alexandros:
It might be possible to merge EventQueue into EventSink. Although I would be inclined not to because:
  1. If EventQueue adds threading and background queuing in future then I don't want to have to touch any event sources. So the extra abstraction is useful.
  2. If EventQueue adds support for multiple simultaneous sinks (e.g. in-server, logging, or none like you can now), then you need the EventQueue class. You can't let your event sources know how many, if any, sinks there are.
  3. The class relationships might make it very difficult to implement that way. Not sure.

Although, if we're talking about unused functionality then the "EventSequence" message is a better example. It's presently always length 1. :)

I did think about passing things in to constructors, but decided against it in the name of:
  simplicity - no long constructor arg lists to steepen the learning curve and trigger changes to lots of test code; and
  flexibility - it's more clear this way that id and event_sink are optional things, and that the Surface class does not require them to remain fixed or be present at all.

Kevin:
Testing something three times is a good thing. It means you're verifying that the class does not stop functioning for some reason after the first use (a common programming mistake). That's a very useful test for event queuing.

Acceptance test names; I disagree they always need to be verbose. Sometimes if you're writing a test that covers _all_ the functionality of some minor feature (surface_states) then it makes sense to name the test after that feature. But I've now renamed the acceptance tests to make them more verbose. I know other people like it that way too.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:628
http://jenkins.qa.ubuntu.com/job/mir-ci/383/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/378
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/261
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/388
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/388/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

267 + * MirSurfaceState: "minimized" or "minimised"?
268 + * A review of 7 leading dictionaries shows "z" is the most common spelling
269 + * (listed in all dictionaries), and "s" only appears in 4. And those that list
270 + * both "z" and "s" usually point out that "z" is the main spelling. Only 2
271 + * dictionaries say that "s" is preferred, and even still all dictionaries seem
272 + * to agree that "s" is only used in the UK.
273 + * The same is true for "maximized" vs "maximised".

All very interesting - but do we need it? Surely Canonical has decided which dialect of English to use?

[Alan Griffiths (alan-griffiths)]

719 + assert(configure_result.has_error());

I'm not keen on assert() - which has different behavior according to the build options.

In any case, if we check there is an error shouldn't we be doing something with it? (Write it to log->debug() perhaps?)

[Alan Griffiths (alan-griffiths)]

363 + // Allocate a non-zero unique ID. We reserve zero for special use.

What special use? Oh, I see elsewhere:

591 + if (result.id() == 0) // It's an event sequence

This seems like unnecessary coupling to me. There wouldn't be a need for this special case handling in "on the wire" code if the client library made blocking "get events" calls to the server (these would simply complete like any other call when the server provides some events).

~~~~

I agree with Alexandros that the relationship between EventQueue and EventSink seems wrong. They are both classes that handle/post events which is commonality that I'd expect to see in the design. E.g.

class EventHandler ...

class EventSink : public EventHandler ...

class EventQueue : public EventHandler ...

As Alexandros says a surface only needs an EventHandler - it doesn't need to know if it is a queue or a sink.

review: Needs Information

[Robert Ancell (robert-ancell)]

> 267 + * MirSurfaceState: "minimized" or "minimised"?
>
> All very interesting - but do we need it? Surely Canonical has decided which
> dialect of English to use?

Yeah, this is a bit over the top - we should only support one set of names, and the convention would be to use the US dialect - minimized.

[Robert Ancell (robert-ancell)]

Approving on the grounds this is an improvement on what we currently have. I can imagine some refactoring could be done in the future, but the client API seems appropriate and this can be done transparently.

review: Approve

[Daniel van Vugt (vanvugt)]

"All very interesting - but do we need it? Surely Canonical has decided which dialect of English to use?"

Actually no one has decided. Our code like most APIs uses a lot of American spelling ("color"). And our design docs use the (rare) UK spelling of minimise/maximise. So the confusion already exists.

AFAIK, most existing toolkits use American spelling for minimize/maximize. And those forms are valid to UK English too. So if you had to choose one then choose "z" (despite conflicting with the design docs). Otherwise, I recommend keeping both forms. It does not hurt to keep both.

[Daniel van Vugt (vanvugt)]

> Asserts...
Yeah there is only one assert. It is correct and it is harmless. It might not be compiled in for release builds and that's fine. Because correct behaviour in that region of code is to ignore error responses (for now). I could happily remove the assert completely if it's bothersome.

> "There wouldn't be a need for this special case handling in "on the wire" code if the client library made blocking "get events" calls to the server (these would simply complete like any other call when the server provides some events)."
I thought exactly the same in the beginning, wanting to reuse the query/response model. However I then realized that the client-side MirSurface needs to receive and handle events regardless of whether the client app is interested in events. This is so the client API can have up-to-date state information cached for each surface, and almost never needs to query the server when mir_surface_get_state is called. No blocking, no round-trips, and no requirement than an event-handling client app needs to be threaded. And of course, this approach will conceivably be useful for other things later.

A second reason for asynchronous event delivery is that it makes little sense to impose a new restriction; that your client has to be designed to block and wait for the next event, if you can generalize with a non-blocking approach. The blocking approach can still be emulated by adding new client API functions later if desired.

[Daniel van Vugt (vanvugt)]

> 363 + // Allocate a non-zero unique ID. We reserve zero for special use.
> What special use? Oh, I see elsewhere:
> 591 + if (result.id() == 0) // It's an event sequence

This is related to the unique-id branch, which is a nicer approach.

[Daniel van Vugt (vanvugt)]

Decoupled shell::Surface from a concrete EventQueue. Now shell::Surface only gets and uses the EventSink interface.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:631
http://jenkins.qa.ubuntu.com/job/mir-ci/397/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/396
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/279
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/402
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/402/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/181

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> > "There wouldn't be a need for this special case handling in "on the wire"
> code if the client library made blocking "get events" calls to the server
> (these would simply complete like any other call when the server provides some
> events)."
> I thought exactly the same in the beginning, wanting to reuse the
> query/response model. However I then realized that the client-side MirSurface
> needs to receive and handle events regardless of whether the client app is
> interested in events. This is so the client API can have up-to-date state
> information cached for each surface, and almost never needs to query the
> server when mir_surface_get_state is called. No blocking, no round-trips, and
> no requirement than an event-handling client app needs to be threaded. And of
> course, this approach will conceivably be useful for other things later.
>
> A second reason for asynchronous event delivery is that it makes little sense
> to impose a new restriction; that your client has to be designed to block and
> wait for the next event, if you can generalize with a non-blocking approach.
> The blocking approach can still be emulated by adding new client API functions
> later if desired.

To clarify I meant the calls could originate *in* the library, not through the API. (Still thinking about whether the proposed approach is good enough - will update vote shortly.)

[Alan Griffiths (alan-griffiths)]

237 +class EventSink
238 +{
239 +public:
240 + virtual ~EventSink() {}
241 + virtual void handle_event(MirEvent const& e) = 0;
242 +};

Destructor should be nothrow. CopyAssign should be disabled.

~~~~

state_value is effectively atomic on Intel, but as it is can be accessed by different threads we should make that explicit with "std::atomic<MirSurfaceState> state_value;" etc.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Mir's unclear use of threads scares me. If state_value needs to be atomic then so does type_value. That's an existing problem which pre-dates this proposal. If it is a problem at all...

Please document Mir's thread usage and which classes may be accessed concurrently.

I'm not going to fix the atomicity of state_value while adjacent members are not atomic, and I'm not entirely sure or convinced that concurrent access is required yet.

[Daniel van Vugt (vanvugt)]

Fixed EventSink destructor and assignment.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

612 + if (event_handler)
613 + {
...
647 + } // else ignore incoming events

I think that this is rather a big chunk of logic to be buried in the middle of MirSocketRpcChannel::read_message() - I'd be happier to see a separate handle_event() member function. (I'm still not totally convinced that MirSocketRpcChannel is the right place for this logic but PendingCallCache doesn't seem any better - although a rename might fix that.)

Actually, if we're going to stick event handling into the "wire" decoding logic I'd rather be changing wire::Result to have an optional event field instead of have a special value in the id field and reusing the response field.

[Alan Griffiths (alan-griffiths)]

> I'm not going to fix the atomicity of state_value while adjacent members are
> not atomic, and I'm not entirely sure or convinced that concurrent access is
> required yet.

OK as state() isn't called yet I guess concurrent access isn't needed yet.

[Alan Griffiths (alan-griffiths)]

88 + virtual void set_event_sink(std::weak_ptr<EventSink> const&) {}

Session is an interface, it should only have pure virtual functions.

~~~~

The chain of set_event_sink() calls feels a bit odd - they fight against the system architecture. Especially:

SessionMediator is just an implementation of the mir::protobuf::DisplayServer interface. But with this MP it introduces an additional public set_event_sink() member function to the interface. This is a bad idea as client code (which, correctly, uses the interface and has no access to the function). This leads to:

927 + // A dynamic cast is not ideal. But the alternatives all seem to require
928 + // non-trivial architectural changes. Another day...
929 + mf::SessionMediator *sm = dynamic_cast<mf::SessionMediator*>(ds.get());
930 + if (sm)
931 + sm->set_event_sink(proc);

Which we can all agree is horrid. (Especially the silently different behavior when another type is supplied.)

~~~~

I can understand that adding an EventSink interface to ProtobufMessageProcessor minimizes the size of the diff, but it dilutes the purpose of the class. It would be better to separate out a ProtobufResponseSender and use that both in the ProtobufMessageProcessor and in a new ProtobufEventSink.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Alan,

612 + if (event_handler)
I thought of moving it too. But I prefer the cohesion of having code in the one and only place where it is used. And it's easier for the reader to comprehend when there is one fewer function name to look up and find. Should that logic ever be reused then definitely separate it, sure. It's not like that block of code is too big to fit on anyone's screen.

My first attempt (weeks ago) did work on the wire protocol as you say, and did not piggyback on Result. However there was a major problem with that -- Protobuf cannot tell you what kind of message you are receiving before you try to decode it. You just construct an object of every possible message type and try to parse the data for it. Half the time you will get an error (wrong message type) and waste CPU time in doing so. The simple and fast solution is to guarantee that incoming messages are of a type that you know in advance (Result). So you can parse them correctly first time. Protobuf won't log any errors and you don't waste CPU time.

The set_event_sink/casting stuff was a big challenge. I spent days (weeks actually) figuring out the class relationships and how to make them talk in a way that makes sense. Reworking it all could take days for me. Maybe simpler for you since you understand these classes. So I would appreciate merges proposed if you'd like it structured differently. I am rapidly running out of time and won't be around to do major reworks until June.

[Daniel van Vugt (vanvugt)]

Re:
    virtual void set_event_sink(std::weak_ptr<EventSink> const&) {}

I know it's an interface so usually consists of pure virtual methods. But there's already a concrete inline destructor:
    virtual ~Session() {}

It's not that much different.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:637
http://jenkins.qa.ubuntu.com/job/mir-ci/420/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/430/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/313/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/425/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/201

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

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Fixed most of Alan's concerns. Except the dynamic_cast. I will look into it again over the next few days.

[Daniel van Vugt (vanvugt)]

Dear Jenkins,

Rebuild!

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:640
http://jenkins.qa.ubuntu.com/job/mir-ci/421/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/431
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/314
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/426
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/426/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/202

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

review: Approve (continuous-integration)

=== modified file 'include/client/mir_toolkit/mir_client_library.h'
--- include/client/mir_toolkit/mir_client_library.h	2013-04-17 07:31:40 +0000
+++ include/client/mir_toolkit/mir_client_library.h	2013-04-18 07:33:25 +0000
@@ -265,6 +265,22 @@
  */
 MirSurfaceType mir_surface_get_type(MirSurface *surface);
 
+/**
+ * Change the state of a surface.
+ *   \param [in] surface  The surface to operate on
+ *   \param [in] state    The new state of the surface
+ *   \return              A wait handle that can be passed to mir_wait_for
+ */
+MirWaitHandle* mir_surface_set_state(MirSurface *surface,
+                                     MirSurfaceState state);
+
+/**
+ * Get the current state of a surface.
+ *   \param [in] surface  The surface to query
+ *   \return              The state of the surface
+ */
+MirSurfaceState mir_surface_get_state(MirSurface *surface);
+
 #ifdef __cplusplus
 }
 /**@}*/

=== added file 'include/server/mir/event_queue.h'
--- include/server/mir/event_queue.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/event_queue.h	2013-04-18 07:33:25 +0000
@@ -0,0 +1,40 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_EVENT_QUEUE_H_
+#define MIR_EVENT_QUEUE_H_
+
+#include "mir_toolkit/event.h"
+#include "mir/event_sink.h"
+#include <memory>
+
+namespace mir
+{
+class EventQueue : public EventSink
+{
+public:
+    void set_target(std::weak_ptr<EventSink> const& s);
+    void handle_event(MirEvent const& e) override;
+
+private:
+    std::weak_ptr<EventSink> target;
+};
+
+} // namespace mir
+
+#endif

=== modified file 'include/server/mir/frontend/protobuf_ipc_factory.h'
--- include/server/mir/frontend/protobuf_ipc_factory.h	2013-04-16 09:08:29 +0000
+++ include/server/mir/frontend/protobuf_ipc_factory.h	2013-04-18 07:33:25 +0000
@@ -23,19 +23,16 @@
 
 namespace mir
 {
-namespace protobuf
-{
-class DisplayServer;
-}
 namespace frontend
 {
+class Server;
 class ResourceCache;
 class MessageProcessorReport;
 
 class ProtobufIpcFactory
 {
 public:
-    virtual std::shared_ptr<protobuf::DisplayServer> make_ipc_server() = 0;
+    virtual std::shared_ptr<Server> make_ipc_server() = 0;
     virtual std::shared_ptr<ResourceCache> resource_cache() = 0;
     virtual std::shared_ptr<MessageProcessorReport> report() = 0;
 

=== added file 'include/server/mir/frontend/server.h'
--- include/server/mir/frontend/server.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/frontend/server.h	2013-04-18 07:33:25 +0000
@@ -0,0 +1,45 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_FRONTEND_SERVER_H_
+#define MIR_FRONTEND_SERVER_H_
+
+#include "mir_protobuf.pb.h"
+#include <memory>
+
+namespace mir
+{
+class EventSink;
+
+namespace frontend
+{
+
+class Server : public mir::protobuf::DisplayServer
+{
+public:
+    virtual ~Server() noexcept;
+    void set_event_sink(std::weak_ptr<EventSink> const& sink);
+
+protected:
+    std::weak_ptr<EventSink> event_sink;
+};
+
+} // namespace frontent
+} // namespace mir
+
+#endif

=== modified file 'include/server/mir/frontend/session.h'
--- include/server/mir/frontend/session.h	2013-04-16 09:08:29 +0000
+++ include/server/mir/frontend/session.h	2013-04-18 07:33:25 +0000
@@ -29,6 +29,7 @@
 
 namespace mir
 {
+class EventSink;
 
 namespace frontend
 {
@@ -52,6 +53,8 @@
 
     virtual int configure_surface(SurfaceId id, MirSurfaceAttrib attrib, int value) = 0;
 
+    virtual void set_event_sink(std::weak_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-16 09:08:29 +0000
+++ include/server/mir/frontend/session_mediator.h	2013-04-18 07:33:25 +0000
@@ -21,6 +21,7 @@
 #define MIR_FRONTEND_SESSION_MEDIATOR_H_
 
 #include "mir_protobuf.pb.h"
+#include "mir/frontend/server.h"
 
 #include <map>
 #include <memory>
@@ -50,7 +51,7 @@
 class ClientBufferTracker;
 
 // SessionMediator relays requests from the client process into the server.
-class SessionMediator : public mir::protobuf::DisplayServer
+class SessionMediator : public mir::frontend::Server
 {
 public:
 

=== modified file 'include/server/mir/shell/application_session.h'
--- include/server/mir/shell/application_session.h	2013-04-16 09:08:29 +0000
+++ include/server/mir/shell/application_session.h	2013-04-18 07:33:25 +0000
@@ -20,11 +20,13 @@
 #define MIR_SHELL_APPLICATION_SESSION_H_
 
 #include "mir/shell/session.h"
+#include "mir/event_queue.h"
 
 #include <map>
 
 namespace mir
 {
+class EventSink;
 
 namespace shell
 {
@@ -52,12 +54,15 @@
 
     int configure_surface(frontend::SurfaceId id, MirSurfaceAttrib attrib, int value);
 
+    void set_event_sink(std::weak_ptr<mir::EventSink> const& sink);
+
 protected:
     ApplicationSession(ApplicationSession const&) = delete;
     ApplicationSession& operator=(ApplicationSession const&) = delete;
 
 private:
     std::shared_ptr<SurfaceFactory> const surface_factory;
+    std::shared_ptr<EventQueue> event_queue;
     std::string const session_name;
 
     frontend::SurfaceId next_id();

=== modified file 'include/server/mir/shell/surface.h'
--- include/server/mir/shell/surface.h	2013-04-16 09:08:29 +0000
+++ include/server/mir/shell/surface.h	2013-04-18 07:33:25 +0000
@@ -21,6 +21,7 @@
 #define MIR_SHELL_SURFACE_H_
 
 #include "mir/frontend/surface.h"
+#include "mir/frontend/surface_id.h"
 #include "mir/surfaces/surface.h"
 #include "mir/input/surface_target.h"
 
@@ -30,6 +31,9 @@
 
 namespace mir
 {
+
+class EventSink;
+
 namespace frontend
 {
 struct SurfaceCreationParameters;
@@ -76,15 +80,25 @@
 
     virtual int configure(MirSurfaceAttrib attrib, int value);
     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);
+    void notify_change(MirSurfaceAttrib attrib, int value);
 
     std::shared_ptr<SurfaceBuilder> const builder;
     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;
+
     MirSurfaceType type_value;
+    MirSurfaceState state_value;
 };
 }
 }

=== added file 'include/shared/mir/event_sink.h'
--- include/shared/mir/event_sink.h	1970-01-01 00:00:00 +0000
+++ include/shared/mir/event_sink.h	2013-04-18 07:33:25 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_EVENT_SINK_H_
+#define MIR_EVENT_SINK_H_
+
+#include "mir_toolkit/event.h"
+
+namespace mir
+{
+
+class EventSink
+{
+public:
+    virtual ~EventSink() noexcept {}
+    virtual void handle_event(MirEvent const& e) = 0;
+
+    EventSink& operator=(EventSink const&) = delete;
+};
+
+} // namespace mir
+
+#endif
+

=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h	2013-03-29 11:39:30 +0000
+++ include/shared/mir_toolkit/common.h	2013-04-18 07:33:25 +0000
@@ -34,6 +34,7 @@
 typedef enum MirSurfaceAttrib
 {
     mir_surface_attrib_type,
+    mir_surface_attrib_state,
     mir_surface_attrib_arraysize_
 } MirSurfaceAttrib;
 
@@ -48,7 +49,32 @@
     mir_surface_type_arraysize_
 } MirSurfaceType;
 
-/* TODO: Surface states here */
+/*
+ * MirSurfaceState: "minimized" or "minimised"?
+ * A review of 7 leading dictionaries shows "z" is the most common spelling
+ * (listed in all dictionaries), and "s" only appears in 4. And those that list
+ * both "z" and "s" usually point out that "z" is the main spelling. Only 2
+ * dictionaries say that "s" is preferred, and even still all dictionaries seem
+ * to agree that "s" is only used in the UK.
+ * The same is true for "maximized" vs "maximised".
+ */
+typedef enum MirSurfaceState
+{
+    mir_surface_state_unknown,
+    mir_surface_state_restored,
+    mir_surface_state_minimized,
+    mir_surface_state_minimised = mir_surface_state_minimized,
+    mir_surface_state_maximized,
+    mir_surface_state_maximised = mir_surface_state_maximized,
+    mir_surface_state_vertmaximized,
+    mir_surface_state_vertmaximised = mir_surface_state_vertmaximized,
+    /* mir_surface_state_semimaximized,
+       Omitted for now, since it's functionally a subset of vertmaximized and
+       differs only in the X coordinate. */
+    mir_surface_state_fullscreen,
+    mir_surface_state_arraysize_
+} MirSurfaceState;
+
 /**@}*/
 
 #endif

=== modified file 'include/shared/mir_toolkit/event.h'
--- include/shared/mir_toolkit/event.h	2013-04-11 22:47:09 +0000
+++ include/shared/mir_toolkit/event.h	2013-04-18 07:33:25 +0000
@@ -21,6 +21,7 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include "mir_toolkit/common.h"
 
 #ifdef __cplusplus
 /**
@@ -37,7 +38,8 @@
 typedef enum
 {
     mir_event_type_key,
-    mir_event_type_motion
+    mir_event_type_motion,
+    mir_event_type_surface
 } MirEventType;
 
 typedef struct
@@ -91,11 +93,21 @@
     } pointer_coordinates[MIR_INPUT_EVENT_MAX_POINTER_COUNT];
 } MirMotionEvent;
 
+typedef struct
+{
+    MirEventType type;
+
+    int id;
+    MirSurfaceAttrib attrib;
+    int value;
+} MirSurfaceEvent;
+
 typedef union
 {
-    MirEventType   type;
-    MirKeyEvent    key;
-    MirMotionEvent motion;
+    MirEventType    type;
+    MirKeyEvent     key;
+    MirMotionEvent  motion;
+    MirSurfaceEvent surface;
 } MirEvent;
 
 #ifdef __cplusplus

=== modified file 'include/test/mir_test/stub_server_tool.h'
--- include/test/mir_test/stub_server_tool.h	2013-04-16 09:08:29 +0000
+++ include/test/mir_test/stub_server_tool.h	2013-04-18 07:33:25 +0000
@@ -20,7 +20,7 @@
 #ifndef MIR_TEST_STUB_SERVER_TOOL_H_
 #define MIR_TEST_STUB_SERVER_TOOL_H_
 
-#include "mir_protobuf.pb.h"
+#include "mir/frontend/server.h"
 #include <condition_variable>
 #include <mutex>
 
@@ -29,7 +29,7 @@
 namespace test
 {
 
-struct StubServerTool : mir::protobuf::DisplayServer
+struct StubServerTool : frontend::Server
 {
     StubServerTool()
         : drm_magic{0}

=== modified file 'include/test/mir_test/test_protobuf_server.h'
--- include/test/mir_test/test_protobuf_server.h	2013-04-16 09:08:29 +0000
+++ include/test/mir_test/test_protobuf_server.h	2013-04-18 07:33:25 +0000
@@ -31,7 +31,7 @@
 struct TestProtobufServer
 {
     TestProtobufServer(std::string socket_name,
-               const std::shared_ptr<protobuf::DisplayServer>& tool);
+               const std::shared_ptr<frontend::Server>& tool);
 
     std::shared_ptr<frontend::Communicator> make_communicator(
         const std::string& socket_name,

=== modified file 'include/test/mir_test_doubles/mock_session.h'
--- include/test/mir_test_doubles/mock_session.h	2013-04-16 09:08:29 +0000
+++ include/test/mir_test_doubles/mock_session.h	2013-04-18 07:33:25 +0000
@@ -43,6 +43,8 @@
     MOCK_METHOD0(show, void());
 
     MOCK_METHOD3(configure_surface, int(frontend::SurfaceId, MirSurfaceAttrib, int));
+
+    MOCK_METHOD1(set_event_sink, void(std::weak_ptr<mir::EventSink> const&));
 };
 
 }

=== modified file 'include/test/mir_test_doubles/stub_ipc_factory.h'
--- include/test/mir_test_doubles/stub_ipc_factory.h	2013-04-16 09:08:29 +0000
+++ include/test/mir_test_doubles/stub_ipc_factory.h	2013-04-18 07:33:25 +0000
@@ -35,13 +35,13 @@
 class StubIpcFactory : public frontend::ProtobufIpcFactory
 {
 public:
-    StubIpcFactory(protobuf::DisplayServer& server) :
+    StubIpcFactory(frontend::Server& server) :
         server(fake_shared(server)),
         cache(std::make_shared<frontend::ResourceCache>())
     {
     }
 
-    std::shared_ptr<protobuf::DisplayServer> make_ipc_server()
+    std::shared_ptr<frontend::Server> make_ipc_server()
     {
         return server;
     }
@@ -57,7 +57,7 @@
         return std::make_shared<frontend::NullMessageProcessorReport>();
     }
 
-    std::shared_ptr<protobuf::DisplayServer> server;
+    std::shared_ptr<frontend::Server> server;
     std::shared_ptr<frontend::ResourceCache> const cache;
 };
 

=== modified file 'include/test/mir_test_doubles/stub_session.h'
--- include/test/mir_test_doubles/stub_session.h	2013-04-16 09:08:29 +0000
+++ include/test/mir_test_doubles/stub_session.h	2013-04-18 07:33:25 +0000
@@ -58,6 +58,9 @@
     {
         return 0;
     }
+    void set_event_sink(std::weak_ptr<EventSink> const&)
+    {
+    }
 };
 
 }

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-04-17 07:31:40 +0000
+++ src/client/mir_client_library.cpp	2013-04-18 07:33:25 +0000
@@ -25,7 +25,7 @@
 #include "native_client_platform_factory.h"
 #include "egl_native_display_container.h"
 #include "mir_logger.h"
-#include "make_rpc_channel.h"
+#include "mir_socket_rpc_channel.h"
 
 #include <set>
 #include <unordered_set>
@@ -57,10 +57,12 @@
         auto log = std::make_shared<mcl::ConsoleLogger>();
         auto client_platform_factory = std::make_shared<mcl::NativeClientPlatformFactory>();
 
-        MirConnection* connection = new MirConnection(
-            mcl::make_rpc_channel(socket_file, log),
-            log,
-            client_platform_factory);
+        auto rpc = std::make_shared<mcl::MirSocketRpcChannel>(socket_file, log);
+
+        MirConnection* connection = new MirConnection(rpc, log,
+                                                      client_platform_factory);
+        
+        rpc->set_event_handler(connection);
 
         return connection->connect(name, callback, context);
     }
@@ -253,10 +255,12 @@
     auto log = std::make_shared<mcl::ConsoleLogger>();
     auto client_platform_factory = std::make_shared<mcl::NativeClientPlatformFactory>();
 
-    MirConnection* connection = new MirConnection(
-        mcl::make_rpc_channel(server, log),
-        log,
-        client_platform_factory);
+    auto rpc = std::make_shared<mcl::MirSocketRpcChannel>(server, log);
+
+    MirConnection* connection = new MirConnection(rpc, log,
+                                                  client_platform_factory);
+
+    rpc->set_event_handler(connection);
 
     return connection->connect(lightdm_id, app_name, callback, client_context);
 }
@@ -298,3 +302,29 @@
 
     return type;
 }
+
+MirWaitHandle* mir_surface_set_state(MirSurface *surf, MirSurfaceState state)
+{
+    return surf ? surf->configure(mir_surface_attrib_state, state) : NULL;
+}
+
+MirSurfaceState mir_surface_get_state(MirSurface *surf)
+{
+    MirSurfaceState state = mir_surface_state_unknown;
+
+    if (surf)
+    {
+        int s = surf->attrib(mir_surface_attrib_state);
+
+        if (s == mir_surface_state_unknown)
+        {
+            surf->configure(mir_surface_attrib_state,
+                            mir_surface_state_unknown)->wait_for_result();
+            s = surf->attrib(mir_surface_attrib_state);
+        }
+
+        state = static_cast<MirSurfaceState>(s);
+    }
+
+    return state;
+}

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-04-11 22:47:09 +0000
+++ src/client/mir_connection.cpp	2013-04-18 07:33:25 +0000
@@ -59,7 +59,7 @@
     connect_result.set_error("connect not called");
 }
 
-MirConnection::~MirConnection()
+MirConnection::~MirConnection() noexcept
 {
     std::lock_guard<std::mutex> lock(connection_guard);
     valid_connections.erase(this);
@@ -120,6 +120,8 @@
 
     SurfaceRelease surf_release{surface, new_wait_handle, callback, context};
 
+    valid_surfaces.erase(surface->id());
+
     mir::protobuf::SurfaceId message;
     message.set_value(surface->id());
     server.release_surface(0, &message, &void_response,
@@ -312,3 +314,44 @@
 {
     return *native_display;
 }
+
+void MirConnection::on_surface_created(int id, MirSurface* surface)
+{
+    valid_surfaces[id] = surface;
+}
+
+void MirConnection::handle_event(MirEvent const& e)
+{
+    switch (e.type)
+    {
+    case mir_event_type_surface:
+        {
+            int id = e.surface.id;
+            SurfaceMap::iterator it = valid_surfaces.find(id);
+            if (it != valid_surfaces.end())
+            {
+                MirSurface *surface = it->second;
+                surface->handle_event(e);
+            }
+            else
+            {
+                log->error() << __PRETTY_FUNCTION__
+                             << ": mir_event_type_surface "
+                             << "received for non-existent surface ID "
+                             << id
+                             << ".\n";
+            }
+        }
+        break;
+    default:
+        // Don't worry. This function only gets called for events from the
+        // RPC channel (not input). So you will never see this error unless
+        // you make a mistake.
+
+        log->error() << __PRETTY_FUNCTION__
+                     << ": Unsupported event type " 
+                     << e.type
+                     << " received from server.\n";
+        break;
+    }
+}

=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2013-04-11 22:47:09 +0000
+++ src/client/mir_connection.h	2013-04-18 07:33:25 +0000
@@ -21,6 +21,7 @@
 #include <string>
 #include <memory>
 #include <unordered_set>
+#include <unordered_map>
 
 #include <mutex>
 
@@ -33,6 +34,7 @@
 #include "client_context.h"
 
 #include "mir_wait_handle.h"
+#include "mir/event_sink.h"
 
 namespace mir
 {
@@ -49,7 +51,8 @@
 }
 }
 
-struct MirConnection : public mir::client::ClientContext
+struct MirConnection : public mir::client::ClientContext,
+                       public mir::EventSink
 {
 public:
     MirConnection();
@@ -57,7 +60,7 @@
     MirConnection(std::shared_ptr<google::protobuf::RpcChannel> const& channel,
                   std::shared_ptr<mir::client::Logger> const & log,
                   std::shared_ptr<mir::client::ClientPlatformFactory> const& client_platform_factory);
-    ~MirConnection();
+    ~MirConnection() noexcept;
 
     MirConnection(MirConnection const &) = delete;
     MirConnection& operator=(MirConnection const &) = delete;
@@ -104,6 +107,9 @@
 
     EGLNativeDisplayType egl_native_display();
 
+    void on_surface_created(int id, MirSurface* surface);
+    void handle_event(MirEvent const&);
+
 private:
     std::shared_ptr<google::protobuf::RpcChannel> channel;
     mir::protobuf::DisplayServer::Stub server;
@@ -132,6 +138,9 @@
     static std::mutex connection_guard;
     static std::unordered_set<MirConnection*> valid_connections;
 
+    typedef std::unordered_map<int, MirSurface*> SurfaceMap;
+    SurfaceMap valid_surfaces;
+
     struct SurfaceRelease;
 
     void done_disconnect();

=== modified file 'src/client/mir_socket_rpc_channel.cpp'
--- src/client/mir_socket_rpc_channel.cpp	2013-04-11 22:47:09 +0000
+++ src/client/mir_socket_rpc_channel.cpp	2013-04-18 07:33:25 +0000
@@ -27,6 +27,7 @@
 #include "ancillary.h"
 
 #include <boost/bind.hpp>
+#include <cstring>
 
 namespace mcl = mir::client;
 namespace mcld = mir::client::detail;
@@ -36,8 +37,15 @@
 {
 }
 
-mcl::MirSocketRpcChannel::MirSocketRpcChannel(std::string const& endpoint, std::shared_ptr<Logger> const& log) :
-    log(log), pending_calls(log), work(io_service), endpoint(endpoint), socket(io_service)
+mcl::MirSocketRpcChannel::MirSocketRpcChannel(
+    std::string const& endpoint,
+    std::shared_ptr<Logger> const& log) :
+    log(log),
+    pending_calls(log),
+    work(io_service),
+    endpoint(endpoint),
+    socket(io_service),
+    event_handler(nullptr)
 {
     socket.connect(endpoint);
 
@@ -245,7 +253,14 @@
 
         log->debug() << __PRETTY_FUNCTION__ << " result.id():" << result.id() << std::endl;
 
-        pending_calls.complete_response(result);
+        if (!result.has_id())  // It's an event sequence
+        {
+            process_event_sequence(result);
+        }
+        else
+        {
+            pending_calls.complete_response(result);
+        }
     }
     catch (std::exception const& x)
     {
@@ -254,6 +269,46 @@
     }
 }
 
+void mcl::MirSocketRpcChannel::process_event_sequence(
+    mir::protobuf::wire::Result const& result)
+{
+    if (!event_handler)
+        return;
+
+    mir::protobuf::EventSequence seq;
+    if (seq.ParseFromString(result.response()))
+    {
+        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())
+            {
+                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()
 {
     const size_t body_size = (header_bytes[0] << 8) + header_bytes[1];
@@ -275,3 +330,13 @@
     result.ParseFromIstream(&in);
     return result;
 }
+
+void mcl::MirSocketRpcChannel::set_event_handler(mir::EventSink *sink)
+{
+    /*
+     * Yes, these have to be regular pointers. Because ownership of the object
+     * (which is actually a MirConnection) is the responsibility of the calling
+     * client. So out of our control.
+     */
+    event_handler = sink;
+}

=== modified file 'src/client/mir_socket_rpc_channel.h'
--- src/client/mir_socket_rpc_channel.h	2013-04-11 22:47:09 +0000
+++ src/client/mir_socket_rpc_channel.h	2013-04-18 07:33:25 +0000
@@ -22,6 +22,7 @@
 
 #include "mir_basic_rpc_channel.h"
 #include "mir_logger.h"
+#include "mir/event_sink.h"
 
 #include <boost/asio.hpp>
 
@@ -51,6 +52,8 @@
     MirSocketRpcChannel(const std::string& endpoint, const std::shared_ptr<Logger>& log);
     ~MirSocketRpcChannel();
 
+    void set_event_handler(EventSink *sink);
+
 private:
     virtual void CallMethod(const google::protobuf::MethodDescriptor* method, google::protobuf::RpcController*,
         const google::protobuf::Message* parameters, google::protobuf::Message* response,
@@ -72,10 +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);
 
     size_t read_message_header();
 
     mir::protobuf::wire::Result read_message_body(const size_t body_size);
+
+    EventSink *event_handler;
 };
 
 }

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-04-11 22:47:09 +0000
+++ src/client/mir_surface.cpp	2013-04-18 07:33:25 +0000
@@ -59,6 +59,7 @@
     for (int i = 0; i < mir_surface_attrib_arraysize_; i++)
         attrib_cache[i] = -1;
     attrib_cache[mir_surface_attrib_type] = mir_surface_type_normal;
+    attrib_cache[mir_surface_attrib_state] = mir_surface_state_unknown;
 }
 
 MirSurface::~MirSurface()
@@ -176,6 +177,7 @@
     auto platform = connection->get_client_platform();
     accelerated_window = platform->create_egl_native_window(this);
 
+    connection->on_surface_created(id(), this);
     callback(this, context);
     
     create_wait_handle.result_received();
@@ -261,14 +263,16 @@
         configure_result.surfaceid().value() == surface.id().value() &&
         configure_result.has_attrib())
     {
-        switch (configure_result.attrib())
+        int a = configure_result.attrib();
+
+        switch (a)
         {
         case mir_surface_attrib_type:
+        case mir_surface_attrib_state:
             if (configure_result.has_ivalue())
-            {
-                int t = configure_result.ivalue();
-                attrib_cache[mir_surface_attrib_type] = t;
-            } // else error is probably set due to an unsupported attrib/value
+                attrib_cache[a] = configure_result.ivalue();
+            else
+                assert(configure_result.has_error());
             break;
         default:
             assert(false);
@@ -307,3 +311,16 @@
         }
     }
 }
+
+void MirSurface::handle_event(MirEvent const& e)
+{
+    if (e.type == mir_event_type_surface)
+    {
+        MirSurfaceAttrib a = e.surface.attrib;
+        if (a < mir_surface_attrib_arraysize_)
+            attrib_cache[a] = e.surface.value;
+    }
+
+    if (handle_event_callback)
+        handle_event_callback(&e);
+}

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2013-04-11 22:47:09 +0000
+++ src/client/mir_surface.h	2013-04-18 07:33:25 +0000
@@ -85,6 +85,7 @@
     int attrib(MirSurfaceAttrib a) const;
 
     void set_event_handler(MirEventDelegate const* delegate);
+    void handle_event(MirEvent const& e);
 
 private:
     void on_configured();

=== modified file 'src/server/CMakeLists.txt'
--- src/server/CMakeLists.txt	2013-04-10 16:23:54 +0000
+++ src/server/CMakeLists.txt	2013-04-18 07:33:25 +0000
@@ -15,6 +15,8 @@
 set(LIBDIR "${CMAKE_INSTALL_PREFIX}/lib")
 set(INCLUDEDIR "${CMAKE_INSTALL_PREFIX}/include/mirserver")
 
+add_library(mireventqueue STATIC event_queue.cpp)
+
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/mirserver.pc.in
   ${CMAKE_CURRENT_BINARY_DIR}/mirserver.pc
@@ -51,6 +53,7 @@
   mirsharedinput
   mirsurfaces
   mirtime
+  mireventqueue
 )
 
 list(APPEND MIRSERVER_LINKS

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-04-17 16:57:53 +0000
+++ src/server/default_server_configuration.cpp	2013-04-18 07:33:25 +0000
@@ -106,7 +106,7 @@
     std::shared_ptr<mg::ViewableArea> const graphics_display;
     std::shared_ptr<mc::GraphicBufferAllocator> const buffer_allocator;
 
-    virtual std::shared_ptr<mir::protobuf::DisplayServer> make_ipc_server()
+    virtual std::shared_ptr<mf::Server> make_ipc_server()
     {
         return std::make_shared<mf::SessionMediator>(
             shell,

=== added file 'src/server/event_queue.cpp'
--- src/server/event_queue.cpp	1970-01-01 00:00:00 +0000
+++ src/server/event_queue.cpp	2013-04-18 07:33:25 +0000
@@ -0,0 +1,37 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: 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)
+{
+    target = s;
+}
+
+void EventQueue::handle_event(MirEvent const& e)
+{
+    // In future, post might put e on a queue and wait for some background
+    // thread to push it through to sink. But that's not required right now.
+
+    std::shared_ptr<EventSink> p = target.lock();
+    if (p)
+        p->handle_event(e);
+}

=== modified file 'src/server/frontend/CMakeLists.txt'
--- src/server/frontend/CMakeLists.txt	2013-04-02 03:03:51 +0000
+++ src/server/frontend/CMakeLists.txt	2013-04-18 07:33:25 +0000
@@ -8,7 +8,7 @@
   protobuf_message_processor.cpp
   null_message_processor.cpp
   surface_creation_parameters.cpp
-
+  server.cpp
   resource_cache.cpp
   ${PROTO_HDRS}
 )

=== modified file 'src/server/frontend/protobuf_message_processor.cpp'
--- src/server/frontend/protobuf_message_processor.cpp	2013-04-16 09:08:29 +0000
+++ src/server/frontend/protobuf_message_processor.cpp	2013-04-18 07:33:25 +0000
@@ -16,6 +16,7 @@
  * Authored by: Alan Griffiths <[e-mail]>
  */
 
+#include "mir_toolkit/event.h"
 #include "protobuf_message_processor.h"
 #include "mir/frontend/message_processor_report.h"
 #include "mir/frontend/resource_cache.h"
@@ -143,6 +144,32 @@
     sender->send(buffer);
 }
 
+void mfd::ProtobufMessageProcessor::send_event(MirEvent const& e)
+{
+    mir::protobuf::EventSequence seq;
+    mir::protobuf::Event *ev = seq.add_event();
+    ev->set_raw(&e, sizeof(MirEvent));
+
+    std::string buffer;
+    seq.SerializeToString(&buffer);
+
+    mir::protobuf::wire::Result result;
+    result.set_response(buffer);
+
+    result.SerializeToString(&buffer);
+
+    sender->send(buffer);
+}
+
+void mfd::ProtobufMessageProcessor::handle_event(MirEvent const& e)
+{
+    // Limit the types of events we wish to send over protobuf, for now.
+    if (e.type == mir_event_type_surface)
+    {
+        send_event(e);
+    }
+}
+
 bool mfd::ProtobufMessageProcessor::dispatch(mir::protobuf::wire::Invocation const& invocation)
 {
     report->received_invocation(display_server.get(), invocation.id(), invocation.method_name());

=== modified file 'src/server/frontend/protobuf_message_processor.h'
--- src/server/frontend/protobuf_message_processor.h	2013-04-16 09:08:29 +0000
+++ src/server/frontend/protobuf_message_processor.h	2013-04-18 07:33:25 +0000
@@ -24,6 +24,7 @@
 
 #include "mir_protobuf.pb.h"
 #include "mir_protobuf_wire.pb.h"
+#include "mir/event_sink.h"
 
 #include <vector>
 #include <memory>
@@ -42,7 +43,8 @@
 namespace detail
 {
 
-struct ProtobufMessageProcessor : MessageProcessor
+struct ProtobufMessageProcessor : MessageProcessor,
+                                  public EventSink
 {
     ProtobufMessageProcessor(
         MessageSender* sender,
@@ -72,6 +74,10 @@
     // OTOH until we have a real requirement it is hard to see how best to generalise.
     void send_response(::google::protobuf::uint32 id, mir::protobuf::Surface* response);
 
+    void send_event(MirEvent const& e);
+
+    void handle_event(MirEvent const& e);
+
     template<class Response>
     std::vector<int32_t> extract_fds_from(Response* response);
 

=== modified file 'src/server/frontend/protobuf_socket_communicator.cpp'
--- src/server/frontend/protobuf_socket_communicator.cpp	2013-04-16 09:08:29 +0000
+++ src/server/frontend/protobuf_socket_communicator.cpp	2013-04-18 07:33:25 +0000
@@ -22,6 +22,7 @@
 
 #include "mir/frontend/protobuf_ipc_factory.h"
 #include "mir/protobuf/google_protobuf_guard.h"
+#include "mir/frontend/session_mediator.h"
 
 #include <boost/signals2.hpp>
 
@@ -52,13 +53,17 @@
         next_id(),
         connected_sessions);
 
-    auto session = std::make_shared<detail::ProtobufMessageProcessor>(
+    std::shared_ptr<mir::frontend::Server> server =
+        ipc_factory->make_ipc_server();
+
+    auto proc = std::make_shared<detail::ProtobufMessageProcessor>(
         socket_session.get(),
-        ipc_factory->make_ipc_server(),
+        server,
         ipc_factory->resource_cache(),
         ipc_factory->report());
-
-    socket_session->set_processor(session);
+    
+    server->set_event_sink(proc);
+    socket_session->set_processor(proc);
 
     acceptor.async_accept(
         socket_session->get_socket(),

=== added file 'src/server/frontend/server.cpp'
--- src/server/frontend/server.cpp	1970-01-01 00:00:00 +0000
+++ src/server/frontend/server.cpp	2013-04-18 07:33:25 +0000
@@ -0,0 +1,30 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#include "mir/frontend/server.h"
+
+using namespace mir::frontend;
+
+Server::~Server() noexcept
+{
+}
+
+void Server::set_event_sink(std::weak_ptr<mir::EventSink> const& sink)
+{
+    event_sink = sink;
+}

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2013-04-16 09:08:29 +0000
+++ src/server/frontend/session_mediator.cpp	2013-04-18 07:33:25 +0000
@@ -64,6 +64,7 @@
     report->session_connect_called(request->application_name());
 
     session = shell->open_session(request->application_name());
+    session->set_event_sink(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-16 09:08:29 +0000
+++ src/server/shell/application_session.cpp	2013-04-18 07:33:25 +0000
@@ -35,6 +35,7 @@
     std::shared_ptr<SurfaceFactory> const& surface_factory,
     std::string const& session_name) :
     surface_factory(surface_factory),
+    event_queue(std::make_shared<EventQueue>()),
     session_name(session_name),
     next_surface_id(0)
 {
@@ -50,6 +51,12 @@
     }
 }
 
+void msh::ApplicationSession::set_event_sink(
+    std::weak_ptr<mir::EventSink> const& sink)
+{
+    event_queue->set_target(sink);
+}
+
 mf::SurfaceId msh::ApplicationSession::next_id()
 {
     return mf::SurfaceId(next_surface_id.fetch_add(1));
@@ -60,6 +67,10 @@
     auto surf = surface_factory->create_surface(params);
     auto const id = next_id();
 
+    std::shared_ptr<EventSink> q(event_queue);
+    surf->set_id(id);
+    surf->set_event_target(q);
+
     std::unique_lock<std::mutex> lock(surfaces_mutex);
     surfaces[id] = surf;
     return id;

=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp	2013-04-16 09:08:29 +0000
+++ src/server/shell/surface.cpp	2013-04-18 07:33:25 +0000
@@ -19,10 +19,13 @@
 #include "mir/shell/surface.h"
 #include "mir/shell/surface_builder.h"
 #include "mir/input/input_channel.h"
+#include "mir_toolkit/event.h"
+#include "mir/event_sink.h"
 
 #include <boost/throw_exception.hpp>
 
 #include <stdexcept>
+#include <cstring>
 
 namespace msh = mir::shell;
 namespace mc = mir::compositor;
@@ -35,7 +38,9 @@
   : builder(builder),
     input_channel(input_channel),
     surface(builder->create_surface(params)),
-    type_value(mir_surface_type_normal)
+    id(0),
+    type_value(mir_surface_type_normal),
+    state_value(mir_surface_state_restored)
 {
 }
 
@@ -47,6 +52,16 @@
     }
 }
 
+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())
@@ -169,6 +184,12 @@
                                                    "type."));
         result = type();
         break;
+    case mir_surface_attrib_state:
+        if (value != mir_surface_state_unknown &&
+            !set_state(static_cast<MirSurfaceState>(value)))
+            BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface state."));
+        result = state();
+        break;
     default:
         BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
                                                "attribute."));
@@ -195,3 +216,44 @@
 
     return valid;
 }
+
+MirSurfaceState msh::Surface::state() const
+{
+    return state_value;
+}
+
+bool msh::Surface::set_state(MirSurfaceState s)
+{
+    bool valid = false;
+
+    if (s > mir_surface_state_unknown &&
+        s < mir_surface_state_arraysize_)
+    {
+        state_value = s;
+        valid = true;
+
+        notify_change(mir_surface_attrib_state, s);
+    }
+
+    return valid;
+}
+
+void msh::Surface::notify_change(MirSurfaceAttrib attrib, int value)
+{
+    if (event_sink)
+    {
+        MirEvent e;
+
+        // This memset is not really required. However it does avoid some
+        // harmless uninitialized memory reads that valgrind will complain
+        // about, due to gaps in MirEvent.
+        memset(&e, 0, sizeof e);
+
+        e.type = mir_event_type_surface;
+        e.surface.id = id.as_value();
+        e.surface.attrib = attrib;
+        e.surface.value = value;
+
+        event_sink->handle_event(e);
+    }
+}

=== modified file 'src/shared/protobuf/mir_protobuf.proto'
--- src/shared/protobuf/mir_protobuf.proto	2013-03-21 03:32:59 +0000
+++ src/shared/protobuf/mir_protobuf.proto	2013-04-18 07:33:25 +0000
@@ -122,6 +122,14 @@
   optional string error = 127;
 }
 
+message Event {
+  optional bytes raw = 1;  // MirEvent structure
+}
+
+message EventSequence {
+  repeated Event event = 1;
+}
+
 service DisplayServer {
   // Platform independent requests
   rpc connect(ConnectParameters) returns (Connection);

=== modified file 'src/shared/protobuf/mir_protobuf_wire.proto'
--- src/shared/protobuf/mir_protobuf_wire.proto	2012-09-14 12:03:52 +0000
+++ src/shared/protobuf/mir_protobuf_wire.proto	2013-04-18 07:33:25 +0000
@@ -7,6 +7,7 @@
 }
 
 message Result {
-  required uint32 id = 1;
+  // Invocation results have id and response. Events only have response.
+  optional uint32 id = 1;
   optional bytes response = 2;
 }

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2013-04-17 07:31:40 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2013-04-18 07:33:25 +0000
@@ -29,6 +29,7 @@
 
 #include <gtest/gtest.h>
 #include <chrono>
+#include <cstring>
 
 namespace mf = mir::frontend;
 namespace mc = mir::compositor;
@@ -279,6 +280,171 @@
     launch_client_process(client_config);
 }
 
+TEST_F(DefaultDisplayServerTestFixture, client_can_set_surface_state)
+{
+    struct ClientConfig : ClientConfigCommon
+    {
+        void exec()
+        {
+            connection = mir_connect_sync(mir_test_socket, __PRETTY_FUNCTION__);
+            ASSERT_TRUE(connection != NULL);
+            EXPECT_TRUE(mir_connection_is_valid(connection));
+            EXPECT_STREQ(mir_connection_get_error_message(connection), "");
+
+            MirSurfaceParameters const request_params =
+            {
+                __PRETTY_FUNCTION__,
+                640, 480,
+                mir_pixel_format_abgr_8888,
+                mir_buffer_usage_hardware
+            };
+
+            surface = mir_connection_create_surface_sync(connection,
+                                                         &request_params);
+            ASSERT_TRUE(surface != NULL);
+            EXPECT_TRUE(mir_surface_is_valid(surface));
+            EXPECT_STREQ(mir_surface_get_error_message(surface), "");
+
+            EXPECT_EQ(mir_surface_state_restored,
+                      mir_surface_get_state(surface));
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                               mir_surface_state_fullscreen));
+            EXPECT_EQ(mir_surface_state_fullscreen,
+                      mir_surface_get_state(surface));
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                           static_cast<MirSurfaceState>(999)));
+            EXPECT_EQ(mir_surface_state_fullscreen,
+                      mir_surface_get_state(surface));
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                               mir_surface_state_minimized));
+            EXPECT_EQ(mir_surface_state_minimised,
+                      mir_surface_get_state(surface));
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                           static_cast<MirSurfaceState>(888)));
+            EXPECT_EQ(mir_surface_state_minimised,
+                      mir_surface_get_state(surface));
+
+            // Stress-test synchronization logic with some flooding
+            for (int i = 0; i < 100; i++)
+            {
+                mir_surface_set_state(surface, mir_surface_state_restored);
+                mir_wait_for(mir_surface_set_state(surface,
+                                                mir_surface_state_fullscreen));
+                ASSERT_EQ(mir_surface_state_fullscreen,
+                          mir_surface_get_state(surface));
+            }
+
+            mir_surface_release_sync(surface);
+            mir_connection_release(connection);
+        }
+    } client_config;
+
+    launch_client_process(client_config);
+}
+
+TEST_F(DefaultDisplayServerTestFixture, client_receives_surface_state_events)
+{
+    struct ClientConfig : ClientConfigCommon
+    {
+        static void event_callback(MirSurface* surface, MirEvent const* event,
+                                   void* ctx)
+        {
+            ClientConfig* self = static_cast<ClientConfig*>(ctx);
+            self->last_event = *event;
+            self->last_event_surface = surface;
+        }
+
+        void exec()
+        {
+            connection = mir_connect_sync(mir_test_socket, __PRETTY_FUNCTION__);
+            ASSERT_TRUE(connection != NULL);
+            ASSERT_TRUE(mir_connection_is_valid(connection));
+
+            MirSurfaceParameters const request_params =
+            {
+                __PRETTY_FUNCTION__,
+                640, 480,
+                mir_pixel_format_abgr_8888,
+                mir_buffer_usage_hardware
+            };
+
+            memset(&last_event, 0, sizeof last_event);
+            last_event_surface = nullptr;
+
+            MirEventDelegate delegate{&event_callback, this};
+            MirSurface* other_surface =
+                mir_connection_create_surface_sync(connection, &request_params);
+            ASSERT_TRUE(other_surface != NULL);
+            ASSERT_TRUE(mir_surface_is_valid(other_surface));
+            mir_surface_set_event_handler(other_surface, nullptr);
+
+            surface =
+                mir_connection_create_surface_sync(connection, &request_params);
+            ASSERT_TRUE(surface != NULL);
+            ASSERT_TRUE(mir_surface_is_valid(surface));
+
+            mir_surface_set_event_handler(surface, &delegate);
+
+            int surface_id = mir_surface_get_id(surface);
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                               mir_surface_state_fullscreen));
+            mir_wait_for(mir_surface_set_state(other_surface,
+                                               mir_surface_state_minimized));
+            EXPECT_EQ(surface, last_event_surface);
+            EXPECT_EQ(mir_event_type_surface, last_event.type);
+            EXPECT_EQ(surface_id, last_event.surface.id);
+            EXPECT_EQ(mir_surface_attrib_state, last_event.surface.attrib);
+            EXPECT_EQ(mir_surface_state_fullscreen, last_event.surface.value);
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                           static_cast<MirSurfaceState>(999)));
+            EXPECT_EQ(surface, last_event_surface);
+            EXPECT_EQ(mir_event_type_surface, last_event.type);
+            EXPECT_EQ(surface_id, last_event.surface.id);
+            EXPECT_EQ(mir_surface_attrib_state, last_event.surface.attrib);
+            EXPECT_EQ(mir_surface_state_fullscreen, last_event.surface.value);
+
+            memset(&last_event, 0, sizeof last_event);
+            last_event_surface = nullptr;
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                               mir_surface_state_minimized));
+            EXPECT_EQ(surface, last_event_surface);
+            EXPECT_EQ(mir_event_type_surface, last_event.type);
+            EXPECT_EQ(surface_id, last_event.surface.id);
+            EXPECT_EQ(mir_surface_attrib_state, last_event.surface.attrib);
+            EXPECT_EQ(mir_surface_state_minimized, last_event.surface.value);
+
+            memset(&last_event, 0, sizeof last_event);
+            last_event_surface = nullptr;
+
+            mir_wait_for(mir_surface_set_state(surface,
+                                           static_cast<MirSurfaceState>(777)));
+            mir_wait_for(mir_surface_set_state(other_surface,
+                                               mir_surface_state_maximized));
+            EXPECT_EQ(0, last_event_surface);
+            EXPECT_EQ(0, last_event.type);
+            EXPECT_EQ(0, last_event.surface.id);
+            EXPECT_EQ(0, last_event.surface.attrib);
+            EXPECT_EQ(0, last_event.surface.value);
+
+            mir_surface_release_sync(surface);
+            mir_surface_release_sync(other_surface);
+            mir_connection_release(connection);
+        }
+
+        MirEvent last_event;
+        MirSurface* last_event_surface;
+    } client_config;
+
+    launch_client_process(client_config);
+}
+
 TEST_F(DefaultDisplayServerTestFixture, client_library_creates_multiple_surfaces)
 {
     int const n_surfaces = 13;

=== modified file 'tests/integration-tests/test_error_reporting.cpp'
--- tests/integration-tests/test_error_reporting.cpp	2013-04-17 07:31:40 +0000
+++ tests/integration-tests/test_error_reporting.cpp	2013-04-18 07:33:25 +0000
@@ -23,6 +23,7 @@
 #include "mir/frontend/protobuf_ipc_factory.h"
 #include "mir/frontend/resource_cache.h"
 #include "mir/frontend/communicator.h"
+#include "mir/frontend/server.h"
 
 #include "mir_protobuf.pb.h"
 
@@ -47,7 +48,7 @@
 {
 char const* const mir_test_socket = mtf::test_socket_file().c_str();
 
-struct ErrorServer : mir::protobuf::DisplayServer
+struct ErrorServer : mf::Server
 {
     static std::string const test_exception_text;
 

=== modified file 'tests/mir_test_doubles/test_protobuf_socket_server.cpp'
--- tests/mir_test_doubles/test_protobuf_socket_server.cpp	2013-03-13 08:09:52 +0000
+++ tests/mir_test_doubles/test_protobuf_socket_server.cpp	2013-04-18 07:33:25 +0000
@@ -26,7 +26,7 @@
 
 mt::TestProtobufServer::TestProtobufServer(
     std::string socket_name,
-    const std::shared_ptr<protobuf::DisplayServer>& tool) :
+    const std::shared_ptr<frontend::Server>& tool) :
     factory(std::make_shared<mtd::StubIpcFactory>(*tool)),
     comm(make_communicator(socket_name, factory))
 {

=== modified file 'tests/unit-tests/CMakeLists.txt'
--- tests/unit-tests/CMakeLists.txt	2013-04-08 16:20:19 +0000
+++ tests/unit-tests/CMakeLists.txt	2013-04-18 07:33:25 +0000
@@ -20,7 +20,7 @@
 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-09 08:50:40 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2013-04-18 07:33:25 +0000
@@ -63,7 +63,7 @@
 
         input_fd = open("/dev/null", O_APPEND);
     }
-    ~MockServerPackageGenerator()
+    ~MockServerPackageGenerator() noexcept
     {
         close(input_fd);
         for (int i = 0; i < server_package.fd_items; i++)
@@ -540,3 +540,21 @@
     EXPECT_EQ(mir_surface_type_normal,
               surface->attrib(mir_surface_attrib_type));
 }
+
+TEST_F(MirClientSurfaceTest, default_surface_state)
+{
+    auto surface = std::make_shared<MirSurface> (connection.get(),
+                                                 *client_comm_channel,
+                                                 logger,
+                                                 mock_buffer_factory,
+                                                 input_platform,
+                                                 params,
+                                                 &empty_callback,
+                                                 nullptr);
+    surface->get_create_wait_handle()->wait_for_result();
+
+    // Test the default cached state value. It is always unknown until we
+    // get a real answer from the server.
+    EXPECT_EQ(mir_surface_state_unknown,
+              surface->attrib(mir_surface_attrib_state));
+}

=== added directory 'tests/unit-tests/event'
=== added file 'tests/unit-tests/event/CMakeLists.txt'
--- tests/unit-tests/event/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/event/CMakeLists.txt	2013-04-18 07:33:25 +0000
@@ -0,0 +1,5 @@
+set (UNIT_TEST_SOURCES
+  ${UNIT_TEST_SOURCES}
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_event_queue.cpp
+  PARENT_SCOPE
+)

=== added file 'tests/unit-tests/event/test_event_queue.cpp'
--- tests/unit-tests/event/test_event_queue.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/event/test_event_queue.cpp	2013-04-18 07:33:25 +0000
@@ -0,0 +1,115 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#include <gtest/gtest.h>
+#include "mir/event_queue.h"
+#include "mir/event_sink.h"
+#include <cstring>
+
+TEST(EventQueue, no_sink)
+{
+    EXPECT_NO_FATAL_FAILURE({
+        mir::EventQueue q;
+        MirEvent e;
+
+        e.type = mir_event_type_key;
+        q.handle_event(e);
+
+        e.type = mir_event_type_motion;
+        q.handle_event(e);
+
+        e.type = mir_event_type_surface;
+        q.handle_event(e);
+    });
+}
+
+namespace
+{
+    class TestSink : public mir::EventSink
+    {
+    public:
+        TestSink()
+        {
+            memset(&last_event, 0, sizeof last_event);
+        }
+
+        void handle_event(MirEvent const& e)
+        {
+            last_event = e;
+        }
+
+        MirEvent const& last_event_handled() const
+        {
+            return last_event;
+        }
+
+    private:
+        MirEvent last_event;
+    };
+}
+
+TEST(EventQueue, events_get_handled)
+{
+    mir::EventQueue q;
+    std::shared_ptr<TestSink> s(new TestSink);
+
+    q.set_target(s);
+
+    MirEvent e;
+    memset(&e, 0, sizeof e);
+
+    e.type = mir_event_type_key;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_key, s->last_event_handled().type);
+
+    e.type = mir_event_type_motion;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_motion, s->last_event_handled().type);
+
+    e.type = mir_event_type_surface;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_surface, s->last_event_handled().type);
+}
+
+TEST(EventQueue, sink_is_changeable)
+{
+    mir::EventQueue q;
+    std::shared_ptr<TestSink> a(new TestSink);
+    std::shared_ptr<TestSink> b(new TestSink);
+
+    q.set_target(a);
+
+    MirEvent e;
+    memset(&e, 0, sizeof e);
+
+    e.type = mir_event_type_motion;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_motion, a->last_event_handled().type);
+
+    q.set_target(b);
+
+    e.type = mir_event_type_surface;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_surface, b->last_event_handled().type);
+    EXPECT_EQ(mir_event_type_motion, a->last_event_handled().type);
+
+    e.type = mir_event_type_key;
+    q.handle_event(e);
+    EXPECT_EQ(mir_event_type_key, b->last_event_handled().type);
+    EXPECT_EQ(mir_event_type_motion, a->last_event_handled().type);
+}

=== modified file 'tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp'
--- tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp	2013-04-18 07:33:25 +0000
@@ -58,6 +58,7 @@
     MOCK_METHOD0(show, void());
 
     MOCK_METHOD3(configure_surface, int(mf::SurfaceId, MirSurfaceAttrib, int));
+    MOCK_METHOD1(set_event_sink, void(std::weak_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-01 16:11:03 +0000
+++ tests/unit-tests/shell/test_surface.cpp	2013-04-18 07:33:25 +0000
@@ -415,3 +415,38 @@
                              mir_surface_type_freestyle));
     EXPECT_EQ(mir_surface_type_freestyle, surf.type());
 }
+
+TEST_F(ShellSurface, states)
+{
+    using namespace testing;
+
+    msh::Surface surf(
+            mt::fake_shared(surface_builder),
+            mf::a_surface(),
+            null_input_channel);
+
+    EXPECT_EQ(mir_surface_state_restored, surf.state());
+
+    EXPECT_EQ(mir_surface_state_vertmaximized,
+              surf.configure(mir_surface_attrib_state,
+                             mir_surface_state_vertmaximized));
+    EXPECT_EQ(mir_surface_state_vertmaximized, surf.state());
+
+    EXPECT_THROW({
+        surf.configure(mir_surface_attrib_state, 999);
+    }, std::logic_error);
+    EXPECT_THROW({
+        surf.configure(mir_surface_attrib_state, -1);
+    }, std::logic_error);
+    EXPECT_EQ(mir_surface_state_vertmaximized, surf.state());
+
+    EXPECT_EQ(mir_surface_state_minimized,
+              surf.configure(mir_surface_attrib_state,
+                             mir_surface_state_minimized));
+    EXPECT_EQ(mir_surface_state_minimized, surf.state());
+
+    EXPECT_EQ(mir_surface_state_fullscreen,
+              surf.configure(mir_surface_attrib_state,
+                             mir_surface_state_fullscreen));
+    EXPECT_EQ(mir_surface_state_fullscreen, surf.state());
+}