[On-device adaptive brightness] Only start sys for signed-in users.

Controller will only create the adaptive brightness system after a user
session has started. The model will be personalized to the primary user
profile, ie. the first signed-in user.

Bug: 881215
Change-Id: Ib54fb5c8a05c2a47eec4ca1035d72e24e8183921
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984794Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jia Meng <jiameng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728305}

chrome/browser/chromeos/power/auto_screen_brightness/adapter.cc

@@ -553,13 +553,15 @@ Adapter::AdapterDecision Adapter::CanAdjustBrightness(base::TimeTicks now) {
 
   // Do not change brightness if it's set by the policy, but do not completely
   // disable the model as the policy could change.
-  if (profile_->GetPrefs()->GetInteger(
-          ash::prefs::kPowerAcScreenBrightnessPercent) >= 0 ||
-      profile_->GetPrefs()->GetInteger(
-          ash::prefs::kPowerBatteryScreenBrightnessPercent) >= 0) {
-    decision.no_brightness_change_cause =
-        NoBrightnessChangeCause::kBrightnessSetByPolicy;
-    return decision;
+  auto* prefs = profile_->GetPrefs();
+  if (prefs) {
+    if (prefs->GetInteger(ash::prefs::kPowerAcScreenBrightnessPercent) >= 0 ||
+        prefs->GetInteger(ash::prefs::kPowerBatteryScreenBrightnessPercent) >=
+            0) {
+      decision.no_brightness_change_cause =
+          NoBrightnessChangeCause::kBrightnessSetByPolicy;
+      return decision;
+    }
   }
 
   if (!new_model_arrived_) {

chrome/browser/chromeos/power/auto_screen_brightness/controller.cc

@@ -16,12 +16,54 @@
 #include "chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "components/session_manager/core/session_manager.h"
 
 namespace chromeos {
 namespace power {
 namespace auto_screen_brightness {
 
 Controller::Controller() {
+  auto* session_manager = session_manager::SessionManager::Get();
+  DCHECK(session_manager);
+
+  if (!session_manager->sessions().empty()) {
+    // If a user session has been created, we can use the primary user profile
+    // to initialize all components immediately without waiting for
+    // |OnUserSessionStarted| to be called.
+    InitializeComponents();
+    return;
+  }
+
+  // Wait for a user session to be created.
+  session_manager->AddObserver(this);
+  observing_session_manager_ = true;
+}
+
+Controller::~Controller() {
+  if (observing_session_manager_) {
+    auto* session_manager = session_manager::SessionManager::Get();
+    if (session_manager)
+      session_manager->RemoveObserver(this);
+  }
+}
+
+void Controller::OnUserSessionStarted(bool /* is_primary_user */) {
+  // The first sign-in user is the primary user, hence if |OnUserSessionStarted|
+  // is called, the primary user profile should have been created. We will
+  // ignore |is_primary_user|.
+  if (is_initialized_)
+    return;
+
+  InitializeComponents();
+}
+
+void Controller::InitializeComponents() {
+  DCHECK(!is_initialized_);
+  is_initialized_ = true;
+
+  Profile* const profile = ProfileManager::GetPrimaryUserProfile();
+  DCHECK(profile);
+
   chromeos::PowerManagerClient* power_manager_client =
       chromeos::PowerManagerClient::Get();
   DCHECK(power_manager_client);
@@ -41,8 +83,6 @@ Controller::Controller() {
       ui::UserActivityDetector::Get();
   DCHECK(user_activity_detector);
 
-  Profile* const profile = ProfileManager::GetPrimaryUserProfile();
-  DCHECK(profile);
   modeller_ = std::make_unique<ModellerImpl>(
       profile, als_reader_.get(), brightness_monitor_.get(),
       model_config_loader_.get(), user_activity_detector,
@@ -54,8 +94,6 @@ Controller::Controller() {
   adapter_->Init();
 }
 
-Controller::~Controller() = default;
-
 }  // namespace auto_screen_brightness
 }  // namespace power
 }  // namespace chromeos

chrome/browser/chromeos/power/auto_screen_brightness/controller.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
+#include "components/session_manager/core/session_manager_observer.h"
 
 namespace chromeos {
 namespace power {
@@ -24,12 +25,28 @@ class ModellerImpl;
 
 // This controller class sets up and destroys all components needed for the auto
 // screen brightness feature.
-class Controller {
+class Controller : public session_manager::SessionManagerObserver {
  public:
   Controller();
-  ~Controller();
+  ~Controller() override;
+
+  // session_manager::SessionManagerObserver overrides:
+  void OnUserSessionStarted(bool is_primary_user) override;
 
  private:
+  // Initializes all components of the adaptive brightness system.
+  // Controller will initialize the components once only, when the first user
+  // signs in. By definition, the first sign-in user is the primary user, the
+  // model will be personalized to this primary user.
+  void InitializeComponents();
+
+  // Whether all components of the adaptive brightness system are initialized.
+  bool is_initialized_ = false;
+
+  // Whether Controller is waiting for session manager's notification about
+  // user sign-ins.
+  bool observing_session_manager_ = false;
+
   std::unique_ptr<MetricsReporter> metrics_reporter_;
   std::unique_ptr<AlsReaderImpl> als_reader_;
   std::unique_ptr<BrightnessMonitorImpl> brightness_monitor_;

chrome/browser/chromeos/power/auto_screen_brightness/model_config_loader_impl.cc

@@ -27,14 +27,15 @@ namespace auto_screen_brightness {
 
 namespace {
 
-// Reads string content from |model_params_path|, which should exist.
+// Reads string content from |model_params_path| if it exists.
 // This should run in another thread to be non-blocking to the main thread (if
 // |is_testing| is false).
 std::string LoadModelParamsFromDisk(const base::FilePath& model_params_path,
                                     bool is_testing) {
   DCHECK(is_testing ||
          !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK(base::PathExists(model_params_path));
+  if (!base::PathExists(model_params_path))
+    return std::string();
 
   std::string content;
   if (!base::ReadFileToString(model_params_path, &content)) {
@@ -138,13 +139,6 @@ ModelConfigLoaderImpl::ModelConfigLoaderImpl(
 void ModelConfigLoaderImpl::Init() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (!base::PathExists(model_params_path_)) {
-    // Allow experiment flags to provide configs if there isn't any config from
-    // the disk.
-    InitFromParams();
-    return;
-  }
-
   base::PostTaskAndReplyWithResult(
       blocking_task_runner_.get(), FROM_HERE,
       base::BindOnce(&LoadModelParamsFromDisk, model_params_path_, is_testing_),

chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.cc

@@ -403,17 +403,26 @@ ModellerImpl::ModellerImpl(
     return;
   }
 
-  model_saving_spec_ = GetModelSavingSpecFromProfile(profile);
-  if (model_saving_spec_.global_curve.empty()) {
-    is_modeller_enabled_ = false;
-    return;
-  }
-
   als_reader_observer_.Add(als_reader);
   brightness_monitor_observer_.Add(brightness_monitor);
   model_config_loader_observer_.Add(model_config_loader);
 
   user_activity_observer_.Add(user_activity_detector);
+
+  base::PostTaskAndReplyWithResult(
+      blocking_task_runner_.get(), FROM_HERE,
+      base::BindOnce(&ModellerImpl::GetModelSavingSpecFromProfile, profile),
+      base::BindOnce(&ModellerImpl::OnModelSavingSpecReadFromProfile,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ModellerImpl::OnModelSavingSpecReadFromProfile(
+    const ModelSavingSpec& spec) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!model_saving_spec_.has_value());
+
+  model_saving_spec_ = spec;
+  HandleStatusUpdate();
 }
 
 void ModellerImpl::HandleStatusUpdate() {
@@ -421,6 +430,15 @@ void ModellerImpl::HandleStatusUpdate() {
   if (is_modeller_enabled_.has_value())
     return;
 
+  if (!model_saving_spec_.has_value())
+    return;
+
+  if (model_saving_spec_->global_curve.empty()) {
+    is_modeller_enabled_ = false;
+    OnInitializationComplete();
+    return;
+  }
+
   if (!als_init_status_.has_value())
     return;
 
@@ -458,7 +476,7 @@ void ModellerImpl::HandleStatusUpdate() {
 
   base::PostTaskAndReplyWithResult(
       blocking_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&LoadModelFromDisk, model_saving_spec_, is_testing_),
+      base::BindOnce(&LoadModelFromDisk, *model_saving_spec_, is_testing_),
       base::BindOnce(&ModellerImpl::OnModelLoadedFromDisk,
                      weak_ptr_factory_.GetWeakPtr()));
 }
@@ -651,7 +669,7 @@ void ModellerImpl::OnTrainingFinished(const TrainingResult& result) {
 
   base::PostTaskAndReplyWithResult(
       blocking_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&SaveModelToDisk, model_saving_spec_, model_,
+      base::BindOnce(&SaveModelToDisk, *model_saving_spec_, model_,
                      global_curve_reset_, export_personal_curve, is_testing_),
       base::BindOnce(&ModellerImpl::OnModelSavedToDisk,
                      weak_ptr_factory_.GetWeakPtr()));

chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.h

@@ -141,6 +141,10 @@ class ModellerImpl : public Modeller,
                const base::TickClock* tick_clock,
                bool is_testing = false);
 
+  // Called after we've attempted to read model saving spec from the user
+  // profile.
+  void OnModelSavingSpecReadFromProfile(const ModelSavingSpec& spec);
+
   // Called to handle a status change in one of the dependencies (ALS,
   // brightness monitor, model config loader) of the modeller. If all
   // dependencies are successfully initialized, attempts initialization of
@@ -247,7 +251,7 @@ class ModellerImpl : public Modeller,
   // |OnModelLoadedFromDisk|.
   base::Optional<bool> is_modeller_enabled_;
 
-  ModelSavingSpec model_saving_spec_;
+  base::Optional<ModelSavingSpec> model_saving_spec_;
 
   // Whether the initial global curve is reset to the one constructed from
   // model config. It is true if there is no saved model loaded from the disk