[On-device adaptive brightness] Integrate ModelConfigLoader with ModellerImpl

This cl contains the following changes
1. ModellerImpl will become an observer of ModelConfigLoader.
2. ModellerImpl will receive some parameters from ModelConfigLoader:
(i). global curve spec
(ii). model_als_horizon_seconds
3. ModellerImpl will have "average_log_als" param removed.
4. An equality operator for ModelConfig.


Bug: 881215
Change-Id: I17cc5f2962b57e70921591bd84d862658a82a45d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1496412
Commit-Queue: Jia Meng <jiameng@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638029}

chrome/browser/chromeos/BUILD.gn

@@ -1692,6 +1692,8 @@ source_set("chromeos") {
     "power/auto_screen_brightness/fake_als_reader.h",
     "power/auto_screen_brightness/fake_brightness_monitor.cc",
     "power/auto_screen_brightness/fake_brightness_monitor.h",
+    "power/auto_screen_brightness/fake_model_config_loader.cc",
+    "power/auto_screen_brightness/fake_model_config_loader.h",
     "power/auto_screen_brightness/gaussian_trainer.cc",
     "power/auto_screen_brightness/gaussian_trainer.h",
     "power/auto_screen_brightness/metrics_reporter.cc",

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

@@ -12,6 +12,7 @@
 #include "chrome/browser/chromeos/power/auto_screen_brightness/brightness_monitor_impl.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/gaussian_trainer.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/metrics_reporter.h"
+#include "chrome/browser/chromeos/power/auto_screen_brightness/model_config_loader_impl.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -35,6 +36,8 @@ Controller::Controller() {
   brightness_monitor_ =
       std::make_unique<BrightnessMonitorImpl>(power_manager_client);
 
+  model_config_loader_ = std::make_unique<ModelConfigLoaderImpl>();
+
   ui::UserActivityDetector* user_activity_detector =
       ui::UserActivityDetector::Get();
   DCHECK(user_activity_detector);
@@ -43,7 +46,8 @@ Controller::Controller() {
   DCHECK(profile);
   modeller_ = std::make_unique<ModellerImpl>(
       profile, als_reader_.get(), brightness_monitor_.get(),
-      user_activity_detector, std::make_unique<GaussianTrainer>());
+      model_config_loader_.get(), user_activity_detector,
+      std::make_unique<GaussianTrainer>());
 
   adapter_ = std::make_unique<Adapter>(
       profile, als_reader_.get(), brightness_monitor_.get(), modeller_.get(),

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

@@ -19,6 +19,7 @@ class Adapter;
 class AlsReaderImpl;
 class BrightnessMonitorImpl;
 class MetricsReporter;
+class ModelConfigLoaderImpl;
 class ModellerImpl;
 
 // This controller class sets up and destroys all components needed for the auto
@@ -32,6 +33,7 @@ class Controller {
   std::unique_ptr<MetricsReporter> metrics_reporter_;
   std::unique_ptr<AlsReaderImpl> als_reader_;
   std::unique_ptr<BrightnessMonitorImpl> brightness_monitor_;
+  std::unique_ptr<ModelConfigLoaderImpl> model_config_loader_;
   std::unique_ptr<ModellerImpl> modeller_;
   std::unique_ptr<Adapter> adapter_;
 

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/power/auto_screen_brightness/fake_model_config_loader.h"
+
+namespace chromeos {
+namespace power {
+namespace auto_screen_brightness {
+
+FakeModelConfigLoader::FakeModelConfigLoader() : weak_ptr_factory_(this) {}
+
+FakeModelConfigLoader::~FakeModelConfigLoader() = default;
+
+void FakeModelConfigLoader::ReportModelConfigLoaded() {
+  DCHECK(is_initialized_);
+  for (auto& observer : observers_) {
+    NotifyObserver(&observer);
+  }
+}
+
+void FakeModelConfigLoader::AddObserver(Observer* const observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+  if (is_initialized_) {
+    NotifyObserver(observer);
+  }
+}
+
+void FakeModelConfigLoader::RemoveObserver(Observer* const observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
+void FakeModelConfigLoader::NotifyObserver(Observer* const observer) {
+  DCHECK(observer);
+  observer->OnModelConfigLoaded(is_model_config_valid_
+                                    ? base::Optional<ModelConfig>(model_config_)
+                                    : base::nullopt);
+}
+
+}  // namespace auto_screen_brightness
+}  // namespace power
+}  // namespace chromeos

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_POWER_AUTO_SCREEN_BRIGHTNESS_FAKE_MODEL_CONFIG_LOADER_H_
+#define CHROME_BROWSER_CHROMEOS_POWER_AUTO_SCREEN_BRIGHTNESS_FAKE_MODEL_CONFIG_LOADER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "chrome/browser/chromeos/power/auto_screen_brightness/model_config_loader.h"
+
+namespace chromeos {
+namespace power {
+namespace auto_screen_brightness {
+
+// This is a fake ModelConfigLoader used for testing only. To use it, we need to
+// call its |set_model_config| first to set a specific model config, and then we
+// can call |ReportModelConfigLoaded| so that it will notify its observers the
+// set model config.
+class FakeModelConfigLoader : public ModelConfigLoader {
+ public:
+  FakeModelConfigLoader();
+  ~FakeModelConfigLoader() override;
+
+  void set_model_config(const ModelConfig& model_config) {
+    model_config_ = model_config;
+    is_model_config_valid_ = IsValidModelConfig(model_config_);
+    is_initialized_ = true;
+  }
+
+  // Notifies its observers the pre-specified model config.
+  void ReportModelConfigLoaded();
+
+  // ModelConfigLoader overrides:
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
+
+ private:
+  // Notifies |observer| the specified model config if it's valid. Otherwise the
+  // |observer| will receive a nullopt.
+  void NotifyObserver(Observer* observer);
+
+  bool is_initialized_ = false;
+  bool is_model_config_valid_ = false;
+  ModelConfig model_config_;
+  base::ObserverList<Observer> observers_;
+  base::WeakPtrFactory<FakeModelConfigLoader> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeModelConfigLoader);
+};
+
+}  // namespace auto_screen_brightness
+}  // namespace power
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_POWER_AUTO_SCREEN_BRIGHTNESS_FAKE_MODEL_CONFIG_LOADER_H_

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

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <cmath>
+
 #include "chrome/browser/chromeos/power/auto_screen_brightness/model_config.h"
 
 namespace chromeos {
@@ -21,6 +23,38 @@ ModelConfig::ModelConfig(const ModelConfig& config) {
 
 ModelConfig::~ModelConfig() = default;
 
+bool ModelConfig::operator==(const ModelConfig& config) const {
+  const double kTol = 1e-10;
+  if (std::abs(auto_brightness_als_horizon_seconds -
+               config.auto_brightness_als_horizon_seconds) >= kTol)
+    return false;
+
+  if (log_lux.size() != config.log_lux.size())
+    return false;
+
+  for (size_t i = 0; i < log_lux.size(); ++i) {
+    if (std::abs(log_lux[i] - config.log_lux[i]) >= kTol)
+      return false;
+  }
+
+  if (brightness.size() != config.brightness.size())
+    return false;
+
+  for (size_t i = 0; i < brightness.size(); ++i) {
+    if (std::abs(brightness[i] - config.brightness[i]) >= kTol)
+      return false;
+  }
+
+  if (metrics_key != config.metrics_key)
+    return false;
+
+  if (std::abs(model_als_horizon_seconds - config.model_als_horizon_seconds) >=
+      kTol)
+    return false;
+
+  return true;
+}
+
 bool IsValidModelConfig(const ModelConfig& model_config) {
   if (model_config.auto_brightness_als_horizon_seconds <= 0)
     return false;

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

@@ -20,9 +20,12 @@ struct ModelConfig {
   std::vector<double> brightness;
   std::string metrics_key;
   double model_als_horizon_seconds = -1.0;
+
   ModelConfig();
   ModelConfig(const ModelConfig& config);
   ~ModelConfig();
+
+  bool operator==(const ModelConfig& config) const;
 };
 
 bool IsValidModelConfig(const ModelConfig& model_config);

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

@@ -27,23 +27,6 @@ namespace auto_screen_brightness {
 
 namespace {
 
-void CheckModelConfig(const ModelConfig& result, const ModelConfig& expected) {
-  EXPECT_DOUBLE_EQ(result.auto_brightness_als_horizon_seconds,
-                   expected.auto_brightness_als_horizon_seconds);
-  EXPECT_EQ(result.log_lux.size(), expected.log_lux.size());
-  for (size_t i = 0; i < result.log_lux.size(); ++i) {
-    EXPECT_DOUBLE_EQ(result.log_lux[i], expected.log_lux[i]);
-  }
-  EXPECT_EQ(result.brightness.size(), expected.brightness.size());
-  for (size_t i = 0; i < result.brightness.size(); ++i) {
-    EXPECT_DOUBLE_EQ(result.brightness[i], expected.brightness[i]);
-  }
-
-  EXPECT_EQ(result.metrics_key, expected.metrics_key);
-  EXPECT_DOUBLE_EQ(result.model_als_horizon_seconds,
-                   expected.model_als_horizon_seconds);
-}
-
 class TestObserver : public ModelConfigLoader::Observer {
  public:
   TestObserver() {}
@@ -158,7 +141,7 @@ TEST_F(ModelConfigLoaderImplTest, ValidModelParamsLoaded) {
   expected_model_config.metrics_key = "abc";
   expected_model_config.model_als_horizon_seconds = 5;
   EXPECT_TRUE(test_observer_->model_config());
-  CheckModelConfig(*test_observer_->model_config(), expected_model_config);
+  EXPECT_EQ(*test_observer_->model_config(), expected_model_config);
 }
 
 TEST_F(ModelConfigLoaderImplTest, ValidModelParamsLoadedThenOverriden) {
@@ -202,7 +185,7 @@ TEST_F(ModelConfigLoaderImplTest, ValidModelParamsLoadedThenOverriden) {
   expected_model_config.metrics_key = "abc";
   expected_model_config.model_als_horizon_seconds = 20.0;
   EXPECT_TRUE(test_observer_->model_config());
-  CheckModelConfig(*test_observer_->model_config(), expected_model_config);
+  EXPECT_EQ(*test_observer_->model_config(), expected_model_config);
 }
 
 TEST_F(ModelConfigLoaderImplTest, InvalidModelParamsLoaded) {
@@ -269,7 +252,7 @@ TEST_F(ModelConfigLoaderImplTest, InvalidModelParamsLoadedThenOverriden) {
   expected_model_config.metrics_key = "abc";
   expected_model_config.model_als_horizon_seconds = 20.0;
   EXPECT_TRUE(test_observer_->model_config());
-  CheckModelConfig(*test_observer_->model_config(), expected_model_config);
+  EXPECT_EQ(*test_observer_->model_config(), expected_model_config);
 }
 
 TEST_F(ModelConfigLoaderImplTest, MissingModelParams) {

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

@@ -20,6 +20,8 @@
 #include "chrome/browser/chromeos/power/auto_screen_brightness/als_reader.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/als_samples.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/brightness_monitor.h"
+#include "chrome/browser/chromeos/power/auto_screen_brightness/model_config.h"
+#include "chrome/browser/chromeos/power/auto_screen_brightness/model_config_loader.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/modeller.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/trainer.h"
 #include "chrome/browser/chromeos/power/auto_screen_brightness/utils.h"
@@ -40,6 +42,7 @@ namespace auto_screen_brightness {
 class ModellerImpl : public Modeller,
                      public AlsReader::Observer,
                      public BrightnessMonitor::Observer,
+                     public ModelConfigLoader::Observer,
                      public ui::UserActivityObserver {
  public:
   static constexpr char kModelDir[] = "autobrightness";
@@ -49,6 +52,7 @@ class ModellerImpl : public Modeller,
   ModellerImpl(const Profile* profile,
                AlsReader* als_reader,
                BrightnessMonitor* brightness_monitor,
+               ModelConfigLoader* model_config_loader,
                ui::UserActivityDetector* user_activity_detector,
                std::unique_ptr<Trainer> trainer);
   ~ModellerImpl() override;
@@ -67,6 +71,9 @@ class ModellerImpl : public Modeller,
                                double new_brightness_percent) override;
   void OnUserBrightnessChangeRequested() override;
 
+  // ModelConfigLoader::Observer overrides:
+  void OnModelConfigLoaded(base::Optional<ModelConfig> model_config) override;
+
   // ui::UserActivityObserver overrides:
   void OnUserActivity(const ui::Event* event) override;
 
@@ -75,6 +82,7 @@ class ModellerImpl : public Modeller,
       const Profile* profile,
       AlsReader* als_reader,
       BrightnessMonitor* brightness_monitor,
+      ModelConfigLoader* model_config_loader,
       ui::UserActivityDetector* user_activity_detector,
       std::unique_ptr<Trainer> trainer,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner,
@@ -95,6 +103,8 @@ class ModellerImpl : public Modeller,
 
   base::TimeDelta GetTrainingDelayForTesting() const;
 
+  ModelConfig GetModelConfigForTesting() const;
+
   // Returns the path that will be used to store curves. It also creates
   // intermediate directories if they do not exist. Returns an empty path on
   // failures.
@@ -105,6 +115,7 @@ class ModellerImpl : public Modeller,
   ModellerImpl(const Profile* profile,
                AlsReader* als_reader,
                BrightnessMonitor* brightness_monitor,
+               ModelConfigLoader* model_config_loader,
                ui::UserActivityDetector* user_activity_detector,
                std::unique_ptr<Trainer> trainer,
                scoped_refptr<base::SequencedTaskRunner> task_runner,
@@ -123,6 +134,9 @@ class ModellerImpl : public Modeller,
   // notified about the status and the curve.
   void HandleStatusUpdate();
 
+  // Load customizations from model configs.
+  void RunCustomization();
+
   // Notifies its observers on the status of the model. It will be called either
   // when HandleStatusUpdate is called and |model_status_| is no longer
   // |kInitializing|, or when an observer is added to the modeller, and
@@ -184,6 +198,9 @@ class ModellerImpl : public Modeller,
   ScopedObserver<BrightnessMonitor, BrightnessMonitor::Observer>
       brightness_monitor_observer_;
 
+  ScopedObserver<ModelConfigLoader, ModelConfigLoader::Observer>
+      model_config_loader_observer_;
+
   ScopedObserver<ui::UserActivityDetector, ui::UserActivityObserver>
       user_activity_observer_;
 
@@ -200,6 +217,12 @@ class ModellerImpl : public Modeller,
   base::Optional<AlsReader::AlsInitStatus> als_init_status_;
   base::Optional<bool> brightness_monitor_success_;
 
+  // |model_config_exists_| will remain nullopt until |OnModelConfigLoaded| is
+  // called. Its value will then be set to true if the input model config exists
+  // (not nullopt), else its value will be false.
+  base::Optional<bool> model_config_exists_;
+  ModelConfig model_config_;
+
   // Whether this modeller has initialized successfully, including connecting
   // to AlsReader, BrightnessMonitor and loading a Trainer.
   // Initially has no value. Guaranteed to have a value after the completion of
@@ -212,8 +235,11 @@ class ModellerImpl : public Modeller,
   // Trainer and Trainer reported it was valid.
   bool has_initial_personal_curve_ = false;
 
-  // Global curve constructed from predefined params.
-  const MonotoneCubicSpline global_curve_;
+  // Global curve constructed from predefined params. It will remain nullopt
+  // until |OnModelConfigLoaded| is called. If input model config is nullopt
+  // then |global_curve_| will remain nullopt, else it will be created based on
+  // the model config.
+  base::Optional<MonotoneCubicSpline> global_curve_;
 
   // Current personal curve. Initially it could be either the global curve or
   // loaded curve. After training, it will be updated each time trainer
@@ -223,10 +249,6 @@ class ModellerImpl : public Modeller,
   // Recent ambient values.
   std::unique_ptr<AmbientLightSampleBuffer> ambient_light_values_;
 
-  // Whether we calculate average log ALS values. This should be the same as
-  // that used by the adapter.
-  bool average_log_als_ = false;
-
   std::vector<TrainingDataPoint> data_cache_;
 
   base::ObserverList<Modeller::Observer> observers_;