chromeos: Add delay between screen off and lock.

This adds a ten-second delay between the time at which the
screen is turned off due to user inactivity and the time at
which it's locked.

It also fixes an issue where screen wake locks (as used by
video playback or by the chrome.power extension API) wouldn't
clear lock delays.

BUG=239486,246008
R=bartfab@chromium.org

Review URL: https://codereview.chromium.org/15734010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203774 0039d316-1c4b-4281-b951-d872f2087c98

chromeos/dbus/power_policy_controller.cc

@@ -58,6 +58,8 @@ power_manager::PowerManagementPolicy_Action GetProtoAction(
 
 }  // namespace
 
+const int PowerPolicyController::kScreenLockAfterOffDelayMs = 10000;  // 10 sec.
+
 // -1 is interpreted as "unset" by powerd, resulting in powerd's default
 // delays being used instead.  There are no similarly-interpreted values
 // for the other fields, unfortunately (but the constructor-assigned values
@@ -147,13 +149,13 @@ void PowerPolicyController::ApplyPrefs(const PrefValues& values) {
   delays->set_idle_warning_ms(values.ac_idle_warning_delay_ms);
   delays->set_idle_ms(values.ac_idle_delay_ms);
 
-  // If screen-locking is enabled, ensure that the screen is locked when
-  // it's turned off due to user inactivity.
+  // If screen-locking is enabled, ensure that the screen is locked soon
+  // after it's turned off due to user inactivity.
+  int64 lock_ms = delays->screen_off_ms() + kScreenLockAfterOffDelayMs;
   if (values.enable_screen_lock && delays->screen_off_ms() > 0 &&
-      (delays->screen_lock_ms() <= 0 ||
-       delays->screen_off_ms() < delays->screen_lock_ms())) {
-    delays->set_screen_lock_ms(delays->screen_off_ms());
-  }
+      (delays->screen_lock_ms() <= 0 || lock_ms < delays->screen_lock_ms()) &&
+      lock_ms < delays->idle_ms())
+    delays->set_screen_lock_ms(lock_ms);
 
   delays = prefs_policy_.mutable_battery_delays();
   delays->set_screen_dim_ms(values.battery_screen_dim_delay_ms);
@@ -161,11 +163,12 @@ void PowerPolicyController::ApplyPrefs(const PrefValues& values) {
   delays->set_screen_lock_ms(values.battery_screen_lock_delay_ms);
   delays->set_idle_warning_ms(values.battery_idle_warning_delay_ms);
   delays->set_idle_ms(values.battery_idle_delay_ms);
+
+  lock_ms = delays->screen_off_ms() + kScreenLockAfterOffDelayMs;
   if (values.enable_screen_lock && delays->screen_off_ms() > 0 &&
-      (delays->screen_lock_ms() <= 0 ||
-       delays->screen_off_ms() < delays->screen_lock_ms())) {
-    delays->set_screen_lock_ms(delays->screen_off_ms());
-  }
+      (delays->screen_lock_ms() <= 0 || lock_ms < delays->screen_lock_ms()) &&
+      lock_ms < delays->idle_ms())
+    delays->set_screen_lock_ms(lock_ms);
 
   prefs_policy_.set_idle_action(GetProtoAction(values.idle_action));
   prefs_policy_.set_lid_closed_action(GetProtoAction(values.lid_closed_action));
@@ -223,8 +226,10 @@ void PowerPolicyController::SendCurrentPolicy() {
   if (honor_screen_wake_locks_ && !screen_wake_locks_.empty()) {
     policy.mutable_ac_delays()->set_screen_dim_ms(0);
     policy.mutable_ac_delays()->set_screen_off_ms(0);
+    policy.mutable_ac_delays()->set_screen_lock_ms(0);
     policy.mutable_battery_delays()->set_screen_dim_ms(0);
     policy.mutable_battery_delays()->set_screen_off_ms(0);
+    policy.mutable_battery_delays()->set_screen_lock_ms(0);
   }
 
   if ((!screen_wake_locks_.empty() || !system_wake_locks_.empty()) &&

chromeos/dbus/power_policy_controller.h

@@ -61,6 +61,12 @@ class CHROMEOS_EXPORT PowerPolicyController
   static std::string GetPolicyDebugString(
       const power_manager::PowerManagementPolicy& policy);
 
+  // Delay in milliseconds between the screen being turned off and the
+  // screen being locked. Used if the |enable_screen_lock| pref is set but
+  // |*_screen_lock_delay_ms| are unset or set to higher values than what
+  // this constant would imply.
+  const static int kScreenLockAfterOffDelayMs;
+
   PowerPolicyController(DBusThreadManager* manager, PowerManagerClient* client);
   virtual ~PowerPolicyController();
 

chromeos/dbus/power_policy_controller_unittest.cc

@@ -95,17 +95,20 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
                 fake_power_client_.get_policy()));
 
   // The enable-screen-lock pref should force the screen-lock delays to
-  // match the screen-off delays.
+  // match the screen-off delays plus a constant value.
   prefs.enable_screen_lock = true;
   policy_controller_->ApplyPrefs(prefs);
-  expected_policy.mutable_ac_delays()->set_screen_lock_ms(660000);
-  expected_policy.mutable_battery_delays()->set_screen_lock_ms(360000);
+  expected_policy.mutable_ac_delays()->set_screen_lock_ms(
+      660000 + PowerPolicyController::kScreenLockAfterOffDelayMs);
+  expected_policy.mutable_battery_delays()->set_screen_lock_ms(
+      360000 + PowerPolicyController::kScreenLockAfterOffDelayMs);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
                 fake_power_client_.get_policy()));
 
   // If the screen-lock-delay prefs are set to lower values than the
-  // screen-off delays, the lock prefs should take precedence.
+  // screen-off delays plus the constant, the lock prefs should take
+  // precedence.
   prefs.ac_screen_lock_delay_ms = 70000;
   prefs.battery_screen_lock_delay_ms = 60000;
   policy_controller_->ApplyPrefs(prefs);
@@ -115,6 +118,24 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
             PowerPolicyController::GetPolicyDebugString(
                 fake_power_client_.get_policy()));
 
+  // If the artificial screen-lock delays would exceed the idle delay, they
+  // shouldn't be set -- the power manager would ignore them since the
+  // idle action should lock the screen in this case.
+  prefs.ac_screen_off_delay_ms = prefs.ac_idle_delay_ms - 1;
+  prefs.battery_screen_off_delay_ms = prefs.battery_idle_delay_ms - 1;
+  prefs.ac_screen_lock_delay_ms = -1;
+  prefs.battery_screen_lock_delay_ms = -1;
+  policy_controller_->ApplyPrefs(prefs);
+  expected_policy.mutable_ac_delays()->set_screen_off_ms(
+      prefs.ac_screen_off_delay_ms);
+  expected_policy.mutable_battery_delays()->set_screen_off_ms(
+      prefs.battery_screen_off_delay_ms);
+  expected_policy.mutable_ac_delays()->set_screen_lock_ms(-1);
+  expected_policy.mutable_battery_delays()->set_screen_lock_ms(-1);
+  EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
+            PowerPolicyController::GetPolicyDebugString(
+                fake_power_client_.get_policy()));
+
   // Set the "allow screen wake locks" pref to false.  The system should be
   // prevented from suspending due to user inactivity but the pref-supplied
   // screen-related delays should be left untouched.
@@ -146,8 +167,10 @@ TEST_F(PowerPolicyControllerTest, WakeLocks) {
       kScreenWakeLockReason);
   expected_policy.mutable_ac_delays()->set_screen_dim_ms(0);
   expected_policy.mutable_ac_delays()->set_screen_off_ms(0);
+  expected_policy.mutable_ac_delays()->set_screen_lock_ms(0);
   expected_policy.mutable_battery_delays()->set_screen_dim_ms(0);
   expected_policy.mutable_battery_delays()->set_screen_off_ms(0);
+  expected_policy.mutable_battery_delays()->set_screen_lock_ms(0);
   expected_policy.set_reason(
       std::string(kScreenWakeLockReason) + ", " + kSystemWakeLockReason);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),