Merge lp:~afrantzis/unity-system-compositor/fix-1485737-notification-with-inactivity-zero into lp:unity-system-compositor

[Alexandros Frantzis]

Cancel active timeouts and ignore notification timeouts when inactivity timeouts are zero (LP: #1485737)

Cancel active timeouts and ignore notification timeouts when inactivity timeouts are zero (LP: #1485737)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Michael Terry (mterry)]

Thanks for taking this branch over for me while I was EOD! :) Efficient.

Instead of directly comparing timeouts_inactivity (and making adding more timeout types even more complicated), might this be more generic if in the new "else" case we did:

else
{
   power_off_alarm->cancel();
   next_power_off = mir::time::Timestamp::max;
}

Instead of {}. This way, when a notification happens, the "new_next_power_off > next_power_off" check won't override the inactivity "never" timeout. But inactivity will still be turned off when the screen is manually turned off (as we do a next_power_off = {} when cancelling timers).

And of course the same for the dimmer logic.

Then we can drop all the extra if logic in this branch, I think?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[lp:~afrantzis/unity-system-compositor/fix-1485737-notification-with-inactivity-zero]

247. By Alexandros Frantzis on 2015-08-18

Simplify code based on mterry's suggestion

[Alexandros Frantzis (afrantzis)]

> Then we can drop all the extra if logic in this branch, I think?

Good idea. Applied.

[Michael Terry (mterry)]

LGTM, but I'm not a Mir guy.

review: Approve

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

=== modified file 'src/mir_screen.cpp'
--- src/mir_screen.cpp	2015-07-27 17:01:28 +0000
+++ src/mir_screen.cpp	2015-08-18 13:15:46 +0000
@@ -257,6 +257,11 @@
             next_power_off = new_next_power_off;
         }
     }
+    else
+    {
+        power_off_alarm->cancel();
+        next_power_off = mir::time::Timestamp::max();
+    }
 
     if (timeouts.dimming_timeout.count() > 0)
     {
@@ -267,6 +272,11 @@
             next_dimming = new_next_dimming;
         }
     }
+    else
+    {
+        dimmer_alarm->cancel();
+        next_dimming = mir::time::Timestamp::max();
+    }
 }
 
 void usc::MirScreen::enable_inactivity_timers_l(bool enable)

=== modified file 'tests/unit-tests/test_mir_screen.cpp'
--- tests/unit-tests/test_mir_screen.cpp	2015-07-27 17:01:28 +0000
+++ tests/unit-tests/test_mir_screen.cpp	2015-08-18 13:15:46 +0000
@@ -730,3 +730,52 @@
         MirPowerMode::mir_power_mode_on,
         PowerStateChangeReason::proximity);
 }
+
+TEST_F(AMirScreen, disables_active_timeout_when_setting_zero_inactivity_timeouts)
+{
+    std::chrono::hours const ten_hours{10};
+
+    expect_no_reconfiguration();
+    mir_screen.set_inactivity_timeouts(0, 0);
+    timer->advance_by(ten_hours);
+    verify_and_clear_expectations();
+}
+
+TEST_F(AMirScreen, notification_timeout_is_ignored_if_inactivity_timeouts_are_zero)
+{
+    std::chrono::hours const ten_hours{10};
+
+    expect_no_reconfiguration();
+    mir_screen.set_inactivity_timeouts(0, 0);
+    receive_notification();
+    timer->advance_by(ten_hours);
+    verify_and_clear_expectations();
+}
+
+TEST_F(AMirScreen, notification_timeout_is_respected_when_screen_is_off_if_inactivity_timeouts_are_zero)
+{
+    mir_screen.set_inactivity_timeouts(0, 0);
+    turn_screen_off();
+    receive_notification();
+    verify_and_clear_expectations();
+
+    expect_screen_is_turned_off();
+    timer->advance_by(notification_power_off_timeout);
+    verify_and_clear_expectations();
+}
+
+TEST_F(AMirScreen, keep_display_on_temporarily_after_notification_keeps_display_on_forever_if_inactivity_timeouts_are_zero)
+{
+    std::chrono::hours const ten_hours{10};
+
+    mir_screen.set_inactivity_timeouts(0, 0);
+    turn_screen_off();
+    
+    receive_notification();
+    verify_and_clear_expectations();
+
+    expect_no_reconfiguration();
+    mir_screen.keep_display_on_temporarily();
+    timer->advance_by(ten_hours);
+    verify_and_clear_expectations();
+}