power: Allow user to override screen brightness policy.

Allow user to change screen brightness defined by screen brightness policy.
User defined brightness value will be preserved till the end of the user session,
even if policy gets updated. The policy brightness values will not be sent to
power manager in that situation.

Bug: 890068, 310429
Test: chromeos_unittests and manually with local policy server
Change-Id: Ia048b4e328d739347cff7b49a0128db7869165a3
Reviewed-on: https://chromium-review.googlesource.com/c/1285730
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600852}

chromeos/dbus/power_policy_controller.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/chromeos_features.h"
+#include "chromeos/dbus/power_manager/backlight.pb.h"
 
 // Avoid some ugly line-wrapping later.
 using base::StringAppendF;
@@ -234,11 +235,14 @@ void PowerPolicyController::ApplyPrefs(const PrefValues& values) {
   prefs_policy_.set_lid_closed_action(GetProtoAction(values.lid_closed_action));
   prefs_policy_.set_use_audio_activity(values.use_audio_activity);
   prefs_policy_.set_use_video_activity(values.use_video_activity);
-  if (values.ac_brightness_percent >= 0.0)
-    prefs_policy_.set_ac_brightness_percent(values.ac_brightness_percent);
-  if (values.battery_brightness_percent >= 0.0) {
-    prefs_policy_.set_battery_brightness_percent(
-        values.battery_brightness_percent);
+
+  if (!per_session_brightness_override_) {
+    if (values.ac_brightness_percent >= 0.0)
+      prefs_policy_.set_ac_brightness_percent(values.ac_brightness_percent);
+    if (values.battery_brightness_percent >= 0.0) {
+      prefs_policy_.set_battery_brightness_percent(
+          values.battery_brightness_percent);
+    }
   }
 
   // Screen-dim deferral in response to user activity predictions can
@@ -295,6 +299,17 @@ void PowerPolicyController::PowerManagerRestarted() {
   SendCurrentPolicy();
 }
 
+void PowerPolicyController::ScreenBrightnessChanged(
+    const power_manager::BacklightBrightnessChange& change) {
+  if (prefs_were_set_ &&
+      (prefs_policy_.has_ac_brightness_percent() ||
+       prefs_policy_.has_battery_brightness_percent()) &&
+      change.cause() ==
+          power_manager::BacklightBrightnessChange_Cause_USER_REQUEST) {
+    per_session_brightness_override_ = true;
+  }
+}
+
 void PowerPolicyController::NotifyChromeIsExiting() {
   if (chrome_is_exiting_)
     return;
@@ -311,13 +326,7 @@ void PowerPolicyController::SetEncryptionMigrationActive(bool active) {
 }
 
 PowerPolicyController::PowerPolicyController(PowerManagerClient* client)
-    : client_(client),
-      prefs_were_set_(false),
-      honor_wake_locks_(true),
-      honor_screen_wake_locks_(true),
-      next_wake_lock_id_(1),
-      chrome_is_exiting_(false),
-      encryption_migration_active_(false) {
+    : client_(client) {
   DCHECK(client_);
   client_->AddObserver(this);
 }

chromeos/dbus/power_policy_controller.h

@@ -129,6 +129,8 @@ class CHROMEOS_EXPORT PowerPolicyController
 
   // PowerManagerClient::Observer implementation:
   void PowerManagerRestarted() override;
+  void ScreenBrightnessChanged(
+      const power_manager::BacklightBrightnessChange& change) override;
 
  private:
   explicit PowerPolicyController(PowerManagerClient* client);
@@ -172,7 +174,7 @@ class CHROMEOS_EXPORT PowerPolicyController
   power_manager::PowerManagementPolicy prefs_policy_;
 
   // Was ApplyPrefs() called?
-  bool prefs_were_set_;
+  bool prefs_were_set_ = false;
 
   // Maps from an ID representing a request to prevent the screen from
   // getting dimmed or turned off or to prevent the system from suspending
@@ -180,21 +182,25 @@ class CHROMEOS_EXPORT PowerPolicyController
   WakeLockMap wake_locks_;
 
   // Should |wake_locks_| be honored?
-  bool honor_wake_locks_;
+  bool honor_wake_locks_ = true;
 
   // If wake locks are honored, should TYPE_SCREEN or TYPE_DIM entries in
   // |wake_locks_| be honored?
   // If false, screen wake locks are just treated as TYPE_SYSTEM instead.
-  bool honor_screen_wake_locks_;
+  bool honor_screen_wake_locks_ = true;
 
   // Next ID to be used by an Add*WakeLock() request.
-  int next_wake_lock_id_;
+  int next_wake_lock_id_ = 1;
 
   // True if Chrome is in the process of exiting.
-  bool chrome_is_exiting_;
+  bool chrome_is_exiting_ = false;
 
   // True if a user homedir is in the process of migrating encryption formats.
-  bool encryption_migration_active_;
+  bool encryption_migration_active_ = false;
+
+  // Whether brightness policy value was overridden by a user adjustment in the
+  // current user session.
+  bool per_session_brightness_override_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(PowerPolicyController);
 };

chromeos/dbus/power_policy_controller_unittest.cc

@@ -7,9 +7,11 @@
 #include <memory>
 
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/chromeos_features.h"
 #include "chromeos/dbus/fake_power_manager_client.h"
+#include "chromeos/dbus/power_manager/backlight.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -375,4 +377,51 @@ TEST_F(PowerPolicyControllerTest, SmartDimEnabledExperimentEnabled) {
                 fake_power_client_->policy()));
 }
 
+TEST_F(PowerPolicyControllerTest, PerSessionScreenBrightnessOverride) {
+  const double kAcBrightness = 99.0;
+  const double kBatteryBrightness = 77.0;
+
+  PowerPolicyController::PrefValues prefs;
+  prefs.ac_brightness_percent = kAcBrightness;
+  prefs.battery_brightness_percent = kBatteryBrightness;
+  policy_controller_->ApplyPrefs(prefs);
+
+  EXPECT_EQ(kAcBrightness,
+            fake_power_client_->policy().ac_brightness_percent());
+  EXPECT_EQ(kBatteryBrightness,
+            fake_power_client_->policy().battery_brightness_percent());
+
+  // Simulate model triggered brightness change - shouldn't override the policy.
+  power_manager::SetBacklightBrightnessRequest request;
+  request.set_percent(80.0);
+  request.set_cause(power_manager::SetBacklightBrightnessRequest_Cause_MODEL);
+  fake_power_client_->SetScreenBrightness(request);
+  base::RunLoop().RunUntilIdle();
+  policy_controller_->ApplyPrefs(prefs);
+
+  EXPECT_EQ(kAcBrightness,
+            fake_power_client_->policy().ac_brightness_percent());
+  EXPECT_EQ(kBatteryBrightness,
+            fake_power_client_->policy().battery_brightness_percent());
+
+  // Simulate user triggered brightness change - should override the policy.
+  request.set_percent(80.0);
+  request.set_cause(
+      power_manager::SetBacklightBrightnessRequest_Cause_USER_REQUEST);
+  fake_power_client_->SetScreenBrightness(request);
+  base::RunLoop().RunUntilIdle();
+  policy_controller_->ApplyPrefs(prefs);
+
+  EXPECT_FALSE(fake_power_client_->policy().has_ac_brightness_percent());
+  EXPECT_FALSE(fake_power_client_->policy().has_battery_brightness_percent());
+
+  // Simulate policy values update that should be ignored.
+  prefs.ac_brightness_percent = 98.0;
+  prefs.battery_brightness_percent = 76.0;
+  policy_controller_->ApplyPrefs(prefs);
+
+  EXPECT_FALSE(fake_power_client_->policy().has_ac_brightness_percent());
+  EXPECT_FALSE(fake_power_client_->policy().has_battery_brightness_percent());
+}
+
 }  // namespace chromeos