Merge lp:~mir-team/mir/fix-new-input-dispatcher-and-hang-head-in-shame into lp:mir

[Robert Carr]

Fix key repeat dispatcher (touch events not working at all)
(LP: #1464174)

Correct: https://bugs.launchpad.net/mir/+bug/1464174

The error is obvious if you look at the key_repeat_dispatcher.cpp diff...it just wasn't forwarding touch events to the next dispatcher! I really did test this at some point but I guess I reverted to an earlier version of the code when rebasing after --overwrite branch catastrophe in Dallas.

It wasn't caught in the tests because the KeyRepeatDispatcher wasn't installed (due to key repeat being disabled to not make the tests timing dependent). I've changed the way key repeat is configured in a way which would have caught this bug. I think the value of this could be argued about though (as the way repeat_enabled is implemented makes it unlikely this would catch future bugs).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2655
http://jenkins.qa.ubuntu.com/job/mir-ci/4084/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2824
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/337
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2772
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/240
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/240/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2772
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2772/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5606
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21150

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

LGTM.

Nit:

69 + bool repeat_enabled,

Not aligned

review: Approve

[Daniel van Vugt (vanvugt)]

Yeah wrong indentation in a couple of places.

But more importantly, tested on krillin and bug fixed.

review: Approve

=== modified file 'src/server/input/default_configuration.cpp'
--- src/server/input/default_configuration.cpp	2015-06-08 17:22:32 +0000
+++ src/server/input/default_configuration.cpp	2015-06-11 18:30:00 +0000
@@ -223,10 +223,11 @@
     std::chrono::milliseconds const key_repeat_timeout{20};
 
     auto const options = the_options();
-    if (!options->get<bool>(options::enable_key_repeat_opt))
-        return the_event_filter_chain_dispatcher();
-    else
-        return std::make_shared<mi::KeyRepeatDispatcher>(the_event_filter_chain_dispatcher(), the_main_loop(), key_repeat_timeout);
+    auto enable_repeat = options->get<bool>(options::enable_key_repeat_opt);
+
+    return std::make_shared<mi::KeyRepeatDispatcher>(
+        the_event_filter_chain_dispatcher(), the_main_loop(), enable_repeat,
+	key_repeat_timeout);
 }
 
 std::shared_ptr<droidinput::EventHubInterface>

=== modified file 'src/server/input/key_repeat_dispatcher.cpp'
--- src/server/input/key_repeat_dispatcher.cpp	2015-06-08 20:18:00 +0000
+++ src/server/input/key_repeat_dispatcher.cpp	2015-06-11 18:30:00 +0000
@@ -32,9 +32,11 @@
 mi::KeyRepeatDispatcher::KeyRepeatDispatcher(
     std::shared_ptr<mi::InputDispatcher> const& next_dispatcher,
     std::shared_ptr<mir::time::AlarmFactory> const& factory,
+    bool repeat_enabled,
     std::chrono::milliseconds repeat_timeout)
     : next_dispatcher(next_dispatcher),
       alarm_factory(factory),
+      repeat_enabled(repeat_enabled),
       repeat_timeout(repeat_timeout)
 {
 }
@@ -47,17 +49,22 @@
 
 bool mi::KeyRepeatDispatcher::dispatch(MirEvent const& event)
 {
+    if (!repeat_enabled) // if we made this mutable we'd need a guard
+    {
+	return next_dispatcher->dispatch(event);
+    }
+    
     if (mir_event_get_type(&event) == mir_event_type_input)
     {
         auto iev = mir_event_get_input_event(&event);
         if (mir_input_event_get_type(iev) != mir_input_event_type_key)
-            return false;
+            return next_dispatcher->dispatch(event);
         if (!handle_key_input(mir_input_event_get_device_id(iev), mir_input_event_get_keyboard_event(iev)))
             return next_dispatcher->dispatch(event);
         else
             return true;
     }
-    return false;
+    return next_dispatcher->dispatch(event);
 }
 
 namespace

=== modified file 'src/server/input/key_repeat_dispatcher.h'
--- src/server/input/key_repeat_dispatcher.h	2015-06-08 17:28:38 +0000
+++ src/server/input/key_repeat_dispatcher.h	2015-06-11 18:30:00 +0000
@@ -41,6 +41,7 @@
 public:
     KeyRepeatDispatcher(std::shared_ptr<InputDispatcher> const& next_dispatcher,
                         std::shared_ptr<time::AlarmFactory> const& factory,
+			bool repeat_enabled,
                         std::chrono::milliseconds repeat_timeout);
 
     // InputDispatcher
@@ -53,6 +54,7 @@
 
     std::shared_ptr<InputDispatcher> const next_dispatcher;
     std::shared_ptr<time::AlarmFactory> const alarm_factory;
+    bool const repeat_enabled;
     std::chrono::milliseconds repeat_timeout;
 
     struct KeyboardState

=== modified file 'tests/unit-tests/input/test_key_repeat_dispatcher.cpp'
--- tests/unit-tests/input/test_key_repeat_dispatcher.cpp	2015-06-03 16:34:34 +0000
+++ tests/unit-tests/input/test_key_repeat_dispatcher.cpp	2015-06-11 18:30:00 +0000
@@ -61,7 +61,7 @@
 struct KeyRepeatDispatcher : public testing::Test
 {
     KeyRepeatDispatcher()
-        : dispatcher(mock_next_dispatcher, mock_alarm_factory, repeat_time)
+        : dispatcher(mock_next_dispatcher, mock_alarm_factory, true, repeat_time)
     {
     }
     std::shared_ptr<mtd::MockInputDispatcher> mock_next_dispatcher = std::make_shared<mtd::MockInputDispatcher>();