Add chromeos::SchedulerConfigurationManagerBase

This allows code outside chrome/ (e.g. components/arc/session/) to
observe the scheduler configuration status.

BUG=b:139752657
TEST=./out/Release/unit_tests --gtest_filter='*SchedulerConfig*'

Change-Id: I15873cedeaa0c66142b0f35947e4a8acf21e003c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815517
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Auto-Submit: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701245}

chrome/browser/chromeos/scheduler_configuration_manager.cc

@@ -98,8 +98,8 @@ void SchedulerConfigurationManager::OnConfigurationSet(
   } else {
     LOG(ERROR) << "Failed to update scheduler configuration";
   }
-  // TODO(b/139752657): Add an observer class for monitoring |result| and
-  // |num_cores_disabled|.
+  for (Observer& obs : observer_list_)
+    obs.OnConfigurationSet(result, num_cores_disabled);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/scheduler_configuration_manager.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chromeos/system/scheduler_configuration_manager_base.h"
 #include "components/prefs/pref_change_registrar.h"
 
 class PrefRegistrySimple;
@@ -25,11 +26,11 @@ class DebugDaemonClient;
 // For more information on why H/T is configurable, see
 // https://www.chromium.org/chromium-os/mds-on-chromeos
 //
-class SchedulerConfigurationManager {
+class SchedulerConfigurationManager : public SchedulerConfigurationManagerBase {
  public:
   SchedulerConfigurationManager(DebugDaemonClient* debug_daemon_client,
                                 PrefService* local_state);
-  ~SchedulerConfigurationManager();
+  ~SchedulerConfigurationManager() override;
 
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
 

chrome/browser/chromeos/scheduler_configuration_manager_unittest.cc

@@ -17,17 +17,26 @@
 
 namespace chromeos {
 
-class SchedulerConfigurationManagerTest : public testing::Test {
+class SchedulerConfigurationManagerTest
+    : public testing::Test,
+      public SchedulerConfigurationManagerBase::Observer {
  public:
   SchedulerConfigurationManagerTest() {
     SchedulerConfigurationManager::RegisterLocalStatePrefs(
         local_state_.registry());
   }
 
+  // SchedulerConfigurationManagerBase::Observer:
+  void OnConfigurationSet(bool success, size_t num_cores_disabled) override {
+    ++configuration_set_count_;
+  }
+
   base::test::TaskEnvironment task_environment_;
 
   FakeDebugDaemonClient debug_daemon_client_;
   TestingPrefServiceSimple local_state_;
+
+  size_t configuration_set_count_ = 0;
 };
 
 TEST_F(SchedulerConfigurationManagerTest, Startup) {
@@ -36,49 +45,61 @@ TEST_F(SchedulerConfigurationManagerTest, Startup) {
 
   // Manager waits on initialization for service to be available.
   SchedulerConfigurationManager manager(&debug_daemon_client_, &local_state_);
+  manager.AddObserver(this);
+
   task_environment_.RunUntilIdle();
   EXPECT_EQ("", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(0u, configuration_set_count_);
 
   // Config changes don't lead to updates while debugd isn't ready.
   local_state_.SetString(prefs::kSchedulerConfiguration, "config");
   task_environment_.RunUntilIdle();
   EXPECT_EQ("", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(0u, configuration_set_count_);
 
   // Once the debugd service becomes available, the config gets set.
   debug_daemon_client_.SetServiceIsAvailable(true);
   task_environment_.RunUntilIdle();
   EXPECT_EQ("config", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(1u, configuration_set_count_);
 }
 
 TEST_F(SchedulerConfigurationManagerTest, ConfigChange) {
   // Correct default is used when there is no configured value.
   SchedulerConfigurationManager manager(&debug_daemon_client_, &local_state_);
+  manager.AddObserver(this);
+
   task_environment_.RunUntilIdle();
   EXPECT_EQ(debugd::scheduler_configuration::kConservativeScheduler,
             debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(1u, configuration_set_count_);
 
   // Change user pref, which should trigger a config change.
   local_state_.SetUserPref(prefs::kSchedulerConfiguration,
                            std::make_unique<base::Value>("user"));
   task_environment_.RunUntilIdle();
   EXPECT_EQ("user", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(2u, configuration_set_count_);
 
   // Set a policy, which should override the user setting
   local_state_.SetManagedPref(prefs::kSchedulerConfiguration,
                               std::make_unique<base::Value>("policy"));
   task_environment_.RunUntilIdle();
   EXPECT_EQ("policy", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(3u, configuration_set_count_);
 
   // Dropping the user pref doesn't change anything.
   local_state_.RemoveUserPref(prefs::kSchedulerConfiguration);
   task_environment_.RunUntilIdle();
   EXPECT_EQ("policy", debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(4u, configuration_set_count_);
 
   // Dropping the policy as well reverts to the default configuration.
   local_state_.RemoveManagedPref(prefs::kSchedulerConfiguration);
   task_environment_.RunUntilIdle();
   EXPECT_EQ(debugd::scheduler_configuration::kConservativeScheduler,
             debug_daemon_client_.scheduler_configuration_name());
+  EXPECT_EQ(5u, configuration_set_count_);
 }
 
 TEST_F(SchedulerConfigurationManagerTest, FinchDefault) {

chromeos/system/BUILD.gn

@@ -26,6 +26,8 @@ component("system") {
     "fake_statistics_provider.h",
     "name_value_pairs_parser.cc",
     "name_value_pairs_parser.h",
+    "scheduler_configuration_manager_base.cc",
+    "scheduler_configuration_manager_base.h",
     "statistics_provider.cc",
     "statistics_provider.h",
   ]

chromeos/system/scheduler_configuration_manager_base.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 "chromeos/system/scheduler_configuration_manager_base.h"
+
+namespace chromeos {
+
+SchedulerConfigurationManagerBase::SchedulerConfigurationManagerBase() =
+    default;
+SchedulerConfigurationManagerBase::~SchedulerConfigurationManagerBase() =
+    default;
+
+void SchedulerConfigurationManagerBase::AddObserver(
+    SchedulerConfigurationManagerBase::Observer* obs) {
+  observer_list_.AddObserver(obs);
+}
+
+void SchedulerConfigurationManagerBase::RemoveObserver(
+    const SchedulerConfigurationManagerBase::Observer* obs) {
+  observer_list_.RemoveObserver(obs);
+}
+
+}  // namespace chromeos

chromeos/system/scheduler_configuration_manager_base.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 CHROMEOS_SYSTEM_SCHEDULER_CONFIGURATION_MANAGER_BASE_H_
+#define CHROMEOS_SYSTEM_SCHEDULER_CONFIGURATION_MANAGER_BASE_H_
+
+#include <stddef.h>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "base/observer_list.h"
+
+namespace chromeos {
+
+// A base class for SchedulerConfigurationManager.
+class COMPONENT_EXPORT(CHROMEOS_SYSTEM) SchedulerConfigurationManagerBase {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    // Called when SetSchedulerConfiguration D-Bus call to debugd returns.
+    virtual void OnConfigurationSet(bool success,
+                                    size_t num_cores_disabled) = 0;
+  };
+
+  SchedulerConfigurationManagerBase();
+  virtual ~SchedulerConfigurationManagerBase();
+
+  void AddObserver(Observer* obs);
+  void RemoveObserver(const Observer* obs);
+
+ protected:
+  base::ObserverList<Observer> observer_list_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SchedulerConfigurationManagerBase);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SYSTEM_SCHEDULER_CONFIGURATION_MANAGER_BASE_H_