Merge lp:~mterry/unity-system-compositor/inactivity-zero-fix into lp:unity-system-compositor

[Michael Terry]

Fix screen turning off in middle of interacting with phone after receiving an SMS with an inactivity value of "never".

This branch cancels power-off and dimmer timeout alarms when resetting for a reason that has zero-timeout value. Historically, we could entirely ignore such resets, because when the zero-timeout value was set (in set_inactivity_timeouts), we cancelled timers. And just never re-enabled them. So the reset() call due to input activity didn't matter.

But now that we have two sets of timeout values [1], the alarms might be active when we try to reset a zero-timer for inactivity. We can't just ignore such an event, we now need to cancel the alarms.

[1] https://code.launchpad.net/~afrantzis/unity-system-compositor/notification-timeouts/+merge/264853

Fix screen turning off in middle of interacting with phone after receiving an SMS with an inactivity value of "never".

This branch cancels power-off and dimmer timeout alarms when resetting for a reason that has zero-timeout value. Historically, we could entirely ignore such resets, because when the zero-timeout value was set (in set_inactivity_timeouts), we cancelled timers. And just never re-enabled them. So the reset() call due to input activity didn't matter.

But now that we have two sets of timeout values [1], the alarms might be active when we try to reset a zero-timer for inactivity. We can't just ignore such an event, we now need to cancel the alarms.

I've assumed that a zero value for *any* timeout reason means "never". I don't think that's a super reasonable value for a notification timeout. But neither is zero milliseconds. So I left it as "never". But if you all don't like that, I can also add a check that reason==inactivity before cancelling the alarms.

I also have not added any unit tests, though this would be an excellent candidate for them. I'll do that as a future MP. I just wanted to get this MP in there soon to increase its chances of landing in OTA6.

[1] https://code.launchpad.net/~afrantzis/unity-system-compositor/notification-timeouts/+merge/264853

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:243
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/258/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-amd64-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/46
        deb: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/46/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-i386-ci/46

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/258/rebuild

review: Approve (continuous-integration)

[Michael Terry (mterry)]

Hrm... There might also be a bug in MirScreen around a notification happening while the screen is on. This would set a timeout of, say, 15s. And if the user didn't cause any activity in those 15s, the screen would turn off, which isn't correct...

This needs a quick additional think.

[Josh Arenson (josharenson)]

> Hrm... There might also be a bug in MirScreen around a notification happening
> while the screen is on. This would set a timeout of, say, 15s. And if the
> user didn't cause any activity in those 15s, the screen would turn off, which
> isn't correct...
>
> This needs a quick additional think.

I was able to reproduce the original bug, and this patch _does_ seems to fix it. However, the screen still does turn off after a period of no activity as you mentioned.

[Alexandros Frantzis (afrantzis)]

> Hrm... There might also be a bug in MirScreen around a notification happening
> while the screen is on. This would set a timeout of, say, 15s. And if the
> user didn't cause any activity in those 15s, the screen would turn off, which
> isn't correct...
>
> This needs a quick additional think.

I have proposed an alternative branch (which incorporates the change proposed in this branch), plus behavioral improvements for the scenario you describe, plus tests:

https://code.launchpad.net/~afrantzis/unity-system-compositor/fix-1485737-notification-with-inactivity-zero

Let me know how it works for you.

[Michael Terry (mterry)]

Rejecting in favor of alf's branch

review: Disapprove

Unmerged revisions

243. By Michael Terry on 2015-08-17

Cancel power timeout alarms when a timeout value is set to zero (never timeout)

=== modified file 'src/mir_screen.cpp'
--- src/mir_screen.cpp	2015-07-27 17:01:28 +0000
+++ src/mir_screen.cpp	2015-08-17 21:35:44 +0000
@@ -256,6 +256,9 @@
             power_off_alarm->reschedule_in(timeouts.power_off_timeout);
             next_power_off = new_next_power_off;
         }
+    } else {
+        power_off_alarm->cancel();
+        next_power_off = {};
     }
 
     if (timeouts.dimming_timeout.count() > 0)
@@ -266,6 +269,9 @@
             dimmer_alarm->reschedule_in(timeouts.dimming_timeout);
             next_dimming = new_next_dimming;
         }
+    } else {
+        dimmer_alarm->cancel();
+        next_dimming = {};
     }
 }