Mir

Merge lp:~raof/mir/dont-kill-the-poor-clients into lp:mir

dont-kill-the-poor-clients
Merge into development-branch

Proposed by Chris Halse Rogers on 2014-09-02

Status:	Work in progress
Proposed branch:	lp:~raof/mir/dont-kill-the-poor-clients
Merge into:	lp:mir
Diff against target:	626 lines (+141/-172) 21 files modified src/client/lifecycle_control.cpp (+13/-0) src/client/lifecycle_control.h (+2/-0) src/client/mir_connection.cpp (+11/-0) src/client/rpc/mir_protobuf_rpc_channel.cpp (+0/-1) tests/acceptance-tests/test_client_cursor_api.cpp (+0/-2) tests/acceptance-tests/test_client_input.cpp (+0/-2) tests/acceptance-tests/test_client_library.cpp (+1/-3) tests/acceptance-tests/test_large_messages.cpp (+0/-2) tests/acceptance-tests/test_nested_mir.cpp (+0/-2) tests/acceptance-tests/test_prompt_session_client_api.cpp (+0/-2) tests/acceptance-tests/test_protobuf.cpp (+0/-2) tests/acceptance-tests/test_test_framework.cpp (+0/-2) tests/include/mir_test_framework/basic_client_server_fixture.h (+0/-2) tests/include/mir_test_framework/using_stub_client_platform.h (+0/-51) tests/integration-tests/client/test_client_render.cpp (+0/-10) tests/mir_test_framework/CMakeLists.txt (+0/-1) tests/mir_test_framework/testing_process_manager.cpp (+0/-2) tests/mir_test_framework/using_stub_client_platform.cpp (+0/-81) tests/unit-tests/client/CMakeLists.txt (+1/-0) tests/unit-tests/client/test_client_mir_surface.cpp (+2/-7) tests/unit-tests/client/test_lifecycle_control.cpp (+111/-0)
To merge this branch:	bzr merge lp:~raof/mir/dont-kill-the-poor-clients
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2014-09-12
Daniel van Vugt			Needs Fixing on 2014-09-12
Chris Halse Rogers			Abstain on 2014-09-10
Cemil Azizoglu (community)			Approve on 2014-09-02
Alexandros Frantzis (community)		2014-09-02	Approve on 2014-09-02
Review via email: mp+232971@code.launchpad.net

Commit message

Don't call the default lifecycle handler after mir_connection_release

The client has deliberately disconnected; they don't need a SIGHUP to kill them because they've been disconnected.

Clients which set a non-default lifecycle handler still get their mir_lifecycle_connection_lost event, as normal.

Description of the change

Don't call the default lifecycle handler during connection cleanup when the client explicitly calls mir_connection_release. They've deliberately disconnected; they don't also need a SIGHUP.

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-02

1886. By Chris Halse Rogers on 2014-09-02: Also remove unnecessary lifecycle setting in Android integration tests
1887. By Chris Halse Rogers on 2014-09-02: Remove some detritus

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-09-02:

Why are we removing UsingStubClientPlatform? Apart from the mir_connection_release() workaround in it (which was a recent addition), it stubs out the client platform which is needed by most tests (since the server platform is also stubbed out).

We should just remove the mir_connection_release() workaround from UsingStubPlatform.

Needs Fixing/Information

review: Needs Fixing

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-02:

But it doesn't stub out the client platform?

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-02:

Hm. Expanding on that - it tries, but fails, to stub out the client platform. It's got at configuration() override, but this is never called.

The DefaultConnectionAPI is constructed in mir_connection_api.cpp, and that uses DefaultMirConnectionAPI's configuration().

Then UsingStubClientPlatform comes along, and interposes StubMirConnectionAPI's connect() method before the default, but then calls the default API's connect(), which calls DefaultConfigurationAPI's configuration().

Since UsingStubClientPlatform wasn't actually doing anything, I removed it.

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-09-02:

> Hm. Expanding on that - it tries, but fails, to stub out the client platform. It's got at
> configuration() override, but this is never called.

Wow, you are right, super-not-inheritance-fail there. Thanks for catching this.

OK, so as long as our tests work without a stubbed platform that's fine. We can add a proper implementation of this as needed.

review: Approve

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2014-09-02:

LGTM

review: Approve

Revision history for this message

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

PASSED: Continuous integration, rev:1887
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2587/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1572
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1578
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1554
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1109
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1109/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/491
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/491/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2624
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12534

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

review: Approve (continuous-integration)

Revision history for this message

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

I like it, but not all clients shut down cleanly after applying this branch:

$ bin/mir_demo_client_egltriangle
Current active output is 1920x1200 +0+0
Server supports 2 of 6 surface pixel formats. Using format: 4
Surface occluded
libEGL warning: unsupported platform (null)
Surface 0 DPI
Surface exposed
1 FPS
23 FPS
60 FPS
60 FPS
Surface occluded
Signal 1 received. Good night.
Caught exception at Mir/EGL driver boundary: /home/dan/bzr/mir/tmp.poor/src/client/rpc/stream_socket_transport.cpp(276): Throw in function virtual void mir::client::rpc::StreamSocketTransport::send_data(const std::vector<unsigned char>&)
Dynamic exception type: N5boost16exception_detail10clone_implINS0_19error_info_injectorIN12_GLOBAL__N_125socket_disconnected_errorEEEEE
std::exception::what: Failed to send message to server: Broken pipe
32, "Broken pipe"

review: Needs Fixing

Revision history for this message

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

To clarify: The regression is the exception. That doesn't happen using development-branch.

Revision history for this message

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

I'm confused as to how you're getting the “Signal 1 received” message; signal 1 is SIGHUP, which we no longer send?

Revision history for this message

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

I tested the branch by merging into development-branch of course...

Just kill the server and the clients receive SIGHUP. In the case of egltriangle it also throws the exception...

Revision history for this message

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

(1) The exception regression described above needs fixing.

and also now:

(2) Text conflict in tests/acceptance-tests/test_client_input.cpp
1 conflicts encountered.

review: Needs Fixing

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-05

1888. By Chris Halse Rogers on 2014-09-05: Detect server disconnect in example EGL apps
1889. By Chris Halse Rogers on 2014-09-05: Merge trunk, resolving conflicts
1890. By Chris Halse Rogers on 2014-09-05: Remove last vestages of UsingStubClientPlatform that crept back in

Revision history for this message

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

FAILED: Continuous integration, rev:1888
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2622/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1618
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1624/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1600/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1144/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/537/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

FAILED: Continuous integration, rev:1890
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2625/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1621
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1627
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1603/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1147
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1147/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/540
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/540/artifact/work/output/*zip*/output.zip
    ABORTED: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2662/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

FAILED: Continuous integration, rev:1888
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2640/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1640/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1646/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1622/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1162/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/559/console

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

review: Needs Fixing (continuous-integration)

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-08

1891. By Chris Halse Rogers on 2014-09-08: Merge trunk, resolving conflicts

Revision history for this message

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

(3) examples/eglapp.c: Please revert the changes to examples/*. They're just hiding the aforementioned regression instead of fixing it. An unmodified client will still suffer from the same regression so it's better to not introduce it than to let it land and then need to create/fix a new bug.

Signal 1 received. Good night.
Caught exception at Mir/EGL driver boundary: /home/dan/bzr/mir/tmp.poor/src/client/rpc/stream_socket_transport.cpp(280): Throw in function virtual void mir::client::rpc::StreamSocketTransport::send_data(const std::vector<unsigned char>&)
Dynamic exception type: N5boost16exception_detail10clone_implINS0_19error_info_injectorIN12_GLOBAL__N_112socket_errorEEEEE
std::exception::what: Failed to send message to server: Bad file descriptor
9, "Bad file descriptor"

review: Needs Fixing

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-08:

So, I can't reproduce the exception you see after reverting the changes to eglapp.c.

I think we want to have a discussion about appropriate error handling on the 2/3rds call today :)

review: Needs Information

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-10:

Ok. So, as I mentioned on the 2/3rds call, this merge proposal doesn't change the code around the exception that you're seeing, which looks to be a race between the signal handler setting ‘running = false’ and the eglSwapBuffers call making a advance_buffer call. I can't reproduce it at the moment.

It isn't an exception escaping across to our C clients, and it's an inherent limitation in eglapp's use of a running flag.

This error is propagated through to clients by eglSwapBuffers returning false, but we don't do anything with that in eglapp.c

review: Abstain

Revision history for this message

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

FAILED: Continuous integration, rev:1891
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2673/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1691
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1697
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1670/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1195/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/607/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

FAILED: Continuous integration, rev:1891
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2678/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1697/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1703
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1676/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1200/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/613
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/613/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2727/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13001

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

FAILED: Continuous integration, rev:1891
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2689/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1713/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1719/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1692/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1211/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/629/console

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

review: Needs Fixing (continuous-integration)

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-11

1892. By Chris Halse Rogers on 2014-09-11: Merge trunk, resolving conflicts

Revision history for this message

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

PASSED: Continuous integration, rev:1892
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2692/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1716
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1723
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1695
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1214
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1214/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/632
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/632/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2738
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13048

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

review: Approve (continuous-integration)

Revision history for this message

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

Today I can't reproduce the exception regression either.

Although I suggest it's still a good idea to revert the examples/* changes. They're not strictly related to what's being fixed according to the description. And it's extra-useful to leave the default behaviour in there so we will see it in case the exception problem returns.

review: Needs Fixing

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-11

1893. By Chris Halse Rogers on 2014-09-11

Revert eglapp changes.

These will be proposed as a part of a wider cleanup of our examples

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

Approved in theory, but now the Jenkins failures appear to be a real regression, easy to reproduce:

$ umockdev-run -- valgrind bin/mir_unit_tests
...
[ RUN ] MirClientSurfaceTest.client_buffer_created_on_next_buffer
pure virtual method called
terminate called without an active exception

$ umockdev-run -- valgrind bin/mir_unit_tests --gtest_filter="MirClientSurfaceTest.*"
...
[ RUN ] MirClientSurfaceTest.create_wait_handle_really_blocks
pure virtual method called
terminate called without an active exception

review: Needs Fixing

Revision history for this message

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

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

review: Needs Fixing (continuous-integration)

lp:~raof/mir/dont-kill-the-poor-clients updated on 2014-09-12

1894. By Chris Halse Rogers on 2014-09-12

Don't force the completion of pending RPC calls on disconnect.

There's nothing sensible to be done there. If the client is simple, they'll be killed with a signal
(that awesomely might race with our force_completion() on the IO thread).

If the client is complex, they'll have a handler for the disconnect.

In neither case is it useful to call the RPC response handlers with a garbage non-reply.
Our response handlers are not generally capable of handling invalid protobuf::Message objects,
so this frequently results in calling a pure-virtual function and termination.

At best this would result in calling a client callback with an error object. But the client
already knows that the connection has died (or the process has been killed by a signal)
so that's not super-useful.

Revision history for this message

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

The first "regression" I was talking about seems to be bug 1368652. Indeed not caused by this branch.

The second (actual) regression that was causing Jenkins failures... I guess we shall see soon if that's fixed.

Revision history for this message

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

FAILED: Continuous integration, rev:1894
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2716/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1748
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1755/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1727/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1238/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/664/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-17:

Urgh. I am the master of submitting branches that require significant infrastructure changes before the entirely work.

Unmerged revisions

1894. By Chris Halse Rogers on 2014-09-12

Don't force the completion of pending RPC calls on disconnect.

There's nothing sensible to be done there. If the client is simple, they'll be killed with a signal
(that awesomely might race with our force_completion() on the IO thread).

If the client is complex, they'll have a handler for the disconnect.

1893. By Chris Halse Rogers on 2014-09-11

Revert eglapp changes.

These will be proposed as a part of a wider cleanup of our examples

1892. By Chris Halse Rogers on 2014-09-11

Merge trunk, resolving conflicts

1891. By Chris Halse Rogers on 2014-09-08

Merge trunk, resolving conflicts

1890. By Chris Halse Rogers on 2014-09-05

Remove last vestages of UsingStubClientPlatform that crept back in

1889. By Chris Halse Rogers on 2014-09-05

Merge trunk, resolving conflicts

1888. By Chris Halse Rogers on 2014-09-05

Detect server disconnect in example EGL apps

1887. By Chris Halse Rogers on 2014-09-02

Remove some detritus

1886. By Chris Halse Rogers on 2014-09-02

Also remove unnecessary lifecycle setting in Android integration tests

1885. By Chris Halse Rogers on 2014-09-02

Remove UsingStubClientConnection.

This is now superfluous; it was only preventing the test code from being SIGHUPed when
it called mir_connection_release, which we no longer do.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Chris Halse Rogers

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'src/client/lifecycle_control.cpp'
 --- src/client/lifecycle_control.cpp	2013-10-07 09:27:32 +0000
 +++ src/client/lifecycle_control.cpp	2014-09-12 08:09:04 +0000
@@ -36,6 +36,19 @@
      handle_lifecycle_event = fn;
+ }
++void mir::client::LifecycleControl::replace_lifecycle_event_handler_if_matches(
++    void (*match)(MirLifecycleState),
++    const std::function<void (MirLifecycleState)> &new_handler)
++{
++    std::lock_guard<decltype(guard)> lock(guard);
++
++    auto current_target = handle_lifecycle_event.target<void(*)(MirLifecycleState)>();
++    if (current_target && *current_target == match)
++    {
++        handle_lifecycle_event = new_handler;
++    }
++}
++
  void mcl::LifecycleControl::call_lifecycle_event_handler(uint32_t state)
+ {
      std::unique_lock<std::mutex> lk(guard);
 === modified file 'src/client/lifecycle_control.h'
 --- src/client/lifecycle_control.h	2013-08-23 12:23:47 +0000
 +++ src/client/lifecycle_control.h	2014-09-12 08:09:04 +0000
@@ -35,6 +35,8 @@
      ~LifecycleControl();
      void set_lifecycle_event_handler(std::function<void(MirLifecycleState)> const&);
++    void replace_lifecycle_event_handler_if_matches(void(*match)(MirLifecycleState),
++                                                    std::function<void(MirLifecycleState)> const& new_handler);
      void call_lifecycle_event_handler(uint32_t state);
  private:
 === modified file 'src/client/mir_connection.cpp'
 --- src/client/mir_connection.cpp	2014-09-10 12:50:53 +0000
 +++ src/client/mir_connection.cpp	2014-09-12 08:09:04 +0000
@@ -298,9 +298,20 @@
      disconnect_wait_handle.result_received();
+ }
++namespace
++{
++void null_lifecycle_handler(MirLifecycleState)
++{
++}
++}
++
  MirWaitHandle* MirConnection::disconnect()
+ {
      disconnect_wait_handle.expect_result();
++    // Don't kill the client with a signal on disconnect when the client has explicitly
++    // requested disconnection.
++    lifecycle_control->replace_lifecycle_event_handler_if_matches(&default_lifecycle_event_handler,
++                                                                  &null_lifecycle_handler);
      server.disconnect(0, &ignored, &ignored,
                        google::protobuf::NewCallback(this, &MirConnection::done_disconnect));
 === modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
 --- src/client/rpc/mir_protobuf_rpc_channel.cpp	2014-09-10 12:50:53 +0000
 +++ src/client/rpc/mir_protobuf_rpc_channel.cpp	2014-09-12 08:09:04 +0000
@@ -79,7 +79,6 @@
+     {
          lifecycle_control->call_lifecycle_event_handler(mir_lifecycle_connection_lost);
+     }
--    pending_calls.force_completion();
+ }
 === modified file 'tests/acceptance-tests/test_client_cursor_api.cpp'
 --- tests/acceptance-tests/test_client_cursor_api.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_client_cursor_api.cpp	2014-09-12 08:09:04 +0000
@@ -24,7 +24,6 @@
  #include "mir_test_framework/in_process_server.h"
  #include "mir_test_framework/fake_event_hub_server_configuration.h"
  #include "mir_test_framework/declarative_placement_strategy.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test/fake_shared.h"
  #include "mir_test/spin_wait.h"
@@ -264,7 +263,6 @@
      std::string const client_cursor_2{"cursor-2"};
      TestServerConfiguration server_configuration_;
      mir::test::WaitCondition expectations_satisfied;
--    mtf::UsingStubClientPlatform using_stub_client_platform;
  };
+ }
 === modified file 'tests/acceptance-tests/test_client_input.cpp'
 --- tests/acceptance-tests/test_client_input.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_client_input.cpp	2014-09-12 08:09:04 +0000
@@ -25,7 +25,6 @@
  #include "src/server/scene/session_container.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test_framework/fake_event_hub_server_configuration.h"
  #include "mir_test_framework/declarative_placement_strategy.h"
  #include "mir_test/wait_condition.h"
@@ -213,7 +212,6 @@
      std::string const test_client_name_2{"client2"};
      geom::Rectangle const screen_geometry{{0, 0}, {1000, 800}};
      TestServerConfiguration server_configuration_{screen_geometry};
--    mtf::UsingStubClientPlatform using_stub_client_platform;
  };
+ }
 === modified file 'tests/acceptance-tests/test_client_library.cpp'
 --- tests/acceptance-tests/test_client_library.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_client_library.cpp	2014-09-12 08:09:04 +0000
@@ -21,7 +21,6 @@
  #include "mir_test_framework/stubbed_server_configuration.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "src/client/client_buffer.h"
@@ -60,7 +59,6 @@
+ {
      mtf::StubbedServerConfiguration server_configuration;
      mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
--    mtf::UsingStubClientPlatform using_stub_client_platform;
      std::set<MirSurface*> surfaces;
      MirConnection* connection = nullptr;
@@ -640,7 +638,7 @@
      mir_connection_release(connection);
+ }
--TEST_F(ClientLibrary, MultiSurfaceClientTracksBufferFdsCorrectly)
++TEST_F(ClientLibrary, multi_surface_client_tracks_buffer_fds_correctly)
+ {
      mir_wait_for(mir_connect(new_connection().c_str(), __PRETTY_FUNCTION__, connection_callback, this));
 === modified file 'tests/acceptance-tests/test_large_messages.cpp'
 --- tests/acceptance-tests/test_large_messages.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_large_messages.cpp	2014-09-12 08:09:04 +0000
@@ -18,7 +18,6 @@
  #include "mir_test_framework/in_process_server.h"
  #include "mir_test_framework/stubbed_server_configuration.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test/wait_object.h"
  #include "mir_test/fake_shared.h"
  #include "mir_test_doubles/stub_display_configuration.h"
@@ -68,7 +67,6 @@
          return large_messages_server_config;
+     }
--    mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
  };
  struct ConnectionContext
 === modified file 'tests/acceptance-tests/test_nested_mir.cpp'
 --- tests/acceptance-tests/test_nested_mir.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_nested_mir.cpp	2014-09-12 08:09:04 +0000
@@ -28,7 +28,6 @@
  #include "mir_test_framework/in_process_server.h"
  #include "mir_test_framework/stubbed_server_configuration.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test/wait_condition.h"
  #include "mir_test_doubles/mock_egl.h"
@@ -267,7 +266,6 @@
      NestedServer() : HostServerConfiguration(display_geometry) {}
      NestedMockEGL mock_egl;
--    mtf::UsingStubClientPlatform using_stub_client_platform;
      virtual mir::DefaultServerConfiguration& server_config()
+     {
 === modified file 'tests/acceptance-tests/test_prompt_session_client_api.cpp'
 --- tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-09-12 08:09:04 +0000
@@ -28,7 +28,6 @@
  #include "mir_test_doubles/stub_session_authorizer.h"
  #include "mir_test_framework/stubbed_server_configuration.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_test/popen.h"
  #include <gtest/gtest.h>
@@ -142,7 +141,6 @@
      std::shared_ptr<mf::Session> application_session;
      std::shared_ptr<ms::PromptSession> server_prompt_session;
--    mtf::UsingStubClientPlatform using_stub_client_platform;
      void SetUp() override
+     {
 === modified file 'tests/acceptance-tests/test_protobuf.cpp'
 --- tests/acceptance-tests/test_protobuf.cpp	2014-09-10 21:37:08 +0000
 +++ tests/acceptance-tests/test_protobuf.cpp	2014-09-12 08:09:04 +0000
@@ -26,7 +26,6 @@
  #include "mir_test_framework/stubbed_server_configuration.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include <gtest/gtest.h>
  #include <gmock/gmock.h>
@@ -158,7 +157,6 @@
      mir::DefaultServerConfiguration& server_config() override { return my_server_config; }
      DemoServerConfiguration my_server_config;
--    mtf::UsingStubClientPlatform using_stub_client_platform;
      std::shared_ptr<DemoConnectionCreator> demo_connection_creator;
 === modified file 'tests/acceptance-tests/test_test_framework.cpp'
 --- tests/acceptance-tests/test_test_framework.cpp	2014-09-10 12:50:53 +0000
 +++ tests/acceptance-tests/test_test_framework.cpp	2014-09-12 08:09:04 +0000
@@ -19,7 +19,6 @@
  #include "mir_test_framework/display_server_test_fixture.h"
  #include "mir_test_framework/testing_server_configuration.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "mir_toolkit/mir_client_library.h"
@@ -75,7 +74,6 @@
      virtual mir::DefaultServerConfiguration& server_config() { return server_config_; }
      mir_test_framework::StubbedServerConfiguration server_config_;
--    mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
  };
+ }
 === modified file 'tests/include/mir_test_framework/basic_client_server_fixture.h'
 --- tests/include/mir_test_framework/basic_client_server_fixture.h	2014-09-10 12:50:53 +0000
 +++ tests/include/mir_test_framework/basic_client_server_fixture.h	2014-09-12 08:09:04 +0000
@@ -22,7 +22,6 @@
  #include "mir_toolkit/mir_client_library.h"
  #include "mir_test_framework/in_process_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "boost/throw_exception.hpp"
@@ -34,7 +33,6 @@
  struct BasicClientServerFixture : InProcessServer
+ {
      TestServerConfiguration server_configuration;
--    UsingStubClientPlatform using_stub_client_platform;
      mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
 === removed file 'tests/include/mir_test_framework/using_stub_client_platform.h'
 --- tests/include/mir_test_framework/using_stub_client_platform.h	2014-09-10 12:50:53 +0000
 +++ tests/include/mir_test_framework/using_stub_client_platform.h	1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
--/*
-- * 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
-- */
--
--#ifndef MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_
--#define MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_
--
--#include <memory>
--
--namespace mir
--{
--namespace client
--{
--class MirConnectionAPI;
--}
--}
--
--namespace mir_test_framework
--{
--
--class UsingStubClientPlatform
--{
--public:
--    UsingStubClientPlatform();
--    ~UsingStubClientPlatform();
--
--private:
--    UsingStubClientPlatform(UsingStubClientPlatform const&) = delete;
--    UsingStubClientPlatform operator=(UsingStubClientPlatform const&) = delete;
--
--    mir::client::MirConnectionAPI* prev_api;
--    std::unique_ptr<mir::client::MirConnectionAPI> stub_api;
--};
--
--}
--
--#endif /* MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_ */
 === modified file 'tests/integration-tests/client/test_client_render.cpp'
 --- tests/integration-tests/client/test_client_render.cpp	2014-09-10 13:27:58 +0000
 +++ tests/integration-tests/client/test_client_render.cpp	2014-09-12 08:09:04 +0000
@@ -57,10 +57,6 @@
  static mtd::DrawPatternCheckered<2,2> draw_pattern1(pattern1);
  static const char socket_file[] = "./test_client_ipc_render_socket";
--void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
--{
--}
--
  struct TestClient
+ {
      static MirPixelFormat select_format_for_visual_id(int visual_id)
@@ -114,9 +110,6 @@
+         }
          mir_surface_release_sync(surface);
--        // Clear the lifecycle callback in order not to get SIGHUP by the
--        // default lifecycle handler during connection teardown
--        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
          mir_connection_release(connection);
          return 0;
+     }
@@ -174,9 +167,6 @@
+         }
          mir_surface_release_sync(mir_surface);
--        // Clear the lifecycle callback in order not to get SIGHUP by the
--        // default lifecycle handler during connection teardown
--        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
          mir_connection_release(connection);
          return 0;
+     }
 === modified file 'tests/mir_test_framework/CMakeLists.txt'
 --- tests/mir_test_framework/CMakeLists.txt	2014-09-10 12:50:53 +0000
 +++ tests/mir_test_framework/CMakeLists.txt	2014-09-12 08:09:04 +0000
@@ -19,7 +19,6 @@
    testing_client_options.cpp
    display_server_test_fixture.cpp
    process.cpp
--  using_stub_client_platform.cpp
    udev_environment.cpp
    declarative_placement_strategy.cpp
    fake_event_hub_server_configuration.cpp
 === modified file 'tests/mir_test_framework/testing_process_manager.cpp'
 --- tests/mir_test_framework/testing_process_manager.cpp	2014-09-10 12:50:53 +0000
 +++ tests/mir_test_framework/testing_process_manager.cpp	2014-09-12 08:09:04 +0000
@@ -19,7 +19,6 @@
  #include "mir_toolkit/client_types.h"
  #include "mir_test_framework/testing_process_manager.h"
  #include "mir_test_framework/detect_server.h"
--#include "mir_test_framework/using_stub_client_platform.h"
  #include "src/client/mir_connection.h"
  #include "mir/run_mir.h"
@@ -107,7 +106,6 @@
          SCOPED_TRACE("Client");
          if (!config.use_real_graphics(test_options))
+         {
--            mtf::UsingStubClientPlatform p;
              config.exec();
+         }
          else
 === removed file 'tests/mir_test_framework/using_stub_client_platform.cpp'
 --- tests/mir_test_framework/using_stub_client_platform.cpp	2014-09-10 12:50:53 +0000
 +++ tests/mir_test_framework/using_stub_client_platform.cpp	1970-01-01 00:00:00 +0000
@@ -1,81 +0,0 @@
--/*
-- * 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
-- */
--
--#include "mir_test_framework/using_stub_client_platform.h"
--#include "mir_test_framework/stub_client_connection_configuration.h"
--#include "mir_toolkit/mir_client_library.h"
--#include "src/client/mir_connection_api.h"
--
--namespace mtf = mir_test_framework;
--namespace mcl = mir::client;
--
--namespace
--{
--
--void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
--{
--}
--
--class StubMirConnectionAPI : public mcl::MirConnectionAPI
--{
--public:
--    StubMirConnectionAPI(mcl::MirConnectionAPI* prev_api)
--        : prev_api{prev_api}
--    {
--    }
--
--    MirWaitHandle* connect(
--        char const* socket_file,
--        char const* name,
--        mir_connected_callback callback,
--        void* context) override
--    {
--        return prev_api->connect(socket_file, name, callback, context);
--    }
--
--    void release(MirConnection* connection) override
--    {
--        // Clear the lifecycle callback in order not to get SIGHUP by the
--        // default lifecycle handler during connection teardown
--        mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
--        return prev_api->release(connection);
--    }
--
--    std::unique_ptr<mcl::ConnectionConfiguration> configuration(std::string const& socket) override
--    {
--        return std::unique_ptr<mcl::ConnectionConfiguration>{
--            new mtf::StubConnectionConfiguration{socket}};
--    }
--
--private:
--    mcl::MirConnectionAPI* const prev_api;
--};
--
--}
--
--mtf::UsingStubClientPlatform::UsingStubClientPlatform()
--    : prev_api{mir_connection_api_impl},
--      stub_api{new StubMirConnectionAPI{prev_api}}
--{
--    mir_connection_api_impl = stub_api.get();
--}
--
--mtf::UsingStubClientPlatform::~UsingStubClientPlatform()
--{
--    mir_connection_api_impl = prev_api;
--}
 === modified file 'tests/unit-tests/client/CMakeLists.txt'
 --- tests/unit-tests/client/CMakeLists.txt	2014-09-10 12:50:53 +0000
 +++ tests/unit-tests/client/CMakeLists.txt	2014-09-12 08:09:04 +0000
@@ -12,6 +12,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_rpc_channel.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_prompt_session.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_event_distributor.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_lifecycle_control.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_periodic_perf_report.cpp
+ )
 === modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
 --- tests/unit-tests/client/test_client_mir_surface.cpp	2014-09-10 12:50:53 +0000
 +++ tests/unit-tests/client/test_client_mir_surface.cpp	2014-09-12 08:09:04 +0000
@@ -392,10 +392,6 @@
+ {
+ }
--void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
--{
--}
--
  MATCHER_P(BufferPackageMatches, package, "")
+ {
      // Can't simply use memcmp() on the whole struct because age is not sent over the wire
@@ -421,9 +417,8 @@
      ~MirClientSurfaceTest()
+     {
--        // Clear the lifecycle callback in order not to get SIGHUP by the
--        // default lifecycle handler during connection teardown
--        connection->register_lifecycle_event_callback(null_lifecycle_callback, nullptr);
++        // Clean up nicely after ourselves...
++        connection->disconnect();
+     }
      void start_test_server()
 === added file 'tests/unit-tests/client/test_lifecycle_control.cpp'
 --- tests/unit-tests/client/test_lifecycle_control.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/client/test_lifecycle_control.cpp	2014-09-12 08:09:04 +0000
@@ -0,0 +1,111 @@
++/*
++ * 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/client/lifecycle_control.h"
++
++#include <gtest/gtest.h>
++
++namespace
++{
++bool a_called{false}, b_called{false};
++
++void handler_a(MirLifecycleState)
++{
++    a_called = true;
++}
++
++void handler_b(MirLifecycleState)
++{
++    b_called = true;
++}
++}
++
++TEST(LifeCycleControl, InvokesSetTarget)
++{
++    mir::client::LifecycleControl control;
++
++    a_called = false;
++    b_called = false;
++
++    control.set_lifecycle_event_handler(&handler_a);
++    control.call_lifecycle_event_handler(mir_lifecycle_connection_lost);
++
++    EXPECT_TRUE(a_called);
++    EXPECT_FALSE(b_called);
++}
++
++TEST(LifeCycleControl, InvokesUpdatedTarget)
++{
++    mir::client::LifecycleControl control;
++
++    a_called = false;
++    b_called = false;
++
++    control.set_lifecycle_event_handler(&handler_a);
++    control.set_lifecycle_event_handler(&handler_b);
++    control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
++
++    EXPECT_FALSE(a_called);
++    EXPECT_TRUE(b_called);
++}
++
++TEST(LifeCycleControl, UpdateReplacesIfMatches)
++{
++    mir::client::LifecycleControl control;
++
++    a_called = false;
++    b_called = false;
++
++    control.set_lifecycle_event_handler(&handler_a);
++    control.replace_lifecycle_event_handler_if_matches(&handler_a, &handler_b);
++    control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
++
++    EXPECT_FALSE(a_called);
++    EXPECT_TRUE(b_called);
++}
++
++TEST(LifeCycleControl, UpdateDoesNotReplaceIfNotMatching)
++{
++    mir::client::LifecycleControl control;
++
++    a_called = false;
++    b_called = false;
++
++    control.set_lifecycle_event_handler(&handler_a);
++    control.replace_lifecycle_event_handler_if_matches(&handler_b, &handler_b);
++    control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
++
++    EXPECT_TRUE(a_called);
++    EXPECT_FALSE(b_called);
++}
++
++TEST(LifeCycleControl, UpdateHandlesDifferingTypes)
++{
++    mir::client::LifecycleControl control;
++
++    a_called = false;
++    b_called = false;
++    auto non_fn_pointer_handler = [](MirLifecycleState){};
++
++    control.set_lifecycle_event_handler(non_fn_pointer_handler);
++    control.replace_lifecycle_event_handler_if_matches(&handler_a, &handler_b);
++    control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
++
++    EXPECT_FALSE(a_called);
++    EXPECT_FALSE(b_called);
++}