Merge 203774 "chromeos: Add delay between screen off and lock."

> 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

TBR=derat@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/branches/1500/src@204010 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 48 insertions, 14 deletions

diff --git a/chromeos/dbus/power_policy_controller.cc b/chromeos/dbus/power_policy_controller.cc
index 3a6de30..ab28a28 100644
--- a/chromeos/dbus/power_policy_controller.cc
+++ b/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()) &&
diff --git a/chromeos/dbus/power_policy_controller.h b/chromeos/dbus/power_policy_controller.h
index 6aa3f71..ce5ed05 100644
--- a/chromeos/dbus/power_policy_controller.h
+++ b/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();
 
diff --git a/chromeos/dbus/power_policy_controller_unittest.cc b/chromeos/dbus/power_policy_controller_unittest.cc
index f09ad61..fe3601ef 100644
--- a/chromeos/dbus/power_policy_controller_unittest.cc
+++ b/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),