Mir

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

Proposed by Chris Halse Rogers
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
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Daniel van Vugt Needs Fixing
Chris Halse Rogers Abstain
Cemil Azizoglu (community) Approve
Alexandros Frantzis (community) Approve
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.

To post a comment you must log in.
1886. By Chris Halse Rogers

Also remove unnecessary lifecycle setting in Android integration tests

1887. By Chris Halse Rogers

Remove some detritus

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

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 :

But it doesn't stub out the client platform?

Revision history for this message
Chris Halse Rogers (raof) wrote :

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 :

> 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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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 :

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

Revision history for this message
Chris Halse Rogers (raof) wrote :

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 :

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 :

(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
1888. By Chris Halse Rogers

Detect server disconnect in example EGL apps

1889. By Chris Halse Rogers

Merge trunk, resolving conflicts

1890. By Chris Halse Rogers

Remove last vestages of UsingStubClientPlatform that crept back in

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1891. By Chris Halse Rogers

Merge trunk, resolving conflicts

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

(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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

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 :

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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1892. By Chris Halse Rogers

Merge trunk, resolving conflicts

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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
1893. By Chris Halse Rogers

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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

or

$ 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 :
review: Needs Fixing (continuous-integration)
1894. By Chris Halse Rogers

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 :

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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

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

Unmerged revisions

1894. By Chris Halse Rogers

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.

1893. By Chris Halse Rogers

Revert eglapp changes.

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

1892. By Chris Halse Rogers

Merge trunk, resolving conflicts

1891. By Chris Halse Rogers

Merge trunk, resolving conflicts

1890. By Chris Halse Rogers

Remove last vestages of UsingStubClientPlatform that crept back in

1889. By Chris Halse Rogers

Merge trunk, resolving conflicts

1888. By Chris Halse Rogers

Detect server disconnect in example EGL apps

1887. By Chris Halse Rogers

Remove some detritus

1886. By Chris Halse Rogers

Also remove unnecessary lifecycle setting in Android integration tests

1885. By Chris Halse Rogers

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
=== 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 @@
36 handle_lifecycle_event = fn;36 handle_lifecycle_event = fn;
37}37}
3838
39void mir::client::LifecycleControl::replace_lifecycle_event_handler_if_matches(
40 void (*match)(MirLifecycleState),
41 const std::function<void (MirLifecycleState)> &new_handler)
42{
43 std::lock_guard<decltype(guard)> lock(guard);
44
45 auto current_target = handle_lifecycle_event.target<void(*)(MirLifecycleState)>();
46 if (current_target && *current_target == match)
47 {
48 handle_lifecycle_event = new_handler;
49 }
50}
51
39void mcl::LifecycleControl::call_lifecycle_event_handler(uint32_t state)52void mcl::LifecycleControl::call_lifecycle_event_handler(uint32_t state)
40{53{
41 std::unique_lock<std::mutex> lk(guard);54 std::unique_lock<std::mutex> lk(guard);
4255
=== 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 @@
35 ~LifecycleControl();35 ~LifecycleControl();
3636
37 void set_lifecycle_event_handler(std::function<void(MirLifecycleState)> const&);37 void set_lifecycle_event_handler(std::function<void(MirLifecycleState)> const&);
38 void replace_lifecycle_event_handler_if_matches(void(*match)(MirLifecycleState),
39 std::function<void(MirLifecycleState)> const& new_handler);
38 void call_lifecycle_event_handler(uint32_t state);40 void call_lifecycle_event_handler(uint32_t state);
3941
40private:42private:
4143
=== 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 @@
298 disconnect_wait_handle.result_received();298 disconnect_wait_handle.result_received();
299}299}
300300
301namespace
302{
303void null_lifecycle_handler(MirLifecycleState)
304{
305}
306}
307
301MirWaitHandle* MirConnection::disconnect()308MirWaitHandle* MirConnection::disconnect()
302{309{
303 disconnect_wait_handle.expect_result();310 disconnect_wait_handle.expect_result();
311 // Don't kill the client with a signal on disconnect when the client has explicitly
312 // requested disconnection.
313 lifecycle_control->replace_lifecycle_event_handler_if_matches(&default_lifecycle_event_handler,
314 &null_lifecycle_handler);
304 server.disconnect(0, &ignored, &ignored,315 server.disconnect(0, &ignored, &ignored,
305 google::protobuf::NewCallback(this, &MirConnection::done_disconnect));316 google::protobuf::NewCallback(this, &MirConnection::done_disconnect));
306317
307318
=== 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 @@
79 {79 {
80 lifecycle_control->call_lifecycle_event_handler(mir_lifecycle_connection_lost);80 lifecycle_control->call_lifecycle_event_handler(mir_lifecycle_connection_lost);
81 }81 }
82 pending_calls.force_completion();
83}82}
8483
8584
8685
=== 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 @@
24#include "mir_test_framework/in_process_server.h"24#include "mir_test_framework/in_process_server.h"
25#include "mir_test_framework/fake_event_hub_server_configuration.h"25#include "mir_test_framework/fake_event_hub_server_configuration.h"
26#include "mir_test_framework/declarative_placement_strategy.h"26#include "mir_test_framework/declarative_placement_strategy.h"
27#include "mir_test_framework/using_stub_client_platform.h"
2827
29#include "mir_test/fake_shared.h"28#include "mir_test/fake_shared.h"
30#include "mir_test/spin_wait.h"29#include "mir_test/spin_wait.h"
@@ -264,7 +263,6 @@
264 std::string const client_cursor_2{"cursor-2"};263 std::string const client_cursor_2{"cursor-2"};
265 TestServerConfiguration server_configuration_;264 TestServerConfiguration server_configuration_;
266 mir::test::WaitCondition expectations_satisfied;265 mir::test::WaitCondition expectations_satisfied;
267 mtf::UsingStubClientPlatform using_stub_client_platform;
268};266};
269267
270}268}
271269
=== 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 @@
25#include "src/server/scene/session_container.h"25#include "src/server/scene/session_container.h"
2626
27#include "mir_test_framework/in_process_server.h"27#include "mir_test_framework/in_process_server.h"
28#include "mir_test_framework/using_stub_client_platform.h"
29#include "mir_test_framework/fake_event_hub_server_configuration.h"28#include "mir_test_framework/fake_event_hub_server_configuration.h"
30#include "mir_test_framework/declarative_placement_strategy.h"29#include "mir_test_framework/declarative_placement_strategy.h"
31#include "mir_test/wait_condition.h"30#include "mir_test/wait_condition.h"
@@ -213,7 +212,6 @@
213 std::string const test_client_name_2{"client2"};212 std::string const test_client_name_2{"client2"};
214 geom::Rectangle const screen_geometry{{0, 0}, {1000, 800}};213 geom::Rectangle const screen_geometry{{0, 0}, {1000, 800}};
215 TestServerConfiguration server_configuration_{screen_geometry};214 TestServerConfiguration server_configuration_{screen_geometry};
216 mtf::UsingStubClientPlatform using_stub_client_platform;
217};215};
218216
219}217}
220218
=== 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 @@
2121
22#include "mir_test_framework/stubbed_server_configuration.h"22#include "mir_test_framework/stubbed_server_configuration.h"
23#include "mir_test_framework/in_process_server.h"23#include "mir_test_framework/in_process_server.h"
24#include "mir_test_framework/using_stub_client_platform.h"
2524
26#include "src/client/client_buffer.h"25#include "src/client/client_buffer.h"
2726
@@ -60,7 +59,6 @@
60{59{
61 mtf::StubbedServerConfiguration server_configuration;60 mtf::StubbedServerConfiguration server_configuration;
62 mir::DefaultServerConfiguration& server_config() override { return server_configuration; }61 mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
63 mtf::UsingStubClientPlatform using_stub_client_platform;
6462
65 std::set<MirSurface*> surfaces;63 std::set<MirSurface*> surfaces;
66 MirConnection* connection = nullptr;64 MirConnection* connection = nullptr;
@@ -640,7 +638,7 @@
640 mir_connection_release(connection);638 mir_connection_release(connection);
641}639}
642640
643TEST_F(ClientLibrary, MultiSurfaceClientTracksBufferFdsCorrectly)641TEST_F(ClientLibrary, multi_surface_client_tracks_buffer_fds_correctly)
644{642{
645 mir_wait_for(mir_connect(new_connection().c_str(), __PRETTY_FUNCTION__, connection_callback, this));643 mir_wait_for(mir_connect(new_connection().c_str(), __PRETTY_FUNCTION__, connection_callback, this));
646644
647645
=== 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 @@
1818
19#include "mir_test_framework/in_process_server.h"19#include "mir_test_framework/in_process_server.h"
20#include "mir_test_framework/stubbed_server_configuration.h"20#include "mir_test_framework/stubbed_server_configuration.h"
21#include "mir_test_framework/using_stub_client_platform.h"
22#include "mir_test/wait_object.h"21#include "mir_test/wait_object.h"
23#include "mir_test/fake_shared.h"22#include "mir_test/fake_shared.h"
24#include "mir_test_doubles/stub_display_configuration.h"23#include "mir_test_doubles/stub_display_configuration.h"
@@ -68,7 +67,6 @@
68 return large_messages_server_config;67 return large_messages_server_config;
69 }68 }
7069
71 mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
72};70};
7371
74struct ConnectionContext72struct ConnectionContext
7573
=== 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 @@
2828
29#include "mir_test_framework/in_process_server.h"29#include "mir_test_framework/in_process_server.h"
30#include "mir_test_framework/stubbed_server_configuration.h"30#include "mir_test_framework/stubbed_server_configuration.h"
31#include "mir_test_framework/using_stub_client_platform.h"
32#include "mir_test/wait_condition.h"31#include "mir_test/wait_condition.h"
3332
34#include "mir_test_doubles/mock_egl.h"33#include "mir_test_doubles/mock_egl.h"
@@ -267,7 +266,6 @@
267 NestedServer() : HostServerConfiguration(display_geometry) {}266 NestedServer() : HostServerConfiguration(display_geometry) {}
268267
269 NestedMockEGL mock_egl;268 NestedMockEGL mock_egl;
270 mtf::UsingStubClientPlatform using_stub_client_platform;
271269
272 virtual mir::DefaultServerConfiguration& server_config()270 virtual mir::DefaultServerConfiguration& server_config()
273 {271 {
274272
=== 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 @@
28#include "mir_test_doubles/stub_session_authorizer.h"28#include "mir_test_doubles/stub_session_authorizer.h"
29#include "mir_test_framework/stubbed_server_configuration.h"29#include "mir_test_framework/stubbed_server_configuration.h"
30#include "mir_test_framework/in_process_server.h"30#include "mir_test_framework/in_process_server.h"
31#include "mir_test_framework/using_stub_client_platform.h"
32#include "mir_test/popen.h"31#include "mir_test/popen.h"
3332
34#include <gtest/gtest.h>33#include <gtest/gtest.h>
@@ -142,7 +141,6 @@
142 std::shared_ptr<mf::Session> application_session;141 std::shared_ptr<mf::Session> application_session;
143142
144 std::shared_ptr<ms::PromptSession> server_prompt_session;143 std::shared_ptr<ms::PromptSession> server_prompt_session;
145 mtf::UsingStubClientPlatform using_stub_client_platform;
146144
147 void SetUp() override145 void SetUp() override
148 {146 {
149147
=== 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 @@
2626
27#include "mir_test_framework/stubbed_server_configuration.h"27#include "mir_test_framework/stubbed_server_configuration.h"
28#include "mir_test_framework/in_process_server.h"28#include "mir_test_framework/in_process_server.h"
29#include "mir_test_framework/using_stub_client_platform.h"
3029
31#include <gtest/gtest.h>30#include <gtest/gtest.h>
32#include <gmock/gmock.h>31#include <gmock/gmock.h>
@@ -158,7 +157,6 @@
158 mir::DefaultServerConfiguration& server_config() override { return my_server_config; }157 mir::DefaultServerConfiguration& server_config() override { return my_server_config; }
159158
160 DemoServerConfiguration my_server_config;159 DemoServerConfiguration my_server_config;
161 mtf::UsingStubClientPlatform using_stub_client_platform;
162160
163 std::shared_ptr<DemoConnectionCreator> demo_connection_creator;161 std::shared_ptr<DemoConnectionCreator> demo_connection_creator;
164162
165163
=== 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 @@
19#include "mir_test_framework/display_server_test_fixture.h"19#include "mir_test_framework/display_server_test_fixture.h"
20#include "mir_test_framework/testing_server_configuration.h"20#include "mir_test_framework/testing_server_configuration.h"
21#include "mir_test_framework/in_process_server.h"21#include "mir_test_framework/in_process_server.h"
22#include "mir_test_framework/using_stub_client_platform.h"
2322
24#include "mir_toolkit/mir_client_library.h"23#include "mir_toolkit/mir_client_library.h"
2524
@@ -75,7 +74,6 @@
75 virtual mir::DefaultServerConfiguration& server_config() { return server_config_; }74 virtual mir::DefaultServerConfiguration& server_config() { return server_config_; }
7675
77 mir_test_framework::StubbedServerConfiguration server_config_;76 mir_test_framework::StubbedServerConfiguration server_config_;
78 mir_test_framework::UsingStubClientPlatform using_stub_client_platform;
79};77};
80}78}
8179
8280
=== 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 @@
22#include "mir_toolkit/mir_client_library.h"22#include "mir_toolkit/mir_client_library.h"
2323
24#include "mir_test_framework/in_process_server.h"24#include "mir_test_framework/in_process_server.h"
25#include "mir_test_framework/using_stub_client_platform.h"
2625
27#include "boost/throw_exception.hpp"26#include "boost/throw_exception.hpp"
2827
@@ -34,7 +33,6 @@
34struct BasicClientServerFixture : InProcessServer33struct BasicClientServerFixture : InProcessServer
35{34{
36 TestServerConfiguration server_configuration;35 TestServerConfiguration server_configuration;
37 UsingStubClientPlatform using_stub_client_platform;
3836
39 mir::DefaultServerConfiguration& server_config() override { return server_configuration; }37 mir::DefaultServerConfiguration& server_config() override { return server_configuration; }
4038
4139
=== 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 @@
1/*
2 * Copyright © 2014 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
17 */
18
19#ifndef MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_
20#define MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_
21
22#include <memory>
23
24namespace mir
25{
26namespace client
27{
28class MirConnectionAPI;
29}
30}
31
32namespace mir_test_framework
33{
34
35class UsingStubClientPlatform
36{
37public:
38 UsingStubClientPlatform();
39 ~UsingStubClientPlatform();
40
41private:
42 UsingStubClientPlatform(UsingStubClientPlatform const&) = delete;
43 UsingStubClientPlatform operator=(UsingStubClientPlatform const&) = delete;
44
45 mir::client::MirConnectionAPI* prev_api;
46 std::unique_ptr<mir::client::MirConnectionAPI> stub_api;
47};
48
49}
50
51#endif /* MIR_TEST_FRAMEWORK_USING_STUB_CLIENT_PLATFORM_H_ */
520
=== 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 @@
57static mtd::DrawPatternCheckered<2,2> draw_pattern1(pattern1);57static mtd::DrawPatternCheckered<2,2> draw_pattern1(pattern1);
58static const char socket_file[] = "./test_client_ipc_render_socket";58static const char socket_file[] = "./test_client_ipc_render_socket";
5959
60void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
61{
62}
63
64struct TestClient60struct TestClient
65{61{
66 static MirPixelFormat select_format_for_visual_id(int visual_id)62 static MirPixelFormat select_format_for_visual_id(int visual_id)
@@ -114,9 +110,6 @@
114 }110 }
115111
116 mir_surface_release_sync(surface);112 mir_surface_release_sync(surface);
117 // Clear the lifecycle callback in order not to get SIGHUP by the
118 // default lifecycle handler during connection teardown
119 mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
120 mir_connection_release(connection);113 mir_connection_release(connection);
121 return 0;114 return 0;
122 }115 }
@@ -174,9 +167,6 @@
174 }167 }
175168
176 mir_surface_release_sync(mir_surface);169 mir_surface_release_sync(mir_surface);
177 // Clear the lifecycle callback in order not to get SIGHUP by the
178 // default lifecycle handler during connection teardown
179 mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
180 mir_connection_release(connection);170 mir_connection_release(connection);
181 return 0;171 return 0;
182 }172 }
183173
=== 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 @@
19 testing_client_options.cpp19 testing_client_options.cpp
20 display_server_test_fixture.cpp20 display_server_test_fixture.cpp
21 process.cpp21 process.cpp
22 using_stub_client_platform.cpp
23 udev_environment.cpp22 udev_environment.cpp
24 declarative_placement_strategy.cpp23 declarative_placement_strategy.cpp
25 fake_event_hub_server_configuration.cpp24 fake_event_hub_server_configuration.cpp
2625
=== 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 @@
19#include "mir_toolkit/client_types.h"19#include "mir_toolkit/client_types.h"
20#include "mir_test_framework/testing_process_manager.h"20#include "mir_test_framework/testing_process_manager.h"
21#include "mir_test_framework/detect_server.h"21#include "mir_test_framework/detect_server.h"
22#include "mir_test_framework/using_stub_client_platform.h"
23#include "src/client/mir_connection.h"22#include "src/client/mir_connection.h"
24#include "mir/run_mir.h"23#include "mir/run_mir.h"
2524
@@ -107,7 +106,6 @@
107 SCOPED_TRACE("Client");106 SCOPED_TRACE("Client");
108 if (!config.use_real_graphics(test_options))107 if (!config.use_real_graphics(test_options))
109 {108 {
110 mtf::UsingStubClientPlatform p;
111 config.exec();109 config.exec();
112 }110 }
113 else111 else
114112
=== 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 @@
1/*
2 * Copyright © 2014 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
17 */
18
19#include "mir_test_framework/using_stub_client_platform.h"
20#include "mir_test_framework/stub_client_connection_configuration.h"
21#include "mir_toolkit/mir_client_library.h"
22#include "src/client/mir_connection_api.h"
23
24namespace mtf = mir_test_framework;
25namespace mcl = mir::client;
26
27namespace
28{
29
30void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
31{
32}
33
34class StubMirConnectionAPI : public mcl::MirConnectionAPI
35{
36public:
37 StubMirConnectionAPI(mcl::MirConnectionAPI* prev_api)
38 : prev_api{prev_api}
39 {
40 }
41
42 MirWaitHandle* connect(
43 char const* socket_file,
44 char const* name,
45 mir_connected_callback callback,
46 void* context) override
47 {
48 return prev_api->connect(socket_file, name, callback, context);
49 }
50
51 void release(MirConnection* connection) override
52 {
53 // Clear the lifecycle callback in order not to get SIGHUP by the
54 // default lifecycle handler during connection teardown
55 mir_connection_set_lifecycle_event_callback(connection, null_lifecycle_callback, nullptr);
56 return prev_api->release(connection);
57 }
58
59 std::unique_ptr<mcl::ConnectionConfiguration> configuration(std::string const& socket) override
60 {
61 return std::unique_ptr<mcl::ConnectionConfiguration>{
62 new mtf::StubConnectionConfiguration{socket}};
63 }
64
65private:
66 mcl::MirConnectionAPI* const prev_api;
67};
68
69}
70
71mtf::UsingStubClientPlatform::UsingStubClientPlatform()
72 : prev_api{mir_connection_api_impl},
73 stub_api{new StubMirConnectionAPI{prev_api}}
74{
75 mir_connection_api_impl = stub_api.get();
76}
77
78mtf::UsingStubClientPlatform::~UsingStubClientPlatform()
79{
80 mir_connection_api_impl = prev_api;
81}
820
=== 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 @@
12 ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_rpc_channel.cpp12 ${CMAKE_CURRENT_SOURCE_DIR}/test_protobuf_rpc_channel.cpp
13 ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_prompt_session.cpp13 ${CMAKE_CURRENT_SOURCE_DIR}/test_mir_prompt_session.cpp
14 ${CMAKE_CURRENT_SOURCE_DIR}/test_event_distributor.cpp14 ${CMAKE_CURRENT_SOURCE_DIR}/test_event_distributor.cpp
15 ${CMAKE_CURRENT_SOURCE_DIR}/test_lifecycle_control.cpp
15 ${CMAKE_CURRENT_SOURCE_DIR}/test_periodic_perf_report.cpp16 ${CMAKE_CURRENT_SOURCE_DIR}/test_periodic_perf_report.cpp
16)17)
1718
1819
=== 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 @@
392{392{
393}393}
394394
395void null_lifecycle_callback(MirConnection*, MirLifecycleState, void*)
396{
397}
398
399MATCHER_P(BufferPackageMatches, package, "")395MATCHER_P(BufferPackageMatches, package, "")
400{396{
401 // Can't simply use memcmp() on the whole struct because age is not sent over the wire397 // Can't simply use memcmp() on the whole struct because age is not sent over the wire
@@ -421,9 +417,8 @@
421417
422 ~MirClientSurfaceTest()418 ~MirClientSurfaceTest()
423 {419 {
424 // Clear the lifecycle callback in order not to get SIGHUP by the420 // Clean up nicely after ourselves...
425 // default lifecycle handler during connection teardown421 connection->disconnect();
426 connection->register_lifecycle_event_callback(null_lifecycle_callback, nullptr);
427 }422 }
428423
429 void start_test_server()424 void start_test_server()
430425
=== 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 @@
1/*
2 * Copyright © 2014 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License version 3 as
6 * published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
17 */
18
19#include "src/client/lifecycle_control.h"
20
21#include <gtest/gtest.h>
22
23namespace
24{
25bool a_called{false}, b_called{false};
26
27void handler_a(MirLifecycleState)
28{
29 a_called = true;
30}
31
32void handler_b(MirLifecycleState)
33{
34 b_called = true;
35}
36}
37
38TEST(LifeCycleControl, InvokesSetTarget)
39{
40 mir::client::LifecycleControl control;
41
42 a_called = false;
43 b_called = false;
44
45 control.set_lifecycle_event_handler(&handler_a);
46 control.call_lifecycle_event_handler(mir_lifecycle_connection_lost);
47
48 EXPECT_TRUE(a_called);
49 EXPECT_FALSE(b_called);
50}
51
52TEST(LifeCycleControl, InvokesUpdatedTarget)
53{
54 mir::client::LifecycleControl control;
55
56 a_called = false;
57 b_called = false;
58
59 control.set_lifecycle_event_handler(&handler_a);
60 control.set_lifecycle_event_handler(&handler_b);
61 control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
62
63 EXPECT_FALSE(a_called);
64 EXPECT_TRUE(b_called);
65}
66
67TEST(LifeCycleControl, UpdateReplacesIfMatches)
68{
69 mir::client::LifecycleControl control;
70
71 a_called = false;
72 b_called = false;
73
74 control.set_lifecycle_event_handler(&handler_a);
75 control.replace_lifecycle_event_handler_if_matches(&handler_a, &handler_b);
76 control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
77
78 EXPECT_FALSE(a_called);
79 EXPECT_TRUE(b_called);
80}
81
82TEST(LifeCycleControl, UpdateDoesNotReplaceIfNotMatching)
83{
84 mir::client::LifecycleControl control;
85
86 a_called = false;
87 b_called = false;
88
89 control.set_lifecycle_event_handler(&handler_a);
90 control.replace_lifecycle_event_handler_if_matches(&handler_b, &handler_b);
91 control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
92
93 EXPECT_TRUE(a_called);
94 EXPECT_FALSE(b_called);
95}
96
97TEST(LifeCycleControl, UpdateHandlesDifferingTypes)
98{
99 mir::client::LifecycleControl control;
100
101 a_called = false;
102 b_called = false;
103 auto non_fn_pointer_handler = [](MirLifecycleState){};
104
105 control.set_lifecycle_event_handler(non_fn_pointer_handler);
106 control.replace_lifecycle_event_handler_if_matches(&handler_a, &handler_b);
107 control.call_lifecycle_event_handler(mir_lifecycle_state_resumed);
108
109 EXPECT_FALSE(a_called);
110 EXPECT_FALSE(b_called);
111}

Subscribers

People subscribed via source and target branches