Merge lp:~mir-team/mir/fix-1438160 into lp:mir

[Chris Halse Rogers]

Don't close() the surface input fd in ~InputDispatcher()

Fixes: https://bugs.launchpad.net/mir/+bug/1438160

Don't close() the surface input fd in ~InputDispatcher().

IntOwnedFd is one of the downsides of us using std::shared_ptr to do mir::Fd. I might at some point see how things look if mir::Fd copies via dup() instead...

[Daniel van Vugt (vanvugt)]

Works for me (manually tested too).

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2448
http://jenkins.qa.ubuntu.com/job/mir-ci/3419/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1906
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1905
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1856
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1416/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1856
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1856/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4808
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19326

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

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

lgtm. shared_ptr<> seemed better than dup when mir::Fd came around, mostly for the sake of less communication with the kernel.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK as a fix. But ideally a test actually passes or fails (not passes or crashes).

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1291/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1914
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1913/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1864
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-autolanding/471/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1864
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1864/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4815
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19353

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2449
http://jenkins.qa.ubuntu.com/job/mir-ci/3433/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1924
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1923
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1874
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1430
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1430/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1874
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1874/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4823
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19370

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

review: Approve (continuous-integration)

=== modified file 'src/client/input/android/android_input_receiver.cpp'
--- src/client/input/android/android_input_receiver.cpp	2015-03-31 02:35:42 +0000
+++ src/client/input/android/android_input_receiver.cpp	2015-04-01 23:12:24 +0000
@@ -91,7 +91,7 @@
         process_and_maybe_send_event();
     });
 
-    dispatcher.add_watch(mir::Fd{input_channel->getFd()},
+    dispatcher.add_watch(mir::Fd{mir::IntOwnedFd{input_channel->getFd()}},
                          [this]() { process_and_maybe_send_event(); });
 }
 

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2015-03-31 02:35:42 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2015-04-01 23:12:24 +0000
@@ -837,3 +837,43 @@
     mir_surface_release_sync(surface);
     mir_connection_release(connection);
 }
+
+namespace
+{
+void dummy_event_handler_one(MirSurface*, MirEvent const*, void*)
+{
+}
+
+void dummy_event_handler_two(MirSurface*, MirEvent const*, void*)
+{
+}
+}
+
+/*
+ * Regression test for LP: 1438160
+ */
+TEST_F(ClientLibrary, can_change_event_delegate)
+{
+    using namespace testing;
+
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+
+    auto surface_spec = mir_connection_create_spec_for_normal_surface(connection,
+                                                                      800, 600,
+                                                                      mir_pixel_format_argb_8888);
+    auto surface = mir_surface_create_sync(surface_spec);
+    mir_surface_spec_release(surface_spec);
+
+    ASSERT_THAT(surface, IsValid());
+
+    /* TODO: When provide-event-fd lands, change this into a better test that actually
+     * tests that the correct event handler is called.
+     *
+     * Without manual dispatch, it's racy to try and test that.
+     */
+    mir_surface_set_event_handler(surface, &dummy_event_handler_one, nullptr);
+    mir_surface_set_event_handler(surface, &dummy_event_handler_two, nullptr);
+
+    mir_surface_release_sync(surface);
+    mir_connection_release(connection);
+}