Merge lp:~afrantzis/unity-system-compositor/fix-1461476-display-off into lp:unity-system-compositor

[Alexandros Frantzis]

Don't restart the inactivity timers when receiving invalid dbus calls or uninteresting dbus events

Previously, invalid dbus calls (e.g., trying to remove a non-existent keepDisplayOn request), or uninteresting dbus events (e.g., being informed about disconnections of dbus clients that hadn't issued any keepDisplayOn requests), would restart the inactivity timers, effectively increasing the delay until display dimming and poweroff.

Don't restart the inactivity timers when receiving invalid dbus calls or uninteresting dbus events

Previously, invalid dbus calls (e.g., trying to remove a non-existent keepDisplayOn request), or uninteresting dbus events (e.g., being informed about disconnections of dbus clients that hadn't issued any keepDisplayOn requests), would restart the inactivity timers, effectively increasing the delay until display dimming and poweroff.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

LGTM

review: Approve

[Andreas Pokorny (andreas-pokorny)]

looks good.

review: Approve

=== modified file 'src/unity_screen_service.cpp'
--- src/unity_screen_service.cpp	2015-03-18 13:06:10 +0000
+++ src/unity_screen_service.cpp	2015-07-08 11:34:49 +0000
@@ -305,6 +305,8 @@
 {
     std::lock_guard<std::mutex> lock{keep_display_on_mutex};
 
+    bool id_removed{false};
+
     auto range = keep_display_on_ids.equal_range(sender);
     for (auto iter = range.first;
          iter != range.second;
@@ -313,11 +315,12 @@
         if (iter->second == id)
         {
             keep_display_on_ids.erase(iter);
+            id_removed = true;
             break;
         }
     }
 
-    if (keep_display_on_ids.empty())
+    if (id_removed && keep_display_on_ids.empty())
         screen->keep_display_on(false);
 }
 
@@ -329,9 +332,14 @@
     if (new_owner.empty() && old_owner == name)
     {
         std::lock_guard<std::mutex> lock{keep_display_on_mutex};
-        keep_display_on_ids.erase(name);
-        if (keep_display_on_ids.empty())
+        // If the disconnected client had issued keepDisplayOn requests
+        // and after removing them there are now no more requests left,
+        // tell the screen we don't need to keep the display on.
+        if (keep_display_on_ids.erase(name) > 0 &&
+            keep_display_on_ids.empty())
+        {
             screen->keep_display_on(false);
+        }
     }
 }
 

=== modified file 'tests/integration-tests/test_unity_screen_service.cpp'
--- tests/integration-tests/test_unity_screen_service.cpp	2015-04-29 14:30:49 +0000
+++ tests/integration-tests/test_unity_screen_service.cpp	2015-07-08 11:34:49 +0000
@@ -365,6 +365,32 @@
     EXPECT_TRUE(request_processed.woken());
 }
 
+TEST_F(AUnityScreenService, ignores_invalid_display_on_removal_request)
+{
+    ut::WaitCondition request_processed;
+
+    int32_t const invalid_id{-1};
+    EXPECT_CALL(*mock_screen, keep_display_on(false)).Times(0);
+
+    client.request_remove_display_on_request(invalid_id);
+    client.disconnect();
+
+    // Allow some time for dbus calls to reach UnityScreenService
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+}
+
+TEST_F(AUnityScreenService, ignores_disconnects_from_clients_without_display_on_request)
+{
+    ut::WaitCondition request_processed;
+
+    EXPECT_CALL(*mock_screen, keep_display_on(false)).Times(0);
+
+    client.disconnect();
+
+    // Allow some time for disconnect notification to reach UnityScreenService
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+}
+
 TEST_F(AUnityScreenService, emits_power_state_change_signal)
 {
     using namespace testing;