Merge lp:~mir-team/mir/reduce-event-pedantism-for-1437357 into lp:mir

[Robert Carr]

It seems it's not reasonable for us to throw an exception here. Android-input doesn't meet it's invariants (it promises it will deliver consistent touches with one touch going up and down at a time). However it seems the variations in the touch stream documented in 1437357 have always been present and a fix is unclear. Therefore removing this exception seems safe (behavior should revert to the previous behavior where we didn't observe any errors) and will reduce the crash.

It seems it's not reasonable for us to throw an exception here. Android-input doesn't meet it's invariants (it promises it will deliver consistent touches with one touch going up and down at a time). However it seems the variations in the touch stream documented in 1437357 have always been present and a fix is unclear. Therefore removing this exception seems safe (behavior should revert to the previous behavior where we didn't observe any errors) and will reduce the crash.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2574
http://jenkins.qa.ubuntu.com/job/mir-ci/3867/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2520
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2468
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/23
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/23/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2468
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2468/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5344
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20593

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

Seems reasonable. How spammy is this log message? I guess not very frequent, as otherwise clients on the relevant devices would die almost immediately?

review: Approve

[Alberto Aguirre (albaguirre)]

OK.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1458/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2535
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/45
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2483/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/13
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/13/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2483
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2483/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5356/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20614

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

OK. I assume our new input stack will not have this problem?

review: Approve

[Alan Griffiths (alan-griffiths)]

Where does the problem originate? In our "adopted" input code or outside our control?

I think "error" is an overreaction and "warning" would be better as it doesn't threaten functionality. (Especially if the cause is outside our control.)

review: Needs Information

[Robert Carr (robertcarr)]

>> Where does the problem originate? In our "adopted" input code or outside our control?
>>
>> I think "error" is an overreaction and "warning" would be better as it doesn't threaten
>> functionality. (Especially if the cause is outside our control.)

I'm not entirely sure yet. It seems the problem can both originate from a broken kernel event stream, or from bugs which cause the InputReader to malform what should have been a functioning input stream...thus I'm not inclined to say it's out of our control.

It actually does threaten functionality...if multiple up downs are encoded in one event the android action masking format will end up erasing some of the actions...I guess this would result in "quirky touch" depending on how valid an input stream the client requires. On the other hand this was apparently always happening and touch has been working pretty well though QtMir had to have a validator of course.

[Kevin DuBois (kdub)]

lgtm

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2575
http://jenkins.qa.ubuntu.com/job/mir-ci/3886/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2552
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/62
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2500
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/42
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/42/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2500
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2500/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5370
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20665

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

review: Approve (continuous-integration)

=== modified file 'src/client/events/event_builders.cpp'
--- src/client/events/event_builders.cpp	2015-05-19 14:02:31 +0000
+++ src/client/events/event_builders.cpp	2015-05-20 14:31:33 +0000
@@ -16,6 +16,9 @@
  * Author: Robert Carr <[e-mail]>
  */
 
+#define MIR_LOG_COMPONENT "event-builders"
+
+#include "mir/log.h"
 #include "mir/events/event_builders.h"
 #include "mir/events/event_private.h"
 
@@ -188,7 +191,7 @@
 
     if (mev.action != mir_motion_action_move && new_mask != mir_motion_action_move)
     {
-        BOOST_THROW_EXCEPTION(std::logic_error("Only one touch up/down may be reported per event"));
+        mir::log_error("Only one touch up/down should be reported per event");
     }
     if (new_mask == mir_motion_action_move)
         return;