Merge into development-branch : 1hz-rendering-always : Code : Mir

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration	2014-04-28	Approve on 2014-05-21
Alan Griffiths			Abstain on 2014-05-21
Daniel van Vugt		2014-04-28	Abstain on 2014-05-21
Andreas Pokorny (community)		2014-04-28	Approve on 2014-05-19
Alexandros Frantzis (community)		2014-04-28	Abstain on 2014-05-16
Alberto Aguirre (community)		2014-04-28	Approve on 2014-05-16
Review via email: mp+217377@code.launchpad.net

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-04-17: Posted in a previous version of this proposal

#

Remove BufferConsumingFunctor from compositor

Now that SwitchingBufferBundle hands out buffers at a minimum, configurable, rate, we no
longer need a fake display consuming buffers at fake vsync.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:1553
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/~raof/mir/1hz-rendering-always/+merge/216246/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1350/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1639/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1637
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1212/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1082
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1082/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1087/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1213/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1350/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:1554
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/~raof/mir/1hz-rendering-always/+merge/216246/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1353/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1643
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1216/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1085/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1090/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1217/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1353/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-04-21: Posted in a previous version of this proposal

#

Code looks fine at first pass...not sure I have entirely thought out all the implications though...maybe we can discuss them on IRC or you can provide some more context here?

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-04-22: Posted in a previous version of this proposal

#

796 + {
797 + if (nfree() <= 0)
798 + {
799 + while (nready == 0)
800 + cond.wait(lock);
801 +
802 + drop_frames(1);
803 + }
804 + complete_client_acquire(std::move(lock));
805 + }
806 + });

This could be private function.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-22: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1368/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-04-22: Posted in a previous version of this proposal

#

(1) Interesting. Is 1Hz not too slow? Is that enough to solve the problem for music-app?

(2) Could you possibly avoid re-introducing timing to SwitchingBundle? It took a lot of effort to rid SwitchingBundle of all time-dependent logic, which I'm glad Alan made me do. And it's obviously more reliable not relying on timing. Or if you do then make sure it's mockable --> mir::time::Clock

(3) Does this solve other problems not explicitly mentioned? The timers plus larger code size makes me a bit nervous.

review: Needs Information

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-04-22: Posted in a previous version of this proposal

#

Sorry still having trouble forming a total opinion as to what I think is best...will continue to ruminate.

I think Daniel is right about the time sources...if that were fixed maybe

+TEST_F(BufferStreamTest, client_swaps_at_at_least_1_Hz)

could be better and not depend on real time.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-04-22: Posted in a previous version of this proposal

#

It's already mockable, via the mir::Timer interface, although I haven't used that mockability. I'll see how it looks without a mock timer.

@Daniel: (1) Gerry said 1Hz would be fast enough; the eventloop doesn't block immediately on renderloop blockage.

(2) I don't see how it's obviously more reliable not to depend on timing. This isn't changing the normal produce/consume case; this is adding a timeout on consume, which is inherently timing based. Alexandros' solution is *also* timing based, although in a different component.

(3) The actual code is not very big - if you remove the tests and test harness stuff, it's:
11 files changed, 307 insertions(+), 146 deletions(-)

If you only look at src/server/compositor:
buffer_stream_factory.cpp | 8 +
buffer_stream_factory.h | 6 -
default_configuration.cpp | 4
multi_threaded_compositor.cpp | 173 +++++++++++-------------------------------
switching_bundle.cpp | 53 ++++++++++--
switching_bundle.h | 13 ++-
6 files changed, 113 insertions(+), 144 deletions(-)

It solves all the same problems solved by Alexandros' non-blocking-swapbuffers branches, and does so in a way that's cleaner (in my opinion, obviously) and more extensible (obvious to all right-thinking humans ☺).

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-04-23: Posted in a previous version of this proposal

#

Hmm. I disagree with adding new complexity and particularly the introduction of timing logic where previously there was none.

I think all the same problems are solved by the combination of two simpler branches:
https://code.launchpad.net/~vanvugt/mir/unocclude/+merge/216690
https://code.launchpad.net/~vanvugt/mir/judder/+merge/216694

Assuming the relevant bugs are indeed all fixable with such simpler solutions, I would prefer that. So this a soft disapproval. I may be out-voted...

review: Disapprove

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1373/rebuild

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-04-23: Posted in a previous version of this proposal

#

Nit: + 819 looks strangely indented - especially with the return statement directly afterwards

Verified with my local setup.

Overall the change does not seem to make SwitchingBundle itself more complex than it already is.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1382/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-24: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1390/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-04-24: Posted in a previous version of this proposal

#

A first pass:

88 +class Alarm
124 +class Timer

Delete CopyAssign.

401 + std::unique_lock<decltype(data->m)> lock(data->m);
412 + std::unique_lock<decltype(data->m)> lock(data->m);

We could use the slightly more efficient lock_guard<> since we don't use any of unique_lock's extra capabilities.

523 + the_main_loop());

Do we want to use the same MainLoop object for both the buffer drop timeouts and the display server main loop? Although I don't see a particular functional problem with doing so, it indicates an intention to serialize events from both sources, which is not really true.

On a related note, perhaps we need a better abstraction for this, one that hides all the timing details from our buffer mechanism, e.g.:

class FrameDroppingPolicy
{
virtual std::unique_ptr<Alarm> schedule_frame_drop(std::function<void()> do_frame_drop) = 0;
};

SwitchingBundle(..., std::shared_ptr<FrameDroppingPolicy>, ...)

Needs Fixing (Needs discussion about the abstraction part)

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-28:

#

FAILED: Continuous integration, rev:1563
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1411/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1720/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1718/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1290/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1143/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1148/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1293/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1411/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-04-28:

#

1) I think we do want to use the same MainLoop object by default, although I could see having a separate the_timer() top-level and returning the mainloop from it might make the intent clearer.

2) I deliberately didn't use a more elaborate abstraction for the initial merge, but if we want to go the whole hog, sure.

In that case, I think the policy interface wants to be something like...

class FrameDroppingPolicy
{
    virtual void swap_now_blocking(std::function<void(void)> do_frame_drop) = 0;
    virtual void swap_unblocked() = 0;
    virtual bool frame_drop_pending() const = 0; /* Possibly don't need this */
};

The SwitchingBundle would then call swap_now_blocking(...) where we currently schedule the alarm, and swap_unblocked() from compositor_release(). If we decide that calling swap_unblocked() when there isn't a swap pending is an error, then we need frame_drop_pending() otherwise we can just unconditionally call swap_unblocked().

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-28:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1412/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-04-28:

#

> In that case, I think the policy interface wants to be something like...

One decision we need to make (we can change it later of course), is whether we want a policy object per bundle or a single central policy object. The advantage of the later is that it allows us to change the drop timeout centrally, which could be useful if we want to set an optimal frame rate depending on the monitors present and power mode. For example we may want to set the drop rate as high as possible (but still below the smallest vsync rate for our timeout logic to work) when the device is plugged in, but drop to a lower rate for power saving modes.

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-04-28:

#

I missed all the discussion about this, but could someone please summarize why this is needed?

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-04-29:

#

Gerry - it's the proper¹ fix for “swapbuffers shouldn't block indefinitely”, moving the responsibility for unblocking clients from the Compositor to the BufferBundle (where the actual blocking logic is).

¹: In my opinion, and it seems the general consensus.

Unless you mean the FrameDroppingPolicy discussion, which is thorourghly theoretical :).

Alexandros - can we split the difference with

virtual MirCookie swap_now_blocking(<>);
virtual swap_unblocked(MirCookie);

? That way we've (a) hidden the underlying Alarm implementation away, which would otherwise leak timeness, and (b) can choose between single-policy-object or policy-object-per-SwitchingBundle at DefaultConfiguration time.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-04-29:

#

45 +// Quite why C++11 doesn't have an existing std::semaphore is a mystery

I don't think it is so much a mystery as an artifact of the standards process. Essentially there wasn't an agreed proposal ready. IIRC there are things like barrier and semaphore in C++14.

In any case, doesn't the canonical semaphore take a count in initialization?

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-04-29:

#

Looks sensible (apart from the following nits). But as it rips out some of Alexandros's stop-gap code I'd like to see his approval before it lands.

~~~~

36 +#ifndef MIR_SEMAPHORE_H_
37 +#define MIR_SEMAPHORE_H_

#ifndef MIR_TEST_FRAMEWORK_...

~~~~

54 + void wait(void);

In C++ we say "void wait();"

~~~~

113 + explicit BufferStreamFactory(const std::shared_ptr<graphics::GraphicBufferAllocator> &gralloc, const std::shared_ptr<time::Timer> timer);

s/const std::shared_ptr<time::Timer>/std::shared_ptr<time::Timer> const&/

review: Needs Fixing

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-04-29:

#

46 +class Semaphore

Perhaps BinarySemaphore?

The framedrop on timeout looks pretty straightforward to me. However I still maintain that the timeout should
only be enabled for the 2 conditions where we actually need it: when a surface is not visible on any display (due to a display off condition) and when a surface is occluded. But that's the FrameDroppingPolicy discussion, which I am okay coming in a separate MP.

~~~
TEST_F(SwitchingBundleTest, compositor_swapping_at_min_rate_gets_oldest_buffer)
~~~

Is this an implicit policy? oldest with respect to what? client-release?

review: Approve

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-04-30:

#

With respect to client-release, yes. The idea being that the compositor shouldn't skip frames - if it takes anything but the oldest frame it's going to have to *drop* all older frames, because it can't present frames out of order.

Should we add the ability to specify the presentation-time (such an API is entirely reasonable in EGL) this wouldn't be true, but then we'll need to rework the interfaces and implementation anyway.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-02:

#

I'm still uncomfortable with the introduction of timing, especially after I went to the trouble of removing all timing a while back. Solving timing problems without measuring time itself is always more robust and elegant, albeit more difficult to conceive. But without looking at the code...

(1) Needs fixing:
Text conflict in src/server/compositor/multi_threaded_compositor.cpp
Text conflict in tests/unit-tests/compositor/test_multi_threaded_compositor.cpp
2 conflicts encountered.

and then after hacking around those, manual testing seems OK.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-06:

#

Now BufferQueue has landed, lots more conflicts:

Text conflict in src/server/compositor/buffer_stream_factory.cpp
Text conflict in src/server/compositor/multi_threaded_compositor.cpp
Contents conflict in src/server/compositor/switching_bundle.cpp
Contents conflict in src/server/compositor/switching_bundle.h
Text conflict in tests/integration-tests/compositor/test_buffer_stream.cpp
Text conflict in tests/integration-tests/compositor/test_swapping_swappers.cpp
Text conflict in tests/integration-tests/graphics/android/test_buffer_integration.cpp
Text conflict in tests/unit-tests/compositor/test_buffer_queue.cpp
Text conflict in tests/unit-tests/compositor/test_multi_threaded_compositor.cpp
9 conflicts encountered.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-06:

#

Needs rebasing on BufferQueue

review: Needs Resubmitting

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-08:

#

FAILED: Continuous integration, rev:1580
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1554/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/94/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/94/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/73/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/71/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/215/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1554/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-08:

#

FAILED: Continuous integration, rev:1581
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1556/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/96/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/97/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/97/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/75/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/73/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/218/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1556/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-08:

#

CI says:

[ 63%] /mir/tests/mir_test_doubles/mock_frame_dropping_policy_factory.cpp:19:65: fatal error: mir_test_doubles/mock_frame_dropping_policy_factory.h: No such file or directory
#include "mir_test_doubles/mock_frame_dropping_policy_factory.h"

~~~~

315 +#endif // MIR_TEST_SEMAPHORE_H_

We're calling it Signal in this revision.

~~~~

2549 + MOCK_METHOD1(create_alarm, std::unique_ptr<mir::time::Alarm>(std::function<void(void)>));

Why create a mock method when we don't set expectations or behavior?

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-09:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1572/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-09:

#

The numbers make me worried: 3226 lines (+1567/-562) 41 files modified
Although it's a fairly serious bug so moving on...

(1) The "Policy" classes don't feel concrete enough to convince me that the design is quite right. I believe it's likely that needs rethinking, but don't yet understand the new architecture enough to make suggestions. Maybe next week.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-09:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1574/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-09:

#

No opinion until prerequisite is dealt with

review: Abstain

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-12:

#

Oops, "Needs fixing" is well out of date now.

review: Abstain

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-05-13:

#

~~~
1179 +void TimeoutFrameDroppingPolicy::swap_now_blocking()
1180 +{
1181 + if (pending_swaps++ == 0)
1182 + alarm->reschedule_in(timeout);
1183 +}
1184 +
1185 +void TimeoutFrameDroppingPolicy::swap_unblocked()
1186 +{
1187 + if (alarm->state() != mir::time::Alarm::Cancelled && alarm->cancel())
1188 + pending_swaps--;
1189 +}
1190 +}
~~~

Let's say the client calls client_acquire twice and there are no buffers available, so the two requests are delegated (pending_client_notifications.size == 2)

swap_now_blocking is called for the first delegated request (pending_swaps == 0) so an alarm is scheduled.
1181 + if (pending_swaps++ == 0)
1182 + alarm->reschedule_in(timeout);

The second call to swap_now_blocking does not create an alarm as pending_swaps == 1. That's ok.

Now let's say the first request is satisfied before the timeout expires- hence swap_unblocked is called and the alarm
is cancelled (pending_swaps == 1 and pending_client_notifications.size == 1)

Let's assume that after that point no requests will be satisfied (compositor blocked or stopped).

The client submits the buffer it got (client_release). The client has a pending request and there are no buffers available, but there is no alarm scheduled to drop the buffer so the client waits indefinitely.

In any case, I've started testing this MP with unity8 and the main case of having the music app respond to volume keys during screen power off works fine.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-13:

#

FAILED: Continuous integration, rev:1591
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1602/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/161/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/162/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/162/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/121/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/119/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/373/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1602/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-05-13:

#

Download full text (4.0 KiB)

Conflicts with lp:~andreas-pokorny/mir/add-timer-to-main-loop

Text conflict in include/server/mir/asio_main_loop.h
Text conflict in include/server/mir/default_server_configuration.h
Text conflict in src/server/asio_main_loop.cpp
Text conflict in tests/mir_test/CMakeLists.txt
Text conflict in tests/unit-tests/test_asio_main_loop.cpp

~~~
2146 + clock->register_time_change_callback([state_copy](mt::FakeClock::time_point now)
2147 + {
2148 + if (state_copy->state == Pending)
2149 + {
2150 + if (now > state_copy->threshold)
2151 + {
2152 + state_copy->state = Triggered;
2153 + state_copy->callback();
2154 + return false;
2155 + }
2156 + return true;
2157 + }
2158 + else
2159 + {
2160 + return false;
2161 + }
2162 + });
~~~

This indentation looks awkward. Maybe a private method for that multi-line lambda?

~~~
2568 + auto buffers_swapped = std::make_shared<mt::Signal>();
2569 +
2570 + q.client_acquire([buffers_swapped](mg::Buffer*){ buffers_swapped->raise(); });
2571 +
2572 + EXPECT_FALSE(buffers_swapped->wait_for(std::chrono::milliseconds{100}));
2573 +
2574 + policy_factory.trigger_policies();
2575 + EXPECT_TRUE(buffers_swapped->wait_for(std::chrono::milliseconds{100}));
~~~

This can be replaced with:

---
auto handle = client_acquire_async(q);
EXPECT_THAT(handle->has_acquired_buffer(), Eq(false));
policy_factory.trigger_policies();
EXPECT_THAT(handle->has_acquired_buffer(), Eq(true));
---

~~~
2598 + auto first_swap = std::make_shared<mt::Signal>();
2599 + auto second_swap = std::make_shared<mt::Signal>();
2600 +
2601 + /* Queue up two pending swaps */
2602 + mg::Buffer* client_buf;
2603 + q.client_acquire([first_swap, &client_buf](mg::Buffer* buffer)
2604 + {
2605 + client_buf = buffer;
2606 + first_swap->raise();
2607 + });
2608 + q.client_acquire([second_swap](mg::Buffer*){ second_swap->raise(); });
2609 +
2610 + ASSERT_FALSE(first_swap->raised());
2611 + ASSERT_FALSE(second_swap->raised());
2612 +
2613 + q.compositor_acquire(nullptr);
2614 +
2615 + EXPECT_TRUE(first_swap->wait_for(std::chrono::milliseconds{100}));
2616 + EXPECT_FALSE(second_swap->raised());
2617 +
2618 + /* We have to release a client buffer here; framedropping or not,
2619 + * a client can't have 2 buffers outstanding in the nbuffers = 2 case.
2620 + */
2621 + q.cli...

Conflicts with lp:~andreas-pokorny/mir/add-timer-to-main-loop

Text conflict in include/server/mir/asio_main_loop.h
Text conflict in include/server/mir/default_server_configuration.h
Text conflict in src/server/asio_main_loop.cpp
Text conflict in tests/mir_test/CMakeLists.txt
Text conflict in tests/unit-tests/test_asio_main_loop.cpp

~~~
2146	+    clock->register_time_change_callback([state_copy](mt::FakeClock::time_point now)
2147	+                                         {
2148	+                                             if (state_copy->state == Pending)
2149	+                                             {
2150	+                                                 if (now > state_copy->threshold)
2151	+                                                 {
2152	+                                                     state_copy->state = Triggered;
2153	+                                                     state_copy->callback();
2154	+                                                     return false;
2155	+                                                 }
2156	+                                                 return true;
2157	+                                             }
2158	+                                             else
2159	+                                             {
2160	+                                                 return false;
2161	+                                             }
2162	+                                         });
~~~

This indentation looks awkward. Maybe a private method for that multi-line lambda?

~~~
2568	+        auto buffers_swapped = std::make_shared<mt::Signal>();
2569	+
2570	+        q.client_acquire([buffers_swapped](mg::Buffer*){ buffers_swapped->raise(); });
2571	+
2572	+        EXPECT_FALSE(buffers_swapped->wait_for(std::chrono::milliseconds{100}));
2573	+
2574	+        policy_factory.trigger_policies();
2575	+        EXPECT_TRUE(buffers_swapped->wait_for(std::chrono::milliseconds{100}));
~~~

This can be replaced with:

---
auto handle = client_acquire_async(q);
EXPECT_THAT(handle->has_acquired_buffer(), Eq(false));
policy_factory.trigger_policies();
EXPECT_THAT(handle->has_acquired_buffer(), Eq(true));
---

~~~
2598	+        auto first_swap = std::make_shared<mt::Signal>();
2599	+        auto second_swap = std::make_shared<mt::Signal>();
2600	+
2601	+        /* Queue up two pending swaps */
2602	+        mg::Buffer* client_buf;
2603	+        q.client_acquire([first_swap, &client_buf](mg::Buffer* buffer)
2604	+        {
2605	+            client_buf = buffer;
2606	+            first_swap->raise();
2607	+        });
2608	+        q.client_acquire([second_swap](mg::Buffer*){ second_swap->raise(); });
2609	+
2610	+        ASSERT_FALSE(first_swap->raised());
2611	+        ASSERT_FALSE(second_swap->raised());
2612	+
2613	+        q.compositor_acquire(nullptr);
2614	+
2615	+        EXPECT_TRUE(first_swap->wait_for(std::chrono::milliseconds{100}));
2616	+        EXPECT_FALSE(second_swap->raised());
2617	+
2618	+        /* We have to release a client buffer here; framedropping or not,
2619	+         * a client can't have 2 buffers outstanding in the nbuffers = 2 case.
2620	+         */
2621	+        q.client_release(client_buf);
2622	+
2623	+        policy_factory.trigger_policies();
2624	+        EXPECT_TRUE(second_swap->wait_for(std::chrono::milliseconds{100}));
~~~

This can be replaced with:
---
/* Queue up two pending swaps */
auto handle1 = client_acquire_async(q);
auto handle2 = client_acquire_async(q);
ASSERT_THAT(handle1->has_acquired_buffer(), Eq(false));
ASSERT_THAT(handle2->has_acquired_buffer(), Eq(false));

q.compositor_acquire(this);

EXPECT_THAT(handle1->has_acquired_buffer(), Eq(true));
EXPECT_THAT(handle2->has_acquired_buffer(), Eq(false));

handle1->release_buffer();
policy_factory.trigger_policies();
EXPECT_THAT(handle2->has_acquired_buffer(), Eq(true));
---

~~~
2630	+TEST_F(BufferQueueTest, compositor_swapping_at_min_rate_gets_oldest_buffer)
~~~

That test implementation still depends on the  WatchDog class.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-14:

#

Text conflict in include/server/mir/asio_main_loop.h
Text conflict in include/server/mir/default_server_configuration.h
Text conflict in src/server/asio_main_loop.cpp
Text conflict in tests/mir_test/CMakeLists.txt
Text conflict in tests/unit-tests/test_asio_main_loop.cpp
5 conflicts encountered.

:)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-14:

#

Merge conflicts

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-15:

#

FAILED: Continuous integration, rev:1594
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1629/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/195/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/196/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/196/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/148/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/147/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/478/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1629/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-15:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1630/rebuild

review: Approve (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) on 2014-05-15:

#

review: Abstain

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-15:

#

Made a first run through the MP:

There already is WaitObject and WaitCondition and now Signal. Two of them provide a boolean to indicate that the wait did not time out, while the first one throws. WaitCondition has Mock Actions, and provides seconds as duration format. Signal provides both a duration and a time point to wait for. Unifying that would be nice. From the three names Signal sounds best. WaitCondition is a bit off as it indicates that there is a condition somewhere.

What about:

inline int max_ownable_buffers(int number_of_available_buffers) { return number_of_available_buffers - 1; }

to avoid the new TODOs.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-05-15:

#

Verified both bugs are fixed.

I'm not a big fan of the new design. Particularly:

(1) framedrop_policy is an object that you modify? Making policy an object is one thing I don't like but can kind of understand. However, modifying a policy object on the fly doesn't sound like the design is quite right.

On the other hand, I am a big fan of fixing bug 1308843 and bug 1308844. And the minimal intrusion to the BufferQueue class is also nice.

review: Abstain (manual testing)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-05-15:

#

114 +class FrameDroppingPolicyFactory
115 +{

Delete CopyAssign (unless we want them for some reason?).

448 +class MockTimer : public mir::time::Timer

This is a FakeTimer not a MockTimer.

review: Needs Fixing

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-05-16:

#

LGTM

review: Approve

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-05-16:

#

@Alexandros: I don't think it _is_ a FakeTimer; it's a Timer that you set the behaviour of (although admittedly via an external parameter).

Updated anyway.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-05-16:

#

I'll propose the merge of WaitObject, WaitCondition, and Signal in a follow-up branch. This is probably big enough :)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-16:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1645/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-16:

#

661 + //TODO: We have to framedrop, but we can't.
662 + //assert(), throw, or ignore?
663 + return;

Is this a request for discussion?

~~~~

61 +class FrameDroppingPolicy
...
70 + /**
71 + * \brief Notify that a swap has blocked
72 + */
73 + virtual void swap_now_blocking() = 0;
74 + /**
75 + * \brief Notify that previous swap is no longer blocking
76 + */
77 + virtual void swap_unblocked() = 0;

I don't immediately understand the role of a "policy" class that doesn't seem to be responsible for doing anything (it only has these two member functions). Based on the functions it has is a SwapBlockageObserver not a FrameDroppingPolicy.

Also based on the comments why isn't "swap_now_blocking()" called "swap_has_blocked()"? And "swap_unblocked()" "swap_has_unblocked()"?

review: Needs Information

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-05-16:

#

Didn't get a chance to review properly today. Abstaining for now to unblock.

review: Abstain

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-05-19:

#

On Fri, May 16, 2014 at 9:57 PM, Alan Griffiths <email address hidden>
wrote:
> Review: Needs Information
>
> 661 + //TODO: We have to framedrop, but we can't.
> 662 + //assert(), throw, or ignore?
> 663 + return;
>
> Is this a request for discussion?

Yes, albeit a low-priority one; this case can't happen the way we
currently use BufferQueue, as it can only occur when a client is
requesting a buffer without first having submitted a buffer. Since we
only allow clients to have a single buffer outstanding, we don't
currently hit this.

Having said this, I think that means we should assert() so that we'll
pick this
up if and when we allow clients to claim multiple buffers.

>
> ~~~~
>
> 61 +class FrameDroppingPolicy
> ...
> 70 + /**
> 71 + * \brief Notify that a swap has blocked
> 72 + */
> 73 + virtual void swap_now_blocking() = 0;
> 74 + /**
> 75 + * \brief Notify that previous swap is no longer blocking
> 76 + */
> 77 + virtual void swap_unblocked() = 0;
>
> I don't immediately understand the role of a "policy" class that
> doesn't seem to be responsible for doing anything (it only has these
> two member functions). Based on the functions it has is a
> SwapBlockageObserver not a FrameDroppingPolicy.

It makes frame dropping decisions based upon notifications from the
buffer queue.

The mechanism it uses for dropping frames is set at creation time,
which is why it doesn't appear in the interface.

>
> Also based on the comments why isn't "swap_now_blocking()" called
> "swap_has_blocked()"? And "swap_unblocked()" "swap_has_unblocked()"?

Hm. I clearly need to reword the comment; it's swap_now_blocking()
because the blocking is ongoing, and will continue to be until the
policy decides to framedrop or the swap_unblocked() is called.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-19:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1657/rebuild

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-19:

#

> [...]
> Having said this, I think that means we should assert() so that we'll
> pick this
> up if and when we allow clients to claim multiple buffers.

This not something a client can trigger in any way - so assert seems right. But It is not asserting right now?

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-05-19:

#

On Mon, May 19, 2014 at 5:54 PM, Andreas Pokorny
<email address hidden> wrote:
>> [...]
>> Having said this, I think that means we should assert() so that
>> we'll
>> pick this
>> up if and when we allow clients to claim multiple buffers.
>
> This not something a client can trigger in any way - so assert seems
> right. But It is not asserting right now?

I ended up deciding that it's not an actual problem. The only way to
trigger it is for the client to hold multiple buffers and then request
another one without submitting a previous buffer (or to use
single-buffering, but I continue to maintain that's inherently broken).

If the client already has a buffer but hasn't submitted it then it's
got something to render to, and so swap_buffers *shouldn't* block, so
we don't need to framedrop.

Revision history for this message

Andreas Pokorny (andreas-pokorny) on 2014-05-19:

#

review: Approve

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-05-19:

#

Download full text (4.3 KiB)

Apologies for meandering thoughts. But this is continuing the *Needs discussion*...

Short version: while I'm in agreement with the approach this branch proposes I've hung up on a lack of design clarity around "FrameDroppingPolicy" which isn't really a policy and doesn't encapsulates the framedropping logic.

> > 61 +class FrameDroppingPolicy
> > ...
> > 70 + /**
> > 71 + * \brief Notify that a swap has blocked
> > 72 + */
> > 73 + virtual void swap_now_blocking() = 0;
> > 74 + /**
> > 75 + * \brief Notify that previous swap is no longer blocking
> > 76 + */
> > 77 + virtual void swap_unblocked() = 0;
> >
> > I don't immediately understand the role of a "policy" class that
> > doesn't seem to be responsible for doing anything (it only has these
> > two member functions). Based on the functions it has is a
> > SwapBlockageObserver not a FrameDroppingPolicy.
>
> It makes frame dropping decisions based upon notifications from the
> buffer queue.
>
> The mechanism it uses for dropping frames is set at creation time,
> which is why it doesn't appear in the interface.

I don't think you've quite understood this concern. It isn't the role of TimeoutFrameDroppingPolicy I'm saying is unclear it is FrameDroppingPolicy that I'm doubtful about. The code that uses it looks like this:

711 + /* Can't give the client a buffer yet; they'll just have to wait
712 + * until the compositor is done with an old frame, or the policy
713 + * says they've waited long enough.
714 + */
715 + framedrop_policy->swap_now_blocking();

and

724 + framedrop_policy->swap_unblocked();
725 give_buffer_to_client(buffer, std::move(lock));

Neither fragment looks to me like a call to be handled by a "policy". (They look a bit more like state transitions - but if that that model is correct "enter_swap_blocked()/exit_swap_blocked() is probably clearer.)

Actually, BufferQueue still has:

    /* Last resort, drop oldest buffer from the ready queue */
    if (frame_dropping_enabled && !ready_to_composite_queue.empty())
    {
        auto const buffer = pop(ready_to_composite_queue);
        give_buffer_to_client(buffer, std::move(lock));

So we have both framedrop_policy and frame_dropping_enabled in BufferQueue. So we're not covering all the policy around framedropping in TimeoutFrameDroppingPolicy.

> > Also based on the comments why isn't "swap_now_blocking()" called
> > "swap_has_blocked()"? And "swap_unblocked()" "swap_has_unblocked()"?
>
> Hm. I clearly need to reword the comment; it's swap_now_blocking()
> because the blocking is ongoing, and will continue to be until the
> policy decides to framedrop or the swap_unblocked() is called.

I think the need for the comment (and that on 711..714) makes it clear that the current name is unsatifactory. C.f.

711 + /* Can't give the client a buffer yet; they'll just have to wait
712 + * until the compositor is done with an old frame, or the policy
713 + * says they've waited long enough.
714 + */
715 + framedrop_policy->handle_swap_buffer_unavailable();
...
724 + framedrop_policy->handle_swap_buffer_available();

However, the more I think about it the more I think the interaction between FrameDroppingPoli...

Mir

Merge lp:~raof/mir/1hz-rendering-always into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'include/platform/mir/graphics/buffer_id.h'
 --- include/platform/mir/graphics/buffer_id.h	2013-08-28 03:41:48 +0000
 +++ include/platform/mir/graphics/buffer_id.h	2014-05-21 00:42:11 +0000
@@ -27,7 +27,7 @@
  class BufferID
+ {
  public:
--    BufferID() : value(id_invalid){}
++    BufferID() noexcept: value(id_invalid){}
      explicit BufferID(uint32_t val) : value(val) {}
      bool is_valid() const { return (id_invalid != value); }
      uint32_t as_uint32_t() const { return value; };
 === modified file 'include/server/mir/asio_main_loop.h'
 --- include/server/mir/asio_main_loop.h	2014-05-19 02:55:29 +0000
 +++ include/server/mir/asio_main_loop.h	2014-05-21 00:42:11 +0000
@@ -53,6 +53,8 @@
                                             std::function<void()> callback) override;
      std::unique_ptr<time::Alarm> notify_at(mir::time::Timestamp time_point,
                                             std::function<void()> callback) override;
++    std::unique_ptr<time::Alarm> create_alarm(std::function<void()> callback) override;
++
      void enqueue(void const* owner, ServerAction const& action);
      void pause_processing_for(void const* owner);
      void resume_processing_for(void const* owner);
 === added file 'include/server/mir/compositor/frame_dropping_policy.h'
 --- include/server/mir/compositor/frame_dropping_policy.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/compositor/frame_dropping_policy.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,69 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_COMPOSITOR_FRAME_DROPPING_POLICY_H_
++#define MIR_COMPOSITOR_FRAME_DROPPING_POLICY_H_
++
++#include <functional>
++
++namespace mir
++{
++namespace compositor
++{
++/**
++ * \brief Policy to determine when to drop a frame from a client
++ *
++ * The FrameDroppingPolicy objects are constructed from a
++ * \ref FrameDroppingPolicyFactory
++ *
++ * The frame dropping mechanism is provided as the
++ * \a drop_frames argument of \ref FrameDroppingPolicyFactory::create_policy
++ *
++ * The policy may decide to drop a frame any time that there is an outstanding
++ * swap - namely, when there have been more calls to \ref swap_now_blocking
++ * than to \ref swap_unblocked
++ */
++class FrameDroppingPolicy
++{
++public:
++    virtual ~FrameDroppingPolicy() = default;
++
++    FrameDroppingPolicy(FrameDroppingPolicy const&) = delete;
++    FrameDroppingPolicy& operator=(FrameDroppingPolicy const&) = delete;
++
++    /**
++     * \brief Notify that a swap is now blocking
++     */
++    virtual void swap_now_blocking() = 0;
++    /**
++     * \brief Notify that previous swap is no longer blocking
++     */
++    virtual void swap_unblocked() = 0;
++
++protected:
++    /**
++     * \note FrameDroppingPolicies should not be constructed directly;
++     *       use a \ref FrameDroppingPolicyFactory
++     */
++    FrameDroppingPolicy() = default;
++};
++
++}
++}
++
++#endif // MIR_COMPOSITOR_FRAME_DROPPING_POLICY_H_
 === added file 'include/server/mir/compositor/frame_dropping_policy_factory.h'
 --- include/server/mir/compositor/frame_dropping_policy_factory.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/compositor/frame_dropping_policy_factory.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,57 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_COMPOSITOR_FRAME_DROPPING_POLICY_FACTORY_H_
++#define MIR_COMPOSITOR_FRAME_DROPPING_POLICY_FACTORY_H_
++
++#include <memory>
++
++namespace mir
++{
++namespace compositor
++{
++class FrameDroppingPolicy;
++
++/**
++ * \brief Creator of FrameDroppingPolicies
++ *
++ * The FrameDroppingPolicyFactory is how you go from a means of dropping frames -
++ * the \a drop_frames parameter of \ref create_policy -
++ * to a \ref FrameDroppingPolicy
++ */
++class FrameDroppingPolicyFactory
++{
++public:
++    FrameDroppingPolicyFactory() = default;
++    virtual ~FrameDroppingPolicyFactory() = default;
++
++    FrameDroppingPolicyFactory(FrameDroppingPolicyFactory const&) = delete;
++    FrameDroppingPolicyFactory& operator=(FrameDroppingPolicyFactory const&) = delete;
++
++    /**
++     * \brief Create a FrameDroppingPolicy that will call \a drop_frame when it decides to drop a frame
++     * \param drop_frame    Function to call when a frame needs to be dropped
++     * \return              The policy object.
++     */
++    virtual std::unique_ptr<FrameDroppingPolicy> create_policy(std::function<void(void)> drop_frame) const = 0;
++};
++
++}
++}
++
++#endif // MIR_COMPOSITOR_FRAME_DROPPING_POLICY_FACTORY_H_
 === modified file 'include/server/mir/default_server_configuration.h'
 --- include/server/mir/default_server_configuration.h	2014-05-20 08:38:05 +0000
 +++ include/server/mir/default_server_configuration.h	2014-05-21 00:42:11 +0000
@@ -46,6 +46,7 @@
  class Compositor;
  class RendererFactory;
  class CompositorReport;
++class FrameDroppingPolicyFactory;
+ }
  namespace frontend
+ {
@@ -195,6 +196,7 @@
       *  @{ */
      virtual std::shared_ptr<graphics::GraphicBufferAllocator> the_buffer_allocator();
      virtual std::shared_ptr<compositor::Scene>                  the_scene();
++    virtual std::shared_ptr<compositor::FrameDroppingPolicyFactory> the_frame_dropping_policy_factory();
      /** @} */
      /** @name frontend configuration - dependencies
@@ -321,6 +323,7 @@
      CachedPtr<frontend::Screencast> screencast;
      CachedPtr<compositor::RendererFactory> renderer_factory;
      CachedPtr<compositor::BufferStreamFactory> buffer_stream_factory;
++    CachedPtr<compositor::FrameDroppingPolicyFactory> frame_dropping_policy_factory;
      CachedPtr<scene::SurfaceStack> surface_stack;
      CachedPtr<scene::SceneReport> scene_report;
 === modified file 'include/server/mir/time/timer.h'
 --- include/server/mir/time/timer.h	2014-04-24 13:58:18 +0000
 +++ include/server/mir/time/timer.h	2014-05-21 00:42:11 +0000
@@ -58,7 +58,14 @@
       */
      virtual std::unique_ptr<Alarm> notify_at(Timestamp time_point,
                                               std::function<void()> callback) = 0;
--
++    /**
++     * \brief Create an Alarm that will not fire until scheduled
++     *
++     * \param callback  Function to call when the Alarm signals
++     *
++     * \return A handle to an Alarm that can later be scheduled
++     */
++    virtual std::unique_ptr<Alarm> create_alarm(std::function<void()> callback) = 0;
      Timer(Timer const&) = delete;
      Timer& operator=(Timer const&) = delete;
 === added file 'include/test/mir_test/fake_clock.h'
 --- include/test/mir_test/fake_clock.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test/fake_clock.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,72 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_TEST_FAKE_CLOCK_H_
++#define MIR_TEST_FAKE_CLOCK_H_
++
++#include <chrono>
++#include <functional>
++#include <list>
++
++namespace mir
++{
++namespace test
++{
++/**
++ * @brief An invasive time source for Mocks/Stubs/Fakes that depend on timing
++ */
++class FakeClock
++{
++public:
++    typedef std::chrono::nanoseconds duration;
++    typedef duration::rep rep;
++    typedef duration::period period;
++    typedef std::chrono::time_point<FakeClock, duration> time_point;
++
++    static constexpr bool is_steady = false;
++    time_point now() const;
++
++    FakeClock();
++    /**
++     * \brief Advance the fake clock
++     * \note Advancing by a negative duration will move the clock backwards
++     */
++    template<typename rep, typename period>
++    void advance_time(std::chrono::duration<rep, period> by)
++    {
++        advance_time_ns(std::chrono::duration_cast<std::chrono::nanoseconds>(by));
++    }
++
++    /**
++     * \brief Register an event callback when the time is changed
++     * \param cb    Function to call when the time is changed.
++     *              This function is called with the new time.
++     *              If the function returns false, it will no longer be called
++     *              on subsequent time changes.
++     */
++    void register_time_change_callback(std::function<bool(time_point)> cb);
++private:
++    void advance_time_ns(std::chrono::nanoseconds by);
++    std::chrono::nanoseconds current_time;
++    std::list<std::function<bool(time_point)>> callbacks;
++};
++
++}
++}
++
++#endif // MIR_TEST_FAKE_CLOCK_H_
 === added file 'include/test/mir_test/signal.h'
 --- include/test/mir_test/signal.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test/signal.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,63 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_TEST_SIGNAL_H_
++#define MIR_TEST_SIGNAL_H_
++
++#include <condition_variable>
++#include <chrono>
++#include <mutex>
++
++namespace mir
++{
++namespace test
++{
++/**
++ * @brief A threadsafe, waitable signal
++ */
++class Signal
++{
++public:
++    Signal();
++
++    void raise();
++    bool raised();
++
++    void wait();
++    template<typename rep, typename period>
++    bool wait_for(std::chrono::duration<rep, period> delay)
++    {
++        std::unique_lock<decltype(mutex)> lock(mutex);
++        return cv.wait_for(lock, delay, [this]() { return signalled; });
++    }
++    template<class Clock, class Duration>
++    bool wait_until(std::chrono::time_point<Clock, Duration> const& time)
++    {
++        std::unique_lock<decltype(mutex)> lock(mutex);
++        return cv.wait_until(lock, time, [this]() { return signalled; });
++    }
++
++private:
++    std::mutex mutex;
++    std::condition_variable cv;
++    bool signalled;
++};
++}
++}
++
++#endif // MIR_TEST_SIGNAL_H_
 === added file 'include/test/mir_test_doubles/mock_frame_dropping_policy_factory.h'
 --- include/test/mir_test_doubles/mock_frame_dropping_policy_factory.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/mock_frame_dropping_policy_factory.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,83 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++
++#ifndef MIR_TEST_DOUBLES_MOCK_FRAME_DROPPING_POLICY_FACTORY_H_
++#define MIR_TEST_DOUBLES_MOCK_FRAME_DROPPING_POLICY_FACTORY_H_
++
++#include "mir/compositor/frame_dropping_policy_factory.h"
++#include "mir/compositor/frame_dropping_policy.h"
++
++#include <unordered_set>
++#include <functional>
++
++#include <gmock/gmock.h>
++
++namespace mc = mir::compositor;
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class MockFrameDroppingPolicyFactory;
++
++class MockFrameDroppingPolicy : public mc::FrameDroppingPolicy
++{
++public:
++    MockFrameDroppingPolicy(std::function<void(void)> callback,
++                            MockFrameDroppingPolicyFactory const* parent);
++    ~MockFrameDroppingPolicy();
++
++    MOCK_METHOD0(swap_now_blocking, void(void));
++    MOCK_METHOD0(swap_unblocked, void(void));
++
++    void trigger();
++
++private:
++    friend class MockFrameDroppingPolicyFactory;
++    void parent_destroyed();
++
++    std::function<void(void)> callback;
++    MockFrameDroppingPolicyFactory const* parent;
++};
++
++class MockFrameDroppingPolicyFactory : public mc::FrameDroppingPolicyFactory
++{
++public:
++    std::unique_ptr<mc::FrameDroppingPolicy> create_policy(std::function<void(void)> drop_frame) const override;
++
++    ~MockFrameDroppingPolicyFactory();
++
++    void trigger_policies() const;
++
++private:
++    friend class MockFrameDroppingPolicy;
++
++    void policy_destroyed(MockFrameDroppingPolicy* policy) const;
++    mutable std::unordered_set<MockFrameDroppingPolicy*> policies;
++};
++
++}
++}
++}
++
++
++#endif // MIR_TEST_DOUBLES_MOCK_FRAME_DROPPING_POLICY_FACTORY_H_
 === added file 'include/test/mir_test_doubles/mock_timer.h'
 --- include/test/mir_test_doubles/mock_timer.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/mock_timer.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,52 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_TEST_DOUBLES_MOCK_TIMER_H_
++#define MIR_TEST_DOUBLES_MOCK_TIMER_H_
++
++#include "mir/time/timer.h"
++#include "mir_test/fake_clock.h"
++#include <memory>
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class FakeTimer : public mir::time::Timer
++{
++public:
++    FakeTimer(std::shared_ptr<FakeClock> const& clock);
++
++    std::unique_ptr<time::Alarm> notify_in(std::chrono::milliseconds delay,
++                                           std::function<void(void)> callback) override;
++    std::unique_ptr<time::Alarm> notify_at(time::Timestamp time_point,
++                                           std::function<void(void)> callback) override;
++    std::unique_ptr<time::Alarm> create_alarm(std::function<void ()> callback) override;
++
++private:
++    std::shared_ptr<FakeClock> const clock;
++};
++
++}
++}
++}
++
++#endif // MIR_TEST_DOUBLES_MOCK_TIMER_H_
 === added file 'include/test/mir_test_doubles/stub_frame_dropping_policy_factory.h'
 --- include/test/mir_test_doubles/stub_frame_dropping_policy_factory.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/stub_frame_dropping_policy_factory.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,58 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++
++#ifndef MIR_TEST_DOUBLES_STUB_FRAME_DROPPING_POLICY_FACTORY_H_
++#define MIR_TEST_DOUBLES_STUB_FRAME_DROPPING_POLICY_FACTORY_H_
++
++#include "mir/compositor/frame_dropping_policy_factory.h"
++#include "mir/compositor/frame_dropping_policy.h"
++
++namespace mc = mir::compositor;
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class StubFrameDroppingPolicy : public mc::FrameDroppingPolicy
++{
++public:
++    void swap_now_blocking()
++    {
++    }
++    void swap_unblocked()
++    {
++    }
++};
++
++class StubFrameDroppingPolicyFactory : public mc::FrameDroppingPolicyFactory
++{
++public:
++    std::unique_ptr<mc::FrameDroppingPolicy> create_policy(std::function<void(void)>) const override
++    {
++        return std::unique_ptr<mc::FrameDroppingPolicy>{new StubFrameDroppingPolicy};
++    }
++};
++
++}
++}
++}
++#endif // TEST_DOUBLES_STUB_FRAME_DROPPING_POLICY_FACTORY_H_
 === modified file 'src/server/asio_main_loop.cpp'
 --- src/server/asio_main_loop.cpp	2014-05-19 02:55:29 +0000
 +++ src/server/asio_main_loop.cpp	2014-05-21 00:42:11 +0000
@@ -172,6 +172,9 @@
                mir::time::Timestamp time_point,
                std::function<void(void)> callback);
++    AlarmImpl(boost::asio::io_service& io,
++              std::function<void(void)> callback);
++
      ~AlarmImpl() noexcept override;
      bool cancel() override;
@@ -200,8 +203,7 @@
  AlarmImpl::AlarmImpl(boost::asio::io_service& io,
                       std::chrono::milliseconds delay,
                       std::function<void ()> callback)
--    : timer{io},
--      data{std::make_shared<InternalState>(callback)}
++    : AlarmImpl(io, callback)
+ {
      reschedule_in(delay);
+ }
@@ -209,12 +211,19 @@
  AlarmImpl::AlarmImpl(boost::asio::io_service& io,
                       mir::time::Timestamp time_point,
                       std::function<void ()> callback)
--    : timer{io},
--      data{std::make_shared<InternalState>(callback)}
++    : AlarmImpl(io, callback)
+ {
      reschedule_for(time_point);
+ }
++AlarmImpl::AlarmImpl(boost::asio::io_service& io,
++                     std::function<void(void)> callback)
++    : timer{io},
++      data{std::make_shared<InternalState>(callback)}
++{
++    data->state = triggered;
++}
++
  AlarmImpl::~AlarmImpl() noexcept
+ {
      AlarmImpl::cancel();
@@ -247,10 +256,10 @@
  bool AlarmImpl::reschedule_for(mir::time::Timestamp time_point)
+ {
++    auto boost_epoch = boost::posix_time::from_time_t(0);
++    auto microseconds_since_epoch = std::chrono::duration_cast<std::chrono::microseconds>(time_point.time_since_epoch()).count();
      bool cancelling =
--        timer.expires_at(boost::posix_time::from_time_t(
--                std::chrono::high_resolution_clock::to_time_t(time_point)
--                ));
++            timer.expires_at(boost_epoch + boost::posix_time::microseconds{microseconds_since_epoch});
      update_timer();
      return cancelling;
+ }
@@ -290,8 +299,13 @@
                                                                 std::function<void()> callback)
+ {
      return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, time_point, callback}};
--
--}
++}
++
++std::unique_ptr<mir::time::Alarm> mir::AsioMainLoop::create_alarm(std::function<void()> callback)
++{
++    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, callback}};
++}
++
  void mir::AsioMainLoop::enqueue(void const* owner, ServerAction const& action)
+ {
+     {
 === modified file 'src/server/compositor/CMakeLists.txt'
 --- src/server/compositor/CMakeLists.txt	2014-05-06 03:17:55 +0000
 +++ src/server/compositor/CMakeLists.txt	2014-05-21 00:42:11 +0000
@@ -14,6 +14,7 @@
    default_configuration.cpp
    screencast_display_buffer.cpp
    compositing_screencast.cpp
++  timeout_frame_dropping_policy_factory.cpp
    buffer_queue.cpp
+ )
 === modified file 'src/server/compositor/buffer_queue.cpp'
 --- src/server/compositor/buffer_queue.cpp	2014-05-19 04:01:20 +0000
 +++ src/server/compositor/buffer_queue.cpp	2014-05-21 00:42:11 +0000
@@ -23,6 +23,7 @@
  #include <boost/throw_exception.hpp>
  #include <stdexcept>
  #include <algorithm>
++#include <cassert>
  namespace mc = mir::compositor;
  namespace mg = mir::graphics;
@@ -93,7 +94,8 @@
  mc::BufferQueue::BufferQueue(
      int nbuffers,
      std::shared_ptr<graphics::GraphicBufferAllocator> const& gralloc,
--    graphics::BufferProperties const& props)
++    graphics::BufferProperties const& props,
++    mc::FrameDroppingPolicyFactory const& policy_provider)
      : nbuffers{nbuffers},
        frame_dropping_enabled{false},
        the_properties{props},
@@ -127,6 +129,30 @@
       */
      if (nbuffers == 1)
          free_buffers.push_back(current_compositor_buffer);
++
++    framedrop_policy = policy_provider.create_policy([this]
++    {
++       std::unique_lock<decltype(guard)> lock{guard};
++       assert(!pending_client_notifications.empty());
++       if (ready_to_composite_queue.empty())
++       {
++           /*
++            * NOTE: This can only happen under two circumstances:
++            * 1) Client is single buffered. Don't do that, it's a bad idea.
++            * 2) Client already has a buffer, and is asking for a new one
++            *    without submitting the old one.
++            *
++            *    This shouldn't be exposed to the client as swap_buffers
++            *    blocking, as the client already has something to render to.
++            *
++            *    Our current implementation will never hit this case. If we
++            *    later want clients to be able to own multiple buffers simultaneously
++            *    then we might want to add an entry to the CompositorReport here.
++            */
++           return;
++       }
++       give_buffer_to_client(pop(ready_to_composite_queue), std::move(lock));
++    });
+ }
  void mc::BufferQueue::client_acquire(mc::BufferQueue::Callback complete)
@@ -161,7 +187,14 @@
+     {
          auto const buffer = pop(ready_to_composite_queue);
          give_buffer_to_client(buffer, std::move(lock));
++        return;
+     }
++
++    /* Can't give the client a buffer yet; they'll just have to wait
++     * until the compositor is done with an old frame, or the policy
++     * says they've waited long enough.
++     */
++    framedrop_policy->swap_now_blocking();
+ }
  void mc::BufferQueue::client_release(graphics::Buffer* released_buffer)
@@ -388,7 +421,10 @@
      std::unique_lock<std::mutex> lock)
+ {
      if (!pending_client_notifications.empty())
++    {
++        framedrop_policy->swap_unblocked();
          give_buffer_to_client(buffer, std::move(lock));
++    }
      else
          free_buffers.push_back(buffer);
+ }
 === modified file 'src/server/compositor/buffer_queue.h'
 --- src/server/compositor/buffer_queue.h	2014-05-19 02:55:29 +0000
 +++ src/server/compositor/buffer_queue.h	2014-05-21 00:42:11 +0000
@@ -18,6 +18,8 @@
  #ifndef MIR_BUFFER_QUEUE_H_
  #define MIR_BUFFER_QUEUE_H_
++#include "mir/compositor/frame_dropping_policy_factory.h"
++#include "mir/compositor/frame_dropping_policy.h"
  #include "buffer_bundle.h"
  #include <mutex>
@@ -42,7 +44,8 @@
      BufferQueue(int nbuffers,
                  std::shared_ptr<graphics::GraphicBufferAllocator> const& alloc,
--                graphics::BufferProperties const& props);
++                graphics::BufferProperties const& props,
++                FrameDroppingPolicyFactory const& policy_provider);
      void client_acquire(Callback complete) override;
      void client_release(graphics::Buffer* buffer) override;
@@ -81,6 +84,7 @@
      int nbuffers;
      bool frame_dropping_enabled;
++    std::unique_ptr<FrameDroppingPolicy> framedrop_policy;
      graphics::BufferProperties the_properties;
      std::condition_variable snapshot_released;
 === modified file 'src/server/compositor/buffer_stream_factory.cpp'
 --- src/server/compositor/buffer_stream_factory.cpp	2014-04-21 15:56:41 +0000
 +++ src/server/compositor/buffer_stream_factory.cpp	2014-05-21 00:42:11 +0000
@@ -34,11 +34,13 @@
  namespace mg = mir::graphics;
  namespace ms = mir::scene;
--mc::BufferStreamFactory::BufferStreamFactory(
--    const std::shared_ptr<mg::GraphicBufferAllocator> &gralloc)
--        : gralloc(gralloc)
++mc::BufferStreamFactory::BufferStreamFactory(std::shared_ptr<mg::GraphicBufferAllocator> const& gralloc,
++                                             std::shared_ptr<mc::FrameDroppingPolicyFactory> const& policy_factory)
++        : gralloc(gralloc),
++          policy_factory{policy_factory}
+ {
      assert(gralloc);
++    assert(policy_factory);
+ }
@@ -46,6 +48,6 @@
      mg::BufferProperties const& buffer_properties)
+ {
      // Note: Framedropping and bypass both require a minimum 3 buffers
--    auto switching_bundle = std::make_shared<mc::BufferQueue>(3, gralloc, buffer_properties);
++    auto switching_bundle = std::make_shared<mc::BufferQueue>(3, gralloc, buffer_properties, *policy_factory);
      return std::make_shared<mc::BufferStreamSurfaces>(switching_bundle);
+ }
 === modified file 'src/server/compositor/buffer_stream_factory.h'
 --- src/server/compositor/buffer_stream_factory.h	2014-03-06 06:05:17 +0000
 +++ src/server/compositor/buffer_stream_factory.h	2014-05-21 00:42:11 +0000
@@ -22,6 +22,7 @@
  #define MIR_COMPOSITOR_BUFFER_STREAM_FACTORY_H_
  #include "mir/scene/buffer_stream_factory.h"
++#include "mir/compositor/frame_dropping_policy_factory.h"
  #include <memory>
@@ -37,9 +38,8 @@
  class BufferStreamFactory : public scene::BufferStreamFactory
+ {
  public:
--
--    explicit BufferStreamFactory(
--        const std::shared_ptr<graphics::GraphicBufferAllocator> &gralloc);
++    BufferStreamFactory(std::shared_ptr<graphics::GraphicBufferAllocator> const& gralloc,
++                        std::shared_ptr<FrameDroppingPolicyFactory> const& policy_factory);
      virtual ~BufferStreamFactory() {}
@@ -48,7 +48,7 @@
  private:
      std::shared_ptr<graphics::GraphicBufferAllocator> gralloc;
--
++    std::shared_ptr<FrameDroppingPolicyFactory> const policy_factory;
  };
+ }
 === modified file 'src/server/compositor/default_configuration.cpp'
 --- src/server/compositor/default_configuration.cpp	2014-04-24 08:42:12 +0000
 +++ src/server/compositor/default_configuration.cpp	2014-05-21 00:42:11 +0000
@@ -23,6 +23,8 @@
  #include "multi_threaded_compositor.h"
  #include "gl_renderer_factory.h"
  #include "compositing_screencast.h"
++#include "timeout_frame_dropping_policy_factory.h"
++#include "mir/main_loop.h"
  #include "mir/frontend/screencast.h"
  #include "mir/options/configuration.h"
@@ -39,7 +41,19 @@
      return buffer_stream_factory(
          [this]()
+         {
--            return std::make_shared<mc::BufferStreamFactory>(the_buffer_allocator());
++            return std::make_shared<mc::BufferStreamFactory>(the_buffer_allocator(),
++                                                             the_frame_dropping_policy_factory());
++        });
++}
++
++std::shared_ptr<mc::FrameDroppingPolicyFactory>
++mir::DefaultServerConfiguration::the_frame_dropping_policy_factory()
++{
++    return frame_dropping_policy_factory(
++        [this]()
++        {
++            return std::make_shared<mc::TimeoutFrameDroppingPolicyFactory>(the_main_loop(),
++                                                                           std::chrono::milliseconds{1000});
          });
+ }
 === modified file 'src/server/compositor/multi_threaded_compositor.cpp'
 --- src/server/compositor/multi_threaded_compositor.cpp	2014-05-19 02:55:29 +0000
 +++ src/server/compositor/multi_threaded_compositor.cpp	2014-05-21 00:42:11 +0000
@@ -30,8 +30,6 @@
  #include <thread>
  #include <condition_variable>
--#include <cstdint>
--#include <chrono>
  namespace mc = mir::compositor;
  namespace mg = mir::graphics;
@@ -89,29 +87,38 @@
  class CompositingFunctor
+ {
  public:
--    CompositingFunctor() : running{true}, frames_scheduled{false} {}
--    virtual ~CompositingFunctor() = default;
--
--    void schedule_compositing()
--    {
--        std::lock_guard<std::mutex> lock{run_mutex};
--
--        frames_scheduled = true;
--        run_cv.notify_one();
--    }
--
--    void stop()
--    {
--        std::lock_guard<std::mutex> lock{run_mutex};
--        running = false;
--        run_cv.notify_one();
--    }
--
--protected:
--    void run_compositing_loop(std::function<bool()> const& composite)
++    CompositingFunctor(std::shared_ptr<mc::DisplayBufferCompositorFactory> const& db_compositor_factory,
++                       mg::DisplayBuffer& buffer,
++                       std::shared_ptr<CompositorReport> const& report)
++        : display_buffer_compositor_factory{db_compositor_factory},
++          buffer(buffer),
++          running{true},
++          frames_scheduled{0},
++          report{report}
++    {
++    }
++
++    void operator()() noexcept  // noexcept is important! (LP: #1237332)
++    try
+     {
          std::unique_lock<std::mutex> lock{run_mutex};
++        /*
++         * Make the buffer the current rendering target, and release
++         * it when the thread is finished.
++         */
++        CurrentRenderingTarget target{buffer};
++
++        auto display_buffer_compositor = display_buffer_compositor_factory->create_compositor_for(buffer);
++
++        CompositorReport::SubCompositorId report_id =
++            display_buffer_compositor.get();
++
++        const auto& r = buffer.view_area();
++        report->added_display(r.size.width.as_int(), r.size.height.as_int(),
++                              r.top_left.x.as_int(), r.top_left.y.as_int(),
++                              report_id);
++
          while (running)
+         {
              /* Wait until compositing has been scheduled or we are stopped */
@@ -125,8 +132,7 @@
+             {
                  frames_scheduled = false;
                  lock.unlock();
--
--                auto more_frames_pending = composite();
++                auto more_frames_pending = display_buffer_compositor->composite();
                  /*
                   * Each surface could have a number of frames ready in its buffer
@@ -140,114 +146,36 @@
+             }
+         }
+     }
++    catch(...)
++    {
++        mir::terminate_with_current_exception();
++    }
++
++    void schedule_compositing()
++    {
++        std::lock_guard<std::mutex> lock{run_mutex};
++
++        frames_scheduled = true;
++        run_cv.notify_one();
++    }
++
++    void stop()
++    {
++        std::lock_guard<std::mutex> lock{run_mutex};
++        running = false;
++        run_cv.notify_one();
++    }
  private:
++    std::shared_ptr<mc::DisplayBufferCompositorFactory> const display_buffer_compositor_factory;
++    mg::DisplayBuffer& buffer;
      bool running;
      bool frames_scheduled;
      std::mutex run_mutex;
      std::condition_variable run_cv;
--};
--
--class DisplayBufferCompositingFunctor : public CompositingFunctor
--{
--public:
--    DisplayBufferCompositingFunctor(
--        std::shared_ptr<mc::DisplayBufferCompositorFactory> const& db_compositor_factory,
--        mg::DisplayBuffer& buffer,
--        std::shared_ptr<mc::CompositorReport> const& report)
--        : display_buffer_compositor_factory{db_compositor_factory},
--          buffer(buffer),
--          report{report}
--    {
--    }
--
--    void operator()() noexcept // noexcept is important! (LP: #1237332)
--    try
--    {
--        /*
--         * Make the buffer the current rendering target, and release
--         * it when the thread is finished.
--         */
--        CurrentRenderingTarget target{buffer};
--
--        auto display_buffer_compositor = display_buffer_compositor_factory->create_compositor_for(buffer);
--
--        CompositorReport::SubCompositorId report_id =
--            display_buffer_compositor.get();
--
--        const auto& r = buffer.view_area();
--        report->added_display(r.size.width.as_int(), r.size.height.as_int(),
--                              r.top_left.x.as_int(), r.top_left.y.as_int(),
--                              report_id);
--
--        run_compositing_loop([&] { return display_buffer_compositor->composite();});
--    }
--    catch (...)
--    {
--        mir::terminate_with_current_exception();
--    }
--
--private:
--    std::shared_ptr<mc::DisplayBufferCompositorFactory> const display_buffer_compositor_factory;
--    mg::DisplayBuffer& buffer;
      std::shared_ptr<CompositorReport> const report;
  };
--class BufferConsumingFunctor : public CompositingFunctor
--{
--public:
--    BufferConsumingFunctor(std::shared_ptr<mc::Scene> const& scene)
--        : scene{scene}
--    {
--    }
--
--    void operator()() noexcept // noexcept is important! (LP: #1237332)
--    try
--    {
--        run_compositing_loop(
--            [this]
--            {
--                std::vector<std::shared_ptr<mg::Buffer>> saved_resources;
--
--                auto const& renderables = scene->renderable_list_for(this);
--
--                for (auto const& r : renderables)
--                {
--                    if (r->buffers_ready_for_compositor() > 0)
--                        saved_resources.push_back(r->buffer());
--                }
--
--                wait_until_next_fake_vsync();
--
--                return false;
--            });
--    }
--    catch (...)
--    {
--        mir::terminate_with_current_exception();
--    }
--
--private:
--    void wait_until_next_fake_vsync()
--    {
--        using namespace std::chrono;
--        typedef duration<int64_t, std::ratio<1, 60>> vsync_periods;
--
--        /* Truncate to vsync periods */
--        auto const previous_vsync =
--            duration_cast<vsync_periods>(steady_clock::now().time_since_epoch());
--        /* Convert back to a timepoint */
--        auto const previous_vsync_tp =
--            time_point<steady_clock, vsync_periods>{previous_vsync};
--        /* Next vsync time point */
--        auto const next_vsync = previous_vsync_tp + vsync_periods(1);
--
--        std::this_thread::sleep_until(next_vsync);
--    }
--
--    std::shared_ptr<mc::Scene> const scene;
--};
--
+ }
+ }
@@ -349,23 +277,13 @@
      /* Start the display buffer compositing threads */
      display->for_each_display_buffer([this](mg::DisplayBuffer& buffer)
+     {
--        auto thread_functor_raw =
--            new mc::DisplayBufferCompositingFunctor{
--                display_buffer_compositor_factory, buffer, report};
--        auto thread_functor =
--            std::unique_ptr<mc::DisplayBufferCompositingFunctor>(thread_functor_raw);
++        auto thread_functor_raw = new mc::CompositingFunctor{display_buffer_compositor_factory, buffer, report};
++        auto thread_functor = std::unique_ptr<mc::CompositingFunctor>(thread_functor_raw);
          threads.push_back(std::thread{std::ref(*thread_functor)});
          thread_functors.push_back(std::move(thread_functor));
      });
--    /* Start the buffer consuming thread */
--    auto thread_functor_raw = new mc::BufferConsumingFunctor{scene};
--    auto thread_functor = std::unique_ptr<mc::BufferConsumingFunctor>(thread_functor_raw);
--
--    threads.push_back(std::thread{std::ref(*thread_functor)});
--    thread_functors.push_back(std::move(thread_functor));
--
      state = CompositorState::started;
+ }
 === added file 'src/server/compositor/timeout_frame_dropping_policy_factory.cpp'
 --- src/server/compositor/timeout_frame_dropping_policy_factory.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/compositor/timeout_frame_dropping_policy_factory.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,92 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir/compositor/frame_dropping_policy.h"
++
++#include "timeout_frame_dropping_policy_factory.h"
++
++#include <cassert>
++#include <atomic>
++#include <chrono>
++#include <boost/throw_exception.hpp>
++
++namespace mc = mir::compositor;
++
++namespace
++{
++class TimeoutFrameDroppingPolicy : public mc::FrameDroppingPolicy
++{
++public:
++    TimeoutFrameDroppingPolicy(std::shared_ptr<mir::time::Timer> const& timer,
++                               std::chrono::milliseconds timeout,
++                               std::function<void(void)> drop_frame);
++
++    void swap_now_blocking() override;
++    void swap_unblocked() override;
++
++private:
++    std::chrono::milliseconds const timeout;
++    std::unique_ptr<mir::time::Alarm> alarm;
++    std::atomic<unsigned int> pending_swaps;
++};
++
++TimeoutFrameDroppingPolicy::TimeoutFrameDroppingPolicy(std::shared_ptr<mir::time::Timer> const& timer,
++                                                       std::chrono::milliseconds timeout,
++                                                       std::function<void(void)> drop_frame)
++    : timeout{timeout},
++      pending_swaps{0}
++{
++    alarm = timer->create_alarm([this, drop_frame]
++    {
++       assert(pending_swaps.load() > 0);
++       drop_frame();
++       if (--pending_swaps > 0)
++           alarm->reschedule_in(this->timeout);
++    });
++}
++
++void TimeoutFrameDroppingPolicy::swap_now_blocking()
++{
++    if (pending_swaps++ == 0)
++        alarm->reschedule_in(timeout);
++}
++
++void TimeoutFrameDroppingPolicy::swap_unblocked()
++{
++    if (alarm->state() != mir::time::Alarm::cancelled && alarm->cancel())
++    {
++        if (--pending_swaps > 0)
++        {
++            alarm->reschedule_in(timeout);
++        }
++    }
++}
++}
++
++mc::TimeoutFrameDroppingPolicyFactory::TimeoutFrameDroppingPolicyFactory(std::shared_ptr<mir::time::Timer> const& timer,
++                                                                         std::chrono::milliseconds timeout)
++    : timer{timer},
++      timeout{timeout}
++{
++}
++
++
++std::unique_ptr<mc::FrameDroppingPolicy> mc::TimeoutFrameDroppingPolicyFactory::create_policy(std::function<void ()> drop_frame) const
++{
++    return std::unique_ptr<mc::FrameDroppingPolicy>{new TimeoutFrameDroppingPolicy{timer, timeout, drop_frame}};
++}
 === added file 'src/server/compositor/timeout_frame_dropping_policy_factory.h'
 --- src/server/compositor/timeout_frame_dropping_policy_factory.h	1970-01-01 00:00:00 +0000
 +++ src/server/compositor/timeout_frame_dropping_policy_factory.h	2014-05-21 00:42:11 +0000
@@ -0,0 +1,56 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++
++#ifndef MIR_COMPOSITOR_TIMEOUT_FRAME_DROPPING_POLICY_FACTORY_H_
++#define MIR_COMPOSITOR_TIMEOUT_FRAME_DROPPING_POLICY_FACTORY_H_
++
++#include "mir/compositor/frame_dropping_policy_factory.h"
++#include "mir/time/timer.h"
++
++#include <memory>
++#include <chrono>
++
++namespace mir
++{
++namespace compositor
++{
++
++/**
++ * \brief Creator of timeout-based FrameDroppingPolicies
++ */
++class TimeoutFrameDroppingPolicyFactory : public FrameDroppingPolicyFactory
++{
++public:
++    /**
++     * \param timer     Timer that the policies constructed will schedule alarms on
++     * \param timeout   Milliseconds that the policies will wait before dropping a frame
++     */
++    TimeoutFrameDroppingPolicyFactory(std::shared_ptr<mir::time::Timer> const& timer,
++                                      std::chrono::milliseconds timeout);
++
++    std::unique_ptr<FrameDroppingPolicy> create_policy(std::function<void(void)> drop_frame) const override;
++private:
++    std::shared_ptr<mir::time::Timer> const timer;
++    std::chrono::milliseconds timeout;
++};
++
++}
++}
++
++#endif // MIR_COMPOSITOR_TIMEOUT_FRAME_DROPPING_POLICY_FACTORY_FACTORY_H_
 === modified file 'tests/acceptance-tests/test_client_surface_swap_buffers.cpp'
 --- tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-04-10 07:44:47 +0000
 +++ tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-05-21 00:42:11 +0000
@@ -22,13 +22,12 @@
  #include "mir_test_framework/in_process_server.h"
  #include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test_doubles/null_display_buffer_compositor_factory.h"
--#include "mir_test/spin_wait.h"
++#include "mir_test/signal.h"
  #include <gtest/gtest.h>
--#include <atomic>
--
  namespace mtf = mir_test_framework;
++namespace mt = mir::test;
  namespace mtd = mir::test::doubles;
  namespace mc = mir::compositor;
@@ -55,8 +54,8 @@
  void swap_buffers_callback(MirSurface*, void* ctx)
+ {
--    auto buffers_swapped = static_cast<std::atomic<bool>*>(ctx);
--    *buffers_swapped = true;
++    auto buffers_swapped = static_cast<mt::Signal*>(ctx);
++    buffers_swapped->raise();
+ }
+ }
@@ -78,23 +77,19 @@
      };
      auto const surface = mir_connection_create_surface_sync(connection, &request_params);
--    ASSERT_NE(nullptr, surface);
++    ASSERT_TRUE(mir_surface_is_valid(surface));
      for (int i = 0; i < 10; ++i)
+     {
--        std::atomic<bool> buffers_swapped{false};
++        mt::Signal buffers_swapped;
          mir_surface_swap_buffers(surface, swap_buffers_callback, &buffers_swapped);
--        mir::test::spin_wait_for_condition_or_timeout(
--            [&] { return buffers_swapped.load(); },
--            std::chrono::seconds{5});
--
--        /*
++        /*
           * ASSERT instead of EXPECT, since if we continue we will block in future
           * mir client calls (e.g mir_connection_release).
           */
--        ASSERT_TRUE(buffers_swapped.load());
++        ASSERT_TRUE(buffers_swapped.wait_for(std::chrono::seconds{5}));
+     }
      mir_surface_release_sync(surface);
 === modified file 'tests/integration-tests/compositor/test_buffer_stream.cpp'
 --- tests/integration-tests/compositor/test_buffer_stream.cpp	2014-04-26 04:42:07 +0000
 +++ tests/integration-tests/compositor/test_buffer_stream.cpp	2014-05-21 00:42:11 +0000
@@ -18,11 +18,14 @@
  #include "src/server/compositor/buffer_stream_surfaces.h"
  #include "mir/graphics/graphic_buffer_allocator.h"
++#include "mir/time/timer.h"
  #include "src/server/compositor/buffer_queue.h"
++#include "src/server/compositor/timeout_frame_dropping_policy_factory.h"
  #include "mir_test_doubles/stub_buffer.h"
  #include "mir_test_doubles/stub_buffer_allocator.h"
--#include "multithread_harness.h"
++#include "mir_test_doubles/mock_timer.h"
++#include "mir_test/signal.h"
  #include <gmock/gmock.h>
@@ -33,12 +36,13 @@
  namespace mc = mir::compositor;
  namespace mg = mir::graphics;
--namespace mt = mir::testing;
  namespace mtd = mir::test::doubles;
++namespace mt =mir::test;
  namespace geom = mir::geometry;
  namespace
+ {
++
  struct BufferStreamSurfaces : mc::BufferStreamSurfaces
+ {
      using mc::BufferStreamSurfaces::BufferStreamSurfaces;
@@ -46,29 +50,29 @@
      // Convenient function to allow tests to be written in linear style
      void swap_client_buffers_blocking(mg::Buffer*& buffer)
+     {
--        std::mutex mutex;
--        std::condition_variable cv;
--        bool done = false;
++        swap_client_buffers_cancellable(buffer, std::make_shared<mt::Signal>());
++    }
++    void swap_client_buffers_cancellable(mg::Buffer*& buffer, std::shared_ptr<mt::Signal> const& signal)
++    {
          swap_client_buffers(buffer,
--            [&](mg::Buffer* new_buffer)
++            [signal, &buffer](mg::Buffer* new_buffer)
+              {
--                std::unique_lock<decltype(mutex)> lock(mutex);
                  buffer = new_buffer;
--                done = true;
--                cv.notify_one();
++                signal->raise();
               });
--        std::unique_lock<decltype(mutex)> lock(mutex);
--
--        cv.wait(lock, [&]{ return done; });
++        signal->wait();
+     }
  };
  struct BufferStreamTest : public ::testing::Test
+ {
      BufferStreamTest()
--        : nbuffers{3},
++        : clock{std::make_shared<mt::FakeClock>()},
++          timer{std::make_shared<mtd::FakeTimer>(clock)},
++          frame_drop_timeout{1000},
++          nbuffers{3},
            buffer_stream{create_bundle()}
+     {
+     }
@@ -79,12 +83,18 @@
          mg::BufferProperties properties{geom::Size{380, 210},
                                          mir_pixel_format_abgr_8888,
                                          mg::BufferUsage::hardware};
++        mc::TimeoutFrameDroppingPolicyFactory policy_factory{timer,
++                                                             frame_drop_timeout};
          return std::make_shared<mc::BufferQueue>(nbuffers,
--                                                     allocator,
--                                                     properties);
++                                                 allocator,
++                                                 properties,
++                                                 policy_factory);
+     }
++    std::shared_ptr<mt::FakeClock> clock;
++    std::shared_ptr<mtd::FakeTimer> timer;
++    std::chrono::milliseconds const frame_drop_timeout;
      const int nbuffers;
      BufferStreamSurfaces buffer_stream;
  };
@@ -338,3 +348,23 @@
      for (auto &s : snapshotters)
          s->join();
+ }
++
++TEST_F(BufferStreamTest, blocked_client_is_released_on_timeout)
++{
++    using namespace testing;
++
++    mg::Buffer* placeholder{nullptr};
++
++    // Grab all the buffers...
++    // TODO: the magic “nbuffers - 1” number should be removed
++    for (int i = 0; i < nbuffers - 1; ++i)
++        buffer_stream.swap_client_buffers_blocking(placeholder);
++
++    auto swap_completed = std::make_shared<mt::Signal>();
++    buffer_stream.swap_client_buffers(placeholder, [swap_completed](mg::Buffer*) {swap_completed->raise();});
++
++    EXPECT_FALSE(swap_completed->raised());
++    clock->advance_time(frame_drop_timeout + std::chrono::milliseconds{1});
++
++    EXPECT_TRUE(swap_completed->wait_for(std::chrono::milliseconds{100}));
++}
 === modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
 --- tests/integration-tests/compositor/test_swapping_swappers.cpp	2014-04-21 15:56:41 +0000
 +++ tests/integration-tests/compositor/test_swapping_swappers.cpp	2014-05-21 00:42:11 +0000
@@ -17,6 +17,7 @@
   */
  #include "mir_test_doubles/stub_buffer_allocator.h"
++#include "mir_test_doubles/stub_frame_dropping_policy_factory.h"
  #include "multithread_harness.h"
  #include "src/server/compositor/buffer_queue.h"
@@ -47,7 +48,8 @@
          auto properties = mg::BufferProperties{geom::Size{380, 210},
                                            mir_pixel_format_abgr_8888,
                                            mg::BufferUsage::hardware};
--        switching_bundle = std::make_shared<mc::BufferQueue>(3, allocator, properties);
++        mtd::StubFrameDroppingPolicyFactory policy_factory;
++        switching_bundle = std::make_shared<mc::BufferQueue>(3, allocator, properties, policy_factory);
+     }
      std::shared_ptr<mc::BufferQueue> switching_bundle;
 === modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
 --- tests/integration-tests/graphics/android/test_buffer_integration.cpp	2014-04-21 15:56:41 +0000
 +++ tests/integration-tests/graphics/android/test_buffer_integration.cpp	2014-05-21 00:42:11 +0000
@@ -26,6 +26,8 @@
  #include "testdraw/graphics_region_factory.h"
  #include "testdraw/patterns.h"
++#include "mir_test_doubles/stub_frame_dropping_policy_factory.h"
++
  #include <gtest/gtest.h>
  namespace mc=mir::compositor;
@@ -54,6 +56,7 @@
      MirPixelFormat pf;
      mg::BufferProperties buffer_properties;
      std::shared_ptr<mtd::GraphicsRegionFactory> graphics_region_factory;
++    mir::test::doubles::StubFrameDroppingPolicyFactory policy_factory;
  };
  auto client_acquire_blocking(mc::BufferQueue& switching_bundle)
@@ -116,7 +119,7 @@
      auto allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(null_buffer_initializer);
--    mc::BufferQueue swapper(2, allocator, buffer_properties);
++    mc::BufferQueue swapper(2, allocator, buffer_properties, policy_factory);
      auto returned_buffer = client_acquire_blocking(swapper);
 === modified file 'tests/integration-tests/graphics/android/test_internal_client.cpp'
 --- tests/integration-tests/graphics/android/test_internal_client.cpp	2014-05-19 02:55:29 +0000
 +++ tests/integration-tests/graphics/android/test_internal_client.cpp	2014-05-21 00:42:11 +0000
@@ -40,6 +40,7 @@
  #include "mir_test_doubles/stub_input_channel.h"
  #include "mir_test_doubles/null_surface_configurator.h"
++#include "mir_test_doubles/stub_frame_dropping_policy_factory.h"
  #include <EGL/egl.h>
  #include <gtest/gtest.h>
@@ -99,7 +100,7 @@
      auto stub_input_factory = std::make_shared<StubInputFactory>();
      auto null_buffer_initializer = std::make_shared<mg::NullBufferInitializer>();
      auto allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(null_buffer_initializer);
--    auto buffer_stream_factory = std::make_shared<mc::BufferStreamFactory>(allocator);
++    auto buffer_stream_factory = std::make_shared<mc::BufferStreamFactory>(allocator, std::make_shared<mtd::StubFrameDroppingPolicyFactory>());
      auto scene_report = mr::null_scene_report();
      auto const surface_configurator = std::make_shared<mtd::NullSurfaceConfigurator>();
      auto surface_allocator = std::make_shared<ms::SurfaceAllocator>(buffer_stream_factory, stub_input_factory, surface_configurator, scene_report);
 === modified file 'tests/mir_test/CMakeLists.txt'
 --- tests/mir_test/CMakeLists.txt	2014-05-19 02:55:29 +0000
 +++ tests/mir_test/CMakeLists.txt	2014-05-21 00:42:11 +0000
@@ -5,6 +5,8 @@
    display_config_matchers.cpp
    cross_process_action.cpp
    spin_wait.cpp
++  signal.cpp
++  fake_clock.cpp
    popen.cpp
    wait_object.cpp
+ )
 === added file 'tests/mir_test/fake_clock.cpp'
 --- tests/mir_test/fake_clock.cpp	1970-01-01 00:00:00 +0000
 +++ tests/mir_test/fake_clock.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,53 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir_test/fake_clock.h"
++
++namespace mt = mir::test;
++
++mt::FakeClock::FakeClock()
++    : current_time{std::chrono::nanoseconds::zero()}
++{
++}
++
++mt::FakeClock::time_point mt::FakeClock::now() const
++{
++    return time_point{current_time};
++}
++
++void mt::FakeClock::register_time_change_callback(std::function<bool(mt::FakeClock::time_point)> cb)
++{
++    callbacks.push_back(cb);
++}
++
++void mt::FakeClock::advance_time_ns(std::chrono::nanoseconds by)
++{
++    current_time += by;
++    auto next = callbacks.begin();
++    while (next != callbacks.end())
++    {
++        if (!(*next)(now()))
++        {
++            next = callbacks.erase(next);
++        }
++        else
++        {
++            next++;
++        }
++    }
++}
 === added file 'tests/mir_test/signal.cpp'
 --- tests/mir_test/signal.cpp	1970-01-01 00:00:00 +0000
 +++ tests/mir_test/signal.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,46 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir_test/signal.h"
++
++namespace mt = mir::test;
++
++mt::Signal::Signal()
++    : signalled{false}
++{
++}
++
++void mt::Signal::raise()
++{
++    std::unique_lock<decltype(mutex)> lock(mutex);
++    signalled = true;
++    lock.unlock();
++    cv.notify_all();
++}
++
++bool mt::Signal::raised()
++{
++    std::lock_guard<decltype(mutex)> lock(mutex);
++    return signalled;
++}
++
++void mt::Signal::wait()
++{
++    std::unique_lock<decltype(mutex)> lock(mutex);
++    cv.wait(lock, [this]() { return signalled; });
++}
 === modified file 'tests/mir_test_doubles/CMakeLists.txt'
 --- tests/mir_test_doubles/CMakeLists.txt	2014-03-06 06:05:17 +0000
 +++ tests/mir_test_doubles/CMakeLists.txt	2014-05-21 00:42:11 +0000
@@ -11,6 +11,8 @@
    fake_event_hub.cpp
    fake_event_hub_input_configuration.cpp
    test_protobuf_socket_server.cpp
++  mock_timer.cpp
++  mock_frame_dropping_policy_factory.cpp
+ )
  set(
 === added file 'tests/mir_test_doubles/mock_frame_dropping_policy_factory.cpp'
 --- tests/mir_test_doubles/mock_frame_dropping_policy_factory.cpp	1970-01-01 00:00:00 +0000
 +++ tests/mir_test_doubles/mock_frame_dropping_policy_factory.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,70 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir_test_doubles/mock_frame_dropping_policy_factory.h"
++
++namespace mc = mir::compositor;
++namespace mtd = mir::test::doubles;
++
++mtd::MockFrameDroppingPolicy::MockFrameDroppingPolicy(std::function<void(void)> callback,
++                                                      MockFrameDroppingPolicyFactory const* parent)
++        : callback{callback},
++          parent{parent}
++{
++}
++
++mtd::MockFrameDroppingPolicy::~MockFrameDroppingPolicy()
++{
++    if (parent)
++        parent->policy_destroyed(this);
++}
++
++void mtd::MockFrameDroppingPolicy::trigger()
++{
++    callback();
++}
++
++void mtd::MockFrameDroppingPolicy::parent_destroyed()
++{
++    parent = nullptr;
++}
++
++std::unique_ptr<mc::FrameDroppingPolicy>
++mtd::MockFrameDroppingPolicyFactory::create_policy(std::function<void(void)> drop_frame) const
++{
++    auto policy = new ::testing::NiceMock<MockFrameDroppingPolicy>{drop_frame, this};
++    policies.insert(policy);
++    return std::unique_ptr<mc::FrameDroppingPolicy>{policy};
++}
++
++mtd::MockFrameDroppingPolicyFactory::~MockFrameDroppingPolicyFactory()
++{
++    for (auto policy : policies)
++        policy->parent_destroyed();
++}
++
++void mtd::MockFrameDroppingPolicyFactory::trigger_policies() const
++{
++    for (auto policy : policies)
++        policy->trigger();
++}
++
++void mtd::MockFrameDroppingPolicyFactory::policy_destroyed(MockFrameDroppingPolicy* policy) const
++{
++    policies.erase(policy);
++}
 === added file 'tests/mir_test_doubles/mock_timer.cpp'
 --- tests/mir_test_doubles/mock_timer.cpp	1970-01-01 00:00:00 +0000
 +++ tests/mir_test_doubles/mock_timer.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,149 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir_test_doubles/mock_timer.h"
++
++namespace mt = mir::test;
++namespace mtd = mir::test::doubles;
++
++namespace
++{
++
++class FakeAlarm : public mir::time::Alarm
++{
++public:
++    FakeAlarm(std::function<void(void)> callback, std::shared_ptr<mt::FakeClock> const& clock);
++    ~FakeAlarm() override;
++
++
++    bool cancel() override;
++    State state() const override;
++    bool reschedule_in(std::chrono::milliseconds delay) override;
++    bool reschedule_for(mir::time::Timestamp timeout) override;
++
++private:
++    struct InternalState
++    {
++        explicit InternalState(std::function<void(void)> callback);
++        State state;
++        std::function<void(void)> const callback;
++        mt::FakeClock::time_point threshold;
++    };
++
++    bool handle_time_change(InternalState& state, mt::FakeClock::time_point now);
++
++    std::shared_ptr<InternalState> const internal_state;
++    std::shared_ptr<mt::FakeClock> const clock;
++};
++
++FakeAlarm::InternalState::InternalState(std::function<void(void)> callback)
++    : state{mir::time::Alarm::pending}, callback{callback}
++{
++}
++
++FakeAlarm::FakeAlarm(std::function<void(void)> callback, std::shared_ptr<mt::FakeClock> const& clock)
++    : internal_state{std::make_shared<InternalState>(callback)}, clock{clock}
++{
++}
++
++FakeAlarm::~FakeAlarm()
++{
++    cancel();
++}
++
++bool FakeAlarm::cancel()
++{
++    if (internal_state->state == triggered)
++        return false;
++
++    internal_state->state = cancelled;
++    return true;
++}
++
++FakeAlarm::State FakeAlarm::state() const
++{
++    return internal_state->state;
++}
++
++bool FakeAlarm::handle_time_change(InternalState& state,
++                                   mir::test::FakeClock::time_point now)
++{
++    if (state.state == pending)
++    {
++        if (now > state.threshold)
++        {
++            state.state = triggered;
++            state.callback();
++            return false;
++        }
++        return true;
++    }
++    else
++    {
++        return false;
++    }
++}
++
++bool FakeAlarm::reschedule_in(std::chrono::milliseconds delay)
++{
++    bool rescheduled = internal_state->state == pending;
++
++    internal_state->state = pending;
++    internal_state->threshold = clock->now() + delay;
++
++    std::shared_ptr<InternalState> state_copy{internal_state};
++    clock->register_time_change_callback([this, state_copy](mt::FakeClock::time_point now)
++    {
++        return handle_time_change(*state_copy, now);
++    });
++    return rescheduled;
++}
++
++bool FakeAlarm::reschedule_for(mir::time::Timestamp timeout)
++{
++    // time::Timestamp is on a different clock, so not directly comparable
++    auto delay = std::chrono::duration_cast<std::chrono::milliseconds>(timeout.time_since_epoch() -
++                                                                       clock->now().time_since_epoch());
++    return reschedule_in(delay);
++}
++}
++
++mtd::FakeTimer::FakeTimer(std::shared_ptr<FakeClock> const& clock) : clock{clock}
++{
++}
++
++std::unique_ptr<mir::time::Alarm> mtd::FakeTimer::notify_in(std::chrono::milliseconds delay,
++                                                            std::function<void(void)> callback)
++{
++    auto alarm = std::unique_ptr<mir::time::Alarm>{new FakeAlarm{callback, clock}};
++    alarm->reschedule_in(delay);
++    return std::move(alarm);
++}
++
++std::unique_ptr<mir::time::Alarm> mtd::FakeTimer::notify_at(time::Timestamp time_point,
++                                                            std::function<void(void)> callback)
++{
++    auto alarm = std::unique_ptr<mir::time::Alarm>{new FakeAlarm{callback, clock}};
++    alarm->reschedule_for(time_point);
++    return std::move(alarm);
++}
++
++std::unique_ptr<mir::time::Alarm> mir::test::doubles::FakeTimer::create_alarm(std::function<void(void)> callback)
++{
++    return std::unique_ptr<mir::time::Alarm>{new FakeAlarm{callback, clock}};
++}
 === modified file 'tests/unit-tests/compositor/CMakeLists.txt'
 --- tests/unit-tests/compositor/CMakeLists.txt	2014-04-21 15:56:41 +0000
 +++ tests/unit-tests/compositor/CMakeLists.txt	2014-05-21 00:42:11 +0000
@@ -9,6 +9,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_occlusion.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_screencast_display_buffer.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_compositing_screencast.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_timeout_frame_dropping_policy.cpp
+ )
  set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)
 === modified file 'tests/unit-tests/compositor/test_buffer_queue.cpp'
 --- tests/unit-tests/compositor/test_buffer_queue.cpp	2014-05-19 07:44:29 +0000
 +++ tests/unit-tests/compositor/test_buffer_queue.cpp	2014-05-21 00:42:11 +0000
@@ -20,6 +20,9 @@
  #include "src/server/compositor/buffer_queue.h"
  #include "mir_test_doubles/stub_buffer_allocator.h"
  #include "mir_test_doubles/stub_buffer.h"
++#include "mir_test_doubles/stub_frame_dropping_policy_factory.h"
++#include "mir_test_doubles/mock_frame_dropping_policy_factory.h"
++#include "mir_test/signal.h"
  #include "mir_test/auto_unblock_thread.h"
  #include <gtest/gtest.h>
@@ -28,6 +31,8 @@
  #include <atomic>
  #include <mutex>
  #include <chrono>
++#include <algorithm>
++#include <map>
  #include <deque>
  #include <unordered_set>
@@ -36,11 +41,13 @@
  namespace mt = mir::test;
  namespace mc=mir::compositor;
  namespace mg = mir::graphics;
++namespace mt=mir::test;
  using namespace testing;
  namespace
+ {
++
  class BufferQueueTest : public ::testing::Test
+ {
  public:
@@ -62,6 +69,7 @@
      int max_nbuffers_to_test;
      std::shared_ptr<mtd::StubBufferAllocator> allocator;
      mg::BufferProperties basic_properties;
++    mtd::StubFrameDroppingPolicyFactory policy_factory;
  };
  class AcquireWaitHandle
@@ -191,7 +199,7 @@
      std::unique_ptr<mc::BufferQueue> q;
      ASSERT_NO_THROW(q = std::move(
          std::unique_ptr<mc::BufferQueue>(
--            new mc::BufferQueue(1, allocator, basic_properties))));
++            new mc::BufferQueue(1, allocator, basic_properties, policy_factory))));
      ASSERT_THAT(q, Ne(nullptr));
      auto handle = client_acquire_async(*q);
@@ -226,7 +234,7 @@
  TEST_F(BufferQueueTest, buffer_queue_of_one_supports_resizing)
+ {
--    mc::BufferQueue q(1, allocator, basic_properties);
++    mc::BufferQueue q(1, allocator, basic_properties, policy_factory);
      const geom::Size expect_size{10, 20};
      q.resize(expect_size);
@@ -251,22 +259,22 @@
  TEST_F(BufferQueueTest, framedropping_is_disabled_by_default)
+ {
--    mc::BufferQueue bundle(2, allocator, basic_properties);
++    mc::BufferQueue bundle(2, allocator, basic_properties, policy_factory);
      EXPECT_THAT(bundle.framedropping_allowed(), Eq(false));
+ }
  TEST_F(BufferQueueTest, throws_when_creating_with_invalid_num_buffers)
+ {
--    EXPECT_THROW(mc::BufferQueue a(0, allocator, basic_properties), std::logic_error);
--    EXPECT_THROW(mc::BufferQueue a(-1, allocator, basic_properties), std::logic_error);
--    EXPECT_THROW(mc::BufferQueue a(-10, allocator, basic_properties), std::logic_error);
++    EXPECT_THROW(mc::BufferQueue a(0, allocator, basic_properties, policy_factory), std::logic_error);
++    EXPECT_THROW(mc::BufferQueue a(-1, allocator, basic_properties, policy_factory), std::logic_error);
++    EXPECT_THROW(mc::BufferQueue a(-10, allocator, basic_properties, policy_factory), std::logic_error);
+ }
  TEST_F(BufferQueueTest, client_can_acquire_and_release_buffer)
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle = client_acquire_async(q);
          ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -278,7 +286,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          int const max_ownable_buffers = nbuffers - 1;
          for (int acquires = 0; acquires < max_ownable_buffers; ++acquires)
+         {
@@ -292,7 +300,7 @@
  TEST_F(BufferQueueTest, clients_can_have_multiple_pending_completions)
+ {
      int const nbuffers = 3;
--    mc::BufferQueue q(nbuffers, allocator, basic_properties);
++    mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
      for (int i = 0; i < nbuffers - 1; ++i)
+     {
@@ -322,7 +330,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          ASSERT_THAT(q.framedropping_allowed(), Eq(false));
          void const* main_compositor = reinterpret_cast<void const*>(0);
@@ -351,7 +359,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          q.allow_framedropping(true);
          for (int i = 0; i < 1000; i++)
@@ -366,7 +374,7 @@
  /* Regression test for LP: #1210042 */
  TEST_F(BufferQueueTest, clients_dont_recycle_startup_buffer)
+ {
--    mc::BufferQueue q(3, allocator, basic_properties);
++    mc::BufferQueue q(3, allocator, basic_properties, policy_factory);
      auto handle = client_acquire_async(q);
      ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -386,7 +394,7 @@
+ {
      for (int nbuffers = 3; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle1 = client_acquire_async(q);
          ASSERT_THAT(handle1->has_acquired_buffer(), Eq(true));
@@ -406,7 +414,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          std::atomic<bool> done(false);
          auto unblock = [&done] { done = true; };
@@ -441,7 +449,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle = client_acquire_async(q);
          ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -459,7 +467,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle = client_acquire_async(q);
          ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -481,7 +489,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          for (int i = 0; i < 10; ++i)
+         {
@@ -515,7 +523,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          for (int i = 0; i < 100; i++)
+         {
@@ -529,7 +537,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          q.allow_framedropping(false);
          std::atomic<bool> done(false);
@@ -567,7 +575,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          mg::BufferID client_id;
@@ -596,7 +604,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle = client_acquire_async(q);
          ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -611,7 +619,7 @@
  /* Regression test for LP#1270964 */
  TEST_F(BufferQueueTest, compositor_client_interleaved)
+ {
--    mc::BufferQueue q(3, allocator, basic_properties);
++    mc::BufferQueue q(3, allocator, basic_properties, policy_factory);
      auto handle = client_acquire_async(q);
      ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -636,7 +644,7 @@
      // This test simulates bypass behaviour
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          std::shared_ptr<mg::Buffer> compositor[2];
@@ -673,7 +681,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto comp_buffer = q.compositor_acquire(this);
          auto snapshot = q.snapshot_acquire();
@@ -687,7 +695,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          int const num_snapshots = 100;
          std::shared_ptr<mg::Buffer> buf[num_snapshots];
@@ -703,7 +711,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          auto handle = client_acquire_async(q);
          ASSERT_THAT(handle->has_acquired_buffer(), Eq(true));
@@ -727,7 +735,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          std::atomic<bool> done(false);
@@ -760,7 +768,7 @@
+ {
      for (int nbuffers = 3; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          std::shared_ptr<mg::Buffer> compositor[2];
@@ -801,7 +809,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          q.allow_framedropping(true);
          int const nframes = 100;
@@ -823,7 +831,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          q.allow_framedropping(false);
          int const max_ownable_buffers = nbuffers - 1;
@@ -848,7 +856,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= 3; nbuffers++)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          unsigned long client_frames = 0;
          const unsigned long compose_frames = 20;
@@ -904,7 +912,7 @@
       */
      for (int nbuffers = 3; nbuffers <= 3; nbuffers++)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          unsigned long client_frames = 0;
          unsigned long const compose_frames = 100;
          auto const frame_time = std::chrono::milliseconds(16);
@@ -962,7 +970,7 @@
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          for (int width = 1; width < 100; ++width)
+         {
@@ -985,11 +993,19 @@
+     }
+ }
++namespace
++{
++int max_ownable_buffers(int nbuffers)
++{
++    return (nbuffers == 1) ? 1 : nbuffers - 1;
++}
++}
++
  TEST_F(BufferQueueTest, compositor_acquires_resized_frames)
+ {
      for (int nbuffers = 1; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          mg::BufferID history[5];
          const int width0 = 123;
@@ -998,9 +1014,8 @@
          const int dy = -3;
          int width = width0;
          int height = height0;
--        int const nbuffers_to_use = nbuffers == 1 ? 1 : nbuffers - 1;
--        for (int produce = 0; produce < nbuffers_to_use; ++produce)
++        for (int produce = 0; produce < max_ownable_buffers(nbuffers); ++produce)
+         {
              geom::Size new_size{width, height};
              width += dx;
@@ -1018,7 +1033,7 @@
          width = width0;
          height = height0;
--        for (int consume = 0; consume < nbuffers_to_use; ++consume)
++        for (int consume = 0; consume < max_ownable_buffers(nbuffers); ++consume)
+         {
              geom::Size expect_size{width, height};
              width += dx;
@@ -1047,12 +1062,79 @@
+     }
+ }
++TEST_F(BufferQueueTest, uncomposited_client_swaps_when_policy_triggered)
++{
++    for (int nbuffers = 2;
++         nbuffers < max_nbuffers_to_test;
++         nbuffers++)
++    {
++        mtd::MockFrameDroppingPolicyFactory policy_factory;
++        mc::BufferQueue q(nbuffers,
++                          allocator,
++                          basic_properties,
++                          policy_factory);
++
++        for (int i = 0; i < max_ownable_buffers(nbuffers); i++)
++        {
++            auto client = client_acquire_sync(q);
++            q.client_release(client);
++        }
++
++        auto handle = client_acquire_async(q);
++
++        EXPECT_FALSE(handle->has_acquired_buffer());
++
++        policy_factory.trigger_policies();
++        EXPECT_TRUE(handle->has_acquired_buffer());
++    }
++}
++
++TEST_F(BufferQueueTest, partially_composited_client_swaps_when_policy_triggered)
++{
++    for (int nbuffers = 2;
++         nbuffers < max_nbuffers_to_test;
++         nbuffers++)
++    {
++        mtd::MockFrameDroppingPolicyFactory policy_factory;
++        mc::BufferQueue q(nbuffers,
++                          allocator,
++                          basic_properties,
++                          policy_factory);
++
++        for (int i = 0; i < max_ownable_buffers(nbuffers); i++)
++        {
++            auto client = client_acquire_sync(q);
++            q.client_release(client);
++        }
++
++        /* Queue up two pending swaps */
++        auto first_swap = client_acquire_async(q);
++        auto second_swap = client_acquire_async(q);
++
++        ASSERT_FALSE(first_swap->has_acquired_buffer());
++        ASSERT_FALSE(second_swap->has_acquired_buffer());
++
++        q.compositor_acquire(nullptr);
++
++        EXPECT_TRUE(first_swap->has_acquired_buffer());
++        EXPECT_FALSE(second_swap->has_acquired_buffer());
++
++        /* We have to release a client buffer here; framedropping or not,
++         * a client can't have 2 buffers outstanding in the nbuffers = 2 case.
++         */
++        first_swap->release_buffer();
++
++        policy_factory.trigger_policies();
++        EXPECT_TRUE(second_swap->has_acquired_buffer());
++    }
++}
++
  TEST_F(BufferQueueTest, with_single_buffer_compositor_acquires_resized_frames_eventually)
+ {
      int const nbuffers{1};
      geom::Size const new_size{123,456};
--    mc::BufferQueue q(nbuffers, allocator, basic_properties);
++    mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
      q.client_release(client_acquire_sync(q));
      q.resize(new_size);
@@ -1072,7 +1154,7 @@
  {  // Regression test for LP: #1319765
      using namespace testing;
--    mc::BufferQueue q{2, allocator, basic_properties};
++    mc::BufferQueue q{2, allocator, basic_properties, policy_factory};
      q.client_release(client_acquire_sync(q));
      auto a = q.compositor_acquire(this);
@@ -1095,7 +1177,7 @@
  {  // Extended regression test for LP: #1319765
      using namespace testing;
--    mc::BufferQueue q{2, allocator, basic_properties};
++    mc::BufferQueue q{2, allocator, basic_properties, policy_factory};
      for (int i = 0; i < 100; ++i)
+     {
@@ -1137,7 +1219,7 @@
      int const nbuffers{3};
--    mc::BufferQueue q{nbuffers, allocator, basic_properties};
++    mc::BufferQueue q{nbuffers, allocator, basic_properties, policy_factory};
      q.allow_framedropping(true);
      std::vector<std::shared_ptr<mg::Buffer>> buffers;
@@ -1183,7 +1265,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          std::mutex client_buffer_guard;
          mg::Buffer* client_buffer = nullptr;
@@ -1231,7 +1313,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          for (int i = 0; i < 100; ++i)
+         {
              auto handle = client_acquire_async(q);
@@ -1267,7 +1349,7 @@
+ {
      for (int nbuffers = 3; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          void const* main_compositor = reinterpret_cast<void const*>(0);
          void const* second_compositor = reinterpret_cast<void const*>(1);
@@ -1327,7 +1409,7 @@
+ {
      for (int nbuffers = 2; nbuffers <= max_nbuffers_to_test; ++nbuffers)
+     {
--        mc::BufferQueue q(nbuffers, allocator, basic_properties);
++        mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
          q.allow_framedropping(false);
          std::atomic<bool> done(false);
@@ -1368,7 +1450,7 @@
  TEST_F(BufferQueueTest, DISABLED_lp_1317801_regression_test)
+ {
      int const nbuffers = 3;
--    mc::BufferQueue q(nbuffers, allocator, basic_properties);
++    mc::BufferQueue q(nbuffers, allocator, basic_properties, policy_factory);
      q.client_release(client_acquire_sync(q));
 === modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
 --- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-05-05 03:36:45 +0000
 +++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-05-21 00:42:11 +0000
@@ -29,9 +29,6 @@
  #include "mir_test_doubles/mock_display_buffer.h"
  #include "mir_test_doubles/mock_compositor_report.h"
  #include "mir_test_doubles/mock_scene.h"
--#include "mir_test_doubles/stub_renderable.h"
--#include "mir_test_doubles/null_display_buffer_compositor_factory.h"
--#include "mir_test/spin_wait.h"
  #include <boost/throw_exception.hpp>
@@ -40,7 +37,6 @@
  #include <thread>
  #include <mutex>
  #include <chrono>
--#include <atomic>
  #include <gmock/gmock.h>
  #include <gtest/gtest.h>
@@ -103,7 +99,7 @@
      mg::RenderableList renderable_list_for(void const*) const
+     {
--        return renderable_list;
++        return mg::RenderableList{};
+     }
      void add_observer(std::shared_ptr<ms::Observer> const& observer_)
@@ -318,35 +314,19 @@
      unsigned int render_count;
  };
--enum class RenderableVisibility { hidden, visible };
--
--class BufferCountingRenderable : public mtd::StubRenderable
++class NullDisplayBufferCompositorFactory : public mc::DisplayBufferCompositorFactory
+ {
  public:
--    BufferCountingRenderable(RenderableVisibility visibility)
--        : buffers_requested_{0}, visibility{visibility}
--    {
--    }
--
--    std::shared_ptr<mg::Buffer> buffer() const override
--    {
--        ++buffers_requested_;
--        return std::make_shared<mtd::StubBuffer>();
--    }
--
--    bool visible() const override
--    {
--        return visibility == RenderableVisibility::visible;
--    }
--
--    int buffers_requested() const
--    {
--        return buffers_requested_;
--    }
--
--private:
--    mutable std::atomic<int> buffers_requested_;
--    RenderableVisibility const visibility;
++    std::unique_ptr<mc::DisplayBufferCompositor> create_compositor_for(mg::DisplayBuffer&)
++    {
++        struct NullDisplayBufferCompositor : mc::DisplayBufferCompositor
++        {
++            bool composite() { return false; }
++        };
++
++        auto raw = new NullDisplayBufferCompositor{};
++        return std::unique_ptr<NullDisplayBufferCompositor>(raw);
++    }
  };
  auto const null_report = mr::null_compositor_report();
@@ -573,7 +553,7 @@
      auto display = std::make_shared<StubDisplayWithMockBuffers>(nbuffers);
      auto scene = std::make_shared<StubScene>();
--    auto db_compositor_factory = std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
++    auto db_compositor_factory = std::make_shared<NullDisplayBufferCompositorFactory>();
      mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
      display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
@@ -590,12 +570,12 @@
  TEST(MultiThreadedCompositor, double_start_or_stop_ignored)
+ {
--    using namespace ::testing;
++    using namespace testing;
      unsigned int const nbuffers{3};
      auto display = std::make_shared<StubDisplayWithMockBuffers>(nbuffers);
      auto mock_scene = std::make_shared<mtd::MockScene>();
--    auto db_compositor_factory = std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
++    auto db_compositor_factory = std::make_shared<NullDisplayBufferCompositorFactory>();
      auto mock_report = std::make_shared<testing::NiceMock<mtd::MockCompositorReport>>();
      EXPECT_CALL(*mock_report, started())
@@ -618,49 +598,6 @@
      compositor.stop();
+ }
--namespace
--{
--struct BufferConsumption : ::testing::TestWithParam<RenderableVisibility> {};
--}
--
--TEST_P(BufferConsumption, consumes_buffers_for_renderables_that_are_not_rendered)
--{
--    using namespace testing;
--
--    unsigned int const nbuffers{2};
--    auto renderable = std::make_shared<BufferCountingRenderable>(GetParam());
--    auto display = std::make_shared<StubDisplay>(nbuffers);
--    auto stub_scene = std::make_shared<StubScene>(mg::RenderableList{renderable});
--    // We use NullDisplayBufferCompositors to simulate DisplayBufferCompositors
--    // not rendering a renderable.
--    auto db_compositor_factory = std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
--
--    mc::MultiThreadedCompositor compositor{
--        display, stub_scene, db_compositor_factory, null_report, true};
--
--    compositor.start();
--
--    mir::test::spin_wait_for_condition_or_timeout(
--        [&] { return renderable->buffers_requested() == 1; },
--        std::chrono::seconds{5});
--
--    EXPECT_THAT(renderable->buffers_requested(), Eq(1));
--
--    stub_scene->emit_change_event();
--
--    mir::test::spin_wait_for_condition_or_timeout(
--        [&] { return renderable->buffers_requested() == 2; },
--        std::chrono::seconds{5});
--
--    EXPECT_THAT(renderable->buffers_requested(), Eq(2));
--
--    compositor.stop();
--}
--
--INSTANTIATE_TEST_CASE_P(
--    MultiThreadedCompositor, BufferConsumption,
--    ::testing::Values(RenderableVisibility::hidden, RenderableVisibility::visible));
--
  TEST(MultiThreadedCompositor, cleans_up_after_throw_in_start)
+ {
      unsigned int const nbuffers{3};
 === added file 'tests/unit-tests/compositor/test_timeout_frame_dropping_policy.cpp'
 --- tests/unit-tests/compositor/test_timeout_frame_dropping_policy.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/compositor/test_timeout_frame_dropping_policy.cpp	2014-05-21 00:42:11 +0000
@@ -0,0 +1,186 @@
++/*
++ * Copyright © 2014 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: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "src/server/compositor/timeout_frame_dropping_policy_factory.h"
++#include "mir/compositor/frame_dropping_policy.h"
++
++#include "mir_test_doubles/mock_timer.h"
++
++#include "mir_test/gmock_fixes.h"
++
++#include <stdexcept>
++
++#include <gtest/gtest.h>
++#include <gmock/gmock.h>
++
++namespace mc = mir::compositor;
++namespace mt = mir::test;
++namespace mtd = mir::test::doubles;
++
++TEST(TimeoutFrameDroppingPolicy, does_not_fire_before_notified_of_block)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++    bool frame_dropped{false};
++
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, schedules_alarm_for_correct_timeout)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++    bool frame_dropped{false};
++
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++    policy->swap_now_blocking();
++
++    clock->advance_time(timeout - std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++    clock->advance_time(std::chrono::milliseconds{2});
++    EXPECT_TRUE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, framedrop_callback_cancelled_by_unblock)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++
++    bool frame_dropped{false};
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    policy->swap_now_blocking();
++    policy->swap_unblocked();
++
++    clock->advance_time(timeout * 10);
++
++    EXPECT_FALSE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, policy_drops_one_frame_per_blocking_swap)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++
++    bool frame_dropped{false};
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    policy->swap_now_blocking();
++    policy->swap_now_blocking();
++    policy->swap_now_blocking();
++
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_TRUE(frame_dropped);
++
++    frame_dropped = false;
++    clock->advance_time(timeout + std::chrono::milliseconds{2});
++    EXPECT_TRUE(frame_dropped);
++
++    frame_dropped = false;
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_TRUE(frame_dropped);
++
++    frame_dropped = false;
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, policy_drops_frames_no_more_frequently_than_timeout)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++
++    bool frame_dropped{false};
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    policy->swap_now_blocking();
++    policy->swap_now_blocking();
++
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_TRUE(frame_dropped);
++
++    frame_dropped = false;
++    clock->advance_time(timeout - std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++    clock->advance_time(std::chrono::milliseconds{2});
++    EXPECT_TRUE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, newly_blocking_frame_doesnt_reset_timeout)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++
++    bool frame_dropped{false};
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    policy->swap_now_blocking();
++    clock->advance_time(timeout - std::chrono::milliseconds{1});
++
++    policy->swap_now_blocking();
++    clock->advance_time(std::chrono::milliseconds{2});
++    EXPECT_TRUE(frame_dropped);
++}
++
++TEST(TimeoutFrameDroppingPolicy, interspersed_timeouts_and_unblocks)
++{
++    auto clock = std::make_shared<mt::FakeClock>();
++    auto timer = std::make_shared<mtd::FakeTimer>(clock);
++    std::chrono::milliseconds const timeout{1000};
++
++    bool frame_dropped{false};
++    mc::TimeoutFrameDroppingPolicyFactory factory{timer, timeout};
++    auto policy = factory.create_policy([&frame_dropped]{ frame_dropped = true; });
++
++    policy->swap_now_blocking();
++    policy->swap_now_blocking();
++    policy->swap_now_blocking();
++
++    /* First frame gets dropped... */
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_TRUE(frame_dropped);
++
++    /* ...Compositor gets its act in order and consumes a frame... */
++    frame_dropped = false;
++    policy->swap_unblocked();
++    clock->advance_time(timeout - std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++
++    /* ...but not a second frame, so third swap should trigger a timeout */
++    clock->advance_time(std::chrono::milliseconds{2});
++    EXPECT_TRUE(frame_dropped);
++
++    frame_dropped = false;
++    clock->advance_time(timeout + std::chrono::milliseconds{1});
++    EXPECT_FALSE(frame_dropped);
++}
 === modified file 'tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp'
 --- tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp	2014-05-19 02:55:29 +0000
 +++ tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp	2014-05-21 00:42:11 +0000
@@ -26,6 +26,7 @@
  #include <gtest/gtest.h>
  #include <gmock/gmock.h>
++#include "mir_test/gmock_fixes.h"
  #include <stdexcept>
 === modified file 'tests/unit-tests/test_asio_main_loop.cpp'
 --- tests/unit-tests/test_asio_main_loop.cpp	2014-05-20 08:38:05 +0000
 +++ tests/unit-tests/test_asio_main_loop.cpp	2014-05-21 00:42:11 +0000
@@ -20,6 +20,7 @@
  #include "mir/time/high_resolution_clock.h"
  #include "mir_test/pipe.h"
  #include "mir_test/auto_unblock_thread.h"
++#include "mir_test/signal.h"
  #include "mir_test/wait_object.h"
  #include <gtest/gtest.h>
@@ -325,58 +326,54 @@
  TEST_F(AsioMainLoopAlarmTest, main_loop_runs_until_stop_called)
+ {
--    std::mutex checkpoint_mutex;
--    std::condition_variable checkpoint;
--    bool hit_checkpoint{false};
++    auto mainloop_started = std::make_shared<mt::Signal>();
      auto fire_on_mainloop_start = ml.notify_in(std::chrono::milliseconds{0},
--                                               [&checkpoint_mutex, &checkpoint, &hit_checkpoint]()
++                                               [mainloop_started]()
+     {
--        std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
--        hit_checkpoint = true;
--        checkpoint.notify_all();
++        mainloop_started->raise();
      });
      UnblockMainLoop unblocker(ml);
--    {
--        std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
--        ASSERT_TRUE(checkpoint.wait_for(lock, std::chrono::milliseconds{100}, [&hit_checkpoint]() { return hit_checkpoint; }));
--    }
++    ASSERT_TRUE(mainloop_started->wait_for(std::chrono::milliseconds{100}));
--    auto alarm = ml.notify_in(std::chrono::milliseconds{10}, [this]
++    auto timer_fired = std::make_shared<mt::Signal>();
++    auto alarm = ml.notify_in(std::chrono::milliseconds{10}, [timer_fired]
+     {
--        wait.notify_ready();
++        timer_fired->raise();
      });
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{500}));
++    EXPECT_TRUE(timer_fired->wait_for(std::chrono::milliseconds{500}));
      ml.stop();
      // Main loop should be stopped now
--    hit_checkpoint = false;
++    timer_fired = std::make_shared<mt::Signal>();
      auto should_not_fire =  ml.notify_in(std::chrono::milliseconds{0},
--                                         [&checkpoint_mutex, &checkpoint, &hit_checkpoint]()
++                                         [timer_fired]()
+     {
--        std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
--        hit_checkpoint = true;
--        checkpoint.notify_all();
++        timer_fired->raise();
      });
--    std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
--    EXPECT_FALSE(checkpoint.wait_for(lock, std::chrono::milliseconds{50}, [&hit_checkpoint]() { return hit_checkpoint; }));
++    EXPECT_FALSE(timer_fired->wait_for(std::chrono::milliseconds{100}));
+ }
  TEST_F(AsioMainLoopAlarmTest, alarm_fires_with_correct_delay)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{50}, [this]()
++    UnblockMainLoop unblocker(ml);
++
++    auto timer_fired = std::make_shared<mt::Signal>();
++    auto alarm = ml.notify_in(std::chrono::milliseconds{100}, [timer_fired]()
+     {
--        wait.notify_ready();
++        timer_fired->raise();
      });
--    UnblockMainLoop unblocker(ml);
++    // Shouldn't fire before timeout
++    EXPECT_FALSE(timer_fired->wait_for(std::chrono::milliseconds{50}));
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{100}));
++    // Give a nice, long wait for our slow ARM valgrind friends
++    EXPECT_TRUE(timer_fired->wait_for(std::chrono::milliseconds{200}));
+ }
  TEST_F(AsioMainLoopAlarmTest, multiple_alarms_fire)
@@ -385,120 +382,129 @@
      std::atomic<int> call_count{0};
      std::array<std::unique_ptr<mir::time::Alarm>, alarm_count> alarms;
++    auto alarms_fired = std::make_shared<mt::Signal>();
++
      for (auto& alarm : alarms)
+     {
--        alarm = ml.notify_in(std::chrono::milliseconds{50}, [this, &call_count]()
++        alarm = ml.notify_in(std::chrono::milliseconds{5}, [alarms_fired, &call_count]()
+         {
--            call_count.fetch_add(1);
++            call_count++;
              if (call_count == alarm_count)
--                wait.notify_ready();
++                alarms_fired->raise();
          });
+     }
      UnblockMainLoop unblocker(ml);
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{100}));
++    EXPECT_TRUE(alarms_fired->wait_for(std::chrono::milliseconds{100}));
++
      for (auto& alarm : alarms)
          EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
+ }
--
  TEST_F(AsioMainLoopAlarmTest, alarm_changes_to_triggered_state)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{50}, [this]()
++    auto alarm_fired = std::make_shared<mt::Signal>();
++    auto alarm = ml.notify_in(std::chrono::milliseconds{5}, [alarm_fired]()
+     {
--        wait.notify_ready();
++        alarm_fired->raise();
      });
      UnblockMainLoop unblocker(ml);
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{100}));
++    ASSERT_TRUE(alarm_fired->wait_for(std::chrono::milliseconds{100}));
      EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
+ }
  TEST_F(AsioMainLoopAlarmTest, alarm_starts_in_pending_state)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{50}, [this]() {});
--
      UnblockMainLoop unblocker(ml);
++    auto alarm = ml.notify_in(std::chrono::milliseconds{5000}, [](){});
++
      EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
+ }
  TEST_F(AsioMainLoopAlarmTest, cancelled_alarm_doesnt_fire)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{200}, [this]()
++    UnblockMainLoop unblocker(ml);
++    auto alarm_fired = std::make_shared<mt::Signal>();
++
++    auto alarm = ml.notify_in(std::chrono::milliseconds{100}, [alarm_fired]()
+     {
--        wait.notify_ready();
++        alarm_fired->raise();
      });
--    UnblockMainLoop unblocker(ml);
--
      EXPECT_TRUE(alarm->cancel());
--    EXPECT_THROW(wait.wait_until_ready(std::chrono::milliseconds{300}), std::runtime_error);
++
++    EXPECT_FALSE(alarm_fired->wait_for(std::chrono::milliseconds{300}));
      EXPECT_EQ(mir::time::Alarm::cancelled, alarm->state());
+ }
  TEST_F(AsioMainLoopAlarmTest, destroyed_alarm_doesnt_fire)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{200}, [this]()
++    auto alarm_fired = std::make_shared<mt::Signal>();
++
++    auto alarm = ml.notify_in(std::chrono::milliseconds{200}, [alarm_fired]()
+     {
--        wait.notify_ready();
++        alarm_fired->raise();
      });
      UnblockMainLoop unblocker(ml);
      alarm.reset(nullptr);
--    EXPECT_THROW(wait.wait_until_ready(std::chrono::milliseconds{300}), std::runtime_error);
++    EXPECT_FALSE(alarm_fired->wait_for(std::chrono::milliseconds{300}));
+ }
  TEST_F(AsioMainLoopAlarmTest, rescheduled_alarm_fires_again)
+ {
--    std::mutex m;
--    std::condition_variable called;
++    auto first_trigger = std::make_shared<mt::Signal>();
++    auto second_trigger = std::make_shared<mt::Signal>();
++    std::atomic<int> call_count{0};
--    auto alarm = ml.notify_in(std::chrono::milliseconds{0}, [this, &m, &called]()
++    auto alarm = ml.notify_in(std::chrono::milliseconds{0}, [first_trigger, second_trigger, &call_count]()
+     {
--        std::unique_lock<decltype(m)> lock(m);
--        call_count++;
--        if (call_count == 2)
--            wait.notify_ready();
--        called.notify_all();
++        auto prev_call_count = call_count++;
++        if (prev_call_count == 0)
++            first_trigger->raise();
++        if (prev_call_count == 1)
++            second_trigger->raise();
++        if (prev_call_count > 1)
++            FAIL() << "Alarm called too many times";
      });
      UnblockMainLoop unblocker(ml);
--    {
--        std::unique_lock<decltype(m)> lock(m);
--        ASSERT_TRUE(called.wait_for(lock,
--                                    std::chrono::milliseconds{50},
--                                    [this](){ return call_count == 1; }));
--    }
--
++    ASSERT_TRUE(first_trigger->wait_for(std::chrono::milliseconds{50}));
      ASSERT_EQ(mir::time::Alarm::triggered, alarm->state());
++
++
      alarm->reschedule_in(std::chrono::milliseconds{100});
++
      EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{500}));
++    EXPECT_TRUE(second_trigger->wait_for(std::chrono::milliseconds{500}));
      EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
+ }
  TEST_F(AsioMainLoopAlarmTest, rescheduled_alarm_cancels_previous_scheduling)
+ {
--    auto alarm = ml.notify_in(std::chrono::milliseconds{100}, [this]()
++    std::atomic<int> call_count{0};
++
++    auto alarm = ml.notify_in(std::chrono::milliseconds{50}, [&call_count]()
+     {
          call_count++;
--        wait.notify_ready();
      });
      UnblockMainLoop unblocker(ml);
--    EXPECT_TRUE(alarm->reschedule_in(std::chrono::milliseconds{150}));
++    EXPECT_TRUE(alarm->reschedule_in(std::chrono::milliseconds{10}));
      EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{500}));
++    std::this_thread::sleep_for(std::chrono::milliseconds{200});
++
      EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
      EXPECT_EQ(1, call_count);
+ }
@@ -507,13 +513,18 @@
+ {
      mir::time::HighResolutionClock clock;
--    mir::time::Timestamp real_soon = clock.sample() + std::chrono::microseconds{120};
++    mir::time::Timestamp real_soon = clock.sample() + std::chrono::milliseconds{120};
++    auto alarm_fired = std::make_shared<mt::Signal>();
--    auto alarm = ml.notify_at(real_soon, [this]() { wait.notify_ready(); });
++    auto alarm = ml.notify_at(real_soon, [alarm_fired]()
++    {
++        alarm_fired->raise();
++    });
      UnblockMainLoop unblocker(ml);
--    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{200}));
++    EXPECT_FALSE(alarm_fired->wait_until(real_soon - std::chrono::milliseconds{50}));
++    EXPECT_TRUE(alarm_fired->wait_until(real_soon + std::chrono::milliseconds{50}));
+ }
  TEST(AsioMainLoopTest, dispatches_action)