Merge lp:~robertcarr/mir/client-focus-notifications into lp:~mir-team/mir/trunk

[Robert Carr]

Send focus notifications to clients.

Another iteration. This time I've removed the dependency on connect-display-request-merge-trunk. I've adapted the handle_surface_created idea from that branch (thanks kevin :D) to enable things to work without the cloggable sink.

Currently the shell mixes responsibilities of a factory interface, and reactive behavior (largely through acting as part of the factory/builder). I hope we can run with this and split out the factory part, in to classes which do that, and be left with a sensible interface like:

mf::Shell
{
handle_session_created
handle_sesssion_destroyed
handle_surface_created
handle_surface_destroyed
etc etc
}

The session mediator (and presumably a to be coming InternalClientMediator) then acts to notify the shell after building the objects.

=== Last description ==
This branch is a new version of client-focus-notifications based on ~mir-team/mir/connect-display-request-merge-trunk which enables removal of the cloggable event sink (notice how focus is now set in handle_surface_created, executed after the surface frontend processing has completed).

I've elected to still use surface attributes for focus, for two reasons:
1. I can't think of anything else to use
2. mir_surface_get_state(mir_surface_attrib_focus...seems to make sense as client API.

I think perhaps though, the default_focus_mechanism should handle the response of surface->configure with surface_attrib_focus. This way the shell can use the surface configurator to interfere and say, actually this surface may not take focus. I'm happy to include it in this MP if it's a popular idea or propose a followup for discussion after.

[Robert Carr (robertcarr)]

There are a few open questions:

Should focus be modeled as a Surface attribute?
Is there any reason for the Surface to keep track of if it has focus.

[kevin gunn (kgunn72)]

> There are a few open questions:
>
> Should focus be modeled as a Surface attribute?
> Is there any reason for the Surface to keep track of if it has focus.

makes sense to me it would be a surface attribute. as to why it might want to keep track....optimizations if there is no focus? and possibly, other client requests that won't work/don't make sense if focused or not.

just curious...is this some convention ?
mir_surface_focus_arraysize_
didn't understand why it was in the surf attrib for focusstate

in general...might be my lack of understanding. there are some mutex/surface locks in the proposed code....but i don't see unlocks (do those happen automagically as the code goes out of scope?)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:725
http://jenkins.qa.ubuntu.com/job/mir-ci/647/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/789
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/671
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/132
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/132/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> just curious...is this some convention ?
> mir_surface_focus_arraysize_
> didn't understand why it was in the surf attrib for focusstate

Yes, it is a sentinel element for the array, and its value is equal to the array size.

> in general...might be my lack of understanding. there are some mutex/surface
> locks in the proposed code....but i don't see unlocks (do those happen
> automagically as the code goes out of scope?)

std::unique_lock<> and std::lock_guard<> are scoped locks. They lock the associated mutex when created, and unlock it when going out of scope.

[Alexandros Frantzis (afrantzis)]

202 + std::unique_lock<std::mutex> lg(lock);
207 + std::unique_lock<std::mutex> lg(lock);

Are these locks necessary? Can a SessionMediator service two requests (in this case, two create_surface requests) concurrently? If it can, then the locks, as they are, have the following problem:

(t1) clog
(t2) clog
(t1) unclog, oops, any events for the surface created in (t2) will be sent to the client, too

214 + void handle_event(MirEvent const& ev)
215 + {
216 + if (clogged)

Taking into account the previous point (assuming the mediator can't service two requests from the same connection concurrently) and since this access to clogged is not protected, I suggest that we make clogged atomic<bool> and get rid of the locks.

315 + notify_change(attrib, value);
316 + break;

Indented one level deeper than required.

374 +namespace
375 +{
376 +
377 + struct ClientConfigCommon : TestingClientConfiguration
378 + {
379 + ClientConfigCommon() :
380 + connection(0),
381 + surface(0)
382 + {
383 + }
...

No need for indentation in namespace block.

415 + surface = NULL;

nullptr

505 +TEST_F(BespokeDisplayServerTestFixture, two_surfaces_are_notified_of_gaining_and_losing_focus)
552 + // We need some synchronization to ensure client two does not connect before client one.

Doesn't waiting for ready_for_second_client semaphore take care of that (it triggers only after the first client has received a surface created event, which means it has already connected)?

However, the test is still fragile. If the second client hasn't managed to connect and create a surface in the 5 seconds the first test has to finish, e.g., because of super high system load, then all_events_received.wait_for_at_most_seconds(5) in the first client will expire and the test will fail.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

CloggableEventSink looks horribly broken:

There is no synchronization around "216 + if (clogged)" - which means it could be in different states in different threads.

"clogged" could change between "if (clogged)" being evaluated and the next statement.

There is nothing to prevent multiple calls to clog() - perhaps clogged should be an atomic counter?

I assume that clog() and unclog() are intended to be paired operations - but the calls I see don't deal with exceptions being thrown (please use RAII for paired operations).

What is the motivation for CloggableEventSink? Is this the right place for synchronization?

review: Needs Fixing

[Robert Carr (robertcarr)]

Hi all! Thanks for review...I think I have improved it some but still not found the final solution.

Starting with Alf's comments:

>> Are these locks necessary? Can a SessionMediator service two requests (in this case, two create_surface
>> requests) concurrently

As it stands the SessionMediator can not service two requests concurrently (the SocketSession only reads a new request after processing the last one). These locks are necessary however, as handle_event may be called from any thread (i.e. the shell configuring a surface from any thread), to prevent a data race on the event buffer.

I would propose we make explicit that the SessionMediator can not service two requests with locking.

>> SNIP

Indentation and nullptr and such fixed.

>> Doesn't waiting for ready_for_second_client semaphore take care of that (it triggers only after
>> the first client has received a surface created event, which means it has already connected)?

Yes sorry I moved the wait without moving the comment making this misleading. This is in fact the point of ready_for_second_client.

>> However, the test is still fragile. If the second client hasn't managed to connect and create a surface in
>> the 5 seconds the first test has to finish, e.g., because of super high system load, then
>> all_events_received.wait_for_at_most_seconds(5) in the first client will expire and the test will fail.

The test will always be fragile or have the possibility to lock indefinitely. I think 5 seconds is perhaps to short though (things have taken longer than this in valgrind on jenkins in the past) so I've bumped the timeouts to 60 seconds. We could consider a standard set of timeout values.

Alan:

>>
>> There is no synchronization around "216 + if (clogged)" - which means it could be in different states in >> different threads.
>>

This is just in error of course. Fixed.

>>
>> What is the motivation for CloggableEventSink? Is this the right place for synchronization?
>>

The motivation is that sending a surface configuration event (i.e. one generated before shell::SessionManager::create_surface returns) before replying to the create_surface request leaves client session in a situation which it can't know how to rectify. I think the SessionMediator acts to arbitrate communication between the Session and the IPC layer, and so this is in fact the right place for synchronization.

>>
>> There is nothing to prevent multiple calls to clog() - perhaps clogged should be an atomic counter?
>> I assume that clog() and unclog() are intended to be paired operations - but the calls I see don't deal
>> with exceptions being thrown (please use RAII for paired operations).
>>

I'll update the API to be safer at the beginning of next week after some feedback from the team on the high level approach.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:731
http://jenkins.qa.ubuntu.com/job/mir-ci/660/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/810
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/692
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/145/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Events should be removed from buffered_events once handled. (Consider also: what should happen if handle_event() throws.)

review: Needs Fixing

[Robert Carr (robertcarr)]

Fixed the failure to clear buffered_events (whoops!)

There is something strange going on with the acceptance tests under valgrind (intermittent failure to complete, client never receives the event). I'll stew on it (and the whole branch) overnight and revisit tomorrow.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:734
http://jenkins.qa.ubuntu.com/job/mir-ci/668/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/820
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/702
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/153/console

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

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

The valgrind-induced race-triggered test-failure was due to the test client code, see the comment and change at l555 (vs the previous location of mir_surface_set_event_handler).

I think EventDelegate should be moved to the surface_create arguments. As it stands now mir_surface_create_sync should be considered dangerous.

[Robert Carr (robertcarr)]

In the process of debugging message_processor_report grew a sent_event method, as did the logging implementation (not sure how to print MirEvent from lttng...). Note the diff to add a single print line amounts to 141 lines...a bit unsatisfying imo.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:738
http://jenkins.qa.ubuntu.com/job/mir-ci/674/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/829
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/711
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/159
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/159/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Nice to see the attribute logic being reused, but I don't think focus is an attribute of a Surface in the traditional sense. But it's kind of OK too because you're only doing notifications here.

I would like to say focus should never be an attribute because that increases the risk and confusion of potentially having multiple focused surfaces. And internally at least the attribute system is one for getting/setting simple values. But being able to set focus per surface would be dangerous.

This risk is mitigated by the absence of a mir_surface_set_focus. It's only a bit wrong internally then [1]. And I agree you need focus to be an attribute to get notifications for it. So I'm glad you're using the attribute notifications as I intended.

I'm not sure though.

If anything I'll just say that "focused" should be the attribute (boolean 0 or 1) instead of focus. And even if you disagree with renaming then please reorder the enum MirSurfaceFocusState so it's usable as a boolean.

[1] The "bit wrong internally" comment is really about this:
437 + case mir_surface_attrib_focus:
438 + notify_change(attrib, value);
439 + break;
You've implemented a stub setter for focus in msh::Surface::configure which doesn't set anything. In fact instead of notify_change and returning the default value 0 (which is wrong BTW) it should be throwing an exception on any attempt to set ("configure") focus. There must be a nicer way to do it, without modifying msh::Surface::configure (hence letting it throw on any attempt to configure focus).

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

198 + virtual ~IPCSemaphore();

Prefer a non-virtual, noexcept destructor

~~~~

549 + client->handler->handle_event(ev);

If "ev" were given the more meaningful name the sheer repetitiveness would be even more apparent:

    client->handler->handle_event(event);

You can see it here too:

948 + event_sink->handle_event(any_event);

s/handle_event/handle/ would be an improvement.

~~~~

98 + std::weak_ptr<Surface> focus_surface;

What is the purpose of this? The name isn't clear, and the usage is entirely at member function scope (so why use a member variable).

review: Needs Fixing

[Robert Carr (robertcarr)]

Thanks both!

Alan: Comments addressed. Name improved to be clear at l98

Daniel: I've flipped the enum values...good point. As for the use of configure...I'm not sure of a better way to do this until a bigger focus refactoring happens. This is "next on my list"

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:742
http://jenkins.qa.ubuntu.com/job/mir-ci/685/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/842
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/724
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/170
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/170/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Again with:
437 + case mir_surface_attrib_focus:
438 + notify_change(attrib, value);
439 + break;

It would be easy to write reasonable test cases that now fail. Similar to:
tests/unit-tests/shell/test_surface.cpp: TEST_F(ShellSurface, states)

Because calling:
    surf.configure(mir_surface_attrib_focus, X) != X
and fails to throw an exception too.

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

> As for the use of configure...I'm not sure of a better way to do this until a bigger focus refactoring happens. This is "next on my list".

If you have in mind a better way to achieve this, perhaps we should treat this MP as a spike, drop it, and move on to a refactoring that will allow implementing a better solution?

review: Abstain

[Alan Griffiths (alan-griffiths)]

I assume that clog() and unclog() are intended to be paired operations - but the calls I see don't deal with exceptions being thrown (please use RAII for paired operations).

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

I really don't like RAII because it's too implicit and hidden from the reader. That said, you kind of need it for exception safety. But I would recommend against ever using exceptions too :)

[Alan Griffiths (alan-griffiths)]

> I really don't like RAII because it's too implicit and hidden from the reader.
> That said, you kind of need it for exception safety. But I would recommend
> against ever using exceptions too :)

You'd recommend against using C++ then?

RAAI and exceptions have been idiomatic C++ for two decades now.

[Daniel van Vugt (vanvugt)]

Well, for many people not using C++ or RAII has been idomatic for almost four decades ;)
</troll>

[Olli Ries (ories)]

you guys... ;)

On Mon, Jun 10, 2013 at 2:28 AM, Daniel van Vugt <
<email address hidden>> wrote:

> Well, for many people not using C++ or RAII has been idomatic for almost
> four decades ;)
> </troll>
> --
>
> https://code.launchpad.net/~robertcarr/mir/client-focus-notifications/+merge/166440
> Your team Mir development team is subscribed to branch lp:mir.
>

[Robert Carr (robertcarr)]

This is non-critical so I am putting it on hold for a few days so I can focus on the Unity stack.

[Robert Carr (robertcarr)]

It returns! With RAII on the event sink. I encountered some intermittent failures (both related, and somewhat unrelated but exposed):

MirConnection::released -> We don't want to remove the surface from valid surfaces until here because we may still be receiving configuration events until the surface is actually released on the server side.

input_testing_server_options: It's not enough to say we are ready for input when the input_registrar returns, because the shell may have not yet granted keyboard focus (fixes 1196744)

MirSurface::release_surface: There was a deadlock going on here:

22:42 < racarr> Client calls mir_surface_release it gets to MirSurface::release_surface which acquires the recursive lock on the MirSurface
22:42 < racarr> then calls MirConnection::release_surface
22:42 < racarr> which trys to acquire the connection lock, but blocks because an RPC thread is
22:42 < racarr> has called MirConnection::handle_event
22:42 < racarr> holding the lock
22:43 < racarr> but the RPC thread while executing MirConnection::handle_event tries to call
22:43 < racarr> MirSurface::handle_event
22:43 < racarr> but the Surface is already locked from the
22:43 < racarr> original client thread

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:746
http://jenkins.qa.ubuntu.com/job/mir-ci/957/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1237
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1121/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/195
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/195/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/597

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

352 + class Impediment
353 + {
354 + public:
355 + ~Impediment()
356 + {
357 + remove_callback();
358 + }
359 + protected:
360 + friend class CloggableEventSink;
361 + Impediment(std::function<void()> const& remove) :
362 + remove_callback(remove)
363 + {
364 + }
365 + private:
366 + std::function<void()> const remove_callback;
367 + };
368 +
369 + std::unique_ptr<Impediment> clog()
370 + {
371 + std::unique_lock<std::mutex> lg(lock);
372 + clogged = true;
373 +
374 + return std::unique_ptr<Impediment>(new Impediment(
375 + std::bind(std::mem_fn(&CloggableEventSink::unclog), this)));
376 + }

This approach seems complicated to me. C.f.

template<class Cloggee>
class Clogger
{
public:
    Clogger(Cloggee& cloggee) : cloggee{cloggee} { cloggee.clog(); }
    ~Clogger() { cloggee.unclog(); }
private:
    Cloggee& cloggee;
    Clogger(Clogger const&) = delete;
    Clogger& operator=(Clogger const&) = delete;
};

[Alan Griffiths (alan-griffiths)]

438 === modified file 'src/server/frontend/socket_session.cpp'
445 + // TODO: It might make sense to use a pool for these.
446 + std::vector<char> whole_message;
...
451 === modified file 'src/server/frontend/socket_session.h'
458 - std::vector<char> whole_message;

This shouldn't be needed.

[Alan Griffiths (alan-griffiths)]

1321 msh::SingleVisibilityFocusMechanism focus_mechanism(mt::fake_shared(model),
1322 - std::make_shared<mtd::StubInputTargeter>());
1323 + std::make_shared<mtd::StubInputTargeter>());

Changes layout from one of our common styles to one we don't use.

[Alan Griffiths (alan-griffiths)]

I'm tempted to suggest -Wno-mismatched-tags to quiet clang's curious insistence that struct X is not a synonym for class X.

I've commented on a couple of oddities above - my biggest concern being that if we really need to move "whole_message" that there is an existing race condition (and that should be fixed instead of hacking around it).

Overall, the MP looks reasonable, but is a bit big for me to feel confident I understand it all.

review: Needs Information

[Robert Carr (robertcarr)]

Fixed clang compile.

I don't think the whole_message bit is required, that was introduced by me in an early iteration to attempt to fix a race that ended up being in the SessionMediator as I recall. It's been removed.

Fixed whitespace around 1321.

I'm not sure I like the Clogger class, the way it is now makes it clear how to call clog.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:749
http://jenkins.qa.ubuntu.com/job/mir-ci/966/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1249
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1133
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/204
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/204/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/605

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I still think that clog()/unclog() are dubious.

What should happen if clog() is called in two threads and then unclog() is called in the first?

Why are you creating a unique_ptr at all? Surely, as you don't want to follow the model used in std (e.g. unique_lock), all that you need to return is a move-only type that calls unclog() in the destructor?

~~~~

234 +protected

line noise

review: Needs Fixing

[Robert Carr (robertcarr)]

Improved the clogging API and added exceptions for some exceptional conditions.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:751
http://jenkins.qa.ubuntu.com/job/mir-ci/1003/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1303
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/241
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/241/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/639

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

352 + ~CloggableEventSink()
353 + {
354 + std::unique_lock<std::mutex> lg(lock);
355 +
356 + if (clogged == true)
357 + BOOST_THROW_EXCEPTION(std::logic_error("EventSink destroyed while clogged leading to lost events"));
358 + }

See "C++ Coding Standards" Guideline 51 (this isn't the guideline I consider wrong).

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

606 +};
607 +struct MockEventObserver
608 +{
609 + MOCK_METHOD1(see, void(MirEvent const*));
610 +};
611 +struct EventObservingClient : ClientConfigCommon

A bit of whitespace between classes aids readability

[Robert Carr (robertcarr)]

Added some whitespace.

Removed the dtor. I thought it might be ok to throw in this case as it likely indicates a programmer error which should never happen.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:753
http://jenkins.qa.ubuntu.com/job/mir-ci/1011/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1312/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1197/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/249/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

On 15/07/13 17:34, Robert Carr wrote:
> Removed the dtor. I thought it might be ok to throw in this case as it likely indicates a programmer error which should never happen.

Better to abort?

--
Alan Griffiths [phone]
Octopull Ltd http://www.octopull.co.uk/

[Alexandros Frantzis (afrantzis)]

219 +#include <initializer_list>
220 +#include <string>

Are these needed?

> 234 +protected
>
> line noise

+1. Just a nit, but there is no reason to add a protected block just for the deleted CopyAssign operations; we can put them under private.

> See "C++ Coding Standards" Guideline 51 (this isn't the guideline I consider wrong).

This also applies to

401 + ~Clog()
402 + {
403 + sink.unclog();
404 + }

since sink.unclog() can throw.

[Alexandros Frantzis (afrantzis)]

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

> On 15/07/13 17:34, Robert Carr wrote:
> > Removed the dtor. I thought it might be ok to throw in this case as it
> likely indicates a programmer error which should never happen.

It is low cost to detect and leads to undefined behavior: just put an assert here.

review: Needs Fixing

[Robert Carr (robertcarr)]

assert(asserts_present_in_revision_755)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:755
http://jenkins.qa.ubuntu.com/job/mir-ci/1025/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1330
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1215/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/262
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/262/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/654

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:755
http://jenkins.qa.ubuntu.com/job/mir-ci/1035/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1343/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1228/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/273/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/663

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

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

...Merged trunk!

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:757
http://jenkins.qa.ubuntu.com/job/mir-ci/1079/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1408
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1293/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/317
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/317/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/697

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

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

Last jenkins failure was just intermittent jenkins infrastructure issue. I would like this to land one day because it conflicts a lot.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:758
http://jenkins.qa.ubuntu.com/job/mir-ci/1098/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1436
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1321
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/336
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/336/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Looks like my review is way out of date now. So no comment.

review: Abstain

[Kevin DuBois (kdub)]

I think lp:~kdub/mir/prepare-for-display-notifications will help with this... EventSink now is a bit more decoupled and the interface can be developed against without messing with ProtobufSocketCommunicator at all.

[Robert Carr (robertcarr)]

Is there a specific portion of the change that people don't want to land?

[Alan Griffiths (alan-griffiths)]

My previous objection is addressed, and I don't have the headspace right now.

recursive mutexes are usually a sign of design issues, but this MP doesn't introduced a new problem.

review: Abstain

[Alexandros Frantzis (afrantzis)]

Looks good overall. One question:

486 + if (current_focus)
487 + current_focus->configure(mir_surface_attrib_focus, mir_surface_unfocused);
488 + surface->configure(mir_surface_attrib_focus, mir_surface_focused);.

Is there a way for the currently focused surface to get the focus again, and receive a confusing [unfocused,focused] sequence of events? We don't seem to guard against this, at least not at this level.

review: Approve

[Alan Griffiths (alan-griffiths)]

NB this needs to be re-sync'ed with trunk before top-approving.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:759
http://jenkins.qa.ubuntu.com/job/mir-ci/1152/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1514/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1399/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/390/console

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

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

Merged trunk

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:760
http://jenkins.qa.ubuntu.com/job/mir-ci/1153/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1516
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1401
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/391
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/391/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> > just curious...is this some convention ?
> > mir_surface_focus_arraysize_
> > didn't understand why it was in the surf attrib for focusstate
>
> Yes, it is a sentinel element for the array, and its value is equal to the
> array size.

Just curious - was there an array when this was written? (There isn't now, and it seems unlikely there ever was.)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/526/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/generic-cleanup-mbs/1194/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1554/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1439/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/180/console

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

The merge conflicts were not entirely trivial so maybe someone wants to give it a once over?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:761
http://jenkins.qa.ubuntu.com/job/mir-ci/1181/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1555
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1440
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/419
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/419/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/530/
Executed test runs:
    None: http://s-jenkins:8080/job/generic-cleanup-mbs/1201/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1563/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1448/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/184/console

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

1. Conflicts...
Text conflict in include/server/mir/shell/default_focus_mechanism.h
Text conflict in src/server/shell/default_focus_mechanism.cpp
Text conflict in tests/unit-tests/shell/test_default_focus_mechanism.cpp
3 conflicts encountered.

2. I think the past tense "sent_event" doesn't feel right, and is a confusing function name despite being similar to those above it. Maybe "on_event_sent".

3. As already mentioned, I still don't think MirSurfaceFocusState should exist at all. Making "focus" an attribute of a surface leads to dangerous and impossible situations:
  (a) Many surfaces focused simultaneously; and
  (b) surf.configure(mir_surface_attrib_focus, X) != X

4. IPCSemaphore: Everyone knows semaphores as having "wait" and "signal" operations. I'd prefer those to the verbose method names you have. But it's not important.

5. Focus/blur should be an event on the connection, not the surface. This would resolve the ambiguities mentioned in 3.

Sorry to keep touching on the same things (3, 5), but I did first raise these concerns 2 months ago. At the time you seem to have agreed saying:
  "I'm not sure of a better way to do this until a bigger focus refactoring happens. This is "next on my list"
I think we should avoid a temporary solution that adds attributes which ultimately we agree should not exist.

review: Needs Fixing

[Robert Carr (robertcarr)]

I don't understand what you mean by point 5. We need to deliver focus based on surface, i.e. you have multiple terminal windows open from the same session and they need to know whether or not to blink the cursor.

[Daniel van Vugt (vanvugt)]

(5) is really an extension of (3). Given that (3) I don't think focus should be a per-surface attribute then one would conclude (5) that the "currently focused surface" should be stored in a single location, single variable. This makes it an attribute of some other class, like maybe the session.

[Chris Halse Rogers (raof)]

High level - I concur with Daniel: having focus being a surface attribute seems dangerous. Current focus is... session local? Hm.

Focus is a per-keyboardish attribute, so it's not strictly speaking unique in a compositor session, but it's pretty close to.

===

Why are we calling the guard value of the enums *_arraysize_? That seems somewhat misleading.

===

Perhaps Clog should take a shared_ptr<CloggableEventSink> rather than a reference? It needs to keep the event sink live while the Clog is live. This is not a problem in the current code, though.

<bikeshed>I'd find "Plug" a more natural name for the Clog class; you stick a plug in your sink to clog it ☺ </bikeshed>

====

I was going to say that lttng::MPR::sent_event should be event_sent to match exception_handled above, but it turns out that above *that* is received_invocation/completed_invocation.

I think event_sent (and invocation_{received,completed}) makes more sense, but there's no consistency there at the moment.

===

812 + ~ProxyShell() noexcept(true) = default;

I *think* the default destructor is noexcept, so the noexcept(true) there is redundant?

===

949 + static int const semaphore_enable_process_shared = 1;

Why is this static? You don't need to track the value of this across entries to IPCSemaphore(), and I presume the compiler will entirely optimise it out.

===

1171 + ~MockEventSink() … {} could be ~MockEventSink() = default; ?

review: Needs Fixing

[Chris Halse Rogers (raof)]

On second thoughts, maybe it's ok?

We're not storing the focus state in a surface attribute, we're storing it in the FocusStrategy and projecting that to the client as a surface state.

It's possible that there's a nicer way of projecting that data, but maybe this isn't too bad.

review: Abstain

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:766
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/client-focus-notifications/+merge/179801/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/1286/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1716
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1601
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/524/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:767
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/client-focus-notifications/+merge/179801/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/1289/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1719
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1604
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/527/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:768
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/client-focus-notifications/+merge/179801/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/1290/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1720/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1605/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/528/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:767
http://jenkins.qa.ubuntu.com/job/mir-ci/1291/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1721/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1606/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/529/console

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

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

BTW, "enum_max_" is not accurate. It goes one past the max value, used to size fixed arrays. Hence "arraysize_".

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:771
http://jenkins.qa.ubuntu.com/job/mir-ci/1295/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1726/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1611/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/533/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:772
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~robertcarr/mir/client-focus-notifications/+merge/180011/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/1296/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1727/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1612/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/534/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:773
http://jenkins.qa.ubuntu.com/job/mir-ci/1298/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1730
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1615
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/536
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/536/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

486 -<<<<<<< TREE
487 MediatingDisplayChanger(
488 std::shared_ptr<graphics::Display> const& display,
489 std::shared_ptr<compositor::Compositor> const& compositor,
490 std::shared_ptr<input::InputManager> const& input_manager,
491 std::shared_ptr<graphics::DisplayConfigurationPolicy> const& display_configuration_policy);
492 -=======
493 - explicit MediatingDisplayChanger(
494 - std::shared_ptr<graphics::Display> const& display,
495 - std::shared_ptr<compositor::Compositor> const& compositor);
496 ->>>>>>> MERGE-SOURCE

The diff is a mess. It is not a good idea to have a prerequisite branch (connect-display-request-merge-trunk) that's been rejected.

review: Needs Resubmitting

[Daniel van Vugt (vanvugt)]

What Alan said.

review: Needs Resubmitting

=== modified file 'examples/demo-inprocess-surface-client/inprocess_egl_client.cpp'
--- examples/demo-inprocess-surface-client/inprocess_egl_client.cpp	2013-08-13 23:40:55 +0000
+++ examples/demo-inprocess-surface-client/inprocess_egl_client.cpp	2013-08-09 08:50:40 +0000
@@ -65,10 +65,9 @@
 me::InprocessEGLClient::~InprocessEGLClient()
 {
     terminate = true;
-    //TODO: this is messy, need a better interface for internal clients to interact with shell
-    if ( auto ses = std::dynamic_pointer_cast<msh::Session>(session))
-        ses->force_requests_to_complete();
-
+    auto session = session_manager->focussed_application().lock();
+    if (session)
+        session->force_requests_to_complete();
     client_thread.join();
 }
 
@@ -81,7 +80,7 @@
         .of_size(surface_size)
         .of_buffer_usage(mg::BufferUsage::hardware)
         .of_pixel_format(geom::PixelFormat::argb_8888);
-    session = session_manager->open_session("Inprocess client",
+    auto session = session_manager->open_session("Inprocess client",
                                                  std::shared_ptr<mf::EventSink>());
     // TODO: Why do we get an ID? ~racarr
     auto surface = session->get_surface(session_manager->create_surface_for(session, params));

=== modified file 'examples/demo-inprocess-surface-client/inprocess_egl_client.h'
--- examples/demo-inprocess-surface-client/inprocess_egl_client.h	2013-08-13 23:40:55 +0000
+++ examples/demo-inprocess-surface-client/inprocess_egl_client.h	2013-08-08 02:34:31 +0000
@@ -39,10 +39,6 @@
 {
 class Platform;
 }
-namespace frontend
-{
-class Session;
-}
 namespace shell
 {
 class SessionManager;
@@ -67,7 +63,6 @@
 private:
     std::shared_ptr<graphics::Platform> const graphics_platform;
     std::shared_ptr<shell::SessionManager> const session_manager;
-    std::shared_ptr<frontend::Session> session;
 
     std::thread client_thread;
     

=== modified file 'examples/demo-shell/demo_shell.cpp'
--- examples/demo-shell/demo_shell.cpp	2013-08-13 23:40:55 +0000
+++ examples/demo-shell/demo_shell.cpp	2013-08-07 07:18:03 +0000
@@ -96,7 +96,7 @@
         {
             // We use this strange two stage initialization to avoid a circular dependency between the EventFilters
             // and the SessionStore
-            wm->set_focus_controller(config.the_shell_focus_setter());
+            wm->set_focus_controller(config.the_focus_controller());
             wm->set_session_manager(config.the_session_manager());
         });
     return 0;

=== modified file 'examples/demo-shell/window_manager.cpp'
--- examples/demo-shell/window_manager.cpp	2013-08-13 23:40:55 +0000
+++ examples/demo-shell/window_manager.cpp	2013-07-10 17:06:37 +0000
@@ -18,7 +18,7 @@
 
 #include "window_manager.h"
 
-#include "mir/shell/focus_setter.h"
+#include "mir/shell/focus_controller.h"
 #include "mir/shell/session_manager.h"
 #include "mir/shell/session.h"
 #include "mir/shell/surface.h"
@@ -41,7 +41,7 @@
 {
 }
 
-void me::WindowManager::set_focus_controller(std::shared_ptr<msh::FocusSetter> const& controller)
+void me::WindowManager::set_focus_controller(std::shared_ptr<msh::FocusController> const& controller)
 {
     focus_controller = controller;
 }
@@ -92,7 +92,7 @@
         MirMotionAction action = static_cast<MirMotionAction>(event.motion.action & ~0xff00);
 
         std::shared_ptr<msh::Session> app =
-            focus_controller->focused_session().lock();
+            session_manager->focussed_application().lock();
 
         int fingers = static_cast<int>(event.motion.pointer_count);
 

=== modified file 'examples/demo-shell/window_manager.h'
--- examples/demo-shell/window_manager.h	2013-08-13 23:40:55 +0000
+++ examples/demo-shell/window_manager.h	2013-06-10 11:26:51 +0000
@@ -28,7 +28,7 @@
 {
 namespace shell
 {
-class FocusSetter;
+class FocusController;
 class SessionManager;
 }
 namespace examples
@@ -40,7 +40,7 @@
     WindowManager();
     ~WindowManager() = default;
 
-    void set_focus_controller(std::shared_ptr<shell::FocusSetter> const& focus_controller);
+    void set_focus_controller(std::shared_ptr<shell::FocusController> const& focus_controller);
     void set_session_manager(std::shared_ptr<shell::SessionManager> const& sm);
     
     bool handle(MirEvent const& event) override;
@@ -50,7 +50,7 @@
     WindowManager& operator=(const WindowManager&) = delete;
 
 private:
-    std::shared_ptr<shell::FocusSetter> focus_controller;
+    std::shared_ptr<shell::FocusController> focus_controller;
     std::shared_ptr<shell::SessionManager> session_manager;
     geometry::Displacement relative_click;  // Click location in window space
     geometry::Point click;                  // Click location in screen space

=== modified file 'include/platform/mir/graphics/display_configuration.h'
--- include/platform/mir/graphics/display_configuration.h	2013-08-12 08:58:59 +0000
+++ include/platform/mir/graphics/display_configuration.h	2013-08-13 23:40:56 +0000
@@ -42,29 +42,6 @@
 struct DisplayConfigurationCard
 {
     DisplayConfigurationCardId id;
-    size_t max_simultaneous_outputs;
-};
-
-/**
- * The type of a display output.
- */
-enum class DisplayConfigurationOutputType
-{
-    unknown,
-    vga,
-    dvii,
-    dvid,
-    dvia,
-    composite,
-    svideo,
-    lvds,
-    component,
-    ninepindin,
-    displayport,
-    hdmia,
-    hdmib,
-    tv,
-    edp
 };
 
 /**
@@ -85,14 +62,10 @@
     DisplayConfigurationOutputId id;
     /** The id of the card the output is connected to. */
     DisplayConfigurationCardId card_id;
-    /** The type of the output. */
-    DisplayConfigurationOutputType type;
     /** The pixel formats supported by the output */
     std::vector<geometry::PixelFormat> pixel_formats;
     /** The modes supported by the output. */
     std::vector<DisplayConfigurationMode> modes;
-    /** The index in the 'modes' vector of the preferred output mode. */
-    size_t preferred_mode_index;
     /** The physical size of the output. */
     geometry::Size physical_size_mm;
     /** Whether the output is connected. */
@@ -107,10 +80,6 @@
     size_t current_format_index;
 };
 
-std::ostream& operator<<(std::ostream& out, DisplayConfigurationCard const& val);
-bool operator==(DisplayConfigurationCard const& val1, DisplayConfigurationCard const& val2);
-bool operator!=(DisplayConfigurationCard const& val1, DisplayConfigurationCard const& val2);
-
 std::ostream& operator<<(std::ostream& out, DisplayConfigurationMode const& val);
 bool operator==(DisplayConfigurationMode const& val1, DisplayConfigurationMode const& val2);
 bool operator!=(DisplayConfigurationMode const& val1, DisplayConfigurationMode const& val2);

=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/default_server_configuration.h	2013-08-13 13:54:24 +0000
@@ -161,6 +161,8 @@
     virtual std::shared_ptr<frontend::DisplayChanger>         the_frontend_display_changer();
     /** @} */
 
+    virtual std::shared_ptr<shell::FocusController> the_focus_controller();
+
     /** @name shell configuration - customization
      * configurable interfaces for modifying shell
      *  @{ */

=== modified file 'include/server/mir/frontend/display_changer.h'
--- include/server/mir/frontend/display_changer.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/frontend/display_changer.h	2013-08-12 17:20:29 +0000
@@ -37,14 +37,7 @@
     virtual ~DisplayChanger() = default;
 
     virtual std::shared_ptr<graphics::DisplayConfiguration> active_configuration() = 0;
-<<<<<<< TREE
     virtual void configure(std::weak_ptr<Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&) = 0;
-=======
-    virtual void store_configuration_for(
-        std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&) = 0;
-    virtual void apply_configuration_of(std::weak_ptr<frontend::Session> const&) = 0;
-    virtual void remove_configuration_for(std::weak_ptr<frontend::Session> const&) = 0;
->>>>>>> MERGE-SOURCE
 
 protected:
     DisplayChanger() = default;

=== modified file 'include/server/mir/frontend/message_processor_report.h'
--- include/server/mir/frontend/message_processor_report.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/message_processor_report.h	2013-08-13 23:40:56 +0000
@@ -20,6 +20,8 @@
 #ifndef MIR_FRONTEND_MESSAGE_PROCESSOR_REPORT_H_
 #define MIR_FRONTEND_MESSAGE_PROCESSOR_REPORT_H_
 
+#include "mir_toolkit/event.h"
+
 #include <string>
 
 namespace mir
@@ -42,6 +44,8 @@
     virtual void exception_handled(void const* mediator, int id, std::exception const& error) = 0;
 
     virtual void exception_handled(void const* mediator, std::exception const& error) = 0;
+    
+    virtual void sent_event(void const* mediator, MirSurfaceEvent const& ev) = 0;
 
 private:
     MessageProcessorReport(MessageProcessorReport const&) = delete;

=== modified file 'include/server/mir/frontend/null_message_processor_report.h'
--- include/server/mir/frontend/null_message_processor_report.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/frontend/null_message_processor_report.h	2013-08-13 23:40:56 +0000
@@ -36,6 +36,8 @@
     void exception_handled(void const*, int, std::exception const&);
 
     void exception_handled(void const*, std::exception const&);
+    
+    void sent_event(void const*, MirSurfaceEvent const& e);
 };
 }
 }

=== modified file 'include/server/mir/frontend/session.h'
--- include/server/mir/frontend/session.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/frontend/session.h	2013-08-08 19:40:41 +0000
@@ -21,7 +21,6 @@
 
 #include "mir_toolkit/common.h"
 #include "mir/frontend/surface_id.h"
-#include "mir/shell/surface_creation_parameters.h"
 
 #include <mutex>
 #include <atomic>

=== modified file 'include/server/mir/frontend/shell.h'
--- include/server/mir/frontend/shell.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/frontend/shell.h	2013-08-13 23:40:56 +0000
@@ -42,13 +42,11 @@
         std::string const& name, std::shared_ptr<EventSink> const& sink) = 0;
     virtual void close_session(std::shared_ptr<Session> const& session)  = 0;
 
-    virtual void handle_display_configuration(std::shared_ptr<Session> const& session) = 0; 
+    virtual SurfaceId create_surface_for(std::shared_ptr<Session> const& session,
+                                         shell::SurfaceCreationParameters const& params) = 0;
+    
     virtual void handle_surface_created(std::shared_ptr<Session> const& session) = 0;
 
-    //TODO: remove, object needed to handle calling appropriate shell notifications for internal clients
-   virtual SurfaceId create_surface_for(std::shared_ptr<Session> const& session,
-                                        shell::SurfaceCreationParameters const& params) = 0;
-
 protected:
     Shell() = default;
     Shell(const Shell&) = delete;

=== modified file 'include/server/mir/logging/message_processor_report.h'
--- include/server/mir/logging/message_processor_report.h	2013-04-24 05:22:20 +0000
+++ include/server/mir/logging/message_processor_report.h	2013-08-13 23:40:56 +0000
@@ -68,6 +68,8 @@
 
     void exception_handled(void const* mediator, std::exception const& error);
 
+    void sent_event(void const* mediator, MirSurfaceEvent const& event);
+
     ~MessageProcessorReport() noexcept(true);
 
 private:

=== modified file 'include/server/mir/lttng/message_processor_report.h'
--- include/server/mir/lttng/message_processor_report.h	2013-06-03 12:15:44 +0000
+++ include/server/mir/lttng/message_processor_report.h	2013-08-13 23:40:56 +0000
@@ -35,6 +35,7 @@
     void unknown_method(void const* mediator, int id, std::string const& method);
     void exception_handled(void const* mediator, int id, std::exception const& error);
     void exception_handled(void const* mediator, std::exception const& error);
+    void sent_event(void const* mediator, MirSurfaceEvent const& event);
 
 private:
     ServerTracepointProvider tp_provider;

=== modified file 'include/server/mir/shell/default_focus_mechanism.h'
--- include/server/mir/shell/default_focus_mechanism.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/shell/default_focus_mechanism.h	2013-08-13 23:40:56 +0000
@@ -27,44 +27,29 @@
 
 namespace shell
 {
+class Surface;
 class InputTargeter;
 class SurfaceController;
-class SessionListener;
-class FocusSequence;
 
 class DefaultFocusMechanism : public FocusSetter
 {
 public:
-    explicit DefaultFocusMechanism(std::shared_ptr<FocusSequence> const& sequence,
-                                   std::shared_ptr<InputTargeter> const& input_targeter,
-                                   std::shared_ptr<SurfaceController> const& surface_controller,
-                                   std::shared_ptr<SessionListener> const& session_listener);
+    explicit DefaultFocusMechanism(std::shared_ptr<InputTargeter> const& input_targeter,
+                                   std::shared_ptr<SurfaceController> const& surface_controller);
     virtual ~DefaultFocusMechanism() = default;
 
-    void surface_created_for(std::shared_ptr<Session> const& session);
-    void session_opened(std::shared_ptr<Session> const& session);
-    void session_closed(std::shared_ptr<Session> const& session);
-    std::weak_ptr<Session> focused_session() const;
-
-    //TODO: this is only used in example code
-    void focus_next();
+    void set_focus_to(std::shared_ptr<shell::Session> const& new_focus);
 
 protected:
-    void focus_next_locked();
-
     DefaultFocusMechanism(const DefaultFocusMechanism&) = delete;
     DefaultFocusMechanism& operator=(const DefaultFocusMechanism&) = delete;
 
 private:
-    void set_focus(std::shared_ptr<Session> const& session);
-
-    std::shared_ptr<FocusSequence> const sequence;
-    std::shared_ptr<SessionListener> const session_listener;
     std::shared_ptr<InputTargeter> const input_targeter;
     std::shared_ptr<SurfaceController> const surface_controller;
 
-    std::mutex mutable mutex;
-    std::shared_ptr<Session> focus_session;
+    // TODO: Protect with mutex
+    std::weak_ptr<Surface> currently_focused_surface;
 };
 
 }

=== added file 'include/server/mir/shell/focus_controller.h'
--- include/server/mir/shell/focus_controller.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/shell/focus_controller.h	2013-04-17 21:33:51 +0000
@@ -0,0 +1,44 @@
+/*
+ * 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: Robert Carr <[e-mail]>
+ */
+
+#ifndef MIR_SHELL_FOCUS_CONTROLLER_H_
+#define MIR_SHELL_FOCUS_CONTROLLER_H_
+
+namespace mir
+{
+
+namespace shell
+{
+
+class FocusController
+{
+public:
+    virtual ~FocusController() {}
+
+    virtual void focus_next() = 0;
+
+protected:
+    FocusController() = default;
+    FocusController(FocusController const&) = delete;
+    FocusController& operator=(FocusController const&) = delete;
+};
+
+}
+} // namespace mir
+
+#endif // MIR_SHELL_FOCUS_CONTROLLER_H_

=== modified file 'include/server/mir/shell/focus_setter.h'
--- include/server/mir/shell/focus_setter.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/shell/focus_setter.h	2013-04-24 05:22:20 +0000
@@ -28,20 +28,14 @@
 {
 class Session;
 
-/// Interface used by the Shell to customize behavior based around focus
+/// Interface used by the Shell to propagate changes in the focus model to interested views
+/// e.g. Input, or Surfaces.
 class FocusSetter
 {
 public:
     virtual ~FocusSetter() {}
 
-    virtual void surface_created_for(std::shared_ptr<Session> const& session) = 0;
-    virtual void session_opened(std::shared_ptr<Session> const& session) = 0;
-    virtual void session_closed(std::shared_ptr<Session> const& session) = 0;
-
-    //TODO: this is only used in example code
-    virtual void focus_next() = 0;
-
-    virtual std::weak_ptr<Session> focused_session() const = 0;
+    virtual void set_focus_to(std::shared_ptr<shell::Session> const& new_focus) = 0;
 
 protected:
     FocusSetter() = default;

=== modified file 'include/server/mir/shell/mediating_display_changer.h'
--- include/server/mir/shell/mediating_display_changer.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/shell/mediating_display_changer.h	2013-08-13 13:54:24 +0000
@@ -19,15 +19,10 @@
 #ifndef MIR_SHELL_MEDIATING_DISPLAY_CHANGER_H_
 #define MIR_SHELL_MEDIATING_DISPLAY_CHANGER_H_
 
-<<<<<<< TREE
 #include "mir/frontend/display_changer.h"
 #include "mir/display_changer.h"
 
 #include <mutex>
-=======
-#include "mir/shell/display_changer.h"
-#include <unordered_map>
->>>>>>> MERGE-SOURCE
 
 namespace mir
 {
@@ -37,41 +32,24 @@
     class Display;
     class DisplayConfigurationPolicy;
 }
-<<<<<<< TREE
 namespace compositor { class Compositor; }
 namespace input { class InputManager; }
 
-=======
-
-namespace compositor
-{
-class Compositor;
-}
-
->>>>>>> MERGE-SOURCE
 namespace shell
 {
-class FocusSetter;
 
 class MediatingDisplayChanger : public frontend::DisplayChanger,
                                 public mir::DisplayChanger
 {
 public:
-<<<<<<< TREE
     MediatingDisplayChanger(
         std::shared_ptr<graphics::Display> const& display,
         std::shared_ptr<compositor::Compositor> const& compositor,
         std::shared_ptr<input::InputManager> const& input_manager,
         std::shared_ptr<graphics::DisplayConfigurationPolicy> const& display_configuration_policy);
-=======
-    explicit MediatingDisplayChanger(
-        std::shared_ptr<graphics::Display> const& display,
-        std::shared_ptr<compositor::Compositor> const& compositor);
->>>>>>> MERGE-SOURCE
 
     /* From mir::frontend::DisplayChanger */
     std::shared_ptr<graphics::DisplayConfiguration> active_configuration();
-<<<<<<< TREE
     void configure(std::weak_ptr<frontend::Session> const& session,
                    std::shared_ptr<graphics::DisplayConfiguration> const& conf);
 
@@ -79,35 +57,16 @@
     void configure_for_hardware_change(
         std::shared_ptr<graphics::DisplayConfiguration> const& conf,
         SystemStateHandling pause_resume_system);
-=======
-
-    void store_configuration_for(std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&);
-    void apply_configuration_of(std::weak_ptr<frontend::Session> const&);
-    void remove_configuration_for(std::weak_ptr<frontend::Session> const&);
->>>>>>> MERGE-SOURCE
 
 private:
-<<<<<<< TREE
     void apply_config(std::shared_ptr<graphics::DisplayConfiguration> const& conf,
                       SystemStateHandling pause_resume_system);
 
-=======
-    void apply_config(std::shared_ptr<graphics::DisplayConfiguration> const&);
-
-
-    std::unordered_map<frontend::Session*, std::shared_ptr<graphics::DisplayConfiguration> > config_map;
->>>>>>> MERGE-SOURCE
     std::shared_ptr<graphics::Display> const display;
-<<<<<<< TREE
     std::shared_ptr<compositor::Compositor> const compositor;
     std::shared_ptr<input::InputManager> const input_manager;
     std::shared_ptr<graphics::DisplayConfigurationPolicy> const display_configuration_policy;
     std::mutex configuration_mutex;
-=======
-    std::shared_ptr<compositor::Compositor> const compositor;
-    std::shared_ptr<graphics::DisplayConfiguration> const base_config;
-    std::shared_ptr<graphics::DisplayConfiguration> active_config;
->>>>>>> MERGE-SOURCE
 };
 
 }

=== modified file 'include/server/mir/shell/session_manager.h'
--- include/server/mir/shell/session_manager.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/shell/session_manager.h	2013-08-13 23:40:56 +0000
@@ -21,6 +21,7 @@
 
 #include "mir/frontend/surface_id.h"
 #include "mir/frontend/shell.h"
+#include "mir/shell/focus_controller.h"
 
 #include <mutex>
 #include <memory>
@@ -38,34 +39,37 @@
 {
 class SurfaceFactory;
 class SessionContainer;
+class FocusSequence;
 class FocusSetter;
 class Session;
 class InputRegistrar;
 class SnapshotStrategy;
 class SessionListener;
 struct SurfaceCreationParameters;
-class DisplayChanger;
 
-class SessionManager : public frontend::Shell
+class SessionManager : public frontend::Shell, public shell::FocusController
 {
 public:
     explicit SessionManager(std::shared_ptr<SurfaceFactory> const& surface_factory,
                             std::shared_ptr<SessionContainer> const& app_container,
+                            std::shared_ptr<FocusSequence> const& focus_sequence,
                             std::shared_ptr<FocusSetter> const& focus_setter,
                             std::shared_ptr<SnapshotStrategy> const& snapshot_strategy,
-                            std::shared_ptr<SessionListener> const& session_listener,
-                            std::shared_ptr<DisplayChanger> const& display_changer);
+                            std::shared_ptr<SessionListener> const& session_listener);
     virtual ~SessionManager();
 
     virtual std::shared_ptr<frontend::Session> open_session(
         std::string const& name, std::shared_ptr<frontend::EventSink> const& sink);
     virtual void close_session(std::shared_ptr<frontend::Session> const& session);
 
-    virtual void handle_display_configuration(std::shared_ptr<frontend::Session> const& session); 
-    virtual void handle_surface_created(std::shared_ptr<frontend::Session> const& session);
+    frontend::SurfaceId create_surface_for(std::shared_ptr<frontend::Session> const& session,
+                                 SurfaceCreationParameters const& params);
 
-    virtual frontend::SurfaceId create_surface_for(std::shared_ptr<frontend::Session> const& session,
-                                             shell::SurfaceCreationParameters const& params);
+    void focus_next();
+    std::weak_ptr<Session> focussed_application() const;
+    void set_focus_to(std::shared_ptr<Session> const& focus);
+    
+    void handle_surface_created(std::shared_ptr<frontend::Session> const& session);
 
 protected:
     SessionManager(const SessionManager&) = delete;
@@ -74,10 +78,15 @@
 private:
     std::shared_ptr<SurfaceFactory> const surface_factory;
     std::shared_ptr<SessionContainer> const app_container;
+    std::shared_ptr<FocusSequence> const focus_sequence;
     std::shared_ptr<FocusSetter> const focus_setter;
     std::shared_ptr<SnapshotStrategy> const snapshot_strategy;
     std::shared_ptr<SessionListener> const session_listener;
-    std::shared_ptr<DisplayChanger> const display_changer;
+
+    std::mutex mutex;
+    std::weak_ptr<Session> focus_application;
+
+    void set_focus_to_locked(std::unique_lock<std::mutex> const& lock, std::shared_ptr<Session> const& next_focus);
 };
 
 }

=== modified file 'include/server/mir/shell/unauthorized_display_changer.h'
--- include/server/mir/shell/unauthorized_display_changer.h	2013-08-13 23:40:55 +0000
+++ include/server/mir/shell/unauthorized_display_changer.h	2013-08-12 17:20:29 +0000
@@ -32,10 +32,7 @@
     explicit UnauthorizedDisplayChanger(std::shared_ptr<frontend::DisplayChanger> const& changer);
 
     std::shared_ptr<graphics::DisplayConfiguration> active_configuration();
-    void store_configuration_for(
-        std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&);
-    void apply_configuration_of(std::weak_ptr<frontend::Session> const&);
-    void remove_configuration_for(std::weak_ptr<frontend::Session> const&);
+    void configure(std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&);
 
 private:
     std::shared_ptr<frontend::DisplayChanger> const changer;

=== modified file 'include/shared/mir_toolkit/client_types.h'
--- include/shared/mir_toolkit/client_types.h	2013-08-12 08:58:59 +0000
+++ include/shared/mir_toolkit/client_types.h	2013-08-13 23:40:56 +0000
@@ -176,34 +176,9 @@
 } MirDisplayInfo;
 
 /**
- * MirDisplayConfiguration provides details of the graphics environment.
+ * MirDisplayOutput provides details of the graphics environment.
  */
 
-typedef struct MirDisplayCard
-{
-    uint32_t card_id;
-    uint32_t max_simultaneous_outputs;
-} MirDisplayCard;
-
-typedef enum MirDisplayOutputType
-{
-    mir_display_output_type_unknown,
-    mir_display_output_type_vga,
-    mir_display_output_type_dvii,
-    mir_display_output_type_dvid,
-    mir_display_output_type_dvia,
-    mir_display_output_type_composite,
-    mir_display_output_type_svideo,
-    mir_display_output_type_lvds,
-    mir_display_output_type_component,
-    mir_display_output_type_ninepindin,
-    mir_display_output_type_displayport,
-    mir_display_output_type_hdmia,
-    mir_display_output_type_hdmib,
-    mir_display_output_type_tv,
-    mir_display_output_type_edp
-} MirDisplayOutputType;
-
 typedef struct MirDisplayMode
 {
     uint32_t vertical_resolution;
@@ -215,7 +190,6 @@
 {
     uint32_t num_modes;
     MirDisplayMode* modes;
-    uint32_t preferred_mode;
     uint32_t current_mode; 
 
     uint32_t num_output_formats;
@@ -224,7 +198,6 @@
 
     uint32_t card_id;
     uint32_t output_id;
-    MirDisplayOutputType type;
 
     int32_t position_x;
     int32_t position_y;
@@ -239,8 +212,6 @@
 {
     uint32_t num_displays;
     MirDisplayOutput* displays;
-    uint32_t num_cards;
-    MirDisplayCard *cards;
 } MirDisplayConfiguration;
 
 /**

=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h	2013-07-09 10:51:59 +0000
+++ include/shared/mir_toolkit/common.h	2013-08-13 23:40:56 +0000
@@ -36,7 +36,8 @@
     mir_surface_attrib_type,
     mir_surface_attrib_state,
     mir_surface_attrib_swapinterval,
-    mir_surface_attrib_arraysize_
+    mir_surface_attrib_focus,
+    mir_surface_attrib_enum_max_
 } MirSurfaceAttrib;
 
 typedef enum MirSurfaceType
@@ -48,7 +49,7 @@
     mir_surface_type_freestyle,
     mir_surface_type_popover,
     mir_surface_type_inputmethod,
-    mir_surface_type_arraysize_
+    mir_surface_type_enum_max_
 } MirSurfaceType;
 
 typedef enum MirSurfaceState
@@ -62,8 +63,16 @@
        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_
+    mir_surface_state_enum_max_
 } MirSurfaceState;
+
+typedef enum MirSurfaceFocusState
+{
+    mir_surface_unfocused = 0,
+    mir_surface_focused,
+    mir_surface_focus_enum_max_
+} MirSurfaceFocusState;
+
 /**@}*/
 
 #endif

=== modified file 'include/test/mir_test/event_matchers.h'
--- include/test/mir_test/event_matchers.h	2013-04-26 16:31:48 +0000
+++ include/test/mir_test/event_matchers.h	2013-08-13 23:40:56 +0000
@@ -19,6 +19,8 @@
 #ifndef MIR_TEST_EVENT_MATCHERS_H_
 #define MIR_TEST_EVENT_MATCHERS_H_
 
+#include "mir_toolkit/event.h"
+
 #include <androidfw/Input.h>
 
 #include <gmock/gmock.h>
@@ -64,6 +66,18 @@
     return (coords->x == dx) && (coords->y == dy);
 }
 
+MATCHER_P2(SurfaceEvent, attrib, value, "")
+{
+    if (arg.type != mir_event_type_surface)
+        return false;
+    auto surface_ev = arg.surface;
+    if (surface_ev.attrib != attrib)
+        return false;
+    if (surface_ev.value != value)
+        return false;
+    return true;
+}
+
 }
 } // namespace mir
 

=== modified file 'include/test/mir_test_doubles/mock_display_changer.h'
--- include/test/mir_test_doubles/mock_display_changer.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/mock_display_changer.h	2013-08-12 17:20:29 +0000
@@ -33,10 +33,8 @@
 {
 public:
     MOCK_METHOD0(active_configuration, std::shared_ptr<graphics::DisplayConfiguration>());
-    MOCK_METHOD2(store_configuration_for,
+    MOCK_METHOD2(configure,
         void(std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&));
-    MOCK_METHOD1(remove_configuration_for, void(std::weak_ptr<frontend::Session> const&));
-    MOCK_METHOD1(apply_configuration_of, void(std::weak_ptr<frontend::Session> const&));
 };
 
 }

=== modified file 'include/test/mir_test_doubles/mock_drm.h'
--- include/test/mir_test_doubles/mock_drm.h	2013-08-12 08:58:58 +0000
+++ include/test/mir_test_doubles/mock_drm.h	2013-08-13 23:40:56 +0000
@@ -48,7 +48,7 @@
 
     void add_crtc(uint32_t id, drmModeModeInfo mode);
     void add_encoder(uint32_t encoder_id, uint32_t crtc_id, uint32_t possible_crtcs_mask);
-    void add_connector(uint32_t connector_id, uint32_t type, drmModeConnection connection,
+    void add_connector(uint32_t connector_id, drmModeConnection connection,
                        uint32_t encoder_id, std::vector<drmModeModeInfo>& modes,
                        std::vector<uint32_t>& possible_encoder_ids,
                        geometry::Size const& physical_size);
@@ -60,10 +60,8 @@
     drmModeEncoder* find_encoder(uint32_t id);
     drmModeConnector* find_connector(uint32_t id);
 
-    enum ModePreference {NormalMode, PreferredMode};
     static drmModeModeInfo create_mode(uint16_t hdisplay, uint16_t vdisplay,
-                                       uint32_t clock, uint16_t htotal, uint16_t vtotal,
-                                       ModePreference preferred);
+                                       uint32_t clock, uint16_t htotal, uint16_t vtotal);
 
 private:
     int pipe_fds[2];

=== modified file 'include/test/mir_test_doubles/mock_focus_setter.h'
--- include/test/mir_test_doubles/mock_focus_setter.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/mock_focus_setter.h	2013-04-24 05:22:20 +0000
@@ -32,11 +32,7 @@
 
 struct MockFocusSetter : public shell::FocusSetter
 {
-    MOCK_METHOD1(session_opened, void(std::shared_ptr<shell::Session> const&));
-    MOCK_METHOD1(session_closed, void(std::shared_ptr<shell::Session> const&));
-    MOCK_METHOD1(surface_created_for, void(std::shared_ptr<shell::Session> const&));
-    MOCK_CONST_METHOD0(focused_session, std::weak_ptr<shell::Session>());
-    MOCK_METHOD0(focus_next, void());
+    MOCK_METHOD1(set_focus_to, void(std::shared_ptr<shell::Session> const&));
 };
 
 }

=== modified file 'include/test/mir_test_doubles/mock_shell.h'
--- include/test/mir_test_doubles/mock_shell.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/mock_shell.h	2013-08-13 23:40:56 +0000
@@ -38,10 +38,8 @@
         std::string const&, std::shared_ptr<frontend::EventSink> const&));
     MOCK_METHOD1(close_session, void(std::shared_ptr<frontend::Session> const&));
 
-    MOCK_METHOD1(handle_display_configuration, void(std::shared_ptr<frontend::Session> const&)); 
-    MOCK_METHOD1(handle_surface_created, void(std::shared_ptr<frontend::Session> const&)); 
-
     MOCK_METHOD2(create_surface_for, frontend::SurfaceId(std::shared_ptr<frontend::Session> const&, shell::SurfaceCreationParameters const&));
+    MOCK_METHOD1(handle_surface_created, void(std::shared_ptr<frontend::Session> const&));
 };
 
 }

=== modified file 'include/test/mir_test_doubles/null_display_changer.h'
--- include/test/mir_test_doubles/null_display_changer.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/null_display_changer.h	2013-08-12 17:20:29 +0000
@@ -36,13 +36,9 @@
     {
         return std::make_shared<NullDisplayConfig>();
     }
-    virtual void store_configuration_for(
-        std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&)
-    {}
-    virtual void apply_configuration_of(std::weak_ptr<frontend::Session> const&)
-    {}
-    virtual void remove_configuration_for(std::weak_ptr<frontend::Session> const&)
-    {}
+    virtual void configure(std::weak_ptr<frontend::Session> const&, std::shared_ptr<graphics::DisplayConfiguration> const&)
+    {
+    }
 };
 }
 }

=== modified file 'include/test/mir_test_doubles/stub_display_configuration.h'
--- include/test/mir_test_doubles/stub_display_configuration.h	2013-08-12 08:58:59 +0000
+++ include/test/mir_test_doubles/stub_display_configuration.h	2013-08-13 23:40:56 +0000
@@ -62,8 +62,7 @@
             graphics::DisplayConfigurationOutput output{
                 graphics::DisplayConfigurationOutputId{static_cast<int>(i)},
                 graphics::DisplayConfigurationCardId{static_cast<int>(i)},
-                graphics::DisplayConfigurationOutputType::vga,
-                pfs, modes, i,
+                pfs, modes,
                 physical_size,
                 ((i % 2) == 0),
                 ((i % 2) == 1),
@@ -72,21 +71,11 @@
             };
 
             outputs.push_back(output);
-
-            graphics::DisplayConfigurationCard card{
-                graphics::DisplayConfigurationCardId{static_cast<int>(i)},
-                i + 1
-            };
-
-            cards.push_back(card);
         }
-
     };
 
-    void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)> f) const
+    void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)>) const
     {
-        for (auto const& card : cards)
-            f(card);
     }
 
     void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)> f) const
@@ -101,7 +90,6 @@
     {
     }
 
-    std::vector<graphics::DisplayConfigurationCard> cards;
     std::vector<graphics::DisplayConfigurationOutput> outputs;
 };
 

=== modified file 'include/test/mir_test_doubles/stub_session.h'
--- include/test/mir_test_doubles/stub_session.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/stub_session.h	2013-08-08 22:16:06 +0000
@@ -32,7 +32,7 @@
 {
     frontend::SurfaceId create_surface(shell::SurfaceCreationParameters const& /* params */)
     {
-        return frontend::SurfaceId{1};
+        return frontend::SurfaceId{0};
     }
     void destroy_surface(frontend::SurfaceId /* surface */)
     {
@@ -58,6 +58,10 @@
     {
         return 0;
     }
+    void set_event_sink(std::shared_ptr<frontend::EventSink> const&)
+    {
+    }
+
     void send_display_config(graphics::DisplayConfiguration const&)
     {
     }

=== modified file 'include/test/mir_test_doubles/stub_shell.h'
--- include/test/mir_test_doubles/stub_shell.h	2013-08-13 23:40:55 +0000
+++ include/test/mir_test_doubles/stub_shell.h	2013-08-13 23:40:56 +0000
@@ -35,24 +35,16 @@
     {
         return std::make_shared<StubSession>();
     }
-
     void close_session(std::shared_ptr<frontend::Session> const& /* session */) override
     {
     }
-
-    virtual void handle_display_configuration(std::shared_ptr<frontend::Session> const&)
-    {
-    }
-
-    virtual void handle_surface_created(std::shared_ptr<frontend::Session> const&)
-    {
-    }
- 
-    virtual frontend::SurfaceId create_surface_for(
-                                        std::shared_ptr<frontend::Session> const& /* session */,
-                                        shell::SurfaceCreationParameters const& /* params */)
-    {
-        return frontend::SurfaceId{4};
+    frontend::SurfaceId create_surface_for(std::shared_ptr<frontend::Session> const& /* session */,
+                                        shell::SurfaceCreationParameters const& /* params */) override
+    {
+        return frontend::SurfaceId{0};
+    }
+    void handle_surface_created(std::shared_ptr<frontend::Session> const& /* session */) override
+    {
     }
 };
 

=== modified file 'include/test/mir_test_framework/input_testing_server_configuration.h'
--- include/test/mir_test_framework/input_testing_server_configuration.h	2013-07-24 05:22:03 +0000
+++ include/test/mir_test_framework/input_testing_server_configuration.h	2013-08-13 23:40:56 +0000
@@ -63,14 +63,13 @@
     void on_exit();
     
     std::shared_ptr<mir::input::InputConfiguration> the_input_configuration() override;
-    std::shared_ptr<mir::surfaces::InputRegistrar> the_input_registrar() override;
+    std::shared_ptr<mir::frontend::Shell> the_frontend_shell() override;
 
 protected:
     virtual void inject_input() = 0;
     mir::input::android::FakeEventHub* fake_event_hub;
 
     void wait_until_client_appears(std::string const& surface_name);
-    void wait_until_client_vanishes(std::string const& surface_name);
 
 private:
     std::mutex lifecycle_lock;
@@ -81,7 +80,7 @@
     std::thread input_injection_thread;
     
     std::shared_ptr<mir::test::doubles::FakeEventHubInputConfiguration> input_configuration;
-    std::shared_ptr<mir::surfaces::InputRegistrar> input_registrar;
+    std::shared_ptr<mir::frontend::Shell> frontend_shell;
 };
 
 }

=== modified file 'src/client/display_configuration.cpp'
--- src/client/display_configuration.cpp	2013-08-12 08:58:59 +0000
+++ src/client/display_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -59,17 +59,10 @@
 namespace
 {
 
-void fill_display_card(MirDisplayCard& card, mp::DisplayCard const& msg)
-{
-    card.card_id = msg.card_id();
-    card.max_simultaneous_outputs = msg.max_simultaneous_outputs();
-}
-
 void fill_display_output(MirDisplayOutput& output, mp::DisplayOutput const& msg)
 {
     output.card_id = msg.card_id();
     output.output_id = msg.output_id();
-    output.type = static_cast<MirDisplayOutputType>(msg.type());
 
     for (auto i = 0u; i < output.num_modes; i++)
     {
@@ -78,7 +71,6 @@
         output.modes[i].vertical_resolution = mode.vertical_resolution(); 
         output.modes[i].refresh_rate = mode.refresh_rate();
     }
-    output.preferred_mode = msg.preferred_mode();
     output.current_mode = msg.current_mode();
 
     for (auto i = 0u; i < output.num_output_formats; i++)
@@ -109,16 +101,6 @@
 void mcl::DisplayConfiguration::set_configuration(mp::DisplayConfiguration const& msg)
 {
     std::unique_lock<std::mutex> lk(guard);
-
-    cards.clear();
-    for (auto i = 0; i < msg.display_card_size(); i++)
-    {
-        auto const& msg_card = msg.display_card(i);
-        MirDisplayCard card;
-        fill_display_card(card, msg_card);
-        cards.push_back(card);
-    }
-
     outputs.clear();
     for (auto i = 0; i < msg.display_output_size(); i++)
     {
@@ -141,22 +123,14 @@
 {
     std::unique_lock<std::mutex> lk(guard);
     auto new_config = new MirDisplayConfiguration;
-
-    /* Cards */
-    new_config->num_cards = cards.size();
-    new_config->cards = new MirDisplayCard[new_config->num_cards];
-
-    for (auto i = 0u; i < cards.size(); i++)
-        new_config->cards[i] = cards[i];
-
-    /* Outputs */
     new_config->num_displays = outputs.size();
     new_config->displays = new MirDisplayOutput[new_config->num_displays];
 
-    for (auto i = 0u; i < outputs.size(); i++)
+    auto i=0u;
+    for (auto const& out : outputs)
     {
-        auto new_info = &new_config->displays[i];
-        MirDisplayOutput* output = outputs[i].get();
+        auto new_info = &new_config->displays[i++];
+        MirDisplayOutput* output = out.get();
         std::memcpy(new_info, output, sizeof(MirDisplayOutput)); 
 
         new_info->output_formats = new MirPixelFormat[new_info->num_output_formats];
@@ -167,7 +141,6 @@
         auto mode_size = sizeof(MirDisplayMode)* new_info->num_modes;
         std::memcpy(new_info->modes, output->modes, mode_size);
     }
-
     return new_config;
 }
 

=== modified file 'src/client/display_configuration.h'
--- src/client/display_configuration.h	2013-08-12 08:58:59 +0000
+++ src/client/display_configuration.h	2013-08-13 23:40:56 +0000
@@ -56,7 +56,6 @@
 
 private:
     std::mutex mutable guard;
-    std::vector<MirDisplayCard> cards;
     std::vector<std::shared_ptr<DisplayOutput>> outputs;
     std::function<void()> notify_change;
 };

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2013-08-09 09:32:07 +0000
+++ src/client/mir_connection.cpp	2013-08-13 23:40:56 +0000
@@ -118,6 +118,10 @@
 
 void MirConnection::released(SurfaceRelease data)
 {
+    {
+        std::lock_guard<std::recursive_mutex> lock(mutex);
+        surface_map->erase(data.surface->id());
+    }
     data.callback(data.surface, data.context);
     data.handle->result_received();
     delete data.surface;
@@ -133,7 +137,6 @@
     auto new_wait_handle = new MirWaitHandle;
 
     SurfaceRelease surf_release{surface, new_wait_handle, callback, context};
-    surface_map->erase(surface->id());
 
     mir::protobuf::SurfaceId message;
     message.set_value(surface->id());

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2013-06-28 09:25:09 +0000
+++ src/client/mir_surface.cpp	2013-08-13 23:40:56 +0000
@@ -54,7 +54,7 @@
     
     server.create_surface(0, &message, &surface, gp::NewCallback(this, &MirSurface::created, callback, context));
 
-    for (int i = 0; i < mir_surface_attrib_arraysize_; i++)
+    for (int i = 0; i < mir_surface_attrib_enum_max_; 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;
@@ -221,8 +221,6 @@
         mir_surface_callback callback,
         void * context)
 {
-    std::lock_guard<std::recursive_mutex> lock(mutex);
-
     return connection->release_surface(this, callback, context);
 }
 
@@ -307,6 +305,7 @@
         {
         case mir_surface_attrib_type:
         case mir_surface_attrib_state:
+        case mir_surface_attrib_focus:
         case mir_surface_attrib_swapinterval:
             if (configure_result.has_ivalue())
                 attrib_cache[a] = configure_result.ivalue();
@@ -362,7 +361,7 @@
     if (e.type == mir_event_type_surface)
     {
         MirSurfaceAttrib a = e.surface.attrib;
-        if (a < mir_surface_attrib_arraysize_)
+        if (a < mir_surface_attrib_enum_max_)
             attrib_cache[a] = e.surface.value;
     }
 

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2013-06-28 11:10:25 +0000
+++ src/client/mir_surface.h	2013-08-13 23:40:56 +0000
@@ -122,7 +122,7 @@
     mir::protobuf::SurfaceSetting configure_result;
 
     // Cache of latest SurfaceSettings returned from the server
-    int attrib_cache[mir_surface_attrib_arraysize_];
+    int attrib_cache[mir_surface_attrib_enum_max_];
 
     std::function<void(MirEvent const*)> handle_event_callback;
     std::shared_ptr<mir::input::receiver::InputReceiverThread> input_thread;

=== modified file 'src/platform/graphics/default_display_configuration_policy.cpp'
--- src/platform/graphics/default_display_configuration_policy.cpp	2013-08-12 08:58:58 +0000
+++ src/platform/graphics/default_display_configuration_policy.cpp	2013-08-13 23:40:56 +0000
@@ -24,15 +24,13 @@
 
 void mg::DefaultDisplayConfigurationPolicy::apply_to(DisplayConfiguration& conf)
 {
+    size_t const preferred_mode_index{0};
+
     conf.for_each_output(
         [&conf](DisplayConfigurationOutput const& conf_output)
         {
             if (conf_output.connected && conf_output.modes.size() > 0)
             {
-                size_t preferred_mode_index{conf_output.preferred_mode_index};
-                if (preferred_mode_index > conf_output.modes.size())
-                    preferred_mode_index = 0;
-
                 conf.configure_output(conf_output.id, true, geom::Point(),
                                       preferred_mode_index);
             }

=== modified file 'src/platform/graphics/display_configuration.cpp'
--- src/platform/graphics/display_configuration.cpp	2013-08-12 11:55:54 +0000
+++ src/platform/graphics/display_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -47,41 +47,6 @@
     std::streamsize const precision;
 };
 
-char const* output_type_to_string(mg::DisplayConfigurationOutputType type)
-{
-    static char const* type_names[] =
-    {
-        "unknown",
-        "vga",
-        "dvii",
-        "dvid",
-        "dvia",
-        "composite",
-        "lvds",
-        "component",
-        "9pindin",
-        "displayport",
-        "hdmia",
-        "hdmib",
-        "tv",
-        "edp"
-    };
-
-    auto index = static_cast<ssize_t>(type);
-    static auto const size = std::distance(std::begin(type_names), std::end(type_names));
-    if (index >= size || index < 0)
-        return "invalid";
-
-    return type_names[index];
-}
-
-}
-
-std::ostream& mg::operator<<(std::ostream& out, mg::DisplayConfigurationCard const& val)
-{
-    return out << "{ id: " << val.id
-               << " max_simultaneous_outputs: " << val.max_simultaneous_outputs << " }"
-               << std::endl;
 }
 
 std::ostream& mg::operator<<(std::ostream& out, mg::DisplayConfigurationMode const& val)
@@ -96,9 +61,7 @@
 
 std::ostream& mg::operator<<(std::ostream& out, mg::DisplayConfigurationOutput const& val)
 {
-    out << "{ id: " << val.id << ", card_id: " << val.card_id
-        << " type: " << output_type_to_string(val.type)
-        << " modes: [";
+    out << "{ id: " << val.id << ", card_id: " << val.card_id << " modes: [";
 
     for (size_t i = 0; i < val.modes.size(); ++i)
     {
@@ -107,8 +70,7 @@
             out << ", ";
     }
 
-    out << "], preferred_mode: " << val.preferred_mode_index;
-    out << " physical_size_mm: " << val.physical_size_mm.width << "x" << val.physical_size_mm.height;
+    out << "], physical_size_mm: " << val.physical_size_mm.width << "x" << val.physical_size_mm.height;
     out << ", connected: " << (val.connected ? "true" : "false");
     out << ", used: " << (val.used ? "true" : "false");
     out << ", top_left: " << val.top_left;
@@ -123,19 +85,6 @@
     return out;
 }
 
-bool mg::operator==(mg::DisplayConfigurationCard const& val1,
-                    mg::DisplayConfigurationCard const& val2)
-{
-    return (val1.id == val2.id) &&
-           (val1.max_simultaneous_outputs == val2.max_simultaneous_outputs);
-}
-
-bool mg::operator!=(mg::DisplayConfigurationCard const& val1,
-                    mg::DisplayConfigurationCard const& val2)
-{
-    return !(val1 == val2);
-}
-
 bool mg::operator==(mg::DisplayConfigurationMode const& val1,
                     mg::DisplayConfigurationMode const& val2)
 {
@@ -154,9 +103,7 @@
 {
     bool equal{(val1.id == val2.id) &&
                (val1.card_id == val2.card_id) &&
-               (val1.type == val2.type) &&
                (val1.physical_size_mm == val2.physical_size_mm) &&
-               (val1.preferred_mode_index == val2.preferred_mode_index) &&
                (val1.connected == val2.connected) &&
                (val1.used == val2.used) &&
                (val1.top_left == val2.top_left) &&

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-08-13 23:40:55 +0000
+++ src/server/default_server_configuration.cpp	2013-08-13 13:54:24 +0000
@@ -362,7 +362,6 @@
         });
 }
 
-<<<<<<< TREE
 std::shared_ptr<msh::MediatingDisplayChanger>
 mir::DefaultServerConfiguration::the_mediating_display_changer()
 {
@@ -388,14 +387,6 @@
 mir::DefaultServerConfiguration::the_display_changer()
 {
     return the_mediating_display_changer();
-=======
-std::shared_ptr<msh::DisplayChanger>
-mir::DefaultServerConfiguration::the_shell_display_changer()
-{
-    return shell_display_changer([this]()
-        { return std::make_shared<msh::MediatingDisplayChanger>(
-            the_display(), the_compositor());});
->>>>>>> MERGE-SOURCE
 }
 
 std::shared_ptr<msh::SessionContainer>
@@ -411,11 +402,7 @@
     return shell_focus_setter(
         [this]
         {
-            return std::make_shared<msh::DefaultFocusMechanism>(
-                the_shell_focus_sequence(),
-                the_input_targeter(),
-                the_surface_controller(),
-                the_shell_session_listener());
+            return std::make_shared<msh::DefaultFocusMechanism>(the_input_targeter(), the_surface_controller());
         });
 }
 
@@ -460,10 +447,10 @@
             return std::make_shared<msh::SessionManager>(
                 the_shell_surface_factory(),
                 the_shell_session_container(),
+                the_shell_focus_sequence(),
                 the_shell_focus_setter(),
                 the_shell_snapshot_strategy(),
-                the_shell_session_listener(),
-                the_shell_display_changer());
+                the_shell_session_listener());
         });
 }
 
@@ -505,6 +492,12 @@
         }); 
 }
 
+std::shared_ptr<msh::FocusController>
+mir::DefaultServerConfiguration::the_focus_controller()
+{
+    return the_session_manager();
+}
+
 std::shared_ptr<mi::CompositeEventFilter>
 mir::DefaultServerConfiguration::the_composite_event_filter()
 {

=== modified file 'src/server/frontend/null_message_processor_report.cpp'
--- src/server/frontend/null_message_processor_report.cpp	2013-04-24 05:22:20 +0000
+++ src/server/frontend/null_message_processor_report.cpp	2013-08-13 23:40:56 +0000
@@ -39,3 +39,7 @@
 void mf::NullMessageProcessorReport::exception_handled(void const*, std::exception const&)
 {
 }
+
+void mf::NullMessageProcessorReport::sent_event(void const*, MirSurfaceEvent const&)
+{
+}

=== modified file 'src/server/frontend/protobuf_buffer_packer.cpp'
--- src/server/frontend/protobuf_buffer_packer.cpp	2013-08-12 08:58:59 +0000
+++ src/server/frontend/protobuf_buffer_packer.cpp	2013-08-13 23:40:56 +0000
@@ -28,19 +28,11 @@
 namespace
 {
 
-void pack_protobuf_display_card(mp::DisplayCard& protobuf_card,
-                                mg::DisplayConfigurationCard const& display_card)
-{
-    protobuf_card.set_card_id(display_card.id.as_value());
-    protobuf_card.set_max_simultaneous_outputs(display_card.max_simultaneous_outputs);
-}
-
 void pack_protobuf_display_output(mp::DisplayOutput& protobuf_output,
                                   mg::DisplayConfigurationOutput const& display_output)
 {
     protobuf_output.set_output_id(display_output.id.as_value());
     protobuf_output.set_card_id(display_output.card_id.as_value());
-    protobuf_output.set_type(static_cast<uint32_t>(display_output.type));
 
     for (auto const& pf : display_output.pixel_formats)
     {
@@ -54,7 +46,6 @@
         protobuf_output_mode->set_vertical_resolution(mode.size.height.as_uint32_t());
         protobuf_output_mode->set_refresh_rate(mode.vrefresh_hz);
     }
-    protobuf_output.set_preferred_mode(display_output.preferred_mode_index);
 
     protobuf_output.set_physical_width_mm(display_output.physical_size_mm.width.as_uint32_t());
     protobuf_output.set_physical_height_mm(display_output.physical_size_mm.height.as_uint32_t());
@@ -72,13 +63,6 @@
 void mfd::pack_protobuf_display_configuration(mp::DisplayConfiguration& protobuf_config,
                                               mg::DisplayConfiguration const& display_config)
 {
-    display_config.for_each_card(
-        [&protobuf_config](mg::DisplayConfigurationCard const& card)
-        {
-            auto protobuf_card = protobuf_config.add_display_card();
-            pack_protobuf_display_card(*protobuf_card, card);
-        });
-
     display_config.for_each_output(
         [&protobuf_config](mg::DisplayConfigurationOutput const& output)
         {

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2013-08-13 23:40:55 +0000
+++ src/server/frontend/session_mediator.cpp	2013-08-13 23:40:56 +0000
@@ -35,17 +35,22 @@
 #include "mir/graphics/display_configuration.h"
 #include "mir/graphics/platform_ipc_package.h"
 #include "mir/frontend/client_constants.h"
+#include "mir/frontend/event_sink.h"
+
 #include "mir/geometry/rectangles.h"
 #include "client_buffer_tracker.h"
 #include "protobuf_buffer_packer.h"
 
 #include <boost/throw_exception.hpp>
 
+#include <mutex>
+#include <functional>
+
 namespace msh = mir::shell;
 namespace mf = mir::frontend;
-namespace mfd = mir::frontend::detail;
+namespace mfd=mir::frontend::detail;
+namespace mg = mir::graphics;
 namespace geom = mir::geometry;
-namespace mg = mir::graphics;
 
 mf::SessionMediator::SessionMediator(
     std::shared_ptr<frontend::Shell> const& shell,
@@ -71,7 +76,6 @@
     if (session)
     {
         report->session_error(session->name(), __PRETTY_FUNCTION__, "connection dropped without disconnect");
-        display_changer->remove_configuration_for(session);
         shell->close_session(session);
     }
 }
@@ -113,50 +117,44 @@
     mir::protobuf::Surface* response,
     google::protobuf::Closure* done)
 {
-    {
-        std::unique_lock<std::mutex> lock(session_mutex);
-
-        if (session.get() == nullptr)
-            BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
-
-        report->session_create_surface_called(session->name());
-
-        auto const id = session->create_surface(
-            msh::SurfaceCreationParameters()
-            .of_name(request->surface_name())
-            .of_size(request->width(), request->height())
-            .of_buffer_usage(static_cast<graphics::BufferUsage>(request->buffer_usage()))
-            .of_pixel_format(static_cast<geometry::PixelFormat>(request->pixel_format()))
-            );
-
-        {
-            auto surface = session->get_surface(id);
-            response->mutable_id()->set_value(id.as_value());
-            response->set_width(surface->size().width.as_uint32_t());
-            response->set_height(surface->size().height.as_uint32_t());
-            response->set_pixel_format((int)surface->pixel_format());
-            response->set_buffer_usage(request->buffer_usage());
-
-            if (surface->supports_input())
-                response->add_fd(surface->client_input_fd());
-
-            client_buffer_resource = surface->advance_client_buffer();
-            auto const& id = client_buffer_resource->id();
-
-            auto buffer = response->mutable_buffer();
-            buffer->set_buffer_id(id.as_uint32_t());
-
-            if (!client_tracker->client_has(id))
-            {
-                auto packer = std::make_shared<mfd::ProtobufBufferPacker>(buffer);
-                graphics_platform->fill_ipc_package(packer, client_buffer_resource);
-            }
-            client_tracker->add(id);
-        }
-    }
-
-    done->Run();
-    shell->handle_surface_created(session);
+    std::unique_lock<std::mutex> lock(session_mutex);
+    
+    if (session.get() == nullptr)
+        BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
+    
+    report->session_create_surface_called(session->name());
+    
+    auto const id = session->create_surface(
+        msh::SurfaceCreationParameters()
+        .of_name(request->surface_name())
+        .of_size(request->width(), request->height())
+        .of_buffer_usage(static_cast<graphics::BufferUsage>(request->buffer_usage()))
+        .of_pixel_format(static_cast<geometry::PixelFormat>(request->pixel_format()))
+        );
+     {
+         auto surface = session->get_surface(id);
+         response->mutable_id()->set_value(id.as_value());
+         response->set_width(surface->size().width.as_uint32_t());
+         response->set_height(surface->size().height.as_uint32_t());
+         response->set_pixel_format((int)surface->pixel_format());
+         response->set_buffer_usage(request->buffer_usage());
+         if (surface->supports_input())
+             response->add_fd(surface->client_input_fd());
+         client_buffer_resource = surface->advance_client_buffer();
+         auto const& id = client_buffer_resource->id();
+         auto buffer = response->mutable_buffer();
+         buffer->set_buffer_id(id.as_uint32_t());
+         if (!client_tracker->client_has(id))
+         {
+             auto packer = std::make_shared<mfd::ProtobufBufferPacker>(buffer);
+             graphics_platform->fill_ipc_package(packer, client_buffer_resource);
+         }
+         client_tracker->add(id);
+     }
+
+     // TODO: We rely on done->Run() sending messages synchronously.
+     done->Run();
+     shell->handle_surface_created(session);
 }
 
 void mf::SessionMediator::next_buffer(
@@ -210,6 +208,7 @@
         session->destroy_surface(id);
     }
 
+    // TODO: We rely on this sending responses synchronously.
     done->Run();
 }
 
@@ -227,7 +226,6 @@
 
         report->session_disconnect_called(session->name());
 
-        display_changer->remove_configuration_for(session);
         shell->close_session(session);
         session.reset();
     }
@@ -283,10 +281,7 @@
                                      output.current_mode());
         }
 
-        display_changer->store_configuration_for(session, config);
-
-        shell->handle_display_configuration(session); 
+        display_changer->configure(session, config);
     }
     done->Run();
-
 }

=== modified file 'src/server/graphics/android/android_display.cpp'
--- src/server/graphics/android/android_display.cpp	2013-08-12 08:58:59 +0000
+++ src/server/graphics/android/android_display.cpp	2013-08-13 23:40:56 +0000
@@ -56,22 +56,17 @@
     AndroidDisplayConfiguration(geom::Size const& display_size)
         : configuration{mg::DisplayConfigurationOutputId{0},
                         mg::DisplayConfigurationCardId{0},
-                        mg::DisplayConfigurationOutputType::lvds,
                         {geom::PixelFormat::abgr_8888},
                         {mg::DisplayConfigurationMode{display_size,0.0f}},
-                        0,
                         geom::Size{0,0},
                         true,
                         true,
                         geom::Point{0,0},
-                        0, 0},
-          card{mg::DisplayConfigurationCardId{0}, 1}
+                        0, 0}
     {
     }
-
-    void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
+    void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)>) const
     {
-        f(card);
     }
 
     void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
@@ -85,8 +80,7 @@
     }
 
 private:
-    mg::DisplayConfigurationOutput const configuration;
-    mg::DisplayConfigurationCard const card;
+    mg::DisplayConfigurationOutput configuration;
 };
 
 EGLDisplay create_and_initialize_display()

=== modified file 'src/server/graphics/gbm/drm_mode_resources.cpp'
--- src/server/graphics/gbm/drm_mode_resources.cpp	2013-08-12 08:58:59 +0000
+++ src/server/graphics/gbm/drm_mode_resources.cpp	2013-08-13 23:40:56 +0000
@@ -95,21 +95,6 @@
     }
 }
 
-size_t mgg::DRMModeResources::num_connectors()
-{
-    return resources->count_connectors;
-}
-
-size_t mgg::DRMModeResources::num_encoders()
-{
-    return resources->count_encoders;
-}
-
-size_t mgg::DRMModeResources::num_crtcs()
-{
-    return resources->count_crtcs;
-}
-
 mgg::DRMModeConnectorUPtr mgg::DRMModeResources::connector(uint32_t id) const
 {
     auto connector_raw = drmModeGetConnector(drm_fd, id);

=== modified file 'src/server/graphics/gbm/drm_mode_resources.h'
--- src/server/graphics/gbm/drm_mode_resources.h	2013-08-12 08:58:59 +0000
+++ src/server/graphics/gbm/drm_mode_resources.h	2013-08-13 23:40:56 +0000
@@ -46,10 +46,6 @@
     void for_each_encoder(std::function<void(DRMModeEncoderUPtr)> const& f) const;
     void for_each_crtc(std::function<void(DRMModeCrtcUPtr)> const& f) const;
 
-    size_t num_connectors();
-    size_t num_encoders();
-    size_t num_crtcs();
-
     DRMModeConnectorUPtr connector(uint32_t id) const;
     DRMModeEncoderUPtr encoder(uint32_t id) const;
     DRMModeCrtcUPtr crtc(uint32_t id) const;

=== modified file 'src/server/graphics/gbm/real_kms_display_configuration.cpp'
--- src/server/graphics/gbm/real_kms_display_configuration.cpp	2013-08-12 08:58:59 +0000
+++ src/server/graphics/gbm/real_kms_display_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -59,12 +59,6 @@
     return round(vrefresh_hz * 10.0) / 10.0;
 }
 
-mg::DisplayConfigurationOutputType
-kms_connector_type_to_output_type(uint32_t connector_type)
-{
-    return static_cast<mg::DisplayConfigurationOutputType>(connector_type);
-}
-
 }
 
 mgg::RealKMSDisplayConfiguration::RealKMSDisplayConfiguration(int drm_fd)
@@ -75,8 +69,7 @@
 
 mgg::RealKMSDisplayConfiguration::RealKMSDisplayConfiguration(
     RealKMSDisplayConfiguration const& conf)
-    : KMSDisplayConfiguration(), drm_fd{conf.drm_fd},
-      card(conf.card), outputs{conf.outputs}
+    : KMSDisplayConfiguration(), drm_fd{conf.drm_fd}, outputs{conf.outputs}
 {
 }
 
@@ -86,7 +79,6 @@
     if (&conf != this)
     {
         drm_fd = conf.drm_fd;
-        card = conf.card;
         outputs = conf.outputs;
     }
 
@@ -96,6 +88,7 @@
 void mgg::RealKMSDisplayConfiguration::for_each_card(
     std::function<void(DisplayConfigurationCard const&)> f) const
 {
+    DisplayConfigurationCard const card{DisplayConfigurationCardId{0}};
     f(card);
 }
 
@@ -161,9 +154,6 @@
 {
     DRMModeResources resources{drm_fd};
 
-    size_t max_outputs = std::min(resources.num_crtcs(), resources.num_connectors());
-    card = {mg::DisplayConfigurationCardId{0}, max_outputs};
-
     resources.for_each_connector([&](DRMModeConnectorUPtr connector)
     {
         add_or_update_output(resources, *connector);
@@ -176,12 +166,9 @@
 {
     DisplayConfigurationOutputId id{static_cast<int>(connector.connector_id)};
     DisplayConfigurationCardId card_id{0};
-    DisplayConfigurationOutputType const type{
-        kms_connector_type_to_output_type(connector.connector_type)};
     geom::Size physical_size{connector.mmWidth, connector.mmHeight};
     bool connected{connector.connection == DRM_MODE_CONNECTED};
     size_t current_mode_index{std::numeric_limits<size_t>::max()};
-    size_t preferred_mode_index{std::numeric_limits<size_t>::max()};
     std::vector<DisplayConfigurationMode> modes;
     std::vector<geom::PixelFormat> formats {geom::PixelFormat::argb_8888,
                                             geom::PixelFormat::xrgb_8888};
@@ -197,7 +184,7 @@
             current_mode_info = crtc->mode;
     }
 
-    /* Add all the available modes and find the current and preferred one */
+    /* Add all the available modes and find the current one */
     for (int m = 0; m < connector.count_modes; m++)
     {
         drmModeModeInfo& mode_info = connector.modes[m];
@@ -210,9 +197,6 @@
 
         if (kms_modes_are_equal(mode_info, current_mode_info))
             current_mode_index = m;
-
-        if ((mode_info.type & DRM_MODE_TYPE_PREFERRED) == DRM_MODE_TYPE_PREFERRED)
-            preferred_mode_index = m;
     }
 
     /* Add or update the output */
@@ -220,16 +204,14 @@
 
     if (iter == outputs.end())
     {
-        outputs.push_back({id, card_id, type, formats, modes, preferred_mode_index,
-                           physical_size, connected, false, geom::Point(),
-                           current_mode_index, 0u});
+        outputs.push_back({id, card_id, formats, modes, physical_size,
+                           connected, false, geom::Point(), current_mode_index, 0u});
     }
     else
     {
         auto& output = *iter;
 
         output.modes = modes;
-        output.preferred_mode_index = preferred_mode_index;
         output.physical_size_mm = physical_size;
         output.connected = connected;
         output.current_mode_index = current_mode_index;

=== modified file 'src/server/graphics/gbm/real_kms_display_configuration.h'
--- src/server/graphics/gbm/real_kms_display_configuration.h	2013-08-12 08:58:59 +0000
+++ src/server/graphics/gbm/real_kms_display_configuration.h	2013-08-13 23:40:56 +0000
@@ -30,6 +30,8 @@
 namespace gbm
 {
 
+class DRMModeResources;
+
 class RealKMSDisplayConfiguration : public KMSDisplayConfiguration
 {
 public:
@@ -52,7 +54,6 @@
     std::vector<DisplayConfigurationOutput>::const_iterator find_output_with_id(DisplayConfigurationOutputId id) const;
 
     int drm_fd;
-    DisplayConfigurationCard card;
     std::vector<DisplayConfigurationOutput> outputs;
 };
 

=== modified file 'src/server/logging/message_processor_report.cpp'
--- src/server/logging/message_processor_report.cpp	2013-04-24 05:22:20 +0000
+++ src/server/logging/message_processor_report.cpp	2013-08-13 23:40:56 +0000
@@ -162,6 +162,9 @@
         mediators.erase(mediator);
 }
 
-
-
-
+void ml::MessageProcessorReport::sent_event(void const* mediator, MirSurfaceEvent const& event)
+{
+    std::ostringstream out;
+    out << "mediator=" << mediator << ", sent event, surface id=" << event.id;
+    log->log<Logger::debug>(out.str(), component);
+}

=== modified file 'src/server/lttng/message_processor_report.cpp'
--- src/server/lttng/message_processor_report.cpp	2013-06-03 12:15:44 +0000
+++ src/server/lttng/message_processor_report.cpp	2013-08-13 23:40:56 +0000
@@ -49,3 +49,8 @@
     void const* /*mediator*/, std::exception const& /*error*/)
 {
 }
+
+void mir::lttng::MessageProcessorReport::sent_event(
+    void const* /*mediator*/, MirSurfaceEvent const&)
+{
+}

=== modified file 'src/server/shell/default_focus_mechanism.cpp'
--- src/server/shell/default_focus_mechanism.cpp	2013-08-13 23:40:55 +0000
+++ src/server/shell/default_focus_mechanism.cpp	2013-08-13 23:40:56 +0000
@@ -22,89 +22,40 @@
 #include "mir/shell/session.h"
 #include "mir/shell/surface.h"
 #include "mir/shell/surface_controller.h"
-#include "mir/shell/session_listener.h"
-#include "mir/shell/focus_sequence.h"
 
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 
-msh::DefaultFocusMechanism::DefaultFocusMechanism(std::shared_ptr<msh::FocusSequence> const& sequence,
-                                                  std::shared_ptr<msh::InputTargeter> const& input_targeter,
-                                                  std::shared_ptr<msh::SurfaceController> const& surface_controller,
-                                                  std::shared_ptr<msh::SessionListener> const& session_listener)
-  : sequence(sequence),
-    session_listener(session_listener),
-    input_targeter(input_targeter),
-    surface_controller(surface_controller),
-    focus_session(nullptr)
-{ 
+msh::DefaultFocusMechanism::DefaultFocusMechanism(std::shared_ptr<msh::InputTargeter> const& input_targeter,
+                                                  std::shared_ptr<msh::SurfaceController> const& surface_controller)
+  : input_targeter(input_targeter),
+    surface_controller(surface_controller)
+{
 }
 
-void msh::DefaultFocusMechanism::set_focus(std::shared_ptr<msh::Session> const& session)
+void msh::DefaultFocusMechanism::set_focus_to(std::shared_ptr<Session> const& focus_session)
 {
-    focus_session = session;
-
+    // TODO: This path should be encapsulated in a seperate clear_focus message
+    if (!focus_session)
+    {
+        input_targeter->focus_cleared();
+        return;
+    }
+    
     auto surface = focus_session->default_surface();
     if (surface)
     {
+        auto current_focus = currently_focused_surface.lock();
+        if (current_focus)
+            current_focus->configure(mir_surface_attrib_focus, mir_surface_unfocused);
+        surface->configure(mir_surface_attrib_focus, mir_surface_focused);
+        currently_focused_surface = surface;
+        
         surface->raise(surface_controller);
         surface->take_input_focus(input_targeter);
-        session_listener->focused(focus_session);
     }
     else
     {
         input_targeter->focus_cleared();
-        session_listener->unfocused();
-    }
-}
-
-void msh::DefaultFocusMechanism::surface_created_for(std::shared_ptr<Session> const& session)
-{
-    std::unique_lock<std::mutex> lock(mutex);
-
-    //just set focus when a new surface is created
-    set_focus(session);
-}
-void msh::DefaultFocusMechanism::session_opened(std::shared_ptr<Session> const& session)
-{
-    std::unique_lock<std::mutex> lock(mutex);
-
-    //just set focus to new session for now
-    set_focus(session);
-}
-
-void msh::DefaultFocusMechanism::session_closed(std::shared_ptr<Session> const& session)
-{
-    std::unique_lock<std::mutex> lock(mutex);
-
-    //cycle through to next focus only if the currently-focused app is being closed 
-    if (focus_session == session)
-    {
-        focus_next_locked();
-    }
-}
-
-void msh::DefaultFocusMechanism::focus_next_locked()
-{
-    try
-    {
-        auto b = sequence->successor_of(focus_session);
-        set_focus(b);
-    } catch (std::logic_error)
-    {
-        input_targeter->focus_cleared();
-        session_listener->unfocused();
-    } 
-}
-
-void msh::DefaultFocusMechanism::focus_next()
-{
-    std::unique_lock<std::mutex> lock(mutex);
-    focus_next_locked();
-}
-
-std::weak_ptr<msh::Session> msh::DefaultFocusMechanism::focused_session() const
-{
-    std::unique_lock<std::mutex> lock(mutex);
-    return focus_session;
+    }
 }

=== modified file 'src/server/shell/mediating_display_changer.cpp'
--- src/server/shell/mediating_display_changer.cpp	2013-08-13 23:40:55 +0000
+++ src/server/shell/mediating_display_changer.cpp	2013-08-12 17:20:29 +0000
@@ -17,9 +17,6 @@
  */
 
 #include "mir/shell/mediating_display_changer.h"
-#include "mir/shell/focus_setter.h"
-#include "mir/shell/session.h"
-#include "mir/compositor/compositor.h"
 #include "mir/graphics/display.h"
 #include "mir/compositor/compositor.h"
 #include "mir/input/input_manager.h"
@@ -28,7 +25,6 @@
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
 namespace mg = mir::graphics;
-<<<<<<< TREE
 namespace mc = mir::compositor;
 namespace mi = mir::input;
 
@@ -115,64 +111,4 @@
     {
         display->configure(*conf);
     }
-=======
-namespace mc = mir::compositor;
-
-msh::MediatingDisplayChanger::MediatingDisplayChanger(
-    std::shared_ptr<mg::Display> const& display,
-    std::shared_ptr<mc::Compositor> const& compositor)
-    : display(display),
-      compositor(compositor),
-      base_config(display->configuration()),
-      active_config(base_config)
-{
-}
-
-std::shared_ptr<mg::DisplayConfiguration> msh::MediatingDisplayChanger::active_configuration()
-{
-    return active_config;
-}
-
-void msh::MediatingDisplayChanger::apply_config(
-    std::shared_ptr<mg::DisplayConfiguration> const& requested_configuration)
-{
-    compositor->stop();
-    display->configure(*requested_configuration);
-    active_config = requested_configuration;
-    compositor->start();
-}
-
-void msh::MediatingDisplayChanger::store_configuration_for(
-    std::weak_ptr<mf::Session> const& app,
-    std::shared_ptr<mg::DisplayConfiguration> const& requested_configuration)
-{
-    auto requesting_application = app.lock();
-    config_map[requesting_application.get()] = requested_configuration;
-}
-
-void msh::MediatingDisplayChanger::apply_configuration_of(std::weak_ptr<mf::Session> const& app)
-{
-    auto it = config_map.find(app.lock().get());
-    if (it == config_map.end())
-    {
-        apply_config(base_config);
-    }
-    else
-    {
-        apply_config(it->second);
-    }
-}
-
-void msh::MediatingDisplayChanger::remove_configuration_for(std::weak_ptr<mf::Session> const& app)
-{
-    auto it = config_map.find(app.lock().get());
-    if (it != config_map.end())
-    {
-        if (it->second == active_config)
-        {
-            apply_config(base_config);
-        }
-        config_map.erase(it);
-    }
->>>>>>> MERGE-SOURCE
 }

=== modified file 'src/server/shell/session_manager.cpp'
--- src/server/shell/session_manager.cpp	2013-08-13 23:40:55 +0000
+++ src/server/shell/session_manager.cpp	2013-08-13 23:40:56 +0000
@@ -18,7 +18,6 @@
 
 #include "mir/shell/session_manager.h"
 #include "mir/shell/application_session.h"
-#include "mir/shell/display_changer.h"
 #include "mir/shell/session_container.h"
 #include "mir/shell/surface_factory.h"
 #include "mir/shell/focus_sequence.h"
@@ -36,18 +35,19 @@
 
 msh::SessionManager::SessionManager(std::shared_ptr<msh::SurfaceFactory> const& surface_factory,
     std::shared_ptr<msh::SessionContainer> const& container,
+    std::shared_ptr<msh::FocusSequence> const& sequence,
     std::shared_ptr<msh::FocusSetter> const& focus_setter,
     std::shared_ptr<msh::SnapshotStrategy> const& snapshot_strategy,
-    std::shared_ptr<msh::SessionListener> const& session_listener,
-    std::shared_ptr<msh::DisplayChanger> const& display_changer) :
+    std::shared_ptr<msh::SessionListener> const& session_listener) :
     surface_factory(surface_factory),
     app_container(container),
+    focus_sequence(sequence),
     focus_setter(focus_setter),
     snapshot_strategy(snapshot_strategy),
-    session_listener(session_listener),
-    display_changer(display_changer)
+    session_listener(session_listener)
 {
     assert(surface_factory);
+    assert(sequence);
     assert(container);
     assert(focus_setter);
     assert(session_listener);
@@ -76,51 +76,92 @@
 std::shared_ptr<mf::Session> msh::SessionManager::open_session(std::string const& name,
                                                 std::shared_ptr<mf::EventSink> const& sender)
 {
-    auto new_session = std::make_shared<msh::ApplicationSession>(
+    std::shared_ptr<msh::Session> new_session =
+        std::make_shared<msh::ApplicationSession>(
             surface_factory, name, snapshot_strategy, session_listener, sender);
 
     app_container->insert_session(new_session);
+    
     session_listener->starting(new_session);
-    focus_setter->session_opened(new_session);
+
+    set_focus_to(new_session);
 
     return new_session;
 }
 
+inline void msh::SessionManager::set_focus_to_locked(std::unique_lock<std::mutex> const&, std::shared_ptr<Session> const& shell_session)
+{
+    auto old_focus = focus_application.lock();
+
+    focus_application = shell_session;
+
+    focus_setter->set_focus_to(shell_session);
+    if (shell_session)
+    {
+        session_listener->focused(shell_session);
+    }
+    else
+    {
+        session_listener->unfocused();
+    }
+}
+
+void msh::SessionManager::set_focus_to(std::shared_ptr<Session> const& shell_session)
+{
+    std::unique_lock<std::mutex> lg(mutex);
+    set_focus_to_locked(lg, shell_session);
+}
 
 void msh::SessionManager::close_session(std::shared_ptr<mf::Session> const& session)
 {
     auto shell_session = std::dynamic_pointer_cast<Session>(session);
 
+    session_listener->stopping(shell_session);
+
     app_container->remove_session(shell_session);
 
-    session_listener->stopping(shell_session);
-
-    focus_setter->session_closed(shell_session);
-}
-
-void msh::SessionManager::handle_display_configuration(std::shared_ptr<mf::Session> const& session) 
-{
-    auto shell_session = std::dynamic_pointer_cast<Session>(session);
-    if (focus_setter->focused_session().lock() == session)
-    {
-        display_changer->apply_configuration_of(session);
-    }
-}
-
-void msh::SessionManager::handle_surface_created(std::shared_ptr<mf::Session> const& session)
-{
-    auto shell_session = std::dynamic_pointer_cast<Session>(session);
-    focus_setter->surface_created_for(shell_session);
-}
-
-//TODO: this function is a kludge for not distributing a SessionMediator-like object to the internal clients.
-//      we should have a the_internal_client_mediator() in the server configuration that would act as a factory
-//      and mediator for internal clients, much like the mf::SessionMediator acts for ipc clients
+    std::unique_lock<std::mutex> lock(mutex);
+    set_focus_to_locked(lock, focus_sequence->default_focus());
+}
+
+void msh::SessionManager::focus_next()
+{
+    std::unique_lock<std::mutex> lock(mutex);
+    auto focus = focus_application.lock();
+    if (!focus)
+    {
+        focus = focus_sequence->default_focus();
+    }
+    else
+    {
+        focus = focus_sequence->successor_of(focus);
+    }
+    set_focus_to_locked(lock, focus);
+}
+
+std::weak_ptr<msh::Session> msh::SessionManager::focussed_application() const
+{
+    return focus_application;
+}
+
+// TODO: We use this to work around the lack of a SessionMediator-like object for internal clients.
+// we could have an internal client mediator which acts as a factory for internal clients, taking responsibility
+// for invoking handle_surface_created.
 mf::SurfaceId msh::SessionManager::create_surface_for(std::shared_ptr<mf::Session> const& session,
     msh::SurfaceCreationParameters const& params)
 {
     auto shell_session = std::dynamic_pointer_cast<Session>(session);
     auto id = shell_session->create_surface(params);
-    handle_surface_created(shell_session);
+    
+    handle_surface_created(session);
+
     return id;
 }
+
+void msh::SessionManager::handle_surface_created(std::shared_ptr<mf::Session> const& session)
+{
+    auto shell_session = std::dynamic_pointer_cast<Session>(session);
+
+    set_focus_to(shell_session);
+}
+                                                 

=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp	2013-08-09 03:39:35 +0000
+++ src/server/shell/surface.cpp	2013-08-13 23:40:56 +0000
@@ -242,6 +242,9 @@
             BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface state."));
         result = state();
         break;
+    case mir_surface_attrib_focus:
+        notify_change(attrib, value);
+        break;
     case mir_surface_attrib_swapinterval:
         allow_dropping = (value == 0);
         allow_framedropping(allow_dropping);
@@ -267,7 +270,7 @@
 {
     bool valid = false;
 
-    if (t >= 0 && t < mir_surface_type_arraysize_)
+    if (t >= 0 && t < mir_surface_type_enum_max_)
     {
         type_value = t;
         valid = true;
@@ -286,7 +289,7 @@
     bool valid = false;
 
     if (s > mir_surface_state_unknown &&
-        s < mir_surface_state_arraysize_)
+        s < mir_surface_state_enum_max_)
     {
         state_value = s;
         valid = true;

=== modified file 'src/server/shell/unauthorized_display_changer.cpp'
--- src/server/shell/unauthorized_display_changer.cpp	2013-08-13 23:40:55 +0000
+++ src/server/shell/unauthorized_display_changer.cpp	2013-08-12 17:20:29 +0000
@@ -33,17 +33,8 @@
     return changer->active_configuration();
 }
 
-void msh::UnauthorizedDisplayChanger::store_configuration_for(
+void msh::UnauthorizedDisplayChanger::configure(
     std::weak_ptr<mf::Session> const&, std::shared_ptr<mg::DisplayConfiguration> const&)
 {
     BOOST_THROW_EXCEPTION(std::runtime_error("not authorized to apply display configurations"));
 }
-
-void msh::UnauthorizedDisplayChanger::apply_configuration_of(std::weak_ptr<mf::Session> const&)
-{
-    BOOST_THROW_EXCEPTION(std::runtime_error("not authorized to apply display configurations"));
-}
-
-void msh::UnauthorizedDisplayChanger::remove_configuration_for(std::weak_ptr<mf::Session> const&)
-{
-}

=== modified file 'src/shared/protobuf/mir_protobuf.proto'
--- src/shared/protobuf/mir_protobuf.proto	2013-08-12 08:58:59 +0000
+++ src/shared/protobuf/mir_protobuf.proto	2013-08-13 23:40:56 +0000
@@ -44,11 +44,6 @@
   optional string error = 127;
 }
 
-message DisplayCard {
-    required uint32 card_id = 1;
-    required uint32 max_simultaneous_outputs = 2;
-}
-
 message DisplayMode {
     optional uint32 vertical_resolution = 1;
     optional uint32 horizontal_resolution = 2;
@@ -68,8 +63,6 @@
   optional uint32 used = 10;
   optional uint32 physical_width_mm = 11;
   optional uint32 physical_height_mm = 12;
-  optional uint32 type = 13;
-  optional uint32 preferred_mode = 14;
 }
 
 // DEPRECATED
@@ -154,7 +147,6 @@
 
 message DisplayConfiguration {
   repeated DisplayOutput display_output = 1;
-  repeated DisplayCard   display_card = 2;
   optional string error = 127;
 }
 

=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2013-08-08 02:34:31 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2013-08-13 23:40:56 +0000
@@ -10,6 +10,7 @@
   test_test_framework.cpp
   test_focus_selection.cpp
   test_server_shutdown.cpp
+  test_client_focus_notification.cpp
   test_client_authorization.cpp
   test_shell_control_of_surface_configuration.cpp
   test_nested_mir.cpp

=== added file 'tests/acceptance-tests/test_client_focus_notification.cpp'
--- tests/acceptance-tests/test_client_focus_notification.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_client_focus_notification.cpp	2013-08-13 23:40:56 +0000
@@ -0,0 +1,231 @@
+/*
+ * 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: Robert Carr <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+
+#include "mir_test/wait_condition.h"
+#include "mir_test/event_matchers.h"
+
+#include "mir_test_framework/display_server_test_fixture.h"
+#include "mir_test_framework/cross_process_sync.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace mt = mir::test;
+namespace mtf = mir_test_framework;
+
+namespace
+{
+    char const* const mir_test_socket = mtf::test_socket_file().c_str();
+}
+
+namespace
+{
+struct ClientConfigCommon : TestingClientConfiguration
+{
+    ClientConfigCommon() :
+        connection(0),
+        surface(0)
+    {
+    }
+    static void connection_callback(MirConnection* connection, void* context)
+    {
+        ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
+        config->connection = connection;
+    }
+    static void create_surface_callback(MirSurface* surface, void* context)
+    {
+        ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
+        config->surface_created(surface);
+    }
+    static void release_surface_callback(MirSurface* surface, void* context)
+    {
+        ClientConfigCommon* config = reinterpret_cast<ClientConfigCommon *>(context);
+        config->surface_released(surface);
+    }
+    virtual void connected(MirConnection* new_connection)
+    {
+        connection = new_connection;
+    }
+    virtual void surface_created(MirSurface* new_surface)
+    {
+        surface = new_surface;
+    }
+    virtual void surface_released(MirSurface* /* released_surface */)
+    {
+        surface = nullptr;
+    }
+    MirConnection* connection;
+    MirSurface* surface;
+};
+
+struct MockEventObserver
+{
+    MOCK_METHOD1(see, void(MirEvent const*));
+};
+
+struct EventObservingClient : ClientConfigCommon
+{
+    EventObservingClient()
+        : observer(std::make_shared<MockEventObserver>())
+    {
+    }
+
+    static void handle_event(MirSurface* /* surface */, MirEvent const* ev, void* context)
+    {
+        auto client = static_cast<EventObservingClient *>(context);
+        client->observer->see(ev);
+    }
+ 
+    virtual void expect_events(mt::WaitCondition* /* all_events_received */) = 0;
+
+    void surface_created(MirSurface *surface_) override
+    {
+        // We need to set the event delegate from the surface_created
+        // callback so we can block the reading of new events
+        // until we are ready
+        MirEventDelegate const event_delegate =
+            {
+                handle_event,
+                this
+            };
+        surface = surface_;
+        mir_surface_set_event_handler(surface, &event_delegate);
+    }
+
+    void exec()
+    {
+        mt::WaitCondition all_events_received;
+        expect_events(&all_events_received);
+        mir_wait_for(mir_connect(mir_test_socket,
+            __PRETTY_FUNCTION__, connection_callback, this));
+        ASSERT_TRUE(connection != NULL);
+        MirSurfaceParameters const request_params =
+            {
+                __PRETTY_FUNCTION__,
+                surface_width, surface_height,
+                mir_pixel_format_abgr_8888,
+                mir_buffer_usage_hardware
+            };
+        mir_wait_for(mir_connection_create_surface(connection, &request_params, create_surface_callback, this));
+        all_events_received.wait_for_at_most_seconds(60);
+        mir_surface_release_sync(surface);
+        mir_connection_release(connection);
+
+        // The ClientConfig is not destroyed before the testing process 
+        // exits.
+        observer.reset();
+    }
+    std::shared_ptr<MockEventObserver> observer;
+    static int const surface_width = 100;
+    static int const surface_height = 100;
+};
+
+}
+
+TEST_F(BespokeDisplayServerTestFixture, a_surface_is_notified_of_receiving_focus)
+{
+    using namespace ::testing;
+
+    TestingServerConfiguration server_config;
+    launch_server_process(server_config);
+
+    struct FocusObservingClient : public EventObservingClient
+    {
+        void expect_events(mt::WaitCondition* all_events_received) override
+        {
+            EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_focused)))).Times(1)
+                .WillOnce(mt::WakeUp(all_events_received));
+        }
+    } client_config;
+    launch_client_process(client_config);
+}
+
+namespace
+{
+
+ACTION_P(SignalFence, fence)
+{
+    fence->try_signal_ready_for();
+}
+
+}
+
+TEST_F(BespokeDisplayServerTestFixture, two_surfaces_are_notified_of_gaining_and_losing_focus)
+{
+    using namespace ::testing;
+    
+    TestingServerConfiguration server_config;
+    launch_server_process(server_config);
+
+    // We use this for synchronization to ensure the two clients
+    // are launched in a defined order.
+    mtf::CrossProcessSync ready_for_second_client;
+
+    struct FocusObservingClientOne : public EventObservingClient
+    {
+        mtf::CrossProcessSync ready_for_second_client;
+        FocusObservingClientOne(mtf::CrossProcessSync const& ready_for_second_client)
+            : ready_for_second_client(ready_for_second_client)
+        {
+        }
+
+        void expect_events(mt::WaitCondition* all_events_received) override
+        {
+            InSequence seq;
+            // We should receive focus as we are created
+            EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus,
+                mir_surface_focused)))).Times(1)
+                    .WillOnce(SignalFence(&ready_for_second_client));
+
+            // And lose it as the second surface is created
+            EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus,
+                                                                        mir_surface_unfocused)))).Times(1);
+            // And regain it when the second surface is closed
+            EXPECT_CALL(*observer, see(Pointee(mt::SurfaceEvent(mir_surface_attrib_focus,
+                                                                        mir_surface_focused)))).Times(1).WillOnce(mt::WakeUp(all_events_received));
+        }
+
+    } client_one_config(ready_for_second_client);
+    launch_client_process(client_one_config);
+
+    struct FocusObservingClientTwo : public EventObservingClient
+    {
+        mtf::CrossProcessSync ready_for_second_client;
+        FocusObservingClientTwo(mtf::CrossProcessSync const& ready_for_second_client)
+            : ready_for_second_client(ready_for_second_client)
+        {
+        }
+        void exec() override
+        {
+            // We need some synchronization to ensure client two does not connect before client one.
+            ready_for_second_client.wait_for_signal_ready_for();
+            EventObservingClient::exec();
+        }
+
+        void expect_events(mt::WaitCondition* all_events_received) override
+        {
+            EXPECT_CALL(*observer, see(Pointee(
+                mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_focused))))
+                    .Times(1).WillOnce(mt::WakeUp(all_events_received));
+        }
+    } client_two_config(ready_for_second_client);
+
+    launch_client_process(client_two_config);
+}

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2013-08-13 23:40:55 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2013-08-13 23:40:56 +0000
@@ -149,12 +149,15 @@
         {
             std::cout << e.what() << std::endl;
         }
+
     }
 
     static void handle_input(MirSurface* /* surface */, MirEvent const* ev, void* context)
     {
+        if (ev->type == mir_event_type_surface)
+            return;
+
         auto client = static_cast<InputClient *>(context);
-
         client->handler->handle_input(ev);
     }
 
@@ -889,7 +892,6 @@
             fake_event_hub->synthesize_event(mis::a_motion_event().with_movement(1,1));
         }
     } server_config{fence, second_client_done_fence};
-
     launch_server_process(server_config);
     
     struct ButtonClientOne : InputClient
@@ -899,10 +901,6 @@
         {
         }
         
-        void exec()
-        {
-            InputClient::exec();
-        }
         void expect_input(mt::WaitCondition& events_received) override
         {
             EXPECT_CALL(*handler, handle_input(HoverEnterEvent())).Times(AnyNumber());

=== modified file 'tests/acceptance-tests/test_focus_selection.cpp'
--- tests/acceptance-tests/test_focus_selection.cpp	2013-08-13 23:40:55 +0000
+++ tests/acceptance-tests/test_focus_selection.cpp	2013-08-13 23:40:56 +0000
@@ -38,6 +38,7 @@
 namespace mi = mir::input;
 namespace mt = mir::test;
 namespace mtd = mt::doubles;
+namespace mt = mir::test;
 namespace mtf = mir_test_framework;
 
 namespace
@@ -117,6 +118,14 @@
 
 }
 
+namespace
+{
+MATCHER(NonNullSession, "")
+{
+    return arg != std::shared_ptr<msh::Session>();
+}
+}
+
 TEST_F(BespokeDisplayServerTestFixture, sessions_creating_surface_receive_focus)
 {
     struct ServerConfig : TestingServerConfiguration
@@ -131,14 +140,11 @@
 
                 auto focus_setter = std::make_shared<mtd::MockFocusSetter>();
                 {
-                    Sequence seq;
-
-                    EXPECT_CALL(*focus_setter, session_opened(_))
-                        .InSequence(seq);
-                    EXPECT_CALL(*focus_setter, surface_created_for(_))
-                        .InSequence(seq);
-                    EXPECT_CALL(*focus_setter, session_closed(_))
-                        .InSequence(seq);
+                    InSequence seq;
+                    // Once on application registration and once on surface creation
+                    EXPECT_CALL(*focus_setter, set_focus_to(NonNullSession())).Times(2);
+                    // Focus is cleared when the session is closed
+                    EXPECT_CALL(*focus_setter, set_focus_to(_)).Times(1);
                 }
                 // TODO: Counterexample ~racarr
 
@@ -174,17 +180,14 @@
 
             if (!expected)
             {
-                Sequence seq;
-                //connect session
-                EXPECT_CALL(*targeter, focus_cleared())
-                    .InSequence(seq);
-                //session has a default surface
-                EXPECT_CALL(*targeter, focus_changed(_))
-                    .InSequence(seq);
-                //session is destroyed
-                EXPECT_CALL(*targeter, focus_cleared())
-                    .InSequence(seq);
-                expected = true;
+                
+                EXPECT_CALL(*targeter, focus_cleared()).Times(AtLeast(0));
+
+                {
+                    InSequence seq;
+                    EXPECT_CALL(*targeter, focus_changed(_)).Times(1);
+                    expected = true;
+                }
             }
 
             return targeter;

=== modified file 'tests/acceptance-tests/test_shell_control_of_surface_configuration.cpp'
--- tests/acceptance-tests/test_shell_control_of_surface_configuration.cpp	2013-07-31 22:17:06 +0000
+++ tests/acceptance-tests/test_shell_control_of_surface_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -81,6 +81,9 @@
         void exec() override
         {
             using namespace ::testing;
+            
+            EXPECT_CALL(mock_configurator, select_attribute_value(_, mir_surface_attrib_focus, _)).Times(AnyNumber());
+            EXPECT_CALL(mock_configurator, attribute_set(_, mir_surface_attrib_focus, _)).Times(AnyNumber());
 
             ON_CALL(mock_configurator, select_attribute_value(_, _, _)).WillByDefault(Return(mir_surface_type_freestyle));
             EXPECT_CALL(mock_configurator, select_attribute_value(_, mir_surface_attrib_type, Eq(mir_surface_type_freestyle))).Times(1);
@@ -116,6 +119,10 @@
         void exec() override
         {
             using namespace ::testing;
+
+            EXPECT_CALL(mock_configurator, select_attribute_value(_, mir_surface_attrib_focus, _)).Times(AnyNumber());
+            EXPECT_CALL(mock_configurator, attribute_set(_, mir_surface_attrib_focus, _)).Times(AnyNumber());
+
             EXPECT_CALL(mock_configurator, select_attribute_value(_, mir_surface_attrib_type, Eq(mir_surface_type_freestyle))).Times(1)
                 .WillOnce(Return(mir_surface_type_normal));
             EXPECT_CALL(mock_configurator, attribute_set(_, mir_surface_attrib_type, Eq(mir_surface_type_normal))).Times(1);

=== modified file 'tests/integration-tests/CMakeLists.txt'
--- tests/integration-tests/CMakeLists.txt	2013-08-12 08:58:58 +0000
+++ tests/integration-tests/CMakeLists.txt	2013-08-13 23:40:56 +0000
@@ -8,7 +8,6 @@
   test_display_server_main_loop_events.cpp
   test_surface_first_frame_sync.cpp
   test_swapinterval.cpp
-  test_server_client_types.cpp
 )
 
 add_subdirectory(client/)

=== modified file 'tests/integration-tests/shell/CMakeLists.txt'
--- tests/integration-tests/shell/CMakeLists.txt	2013-08-13 23:40:55 +0000
+++ tests/integration-tests/shell/CMakeLists.txt	2013-06-25 15:20:05 +0000
@@ -1,6 +1,6 @@
 list(
   APPEND INTEGRATION_TESTS_SRCS
-#  ${CMAKE_CURRENT_SOURCE_DIR}/test_session_manager.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_session_manager.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_session.cpp
 )
 

=== modified file 'tests/integration-tests/test_display_info.cpp'
--- tests/integration-tests/test_display_info.cpp	2013-08-13 23:40:55 +0000
+++ tests/integration-tests/test_display_info.cpp	2013-08-12 17:20:29 +0000
@@ -25,7 +25,6 @@
 #include "mir/frontend/session_mediator.h"
 #include "mir/frontend/global_event_sender.h"
 
-#include "mir_test_framework/cross_process_sync.h"
 #include "mir_test_framework/display_server_test_fixture.h"
 #include "mir_test_framework/cross_process_sync.h"
 #include "mir_test_doubles/stub_buffer.h"
@@ -298,12 +297,13 @@
     launch_client_process(client_config);
 }
 
+
 namespace
 {
 class EventSinkSkimmingIpcFactory : public mf::ProtobufIpcFactory
 {
 public:
-    explicit EventSinkSkimmingIpcFactory(
+    EventSinkSkimmingIpcFactory(
         std::shared_ptr<mf::Shell> const& shell,
         std::shared_ptr<mf::SessionMediatorReport> const& sm_report,
         std::shared_ptr<mf::MessageProcessorReport> const& mr_report,
@@ -321,10 +321,11 @@
     {
     }
 
-    void last_clients_event_sink()
+    std::shared_ptr<mf::EventSink> last_clients_event_sink()
     {
-        last_event_sink.lock()->handle_display_config_change(StubChanger::stub_display_config);
+        return std::move(last_event_sink);
     }
+
 private:
     std::shared_ptr<mf::Shell> shell;
     std::shared_ptr<mf::SessionMediatorReport> const sm_report;
@@ -333,7 +334,7 @@
     std::shared_ptr<mg::Platform> const graphics_platform;
     std::shared_ptr<mf::DisplayChanger> const graphics_changer;
     std::shared_ptr<mg::GraphicBufferAllocator> const buffer_allocator;
-    std::weak_ptr<mf::EventSink> last_event_sink;
+    std::shared_ptr<mf::EventSink> last_event_sink;
 
     virtual std::shared_ptr<mir::protobuf::DisplayServer> make_ipc_server(
         std::shared_ptr<mf::EventSink> const& sink, bool)
@@ -358,86 +359,12 @@
     {
         return mp_report;
     }
-};
-
-class ConfigureNotifyingDisplay : public mtd::NullDisplay
-{
-public:
-    ConfigureNotifyingDisplay(mtf::CrossProcessSync const& configure_fence)
-        : configure_fence(configure_fence)
-    {
-    }
-
-    std::shared_ptr<mg::DisplayConfiguration> configuration() override
-    {
-        return std::make_shared<mtd::StubDisplayConfig>();
-    }
-
-    void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
-    {
-        f(display_buffer);
-    }
-
-    void configure(mg::DisplayConfiguration const&) override
-    {
-        configure_fence.try_signal_ready_for(std::chrono::milliseconds(1000));
-    }
-private:
-    mtd::NullDisplayBuffer display_buffer;
-    mtf::CrossProcessSync configure_fence;
+
 };
 
 mtd::StubDisplayConfig changed_stub_display_config{1};
 }
 
-TEST_F(BespokeDisplayServerTestFixture, display_change_request_for_authorized_client)
-{
-    mtf::CrossProcessSync server_configure_fence;
-
-    struct ServerConfig : TestingServerConfiguration
-    {
-        ServerConfig(mtf::CrossProcessSync const& fence)
-            : configure_fence(fence)
-        {
-        }
-
-        std::shared_ptr<mg::Display> the_display()
-        {
-            if (!display)
-                display = std::make_shared<ConfigureNotifyingDisplay>(configure_fence);
-            return display; 
-        }
-
-        std::shared_ptr<ConfigureNotifyingDisplay> display;
-        mtf::CrossProcessSync configure_fence;
-
-    } server_config(server_configure_fence);
-
-    launch_server_process(server_config);
-    
-    struct Client : TestingClientConfiguration
-    {
-        void exec()
-        {
-            auto connection = mir_connect_sync(mir_test_socket, __PRETTY_FUNCTION__);
-            auto configuration = mir_connection_create_display_config(connection);
-
-            mir_wait_for(mir_connection_apply_display_config(connection, configuration));
-            EXPECT_STREQ(mir_connection_get_error_message(connection),  "");
-            mir_display_config_destroy(configuration);
-
-            mir_connection_release(connection);
-        }
-    } client_config;
-
-    launch_client_process(client_config);
-
-    run_in_test_process([&]
-    {
-        server_configure_fence.wait_for_signal_ready_for(std::chrono::milliseconds(1000));
-    });
-} 
-
 TEST_F(BespokeDisplayServerTestFixture, display_change_notification_reaches_all_clients)
 {
     mtf::CrossProcessSync client_ready_fence;
@@ -462,31 +389,12 @@
             return platform;
         }
 
-        std::shared_ptr<mir::frontend::ProtobufIpcFactory> the_ipc_factory(
-            std::shared_ptr<mf::Shell> const& shell,
-            std::shared_ptr<mg::GraphicBufferAllocator> const& allocator) override
-        {
-            return ipc_factory([&]() {
-                return std::make_shared<EventSinkSkimmingIpcFactory>(
-                                shell,
-                                the_session_mediator_report(),
-                                the_message_processor_report(),
-                                the_graphics_platform(),
-                                the_shell_display_changer(), allocator); });
-        }
-
         void exec() override
         {
             change_thread = std::move(std::thread([this](){
                 auto global_sender = the_global_event_sink();
 
                 send_event_fence.wait_for_signal_ready_for(std::chrono::milliseconds(1000));
-                auto factory = ipc_factory([&]() { return nullptr; });
-                if (factory)
-                {
-                    factory->last_clients_event_sink();
-                } 
-
                 global_sender->handle_display_config_change(changed_stub_display_config);
                 events_sent.try_signal_ready_for(std::chrono::milliseconds(1000));
             })); 
@@ -498,7 +406,6 @@
         }
 
         mtf::CrossProcessSync send_event_fence;
-        mir::CachedPtr<EventSinkSkimmingIpcFactory> ipc_factory;
         mtf::CrossProcessSync events_sent;
         std::shared_ptr<StubPlatform> platform;
         std::thread change_thread;
@@ -539,13 +446,12 @@
 
             client_ready_fence.try_signal_ready_for(std::chrono::milliseconds(1000));
 
-     //       while(!callback_called)
-     //       {
-      //          callback_wait.wait(lk);
-       //     } 
+            while(!callback_called)
+            {
+                callback_wait.wait(lk);
+            } 
 
             mir_connection_release(connection);
-            client_ready_fence.try_signal_ready_for(std::chrono::milliseconds(1000));
         }
 
         mtf::CrossProcessSync client_ready_fence;
@@ -610,7 +516,7 @@
         unsubscribed_client_ready_fence.wait_for_signal_ready_for(std::chrono::milliseconds(1000));
 
         send_event_fence.try_signal_ready_for(std::chrono::milliseconds(1000));
-        client_ready_fence.wait_for_signal_ready_for(std::chrono::milliseconds(1000));
+        events_all_sent.wait_for_signal_ready_for(std::chrono::milliseconds(1000));
 
         unsubscribed_check_fence.try_signal_ready_for(std::chrono::milliseconds(1000));
     });

=== removed file 'tests/integration-tests/test_server_client_types.cpp'
--- tests/integration-tests/test_server_client_types.cpp	2013-08-12 08:58:58 +0000
+++ tests/integration-tests/test_server_client_types.cpp	1970-01-01 00:00:00 +0000
@@ -1,46 +0,0 @@
-/*
- * 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: Alexandros Frantzis <[e-mail]>
- */
-
-#include "mir_toolkit/client_types.h"
-#include "mir/graphics/display_configuration.h"
-
-#include <gtest/gtest.h>
-
-namespace mg = mir::graphics;
-
-#define EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(TYPE) \
-    EXPECT_EQ(static_cast<mg::DisplayConfigurationOutputType>(mir_display_output_type_##TYPE), \
-              mg::DisplayConfigurationOutputType::TYPE)
-
-TEST(ServerClientTypes, display_output_types_match)
-{
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(unknown);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(vga);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(dvid);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(dvia);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(composite);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(svideo);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(lvds);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(component);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(ninepindin);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(displayport);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(hdmia);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(hdmib);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(tv);
-    EXPECT_DISPLAY_OUTPUT_TYPES_MATCH(edp);
-}

=== modified file 'tests/mir_test/display_config_matchers.cpp'
--- tests/mir_test/display_config_matchers.cpp	2013-08-12 11:40:45 +0000
+++ tests/mir_test/display_config_matchers.cpp	2013-08-13 23:40:56 +0000
@@ -35,20 +35,6 @@
 public:
     TestDisplayConfiguration(mp::DisplayConfiguration const& protobuf_config)
     {
-        /* Cards */
-        for (int i = 0; i < protobuf_config.display_card_size(); i++)
-        {
-            auto const& protobuf_card = protobuf_config.display_card(i);
-            mg::DisplayConfigurationCard display_card
-            {
-                mg::DisplayConfigurationCardId(protobuf_card.card_id()),
-                protobuf_card.max_simultaneous_outputs(),
-            };
-
-            cards.push_back(display_card);
-        }
-
-        /* Outputs */
         for (int i = 0; i < protobuf_config.display_output_size(); i++)
         {
             auto const& protobuf_output = protobuf_config.display_output(i);
@@ -56,10 +42,8 @@
             {
                 mg::DisplayConfigurationOutputId(protobuf_output.output_id()),
                 mg::DisplayConfigurationCardId(protobuf_output.card_id()),
-                static_cast<mg::DisplayConfigurationOutputType>(protobuf_output.type()),
-                {},
-                {},
-                protobuf_output.preferred_mode(),
+                {},
+                {},
                 geom::Size{protobuf_output.physical_width_mm(),
                            protobuf_output.physical_height_mm()},
                 static_cast<bool>(protobuf_output.connected()),
@@ -99,20 +83,6 @@
 
     TestDisplayConfiguration(MirDisplayConfiguration const& client_config)
     {
-        /* Cards */
-        for (size_t i = 0; i < client_config.num_cards; i++)
-        {
-            auto const& client_card = client_config.cards[i];
-            mg::DisplayConfigurationCard display_card
-            {
-                mg::DisplayConfigurationCardId(client_card.card_id),
-                client_card.max_simultaneous_outputs,
-            };
-
-            cards.push_back(display_card);
-        }
-
-        /* Outputs */
         for (size_t i = 0; i < client_config.num_displays; i++)
         {
             auto const& client_output = client_config.displays[i];
@@ -120,10 +90,8 @@
             {
                 mg::DisplayConfigurationOutputId(client_output.output_id),
                 mg::DisplayConfigurationCardId(client_output.card_id),
-                static_cast<mg::DisplayConfigurationOutputType>(client_output.type),
-                {},
-                {},
-                client_output.preferred_mode,
+                {},
+                {},
                 geom::Size{client_output.physical_width_mm,
                            client_output.physical_height_mm},
                 static_cast<bool>(client_output.connected),
@@ -161,10 +129,8 @@
         }
     }
 
-    void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
+    void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)>) const
     {
-        for (auto const& card : cards)
-            f(card);
     }
 
     void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
@@ -179,48 +145,12 @@
     }
 
 private:
-    std::vector<mg::DisplayConfigurationCard> cards;
     std::vector<mg::DisplayConfigurationOutput> outputs;
 };
 
 bool compare_display_configurations(mg::DisplayConfiguration const& config1,
                                     mg::DisplayConfiguration const& config2)
 {
-    bool result = true;
-
-    /* cards */
-    std::vector<mg::DisplayConfigurationCard> cards1;
-    std::vector<mg::DisplayConfigurationCard> cards2;
-
-    config1.for_each_card(
-        [&cards1](mg::DisplayConfigurationCard const& card)
-        {
-            cards1.push_back(card);
-        });
-    config2.for_each_card(
-        [&cards2](mg::DisplayConfigurationCard const& card)
-        {
-            cards2.push_back(card);
-        });
-
-    if (cards1.size() == cards2.size())
-    {
-        for (size_t i = 0; i < cards1.size(); i++)
-        {
-            if (cards1[i] != cards2[i])
-            {
-                EXPECT_EQ(cards1[i], cards2[i]);
-                result = false;
-            }
-        }
-    }
-    else
-    {
-        EXPECT_EQ(cards1.size(), cards2.size());
-        result = false;
-    }
-
-    /* Outputs */
     std::vector<mg::DisplayConfigurationOutput> outputs1;
     std::vector<mg::DisplayConfigurationOutput> outputs2;
 
@@ -235,21 +165,21 @@
             outputs2.push_back(output);
         });
 
-    if (outputs1.size() == outputs2.size())
-    {
-        for (size_t i = 0; i < outputs1.size(); i++)
-        {
-            if (outputs1[i] != outputs2[i])
-            {
-                EXPECT_EQ(outputs1[i], outputs2[i]);
-                result = false;
-            }
-        }
-    }
-    else
+    if (outputs1.size() != outputs2.size())
     {
         EXPECT_EQ(outputs1.size(), outputs2.size());
-        result = false;
+        return false;
+    }
+
+    bool result = true;
+
+    for (size_t i = 0; i < outputs1.size(); i++)
+    {
+        if (outputs1[i] != outputs2[i])
+        {
+            EXPECT_EQ(outputs1[i], outputs2[i]);
+            result = false;
+        }
     }
 
     return result;

=== modified file 'tests/mir_test_doubles/mock_drm.cpp'
--- tests/mir_test_doubles/mock_drm.cpp	2013-08-12 08:58:58 +0000
+++ tests/mir_test_doubles/mock_drm.cpp	2013-08-13 23:40:56 +0000
@@ -50,8 +50,8 @@
     uint32_t const connector1_id{31};
     uint32_t const all_crtcs_mask{0x3};
 
-    modes.push_back(create_mode(1920, 1080, 138500, 2080, 1111, PreferredMode));
-    modes.push_back(create_mode(832, 624, 57284, 1152, 667, NormalMode));
+    modes.push_back(create_mode(1920, 1080, 138500, 2080, 1111));
+    modes.push_back(create_mode(832, 624, 57284, 1152, 667));
     connector_encoder_ids.push_back(encoder0_id);
     connector_encoder_ids.push_back(encoder1_id);
 
@@ -61,11 +61,9 @@
     add_encoder(encoder0_id, invalid_id, all_crtcs_mask);
     add_encoder(encoder1_id, crtc1_id, all_crtcs_mask);
 
-    add_connector(connector0_id, DRM_MODE_CONNECTOR_VGA,
-                  DRM_MODE_DISCONNECTED, invalid_id,
+    add_connector(connector0_id, DRM_MODE_DISCONNECTED, invalid_id,
                   modes_empty, connector_encoder_ids, geom::Size());
-    add_connector(connector1_id, DRM_MODE_CONNECTOR_DVID,
-                  DRM_MODE_CONNECTED, encoder1_id,
+    add_connector(connector1_id, DRM_MODE_CONNECTED, encoder1_id,
                   modes, connector_encoder_ids,
                   geom::Size{121, 144});
 
@@ -150,7 +148,6 @@
 }
 
 void mgg::FakeDRMResources::add_connector(uint32_t connector_id,
-                                          uint32_t type,
                                           drmModeConnection connection,
                                           uint32_t encoder_id,
                                           std::vector<drmModeModeInfo>& modes,
@@ -160,7 +157,6 @@
     drmModeConnector connector = drmModeConnector();
 
     connector.connector_id = connector_id;
-    connector.connector_type = type;
     connector.connection = connection;
     connector.encoder_id = encoder_id;
     connector.modes = modes.data();
@@ -206,8 +202,7 @@
 
 drmModeModeInfo mgg::FakeDRMResources::create_mode(uint16_t hdisplay, uint16_t vdisplay,
                                                    uint32_t clock, uint16_t htotal,
-                                                   uint16_t vtotal,
-                                                   ModePreference preferred)
+                                                   uint16_t vtotal)
 {
     drmModeModeInfo mode = drmModeModeInfo();
 
@@ -216,8 +211,6 @@
     mode.clock = clock;
     mode.htotal = htotal;
     mode.vtotal = vtotal;
-    if (preferred)
-        mode.type |= DRM_MODE_TYPE_PREFERRED;
 
     return mode;
 }

=== modified file 'tests/mir_test_framework/input_testing_server_options.cpp'
--- tests/mir_test_framework/input_testing_server_options.cpp	2013-08-07 07:18:03 +0000
+++ tests/mir_test_framework/input_testing_server_options.cpp	2013-08-13 23:40:56 +0000
@@ -21,6 +21,9 @@
 #include "mir/input/input_channel.h"
 #include "mir/surfaces/input_registrar.h"
 #include "mir/input/surface.h"
+#include "mir/shell/surface_creation_parameters.h"
+#include "mir/frontend/shell.h"
+#include "mir/frontend/session.h"
 #include "mir/input/composite_event_filter.h"
 
 #include "mir_test/fake_event_hub.h"
@@ -34,6 +37,8 @@
 namespace mtf = mir_test_framework;
 
 namespace ms = mir::surfaces;
+namespace msh = mir::shell;
+namespace mf = mir::frontend;
 namespace mg = mir::graphics;
 namespace mi = mir::input;
 namespace mia = mi::android;
@@ -48,7 +53,6 @@
     virtual ~SurfaceReadinessListener() = default;
     
     virtual void channel_ready_for_input(std::string const& channel_name) = 0;
-    virtual void channel_finished_for_input(std::string const& channel_name) = 0;
 
 protected:
     SurfaceReadinessListener() = default;
@@ -56,36 +60,43 @@
     SurfaceReadinessListener& operator=(SurfaceReadinessListener const&) = delete;
 };
 
-class ProxyInputRegistrar : public ms::InputRegistrar
+class ProxyShell : public mf::Shell
 {
 public:
-    ProxyInputRegistrar(std::shared_ptr<ms::InputRegistrar> const underlying_registrar,
-                        std::shared_ptr<SurfaceReadinessListener> const listener)
-        : underlying_registrar(underlying_registrar),
+    ProxyShell(std::shared_ptr<mf::Shell> const& underlying_shell,
+               std::shared_ptr<SurfaceReadinessListener> const listener)
+        : underlying_shell(underlying_shell),
           listener(listener)
     {   
     }
 
-    ~ProxyInputRegistrar() noexcept(true) = default;
-    
-    void input_channel_opened(std::shared_ptr<mi::InputChannel> const& opened_channel,
-                              std::shared_ptr<mi::Surface> const& surface,
-                              mi::InputReceptionMode input_mode)
-    {
-        outstanding_channels[opened_channel] = surface->name();
-        underlying_registrar->input_channel_opened(opened_channel, surface, input_mode);
-        listener->channel_ready_for_input(surface->name());
-    }
-    void input_channel_closed(std::shared_ptr<mi::InputChannel> const& closed_channel)
-    {
-        auto channel = outstanding_channels[closed_channel];
-        underlying_registrar->input_channel_closed(closed_channel);
-        listener->channel_finished_for_input(channel);
+    ~ProxyShell() noexcept(true) = default;
+    
+    mf::SurfaceId create_surface_for(std::shared_ptr<mf::Session> const& session,
+        msh::SurfaceCreationParameters const& params)
+    {
+        return underlying_shell->create_surface_for(session, params);
+    }
+
+    std::shared_ptr<mf::Session> open_session(std::string const& name, 
+                                              std::shared_ptr<mf::EventSink> const& sink)
+    {
+        return underlying_shell->open_session(name, sink);
+    }
+
+    void close_session(std::shared_ptr<mf::Session> const& session)
+    {
+        underlying_shell->close_session(session);
+    }
+    
+    void handle_surface_created(std::shared_ptr<mf::Session> const& session)
+    {
+        underlying_shell->handle_surface_created(session);
+        listener->channel_ready_for_input(session->name());
     }
 
 private:
-    std::map<std::shared_ptr<mi::InputChannel>, std::string> outstanding_channels;
-    std::shared_ptr<ms::InputRegistrar> const underlying_registrar;
+    std::shared_ptr<mf::Shell> const underlying_shell;
     std::shared_ptr<SurfaceReadinessListener> const listener;
 };
 
@@ -126,7 +137,7 @@
     return input_configuration;
 }
 
-std::shared_ptr<ms::InputRegistrar> mtf::InputTestingServerConfiguration::the_input_registrar()
+std::shared_ptr<mf::Shell> mtf::InputTestingServerConfiguration::the_frontend_shell()
 {
     struct LifecycleTracker : public SurfaceReadinessListener
     {
@@ -145,27 +156,20 @@
             lifecycle_condition.notify_all();
         }
 
-        void channel_finished_for_input(std::string const& channel_name)
-        {
-            std::unique_lock<std::mutex> lg(lifecycle_lock);
-            client_lifecycles[channel_name] = mtf::ClientLifecycleState::vanished;
-            lifecycle_condition.notify_all();
-        }
         std::mutex &lifecycle_lock;
         std::condition_variable &lifecycle_condition;
         std::map<std::string, mtf::ClientLifecycleState> &client_lifecycles;
     };
     
-    if (!input_registrar)
+    if (!frontend_shell)
     {
-        auto registrar_listener = std::make_shared<LifecycleTracker>(lifecycle_lock,
+        auto readiness_listener = std::make_shared<LifecycleTracker>(lifecycle_lock,
             lifecycle_condition,
             client_lifecycles);
-        input_registrar = std::make_shared<ProxyInputRegistrar>(the_input_configuration()->the_input_registrar(),
-            registrar_listener);
+        frontend_shell = std::make_shared<ProxyShell>(DefaultServerConfiguration::the_frontend_shell(), readiness_listener);
     }
 
-    return input_registrar;
+    return frontend_shell;
 }
 
 void mtf::InputTestingServerConfiguration::wait_until_client_appears(std::string const& channel_name)
@@ -185,22 +189,3 @@
             BOOST_THROW_EXCEPTION(std::runtime_error("Timed out waiting for client (" + channel_name + ") to appear"));
     }
 }
-
-void mtf::InputTestingServerConfiguration::wait_until_client_vanishes(std::string const& channel_name)
-{
-    std::unique_lock<std::mutex> lg(lifecycle_lock);
-
-    std::chrono::seconds timeout(60);
-    auto end_time = std::chrono::system_clock::now() + timeout;
-    
-
-    if (client_lifecycles[channel_name] == appeared)
-    {
-        BOOST_THROW_EXCEPTION(std::runtime_error("Waiting for a client (" + channel_name + ") to vanish but it has already appeared"));
-    }
-    while (client_lifecycles[channel_name] != vanished)
-    {
-        if (lifecycle_condition.wait_until(lg, end_time) == std::cv_status::timeout)
-            BOOST_THROW_EXCEPTION(std::runtime_error("Timed out waiting for client (" + channel_name + ") to vanish"));
-    }
-}

=== modified file 'tests/mir_test_framework/testing_server_options.cpp'
--- tests/mir_test_framework/testing_server_options.cpp	2013-08-12 08:58:58 +0000
+++ tests/mir_test_framework/testing_server_options.cpp	2013-08-13 23:40:56 +0000
@@ -79,9 +79,8 @@
     {
         mg::DisplayConfigurationOutput dummy_output_config{
             mg::DisplayConfigurationOutputId{0}, mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::vga,
             std::vector<geom::PixelFormat>{geom::PixelFormat::abgr_8888},
-            modes, 0, geom::Size{}, true, true, geom::Point{0,0}, 0, 0};
+            modes, geom::Size{}, true, true, geom::Point{0,0}, 0, 0};
 
         f(dummy_output_config);
     }

=== modified file 'tests/unit-tests/client/test_client_display_conf.cpp'
--- tests/unit-tests/client/test_client_display_conf.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/client/test_client_display_conf.cpp	2013-08-13 23:40:56 +0000
@@ -27,12 +27,6 @@
 
 namespace
 {
-void fill(mp::DisplayCard* out)
-{
-    out->set_card_id(7);
-    out->set_max_simultaneous_outputs(3);
-}
-
 void fill(mp::DisplayOutput* out)
 {
     out->add_pixel_format(4);
@@ -58,9 +52,6 @@
     mp::DisplayConfiguration protobuf_config;
     fill(protobuf_config.add_display_output());
     fill(protobuf_config.add_display_output());
-    fill(protobuf_config.add_display_card());
-    fill(protobuf_config.add_display_card());
-    fill(protobuf_config.add_display_card());
  
     mcl::DisplayConfiguration internal_config;
 

=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2013-08-13 23:40:56 +0000
@@ -373,7 +373,7 @@
     output.used = 0;
     output.position_x = 4;
     output.position_y = 6;
-    MirDisplayConfiguration user_config{1, &output, 0, nullptr};
+    MirDisplayConfiguration user_config{1, &output};
 
     auto verify_display_change = [&](mp::DisplayConfiguration const* config)
     {

=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp	2013-08-12 17:20:29 +0000
@@ -181,6 +181,7 @@
         using namespace ::testing;
 
         ON_CALL(*shell, open_session(_, _)).WillByDefault(Return(stubbed_session));
+        ON_CALL(*shell, create_surface_for(_, _)).WillByDefault(Return(mf::SurfaceId{1}));
     }
 
     std::shared_ptr<testing::NiceMock<mtd::MockShell>> const shell;
@@ -486,7 +487,7 @@
         .InSequence(seq);
     EXPECT_CALL(mock_display_config, configure_output(id1, used1, pt1, mode_index1))
         .InSequence(seq);
-    EXPECT_CALL(*mock_display_selector, store_configuration_for(_,_))
+    EXPECT_CALL(*mock_display_selector, configure(_,_))
         .InSequence(seq);
  
     mf::SessionMediator session_mediator{
@@ -515,58 +516,3 @@
 
     session_mediator.disconnect(nullptr, nullptr, nullptr, null_callback.get());
 }
-
-TEST_F(SessionMediatorTest, apply_session_display_config)
-{
-    using namespace testing;
-    mp::ConnectParameters connect_parameters;
-    mp::Connection connection;
-    mp::SurfaceParameters request;
-    mp::Surface response;
-    mp::SurfaceId release_request;
-    mp::Void ignored;
-    mp::DisplayConfiguration configuration; 
-
-    NiceMock<MockConfig> mock_display_config;
-    auto mock_display_selector = std::make_shared<mtd::MockDisplayChanger>();
-
-    EXPECT_CALL(*mock_display_selector, active_configuration())
-        .WillRepeatedly(Return(mt::fake_shared(mock_display_config)));
-
-    EXPECT_CALL(*mock_display_selector, store_configuration_for(_,_))
-        .Times(1);
-    EXPECT_CALL(*mock_display_selector, remove_configuration_for(_))
-        .Times(1);
-
-    mf::SessionMediator session_mediator{
-            shell, graphics_platform, mock_display_selector,
-            buffer_allocator, report, std::make_shared<mtd::NullEventSink>(), resource_cache};
-
-    session_mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
-    session_mediator.configure_display(nullptr, &configuration, &ignored, null_callback.get());
-    session_mediator.disconnect(nullptr, nullptr, nullptr, null_callback.get());
-}
-
-TEST_F(SessionMediatorTest, display_config_removed_if_no_disconnect)
-{
-    using namespace testing;
-    mp::ConnectParameters connect_parameters;
-    mp::Connection connection;
-    mp::SurfaceParameters request;
-    mp::Surface response;
-    mp::SurfaceId release_request;
-
-    NiceMock<MockConfig> mock_display_config;
-    auto mock_display_selector = std::make_shared<mtd::MockDisplayChanger>();
-
-    EXPECT_CALL(*mock_display_selector, active_configuration())
-        .WillOnce(Return(mt::fake_shared(mock_display_config))); 
-    EXPECT_CALL(*mock_display_selector, remove_configuration_for(_))
-        .Times(1);
-
-    mf::SessionMediator session_mediator{
-            shell, graphics_platform, mock_display_selector,
-            buffer_allocator, report, std::make_shared<mtd::NullEventSink>(), resource_cache};
-
-    session_mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
-}

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_cursor.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_cursor.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_cursor.cpp	2013-08-13 23:40:56 +0000
@@ -90,13 +90,11 @@
             {
                 mg::DisplayConfigurationOutputId{10},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
                 {},
                 {
                     {geom::Size{10, 20}, 59.9},
                     {geom::Size{200, 100}, 59.9},
                 },
-                1,
                 geom::Size{324, 642},
                 true,
                 true,
@@ -108,13 +106,11 @@
             {
                 mg::DisplayConfigurationOutputId{11},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
                 {},
                 {
                     {geom::Size{200, 200}, 59.9},
                     {geom::Size{100, 200}, 59.9},
                 },
-                0,
                 geom::Size{566, 111},
                 true,
                 true,
@@ -126,7 +122,7 @@
 
     void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
     {
-        f({card_id, outputs.size()});
+        f({card_id});
     }
 
     void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_display_configuration.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_display_configuration.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_display_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -104,13 +104,11 @@
 
     void setup_sample_modes()
     {
-        using fake = mtd::FakeDRMResources;
-
         /* Add DRM modes */
-        modes0.push_back(fake::create_mode(1920, 1080, 138500, 2080, 1111, fake::NormalMode));
-        modes0.push_back(fake::create_mode(1920, 1080, 148500, 2200, 1125, fake::PreferredMode));
-        modes0.push_back(fake::create_mode(1680, 1050, 119000, 1840, 1080, fake::NormalMode));
-        modes0.push_back(fake::create_mode(832, 624, 57284, 1152, 667, fake::NormalMode));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1920, 1080, 138500, 2080, 1111));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1920, 1080, 148500, 2200, 1125));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1680, 1050, 119000, 1840, 1080));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(832, 624, 57284, 1152, 667));
 
         /* Add the DisplayConfiguration modes corresponding to the DRM modes */
         for (auto const& mode : modes0)
@@ -148,7 +146,6 @@
     geom::Size const connector2_physical_size_mm{};
     std::vector<uint32_t> possible_encoder_ids_empty;
     uint32_t const possible_crtcs_mask_empty{0};
-    size_t const max_simultaneous_outputs{1};
 
     mtd::FakeDRMResources& resources(mock_drm.fake_drm);
 
@@ -159,39 +156,26 @@
     resources.add_encoder(encoder0_id, crtc0_id, possible_crtcs_mask_empty);
     resources.add_encoder(encoder1_id, invalid_id, possible_crtcs_mask_empty);
 
-    resources.add_connector(connector0_id, DRM_MODE_CONNECTOR_HDMIA,
-                            DRM_MODE_CONNECTED, encoder0_id,
+    resources.add_connector(connector0_id, DRM_MODE_CONNECTED, encoder0_id,
                             modes0, possible_encoder_ids_empty,
                             connector0_physical_size_mm);
-    resources.add_connector(connector1_id, DRM_MODE_CONNECTOR_Unknown,
-                            DRM_MODE_DISCONNECTED, invalid_id,
+    resources.add_connector(connector1_id, DRM_MODE_DISCONNECTED, invalid_id,
                             modes_empty, possible_encoder_ids_empty,
                             connector1_physical_size_mm);
-    resources.add_connector(connector2_id, DRM_MODE_CONNECTOR_eDP,
-                            DRM_MODE_DISCONNECTED, encoder1_id,
+    resources.add_connector(connector2_id, DRM_MODE_DISCONNECTED, encoder1_id,
                             modes_empty, possible_encoder_ids_empty,
                             connector2_physical_size_mm);
 
     resources.prepare();
 
     /* Expected results */
-    std::vector<mg::DisplayConfigurationCard> const expected_cards =
-    {
-        {
-            mg::DisplayConfigurationCardId{0},
-            max_simultaneous_outputs
-        }
-    };
-
     std::vector<mg::DisplayConfigurationOutput> const expected_outputs =
     {
         {
             mg::DisplayConfigurationOutputId{connector0_id},
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::hdmia,
             {},
             conf_modes0,
-            1,
             connector0_physical_size_mm,
             true,
             true,
@@ -202,10 +186,8 @@
         {
             mg::DisplayConfigurationOutputId{connector1_id},
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::unknown,
             {},
             std::vector<mg::DisplayConfigurationMode>(),
-            std::numeric_limits<size_t>::max(),
             connector1_physical_size_mm,
             false,
             false,
@@ -216,10 +198,8 @@
         {
             mg::DisplayConfigurationOutputId{connector2_id},
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::edp,
             {},
             std::vector<mg::DisplayConfigurationMode>(),
-            std::numeric_limits<size_t>::max(),
             connector2_physical_size_mm,
             false,
             false,
@@ -234,15 +214,6 @@
 
     auto conf = display->configuration();
 
-    size_t card_count{0};
-
-    conf->for_each_card([&](mg::DisplayConfigurationCard const& card)
-    {
-        ASSERT_LT(card_count, expected_cards.size());
-        EXPECT_EQ(expected_cards[card_count], card) << "card_count: " << card_count;
-        ++card_count;
-    });
-
     size_t output_count{0};
 
     conf->for_each_output([&](mg::DisplayConfigurationOutput const& output)
@@ -272,8 +243,7 @@
     resources.add_encoder(encoder0_id, crtc0_id, possible_crtcs_mask_empty);
     for (auto id : connector_ids)
     {
-        resources.add_connector(id, DRM_MODE_CONNECTOR_DVID,
-                                DRM_MODE_CONNECTED, encoder0_id,
+        resources.add_connector(id, DRM_MODE_CONNECTED, encoder0_id,
                                 modes0, encoder_ids,
                                 geom::Size());
     }
@@ -315,8 +285,7 @@
     resources.add_encoder(encoder0_id, crtc0_id, possible_crtcs_mask_empty);
     for (auto id : connector_ids)
     {
-        resources.add_connector(id, DRM_MODE_CONNECTOR_VGA,
-                                DRM_MODE_CONNECTED, encoder0_id,
+        resources.add_connector(id, DRM_MODE_CONNECTED, encoder0_id,
                                 modes0, encoder_ids,
                                 geom::Size());
     }
@@ -353,26 +322,15 @@
     };
     std::vector<uint32_t> possible_encoder_ids_empty;
     uint32_t const possible_crtcs_mask_empty{0};
-    size_t const max_simultaneous_outputs{1};
 
     /* Expected results */
-    std::vector<mg::DisplayConfigurationCard> const expected_cards =
-    {
-        {
-            mg::DisplayConfigurationCardId{0},
-            max_simultaneous_outputs
-        }
-    };
-
     std::vector<mg::DisplayConfigurationOutput> const expected_outputs_before =
     {
         {
             mg::DisplayConfigurationOutputId(connector_ids[0]),
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::composite,
             {},
             conf_modes0,
-            1,
             connector_physical_sizes_mm_before[0],
             true,
             true,
@@ -383,10 +341,8 @@
         {
             mg::DisplayConfigurationOutputId(connector_ids[1]),
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::vga,
             {},
             std::vector<mg::DisplayConfigurationMode>(),
-            std::numeric_limits<size_t>::max(),
             connector_physical_sizes_mm_before[1],
             false,
             false,
@@ -401,10 +357,8 @@
         {
             mg::DisplayConfigurationOutputId(connector_ids[0]),
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::composite,
             {},
             std::vector<mg::DisplayConfigurationMode>(),
-            std::numeric_limits<size_t>::max(),
             connector_physical_sizes_mm_after[0],
             false,
             true,
@@ -415,10 +369,8 @@
         {
             mg::DisplayConfigurationOutputId(connector_ids[1]),
             mg::DisplayConfigurationCardId{0},
-            mg::DisplayConfigurationOutputType::vga,
             {},
             conf_modes0,
-            1,
             connector_physical_sizes_mm_after[1],
             true,
             false,
@@ -438,12 +390,10 @@
     resources.add_encoder(encoder_ids[0], crtc_ids[0], possible_crtcs_mask_empty);
     resources.add_encoder(encoder_ids[1], invalid_id, possible_crtcs_mask_empty);
 
-    resources.add_connector(connector_ids[0], DRM_MODE_CONNECTOR_Composite,
-                            DRM_MODE_CONNECTED, encoder_ids[0],
+    resources.add_connector(connector_ids[0], DRM_MODE_CONNECTED, encoder_ids[0],
                             modes0, possible_encoder_ids_empty,
                             connector_physical_sizes_mm_before[0]);
-    resources.add_connector(connector_ids[1], DRM_MODE_CONNECTOR_VGA,
-                            DRM_MODE_DISCONNECTED, invalid_id,
+    resources.add_connector(connector_ids[1], DRM_MODE_DISCONNECTED, invalid_id,
                             modes_empty, possible_encoder_ids_empty,
                             connector_physical_sizes_mm_before[1]);
 
@@ -453,15 +403,6 @@
 
     auto conf = display->configuration();
 
-    size_t card_count{0};
-
-    conf->for_each_card([&](mg::DisplayConfigurationCard const& card)
-    {
-        ASSERT_LT(card_count, expected_cards.size());
-        EXPECT_EQ(expected_cards[card_count], card) << "card_count: " << card_count;
-        ++card_count;
-    });
-
     size_t output_count{0};
 
     conf->for_each_output([&](mg::DisplayConfigurationOutput const& output)
@@ -481,12 +422,10 @@
     resources.add_encoder(encoder_ids[0], invalid_id, possible_crtcs_mask_empty);
     resources.add_encoder(encoder_ids[1], crtc_ids[1], possible_crtcs_mask_empty);
 
-    resources.add_connector(connector_ids[0], DRM_MODE_CONNECTOR_Composite,
-                            DRM_MODE_DISCONNECTED, invalid_id,
+    resources.add_connector(connector_ids[0], DRM_MODE_DISCONNECTED, invalid_id,
                             modes_empty, possible_encoder_ids_empty,
                             connector_physical_sizes_mm_after[0]);
-    resources.add_connector(connector_ids[1], DRM_MODE_CONNECTOR_VGA,
-                            DRM_MODE_CONNECTED, encoder_ids[1],
+    resources.add_connector(connector_ids[1], DRM_MODE_CONNECTED, encoder_ids[1],
                             modes0, possible_encoder_ids_empty,
                             connector_physical_sizes_mm_after[1]);
 
@@ -494,15 +433,6 @@
 
     conf = display->configuration();
 
-    card_count = 0;
-
-    conf->for_each_card([&](mg::DisplayConfigurationCard const& card)
-    {
-        ASSERT_LT(card_count, expected_cards.size());
-        EXPECT_EQ(expected_cards[card_count], card) << "card_count: " << card_count;
-        ++card_count;
-    });
-
     output_count = 0;
 
     conf->for_each_output([&](mg::DisplayConfigurationOutput const& output)

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_display_multi_monitor.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_display_multi_monitor.cpp	2013-08-12 08:58:58 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_display_multi_monitor.cpp	2013-08-13 23:40:56 +0000
@@ -51,13 +51,15 @@
 public:
     void apply_to(mg::DisplayConfiguration& conf)
     {
+        size_t const preferred_mode_index{0};
+
         conf.for_each_output(
             [&](mg::DisplayConfigurationOutput const& conf_output)
             {
                 if (conf_output.connected && conf_output.modes.size() > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{0, 0},
-                                          conf_output.preferred_mode_index);
+                                          preferred_mode_index);
                 }
                 else
                 {
@@ -73,6 +75,7 @@
 public:
     void apply_to(mg::DisplayConfiguration& conf)
     {
+        size_t const preferred_mode_index{0};
         int max_x = 0;
 
         conf.for_each_output(
@@ -81,8 +84,8 @@
                 if (conf_output.connected && conf_output.modes.size() > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
-                                          conf_output.preferred_mode_index);
-                    max_x += conf_output.modes[conf_output.preferred_mode_index].size.width.as_int();
+                                          preferred_mode_index);
+                    max_x += conf_output.modes[0].size.width.as_int();
                 }
                 else
                 {
@@ -149,15 +152,13 @@
 
     void setup_outputs(int n)
     {
-        using fake = mtd::FakeDRMResources;
-
         mtd::FakeDRMResources& resources(mock_drm.fake_drm);
 
         modes0.clear();
-        modes0.push_back(fake::create_mode(1920, 1080, 138500, 2080, 1111, fake::NormalMode));
-        modes0.push_back(fake::create_mode(1920, 1080, 148500, 2200, 1125, fake::PreferredMode));
-        modes0.push_back(fake::create_mode(1680, 1050, 119000, 1840, 1080, fake::NormalMode));
-        modes0.push_back(fake::create_mode(832, 624, 57284, 1152, 667, fake::NormalMode));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1920, 1080, 138500, 2080, 1111));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1920, 1080, 148500, 2200, 1125));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(1680, 1050, 119000, 1840, 1080));
+        modes0.push_back(mtd::FakeDRMResources::create_mode(832, 624, 57284, 1152, 667));
 
         geom::Size const connector_physical_size_mm{1597, 987};
 
@@ -185,8 +186,7 @@
             uint32_t const connector_id{connector_base_id + i};
 
             connector_ids.push_back(connector_id);
-            resources.add_connector(connector_id, DRM_MODE_CONNECTOR_VGA,
-                                    DRM_MODE_CONNECTED, encoder_ids[i],
+            resources.add_connector(connector_id, DRM_MODE_CONNECTED, encoder_ids[i],
                                     modes0, encoder_ids, connector_physical_size_mm);
         }
 

=== modified file 'tests/unit-tests/graphics/gbm/test_overlapping_output_grouping.cpp'
--- tests/unit-tests/graphics/gbm/test_overlapping_output_grouping.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/graphics/gbm/test_overlapping_output_grouping.cpp	2013-08-13 23:40:56 +0000
@@ -48,8 +48,7 @@
     {
         mg::DisplayConfigurationCard card
         {
-            mg::DisplayConfigurationCardId{1},
-            outputs.size()
+            mg::DisplayConfigurationCardId{1}
         };
 
         f(card);
@@ -70,10 +69,8 @@
             {
                 mg::DisplayConfigurationOutputId(i),
                 mg::DisplayConfigurationCardId{1},
-                mg::DisplayConfigurationOutputType::svideo,
                 {},
                 modes,
-                i - 1,
                 {100, 100},
                 connected,
                 used,

=== modified file 'tests/unit-tests/graphics/gbm/test_real_kms_output.cpp'
--- tests/unit-tests/graphics/gbm/test_real_kms_output.cpp	2013-08-12 08:58:58 +0000
+++ tests/unit-tests/graphics/gbm/test_real_kms_output.cpp	2013-08-13 23:40:56 +0000
@@ -71,8 +71,7 @@
 
         resources.add_crtc(crtc_ids[0], drmModeModeInfo());
         resources.add_encoder(encoder_ids[0], crtc_ids[0], possible_crtcs_mask);
-        resources.add_connector(connector_ids[0], DRM_MODE_CONNECTOR_VGA,
-                                DRM_MODE_CONNECTED, encoder_ids[0],
+        resources.add_connector(connector_ids[0], DRM_MODE_CONNECTED, encoder_ids[0],
                                 modes_empty, possible_encoder_ids1, geom::Size());
 
         resources.prepare();
@@ -90,11 +89,9 @@
         resources.add_crtc(crtc_ids[1], drmModeModeInfo());
         resources.add_encoder(encoder_ids[0], crtc_ids[0], possible_crtcs_mask1);
         resources.add_encoder(encoder_ids[1], invalid_id, possible_crtcs_mask_all);
-        resources.add_connector(connector_ids[0], DRM_MODE_CONNECTOR_Composite,
-                                DRM_MODE_CONNECTED, invalid_id,
+        resources.add_connector(connector_ids[0], DRM_MODE_CONNECTED, invalid_id,
                                 modes_empty, possible_encoder_ids2, geom::Size());
-        resources.add_connector(connector_ids[1], DRM_MODE_CONNECTOR_DVIA,
-                                DRM_MODE_CONNECTED, encoder_ids[0],
+        resources.add_connector(connector_ids[1], DRM_MODE_CONNECTED, encoder_ids[0],
                                 modes_empty, possible_encoder_ids2, geom::Size());
 
         resources.prepare();

=== modified file 'tests/unit-tests/graphics/test_default_display_configuration_policy.cpp'
--- tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-08-12 08:58:59 +0000
+++ tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-08-13 23:40:56 +0000
@@ -39,7 +39,6 @@
             {
                 mg::DisplayConfigurationOutputId{10},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
                 {
                     {geom::PixelFormat::abgr_8888}
                 },
@@ -48,7 +47,6 @@
                     {geom::Size{123, 111}, 59.9},
                     {geom::Size{123, 111}, 59.9}
                 },
-                2,
                 geom::Size{324, 642},
                 true,
                 false,
@@ -61,10 +59,8 @@
             {
                 mg::DisplayConfigurationOutputId{11},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
-                {},
-                {},
-                std::numeric_limits<size_t>::max(),
+                {},
+                {},
                 geom::Size{566, 111},
                 true,
                 false,
@@ -77,14 +73,12 @@
             {
                 mg::DisplayConfigurationOutputId{12},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
                 {
                     {geom::PixelFormat::abgr_8888}
                 },
                 {
                     {geom::Size{523, 555}, 60.0},
                 },
-                0,
                 geom::Size{324, 642},
                 true,
                 false,
@@ -97,12 +91,10 @@
             {
                 mg::DisplayConfigurationOutputId{13},
                 card_id,
-                mg::DisplayConfigurationOutputType::vga,
                 {
                     {geom::PixelFormat::abgr_8888}
                 },
                 {},
-                0,
                 geom::Size{324, 642},
                 false,
                 false,
@@ -114,7 +106,7 @@
 
     void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
     {
-        f({card_id, outputs.size()});
+        f({card_id});
     }
 
     void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
@@ -137,13 +129,14 @@
 {
     mg::DefaultDisplayConfigurationPolicy policy;
     MockDisplayConfiguration conf;
+    size_t const highest_mode{0};
 
     conf.for_each_output([&conf](mg::DisplayConfigurationOutput const& output)
     {
         if (output.connected && output.modes.size() > 0)
         {
             EXPECT_CALL(conf, configure_output(output.id, true, geom::Point(),
-                                               output.preferred_mode_index));
+                                               highest_mode));
         }
         else
         {

=== modified file 'tests/unit-tests/graphics/test_display_configuration.cpp'
--- tests/unit-tests/graphics/test_display_configuration.cpp	2013-08-12 11:52:13 +0000
+++ tests/unit-tests/graphics/test_display_configuration.cpp	2013-08-13 23:40:56 +0000
@@ -30,7 +30,6 @@
 {
     mg::DisplayConfigurationOutputId{3},
     mg::DisplayConfigurationCardId{2},
-    mg::DisplayConfigurationOutputType::dvid,
     {
         {geom::PixelFormat::abgr_8888}
     },
@@ -39,7 +38,6 @@
         {geom::Size{10, 20}, 59.0},
         {geom::Size{15, 20}, 59.0}
     },
-    0,
     geom::Size{10, 20},
     true,
     true,
@@ -50,39 +48,7 @@
 
 }
 
-TEST(DisplayConfiguration, same_cards_compare_equal)
-{
-    mg::DisplayConfigurationCardId const id{1};
-    size_t const max_outputs{3};
-
-    mg::DisplayConfigurationCard const card1{id, max_outputs};
-    mg::DisplayConfigurationCard const card2 = card1;
-
-    EXPECT_EQ(card1, card1);
-    EXPECT_EQ(card1, card2);
-    EXPECT_EQ(card2, card1);
-}
-
-TEST(DisplayConfiguration, different_cards_compare_unequal)
-{
-    mg::DisplayConfigurationCardId const id1{1};
-    mg::DisplayConfigurationCardId const id2{2};
-    size_t const max_outputs1{3};
-    size_t const max_outputs2{4};
-
-    mg::DisplayConfigurationCard const card1{id1, max_outputs1};
-    mg::DisplayConfigurationCard const card2{id1, max_outputs2};
-    mg::DisplayConfigurationCard const card3{id2, max_outputs1};
-
-    EXPECT_NE(card1, card2);
-    EXPECT_NE(card2, card1);
-    EXPECT_NE(card2, card3);
-    EXPECT_NE(card3, card2);
-    EXPECT_NE(card1, card3);
-    EXPECT_NE(card3, card1);
-}
-
-TEST(DisplayConfiguration, same_modes_compare_equal)
+TEST(DisplayConfiguration, mode_equality)
 {
     geom::Size const size{10, 20};
     double const vrefresh{59.9};
@@ -95,7 +61,7 @@
     EXPECT_EQ(mode2, mode1);
 }
 
-TEST(DisplayConfiguration, different_modes_compare_unequal)
+TEST(DisplayConfiguration, mode_inequality)
 {
     geom::Size const size1{10, 20};
     geom::Size const size2{10, 21};
@@ -114,7 +80,7 @@
     EXPECT_NE(mode3, mode1);
 }
 
-TEST(DisplayConfiguration, same_outputs_compare_equal)
+TEST(DisplayConfiguration, output_equality)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -124,7 +90,7 @@
     EXPECT_EQ(output2, output1);
 }
 
-TEST(DisplayConfiguration, outputs_with_different_ids_compare_unequal)
+TEST(DisplayConfiguration, output_inequality_id)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -141,7 +107,7 @@
     EXPECT_NE(output3, output1);
 }
 
-TEST(DisplayConfiguration, outupts_with_different_modes_compare_unequal)
+TEST(DisplayConfiguration, output_inequality_modes)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -170,7 +136,7 @@
     EXPECT_NE(output3, output1);
 }
 
-TEST(DisplayConfiguration, outputs_with_different_physical_size_compare_unequal)
+TEST(DisplayConfiguration, output_inequality_physical_size)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -183,7 +149,7 @@
     EXPECT_NE(output2, output1);
 }
 
-TEST(DisplayConfiguration, outputs_with_different_connected_status_compare_unequal)
+TEST(DisplayConfiguration, output_inequality_connection)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -194,7 +160,7 @@
     EXPECT_NE(output2, output1);
 }
 
-TEST(DisplayConfiguration, outputs_with_different_current_mode_index_compare_unequal)
+TEST(DisplayConfiguration, output_inequality_current_mode)
 {
     mg::DisplayConfigurationOutput const output1 = tmpl_output;
     mg::DisplayConfigurationOutput output2 = tmpl_output;
@@ -204,14 +170,3 @@
     EXPECT_NE(output1, output2);
     EXPECT_NE(output2, output1);
 }
-
-TEST(DisplayConfiguration, outputs_with_different_preferred_mode_index_compare_unequal)
-{
-    mg::DisplayConfigurationOutput const output1 = tmpl_output;
-    mg::DisplayConfigurationOutput output2 = tmpl_output;
-
-    output2.preferred_mode_index = 1;
-
-    EXPECT_NE(output1, output2);
-    EXPECT_NE(output2, output1);
-}

=== modified file 'tests/unit-tests/shell/CMakeLists.txt'
--- tests/unit-tests/shell/CMakeLists.txt	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/shell/CMakeLists.txt	2013-08-12 17:20:29 +0000
@@ -10,7 +10,6 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_threaded_snapshot_strategy.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gl_pixel_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_graphics_display_layout.cpp
-  ${CMAKE_CURRENT_SOURCE_DIR}/test_mediating_display_changer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_unauthorized_display_changer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_mediating_display_changer.cpp
 )

=== modified file 'tests/unit-tests/shell/test_default_focus_mechanism.cpp'
--- tests/unit-tests/shell/test_default_focus_mechanism.cpp	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/shell/test_default_focus_mechanism.cpp	2013-08-13 23:40:56 +0000
@@ -34,7 +34,6 @@
 #include "mir_test_doubles/stub_surface_controller.h"
 #include "mir_test_doubles/stub_input_targeter.h"
 #include "mir_test_doubles/mock_input_targeter.h"
-#include "mir_test_doubles/mock_session_listener.h"
 #include "mir_test_doubles/stub_surface_controller.h"
 
 #include <gmock/gmock.h>
@@ -45,126 +44,83 @@
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
 namespace mf = mir::frontend;
-namespace mg = mir::graphics;
 namespace mt = mir::test;
 namespace mtd = mir::test::doubles;
 
-namespace
-{
-struct MockShellSession : public msh::Session
-{
-    MOCK_METHOD1(create_surface, mf::SurfaceId(msh::SurfaceCreationParameters const&));
-    MOCK_METHOD1(destroy_surface, void(mf::SurfaceId));
-    MOCK_CONST_METHOD1(get_surface, std::shared_ptr<mf::Surface>(mf::SurfaceId));
-
-    MOCK_METHOD1(take_snapshot, void(msh::SnapshotCallback const&));
-    MOCK_CONST_METHOD0(default_surface, std::shared_ptr<msh::Surface>());
-
-    MOCK_CONST_METHOD0(name, std::string());
-    MOCK_METHOD0(force_requests_to_complete, void());
-
-    MOCK_METHOD0(hide, void());
-    MOCK_METHOD0(show, void());
-    MOCK_METHOD3(configure_surface, int(mf::SurfaceId, MirSurfaceAttrib, int));
-    MOCK_METHOD1(send_display_config, void(mg::DisplayConfiguration const&));
-};
-
-struct MockFocusSequence : public msh::FocusSequence
-{
-    MOCK_CONST_METHOD1(successor_of, std::shared_ptr<msh::Session>(std::shared_ptr<msh::Session> const&));
-    MOCK_CONST_METHOD1(predecessor_of, std::shared_ptr<msh::Session>(std::shared_ptr<msh::Session> const&));
-    MOCK_CONST_METHOD0(default_focus, std::shared_ptr<msh::Session>());
-};
-
-struct DefaultFocusMechanism : public testing::Test
-{
-    void SetUp()
+TEST(DefaultFocusMechanism, raises_default_surface)
+{
+    using namespace ::testing;
+    
+    NiceMock<mtd::MockShellSession> app1;
+    mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
     {
-        controller = std::make_shared<mtd::StubSurfaceController>();
-
-        //TODO: extremely kludgy that a mock needs to be constructed this way 
-        mock_surface1 = std::make_shared<mtd::MockSurface>(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
-        mock_surface2 = std::make_shared<mtd::MockSurface>(&app2, std::make_shared<mtd::StubSurfaceBuilder>());
-        ON_CALL(app1, default_surface())
-            .WillByDefault(testing::Return(mock_surface1));
-        ON_CALL(app2, default_surface())
-            .WillByDefault(testing::Return(mock_surface2));
+        InSequence seq;
+        EXPECT_CALL(app1, default_surface()).Times(1)
+            .WillOnce(Return(mt::fake_shared(mock_surface)));
     }
 
-    std::shared_ptr<mtd::MockSurface> mock_surface1, mock_surface2;
-    testing::NiceMock<MockShellSession> app1;
-    testing::NiceMock<MockShellSession> app2;
-    testing::NiceMock<mtd::MockSessionListener> mock_listener;
-    testing::NiceMock<MockFocusSequence> focus_sequence;
-    std::shared_ptr<msh::SurfaceController> controller;
-
+    auto controller = std::make_shared<mtd::StubSurfaceController>();
+    EXPECT_CALL(mock_surface, raise(Eq(controller))).Times(1);
     mtd::StubInputTargeter targeter;
-};
-
-}
-
-TEST_F(DefaultFocusMechanism, raises_default_surface)
-{
-    using namespace ::testing;
-    
-    EXPECT_CALL(mock_listener, focused(_))
-        .Times(1);
-    EXPECT_CALL(*mock_surface1,
-         raise(Eq(controller))).Times(1);
-
-    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(focus_sequence),
-        mt::fake_shared(targeter), controller, mt::fake_shared(mock_listener));
-    
-    focus_mechanism.surface_created_for(mt::fake_shared(app1));
-}
-
-TEST_F(DefaultFocusMechanism, open_and_close)
-{
-    using namespace ::testing;
-
-    Sequence seq; 
-    EXPECT_CALL(mock_listener, focused(_))
-        .InSequence(seq);
-
-    EXPECT_CALL(focus_sequence, successor_of(_))
-        .InSequence(seq)
-        .WillOnce(Throw(std::logic_error("")));
-    EXPECT_CALL(mock_listener, unfocused())
-        .InSequence(seq);
-
-    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(focus_sequence),
-        mt::fake_shared(targeter), controller, mt::fake_shared(mock_listener));
-
-    auto app = mt::fake_shared(app1); 
-    focus_mechanism.session_opened(app);
-    focus_mechanism.session_closed(app);
-}
-
-TEST_F(DefaultFocusMechanism, unfocus_only_on_close_active_session)
-{
-    using namespace ::testing;
-
-    EXPECT_CALL(mock_listener, focused(_))
-        .Times(2);  
-    EXPECT_CALL(mock_listener, unfocused())
-        .Times(0);
-
-    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(focus_sequence),
-        mt::fake_shared(targeter), controller, mt::fake_shared(mock_listener));
-    
-    focus_mechanism.session_opened(mt::fake_shared(app1));
-    focus_mechanism.session_opened(mt::fake_shared(app2));
-    focus_mechanism.session_closed(mt::fake_shared(app1));
-}
-
-TEST_F(DefaultFocusMechanism, sets_input_focus)
-{
-    using namespace ::testing;
-    
-    EXPECT_CALL(*mock_surface1, take_input_focus(_)).Times(1);
-    
-    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(focus_sequence), mt::fake_shared(targeter), controller,
-        mt::fake_shared(mock_listener));
-    
-    focus_mechanism.surface_created_for(mt::fake_shared(app1));
+    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(targeter), controller);
+    
+    focus_mechanism.set_focus_to(mt::fake_shared(app1));
+}
+
+TEST(DefaultFocusMechanism, mechanism_notifies_default_surface_of_focus_changes)
+{
+    using namespace ::testing;
+
+    NiceMock<mtd::MockShellSession> app1, app2;
+    mtd::MockSurface mock_surface1(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
+    mtd::MockSurface mock_surface2(&app2, std::make_shared<mtd::StubSurfaceBuilder>());
+    
+    ON_CALL(app1, default_surface()).WillByDefault(Return(mt::fake_shared(mock_surface1)));
+    ON_CALL(app2, default_surface()).WillByDefault(Return(mt::fake_shared(mock_surface2)));
+
+    
+    {
+        InSequence seq;
+        EXPECT_CALL(mock_surface1, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
+        EXPECT_CALL(mock_surface1, configure(mir_surface_attrib_focus, mir_surface_unfocused)).Times(1);
+        EXPECT_CALL(mock_surface2, configure(mir_surface_attrib_focus, mir_surface_focused)).Times(1);
+    }
+
+    msh::DefaultFocusMechanism focus_mechanism(std::make_shared<mtd::StubInputTargeter>(),
+                                                        std::make_shared<mtd::StubSurfaceController>());
+
+    focus_mechanism.set_focus_to(mt::fake_shared(app1));
+    focus_mechanism.set_focus_to(mt::fake_shared(app2));
+}
+
+TEST(DefaultFocusMechanism, sets_input_focus)
+{
+    using namespace ::testing;
+    
+    NiceMock<mtd::MockShellSession> app1;
+    mtd::MockSurface mock_surface(&app1, std::make_shared<mtd::StubSurfaceBuilder>());
+    {
+        InSequence seq;
+        EXPECT_CALL(app1, default_surface()).Times(1)
+            .WillOnce(Return(mt::fake_shared(mock_surface)));
+        EXPECT_CALL(app1, default_surface()).Times(1)
+            .WillOnce(Return(std::shared_ptr<msh::Surface>()));
+    }
+
+    mtd::MockInputTargeter targeter;
+    
+    msh::DefaultFocusMechanism focus_mechanism(mt::fake_shared(targeter), std::make_shared<mtd::StubSurfaceController>());
+    
+    {
+        InSequence seq;
+        EXPECT_CALL(mock_surface, take_input_focus(_)).Times(1);
+        // When we have no default surface.
+        EXPECT_CALL(targeter, focus_cleared()).Times(1);
+        // When we have no session.
+        EXPECT_CALL(targeter, focus_cleared()).Times(1);
+    }
+    
+    focus_mechanism.set_focus_to(mt::fake_shared(app1));
+    focus_mechanism.set_focus_to(mt::fake_shared(app1));
+    focus_mechanism.set_focus_to(std::shared_ptr<msh::Session>());
 }

=== renamed file 'tests/unit-tests/shell/test_mediating_display_changer.cpp.moved' => 'tests/unit-tests/shell/test_mediating_display_changer.cpp'
=== removed file 'tests/unit-tests/shell/test_mediating_display_changer.cpp'
--- tests/unit-tests/shell/test_mediating_display_changer.cpp	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/shell/test_mediating_display_changer.cpp	1970-01-01 00:00:00 +0000
@@ -1,179 +0,0 @@
-/*
- * 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: Kevin DuBois <[e-mail]>
- */
-
-#include "mir/shell/mediating_display_changer.h"
-#include "mir/shell/session.h"
-#include "mir/compositor/compositor.h"
-
-#include "mir_test_doubles/mock_display_changer.h"
-#include "mir_test_doubles/mock_session.h"
-#include "mir_test_doubles/stub_session.h"
-#include "mir_test_doubles/mock_display.h"
-#include "mir_test_doubles/null_display_configuration.h"
-#include "mir_test/fake_shared.h"
-#include <gtest/gtest.h>
-
-namespace mc = mir::compositor;
-namespace mf = mir::frontend;
-namespace mg = mir::graphics;
-namespace msh = mir::shell;
-namespace mt = mir::test;
-namespace mtd = mt::doubles;
-
-namespace
-{
-struct MockCompositor : public mc::Compositor
-{
-    MOCK_METHOD0(start, void());
-    MOCK_METHOD0(stop, void());
-};
-
-}
-
-struct MediatingDisplayChangerTest : public ::testing::Test
-{
-    void SetUp()
-    {
-        using namespace testing;
-        ON_CALL(mock_display, configuration())
-            .WillByDefault(Return(mt::fake_shared(default_config)));
-    }
-    mtd::MockDisplay mock_display;
-    MockCompositor mock_compositor;
-    
-    mtd::NullDisplayConfiguration default_config;
-};
-
-TEST_F(MediatingDisplayChangerTest, display_info)
-{
-    using namespace testing;
-    mtd::NullDisplayConfiguration conf;
-    EXPECT_CALL(mock_display, configuration())
-        .Times(1)
-        .WillOnce(Return(mt::fake_shared(conf)));
-
-    msh::MediatingDisplayChanger changer(mt::fake_shared(mock_display),
-                                         mt::fake_shared(mock_compositor));
-    auto returned_conf = changer.active_configuration();
-    EXPECT_EQ(&conf, returned_conf.get());
-}
-
-namespace
-{
-struct StubShellSession : public msh::Session
-{
-    mf::SurfaceId create_surface(msh::SurfaceCreationParameters const&)
-    {
-        return mf::SurfaceId{0};
-    }
-    void destroy_surface(mf::SurfaceId)
-    {
-    }
-    std::shared_ptr<mf::Surface> get_surface(mf::SurfaceId) const
-    {
-        return std::shared_ptr<mf::Surface>();
-    }
-
-    std::string name() const
-    {
-        return std::string();
-    }
-    void force_requests_to_complete()
-    {
-    }
-    void hide()
-    {
-    }
-    void show()
-    {
-    }
-    int configure_surface(mf::SurfaceId, MirSurfaceAttrib, int)
-    {
-        return 0;
-    }
-    void send_display_config(mg::DisplayConfiguration const&)
-    {
-    }
-    void take_snapshot(msh::SnapshotCallback const&)
-    {
-    }
-    std::shared_ptr<msh::Surface> default_surface() const
-    {
-        return std::shared_ptr<msh::Surface>();
-    }
-
-};
-}
-
-namespace
-{
-MATCHER_P(ConfigPtrsMatch, config_ptr, "")
-{
-    return (&arg == config_ptr);
-}
-
-}
-TEST_F(MediatingDisplayChangerTest, display_configure)
-{
-    using namespace testing;
-
-    auto session1 = std::make_shared<StubShellSession>();
-    auto session2 = std::make_shared<StubShellSession>();
-    auto base_config = std::make_shared<mtd::NullDisplayConfiguration>();
-    auto applied_config1 = std::make_shared<mtd::NullDisplayConfiguration>();
-    auto applied_config2 = std::make_shared<mtd::NullDisplayConfiguration>();
-
-    //get base config
-    EXPECT_CALL(mock_display, configuration())
-        .Times(1)
-        .WillOnce(Return(base_config));
-
-    //apply session config
-    Sequence seq;
-    EXPECT_CALL(mock_compositor, stop())
-        .InSequence(seq);
-    EXPECT_CALL(mock_display, configure(ConfigPtrsMatch(applied_config1.get())))
-        .InSequence(seq);
-    EXPECT_CALL(mock_compositor, start())
-        .InSequence(seq);
-
-    EXPECT_CALL(mock_compositor, stop())
-        .InSequence(seq);
-    EXPECT_CALL(mock_display, configure(ConfigPtrsMatch(applied_config2.get())))
-        .InSequence(seq);
-    EXPECT_CALL(mock_compositor, start())
-        .InSequence(seq);
-
-    //apply base config
-    EXPECT_CALL(mock_compositor, stop())
-        .InSequence(seq);
-    EXPECT_CALL(mock_display, configure(ConfigPtrsMatch(base_config.get())))
-        .InSequence(seq);
-    EXPECT_CALL(mock_compositor, start())
-        .InSequence(seq);
-
-    msh::MediatingDisplayChanger changer(mt::fake_shared(mock_display),
-                                         mt::fake_shared(mock_compositor));
-
-    changer.store_configuration_for(session1, applied_config1);
-    changer.store_configuration_for(session2, applied_config2);
-    changer.apply_configuration_of(session1);
-    changer.apply_configuration_of(session2);
-    changer.remove_configuration_for(session2);
-    changer.remove_configuration_for(session1);
-}

=== modified file 'tests/unit-tests/shell/test_session_manager.cpp'
--- tests/unit-tests/shell/test_session_manager.cpp	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/shell/test_session_manager.cpp	2013-08-09 03:39:35 +0000
@@ -26,12 +26,9 @@
 #include "mir/shell/null_session_listener.h"
 #include "mir/shell/surface_creation_parameters.h"
 #include "mir/surfaces/surface.h"
-#include "mir/graphics/display_configuration.h"
 
 #include "mir_test/fake_shared.h"
 #include "mir_test_doubles/mock_buffer_stream.h"
-#include "mir_test_doubles/mock_display_changer.h"
-#include "mir_test_doubles/mock_session.h"
 #include "mir_test_doubles/mock_surface_factory.h"
 #include "mir_test_doubles/mock_focus_setter.h"
 #include "mir_test_doubles/mock_session_listener.h"
@@ -45,7 +42,6 @@
 
 namespace mc = mir::compositor;
 namespace mf = mir::frontend;
-namespace mg = mir::graphics;
 namespace msh = mir::shell;
 namespace ms = mir::surfaces;
 namespace geom = mir::geometry;
@@ -63,15 +59,22 @@
     ~MockSessionContainer() noexcept {}
 };
 
+struct MockFocusSequence : public msh::FocusSequence
+{
+    MOCK_CONST_METHOD1(successor_of, std::shared_ptr<msh::Session>(std::shared_ptr<msh::Session> const&));
+    MOCK_CONST_METHOD1(predecessor_of, std::shared_ptr<msh::Session>(std::shared_ptr<msh::Session> const&));
+    MOCK_CONST_METHOD0(default_focus, std::shared_ptr<msh::Session>());
+};
+
 struct SessionManagerSetup : public testing::Test
 {
     SessionManagerSetup()
       : session_manager(mt::fake_shared(surface_factory),
                         mt::fake_shared(container),
+                        mt::fake_shared(focus_sequence),
                         mt::fake_shared(focus_setter),
                         std::make_shared<mtd::NullSnapshotStrategy>(),
-                        mt::fake_shared(session_listener),
-                        mt::fake_shared(mock_display_changer))
+                        mt::fake_shared(session_listener))
     {
     }
 
@@ -79,85 +82,127 @@
     mtd::StubSurfaceController surface_controller;
     mtd::MockSurfaceFactory surface_factory;
     testing::NiceMock<MockSessionContainer> container;    // Inelegant but some tests need a stub
+    MockFocusSequence focus_sequence;
+    testing::NiceMock<mtd::MockFocusSetter> focus_setter; // Inelegant but some tests need a stub
+    msh::NullSessionListener session_listener;
+
+    msh::SessionManager session_manager;
+};
+
+}
+
+TEST_F(SessionManagerSetup, open_and_close_session)
+{
+    using namespace ::testing;
+
+    EXPECT_CALL(container, insert_session(_)).Times(1);
+    EXPECT_CALL(container, remove_session(_)).Times(1);
+    EXPECT_CALL(focus_setter, set_focus_to(_));
+    EXPECT_CALL(focus_setter, set_focus_to(std::shared_ptr<msh::Session>())).Times(1);
+
+    EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
+
+    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<mf::EventSink>());
+    session_manager.close_session(session);
+}
+
+TEST_F(SessionManagerSetup, closing_session_removes_surfaces)
+{
+    using namespace ::testing;
+
+    EXPECT_CALL(surface_factory, create_surface(_, _, _, _)).Times(1);
+
+    ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
+       Return(std::make_shared<msh::Surface>(
+           nullptr,
+           mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
+           msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
+
+
+    EXPECT_CALL(container, insert_session(_)).Times(1);
+    EXPECT_CALL(container, remove_session(_)).Times(1);
+
+    EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1);
+    EXPECT_CALL(focus_setter, set_focus_to(std::shared_ptr<msh::Session>())).Times(1);
+
+    EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
+
+    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<mf::EventSink>());
+    session->create_surface(msh::a_surface().of_size(geom::Size{geom::Width{1024}, geom::Height{768}}));
+
+    session_manager.close_session(session);
+}
+
+TEST_F(SessionManagerSetup, new_applications_receive_focus)
+{
+    using namespace ::testing;
+    std::shared_ptr<msh::Session> new_session;
+
+    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", std::shared_ptr<mf::EventSink>());
+    EXPECT_EQ(session, new_session);
+}
+
+TEST_F(SessionManagerSetup, create_surface_for_session_forwards_and_then_focuses_session)
+{
+    using namespace ::testing;
+    ON_CALL(surface_factory, create_surface(_, _, _, _)).WillByDefault(
+        Return(std::make_shared<msh::Surface>(
+           nullptr,
+           mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
+           msh::a_surface(),mf::SurfaceId{}, std::shared_ptr<mf::EventSink>())));
+
+    // Once for session creation and once for surface creation
+    {
+        InSequence seq;
+
+        EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1); // Session creation
+        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", std::shared_ptr<mf::EventSink>());
+    session_manager.create_surface_for(session1, msh::a_surface());
+}
+
+namespace 
+{
+
+struct SessionManagerSessionListenerSetup : public testing::Test
+{
+    SessionManagerSessionListenerSetup()
+      : session_manager(mt::fake_shared(surface_factory),
+                        mt::fake_shared(container),
+                        mt::fake_shared(focus_sequence),
+                        mt::fake_shared(focus_setter),
+                        std::make_shared<mtd::NullSnapshotStrategy>(),
+                        mt::fake_shared(session_listener))
+    {
+    }
+
+    mtd::MockSurfaceFactory surface_factory;
+    testing::NiceMock<MockSessionContainer> container;    // Inelegant but some tests need a stub
+    testing::NiceMock<MockFocusSequence> focus_sequence;
     testing::NiceMock<mtd::MockFocusSetter> focus_setter; // Inelegant but some tests need a stub
     mtd::MockSessionListener session_listener;
-    testing::NiceMock<mtd::MockDisplayChanger> mock_display_changer;
 
     msh::SessionManager session_manager;
 };
-
 }
 
-TEST_F(SessionManagerSetup, open_and_close_session)
+TEST_F(SessionManagerSessionListenerSetup, session_listener_is_notified_of_lifecycle_and_focus)
 {
     using namespace ::testing;
 
-    Sequence seq;
-    EXPECT_CALL(container, insert_session(_))
-        .InSequence(seq);
-    EXPECT_CALL(session_listener, starting(_))
-        .InSequence(seq);
-    EXPECT_CALL(focus_setter, session_opened(_))
-        .InSequence(seq);
-
-    EXPECT_CALL(container, remove_session(_))
-        .InSequence(seq);
-    EXPECT_CALL(session_listener, stopping(_))
-        .InSequence(seq);
-    EXPECT_CALL(focus_setter, session_closed(_))
-        .InSequence(seq);
-
-    auto session = session_manager.open_session("Visual Basic Studio", std::shared_ptr<mf::EventSink>());
-    session_manager.close_session(session);
-}
-
-TEST_F(SessionManagerSetup, create_surface)
-{
-    using namespace testing;
-
-    std::shared_ptr<mf::Session> session;
-    EXPECT_CALL(focus_setter, surface_created_for(_))
-        .Times(1);
-
-    session_manager.handle_surface_created(session);
-}
-
-namespace
-{
-struct MockShellSession : public msh::Session
-{
-    MOCK_METHOD1(create_surface, mf::SurfaceId(msh::SurfaceCreationParameters const&));
-    MOCK_METHOD1(destroy_surface, void(mf::SurfaceId));
-    MOCK_CONST_METHOD1(get_surface, std::shared_ptr<mf::Surface>(mf::SurfaceId));
-
-    MOCK_METHOD1(take_snapshot, void(msh::SnapshotCallback const&));
-    MOCK_CONST_METHOD0(default_surface, std::shared_ptr<msh::Surface>());
-
-    MOCK_CONST_METHOD0(name, std::string());
-    MOCK_METHOD0(force_requests_to_complete, void());
-
-    MOCK_METHOD0(hide, void());
-    MOCK_METHOD0(show, void());
+    EXPECT_CALL(session_listener, starting(_)).Times(1);
+    EXPECT_CALL(session_listener, focused(_)).Times(1);
+    EXPECT_CALL(session_listener, stopping(_)).Times(1);
+    EXPECT_CALL(session_listener, unfocused()).Times(1);
+
+    EXPECT_CALL(focus_sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
     
-    MOCK_METHOD3(configure_surface, int(mf::SurfaceId, MirSurfaceAttrib, int));
-    MOCK_METHOD1(send_display_config, void(mg::DisplayConfiguration const&));
-};
-}
-
-TEST_F(SessionManagerSetup, display_change_configuration)
-{
-    using namespace testing;
-
-    auto session1 = std::make_shared<MockShellSession>();
-    auto session2 = std::make_shared<MockShellSession>();
-
-    EXPECT_CALL(focus_setter, focused_session())
-        .Times(2)
-        .WillOnce(Return(session1))
-        .WillOnce(Return(session2));
-    EXPECT_CALL(mock_display_changer, apply_configuration_of(_))
-        .Times(1);
-
-    session_manager.handle_display_configuration(session2);
-    session_manager.handle_display_configuration(session2);
+    auto session = session_manager.open_session("XPlane", std::shared_ptr<mf::EventSink>());
+    session_manager.close_session(session);
 }

=== modified file 'tests/unit-tests/shell/test_surface.cpp'
--- tests/unit-tests/shell/test_surface.cpp	2013-08-09 03:39:35 +0000
+++ tests/unit-tests/shell/test_surface.cpp	2013-08-13 23:40:56 +0000
@@ -21,6 +21,8 @@
 #include "mir/shell/surface_creation_parameters.h"
 #include "mir/surfaces/surface_stack_model.h"
 #include "mir/shell/surface_builder.h"
+#include "mir/frontend/event_sink.h"
+#include "mir/graphics/display_configuration.h"
 
 #include "mir_test_doubles/stub_surface_controller.h"
 #include "mir_test_doubles/mock_surface_controller.h"
@@ -35,6 +37,7 @@
 #include "mir_test_doubles/null_surface_configurator.h"
 #include "mir_test_doubles/mock_surface_configurator.h"
 #include "mir_test/fake_shared.h"
+#include "mir_test/event_matchers.h"
 
 #include <stdexcept>
 #include <gmock/gmock.h>
@@ -52,6 +55,14 @@
 
 namespace
 {
+
+struct MockEventSink : public mf::EventSink
+{
+    ~MockEventSink() noexcept(true) {}
+    MOCK_METHOD1(handle_event, void(MirEvent const&));
+    MOCK_METHOD1(handle_display_config_change, void(mg::DisplayConfiguration const&));
+};
+
 class StubSurfaceBuilder : public msh::SurfaceBuilder
 {
 public:
@@ -454,6 +465,29 @@
     EXPECT_EQ(mir_surface_state_fullscreen, surf.state());
 }
 
+TEST_F(ShellSurface, focus)
+{
+    using namespace testing;
+
+    MockEventSink sink;
+    
+    {
+        InSequence seq;
+        EXPECT_CALL(sink, handle_event(mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_focused)))
+            .Times(1);
+        EXPECT_CALL(sink, handle_event(mt::SurfaceEvent(mir_surface_attrib_focus, mir_surface_unfocused)))
+            .Times(1);
+    }
+
+    msh::Surface surf(
+            nullptr,
+            mt::fake_shared(surface_builder), std::make_shared<mtd::NullSurfaceConfigurator>(),
+            msh::a_surface(), stub_id, mt::fake_shared(sink));
+
+    surf.configure(mir_surface_attrib_focus, mir_surface_focused);
+    surf.configure(mir_surface_attrib_focus, mir_surface_unfocused);
+}
+
 TEST_F(ShellSurface, configurator_selects_attribute_values)
 {
     using namespace testing;

=== modified file 'tests/unit-tests/shell/test_unauthorized_display_changer.cpp'
--- tests/unit-tests/shell/test_unauthorized_display_changer.cpp	2013-08-13 23:40:55 +0000
+++ tests/unit-tests/shell/test_unauthorized_display_changer.cpp	2013-08-12 17:20:29 +0000
@@ -36,17 +36,13 @@
     mtd::MockDisplayChanger underlying_changer;
 };
 
-TEST_F(UnauthorizedDisplayChangerTest, change_store)
+TEST_F(UnauthorizedDisplayChangerTest, change_attempt)
 {
     mtd::NullDisplayConfiguration conf;
     msh::UnauthorizedDisplayChanger changer(mt::fake_shared(underlying_changer));
 
     EXPECT_THROW({
-        changer.store_configuration_for(std::weak_ptr<mf::Session>(), mt::fake_shared(conf));
-    }, std::runtime_error);
-
-    EXPECT_THROW({
-        changer.apply_configuration_of(std::weak_ptr<mf::Session>());
+        changer.configure(std::weak_ptr<mf::Session>(), mt::fake_shared(conf));
     }, std::runtime_error);
 }
 
@@ -62,11 +58,6 @@
     msh::UnauthorizedDisplayChanger changer(mt::fake_shared(underlying_changer));
 
     auto returned_conf = changer.active_configuration();
-<<<<<<< TREE
 
     EXPECT_EQ(&conf, returned_conf.get());
 }
-=======
-    EXPECT_EQ(&conf, returned_conf.get());
-}
->>>>>>> MERGE-SOURCE