Use UserPerformanceTuningManager for battery saver in PM's metrics pr…

…ovider

This CL changes performance manager's metrics provider to use
UserPerformanceTuningManager when determining if battery saver mode is
enabled. This change is required because battery saver mode is changing to
be controlled by multiple signals instead of a single preference.

Achieving this requires moving some files from components to c/b as well as
reworking some tests. UserPerformanceTuningManager also gets some of its
functionality through dependency injection to allow for mocking in tests.

Bug: 1348590
Change-Id: I1ca20ff49d4a1f409953b12c8813efd8b1509bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3821584
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034091}

chrome/browser/BUILD.gn

@@ -4029,6 +4029,8 @@ static_library("browser") {
       "performance_manager/mechanisms/page_discarder.h",
       "performance_manager/mechanisms/page_loader.cc",
       "performance_manager/mechanisms/page_loader.h",
+      "performance_manager/metrics/metrics_provider.cc",
+      "performance_manager/metrics/metrics_provider.h",
       "performance_manager/persistence/site_data/site_data_cache_facade.cc",
       "performance_manager/persistence/site_data/site_data_cache_facade.h",
       "performance_manager/persistence/site_data/site_data_cache_facade_factory.cc",

chrome/browser/DEPS

@@ -500,6 +500,7 @@ include_rules = [
   "+chrome/browser/performance_manager/test_support",
   "+chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.h",
   "+chrome/browser/performance_manager/chrome_content_browser_client_performance_manager_part.h",
+  "+chrome/browser/performance_manager/metrics/metrics_provider.h",
   "+chrome/browser/performance_manager/policies/policy_features.h",
 
   # Explicitly disallow using SyncMessageFilter to prevent browser from

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -20,6 +20,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_macros.h"
@@ -127,7 +128,7 @@
 #include "components/metrics/android_metrics_provider.h"
 #else
 #include "chrome/browser/metrics/browser_activity_watcher.h"
-#include "components/performance_manager/public/metrics/metrics_provider.h"
+#include "chrome/browser/performance_manager/metrics/metrics_provider.h"
 #endif
 
 #if BUILDFLAG(IS_POSIX)
@@ -785,7 +786,7 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
       std::make_unique<FamilyLinkUserMetricsProvider>());
 #else
   metrics_service_->RegisterMetricsProvider(
-      std::make_unique<performance_manager::MetricsProvider>(local_state));
+      base::WrapUnique(new performance_manager::MetricsProvider(local_state)));
 #endif  // BUILDFLAG(IS_ANDROID)
 
 #if BUILDFLAG(IS_WIN)

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/performance_manager/decorators/helpers/page_live_state_decorator_helper.h"
 #include "chrome/browser/performance_manager/decorators/page_aggregator.h"
 #include "chrome/browser/performance_manager/metrics/memory_pressure_metrics.h"
+#include "chrome/browser/performance_manager/metrics/metrics_provider.h"
 #include "chrome/browser/performance_manager/observers/page_load_metrics_observer.h"
 #include "chrome/browser/performance_manager/policies/background_tab_loading_policy.h"
 #include "chrome/browser/performance_manager/policies/policy_features.h"
@@ -211,9 +212,15 @@ void ChromeBrowserMainExtraPartsPerformanceManager::PreMainMessageLoopRun() {
           performance_manager::features::kHighEfficiencyModeAvailable) ||
       base::FeatureList::IsEnabled(
           performance_manager::features::kBatterySaverModeAvailable)) {
-    user_performance_tuning_manager_ = std::make_unique<
+    user_performance_tuning_manager_ = std::unique_ptr<
         performance_manager::user_tuning::UserPerformanceTuningManager>(
-        g_browser_process->local_state());
+        new performance_manager::user_tuning::UserPerformanceTuningManager(
+            g_browser_process->local_state()));
+
+    // This object is created by the metrics service before threads, but it
+    // needs the UserPerformanceTuningManager to exist. At this point it's
+    // instantiated, but still needs to be initialized.
+    performance_manager::MetricsProvider::GetInstance()->Initialize();
   }
 #endif
 }

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.h

@@ -35,12 +35,10 @@ class ExtensionWatcher;
 #endif
 
 namespace user_tuning {
+class ProfileDiscardOptOutListHelper;
 class UserPerformanceTuningManager;
 }
 
-namespace user_tuning {
-class ProfileDiscardOptOutListHelper;
-}
 }  // namespace performance_manager
 
 // Handles the initialization of the performance manager and a few dependent

components/performance_manager/metrics/metrics_provider.cc → chrome/browser/performance_manager/metrics/metrics_provider.cc

@@ -2,33 +2,53 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/performance_manager/public/metrics/metrics_provider.h"
+#include "chrome/browser/performance_manager/metrics/metrics_provider.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "chrome/browser/performance_manager/user_tuning/user_performance_tuning_manager.h"
 #include "components/performance_manager/public/user_tuning/prefs.h"
 #include "components/prefs/pref_service.h"
 
 namespace performance_manager {
 
-MetricsProvider::MetricsProvider(PrefService* local_state)
-    : local_state_(local_state) {
+namespace {
+
+MetricsProvider* g_metrics_provider = nullptr;
+
+}
+
+// static
+MetricsProvider* MetricsProvider::GetInstance() {
+  DCHECK(g_metrics_provider);
+  return g_metrics_provider;
+}
+
+MetricsProvider::~MetricsProvider() {
+  DCHECK_EQ(this, g_metrics_provider);
+  g_metrics_provider = nullptr;
+}
+
+void MetricsProvider::Initialize() {
+  DCHECK(!initialized_);
+
   pref_change_registrar_.Init(local_state_);
   pref_change_registrar_.Add(
       performance_manager::user_tuning::prefs::kHighEfficiencyModeEnabled,
-      base::BindRepeating(&MetricsProvider::OnEfficiencyModeChanged,
-                          base::Unretained(this)));
-  pref_change_registrar_.Add(
-      performance_manager::user_tuning::prefs::kBatterySaverModeEnabled,
-      base::BindRepeating(&MetricsProvider::OnEfficiencyModeChanged,
+      base::BindRepeating(&MetricsProvider::OnTuningModesChanged,
                           base::Unretained(this)));
+  performance_manager::user_tuning::UserPerformanceTuningManager::GetInstance()
+      ->AddObserver(this);
 
+  initialized_ = true;
   current_mode_ = ComputeCurrentMode();
 }
 
-MetricsProvider::~MetricsProvider() = default;
-
 void MetricsProvider::ProvideCurrentSessionData(
     metrics::ChromeUserMetricsExtension* uma_proto) {
+  // It's valid for this to be called when `initialized_` is false if the finch
+  // features controlling battery saver and high efficiency are disabled.
+  // TODO(crbug.com/1348590): CHECK(initialized_) when the features are enabled
+  // and removed.
   base::UmaHistogramEnumeration("PerformanceManager.UserTuning.EfficiencyMode",
                                 current_mode_);
 
@@ -38,7 +58,17 @@ void MetricsProvider::ProvideCurrentSessionData(
   current_mode_ = ComputeCurrentMode();
 }
 
-void MetricsProvider::OnEfficiencyModeChanged() {
+MetricsProvider::MetricsProvider(PrefService* local_state)
+    : local_state_(local_state) {
+  DCHECK(!g_metrics_provider);
+  g_metrics_provider = this;
+}
+
+void MetricsProvider::OnBatterySaverModeChanged(bool is_active) {
+  OnTuningModesChanged();
+}
+
+void MetricsProvider::OnTuningModesChanged() {
   EfficiencyMode new_mode = ComputeCurrentMode();
 
   // If the mode changes between UMA reports, mark it as Mixed for this
@@ -49,10 +79,23 @@ void MetricsProvider::OnEfficiencyModeChanged() {
 }
 
 MetricsProvider::EfficiencyMode MetricsProvider::ComputeCurrentMode() const {
+  // It's valid for this to be uninitialized if the battery saver/high
+  // efficiency modes are unavailable. In that case, the browser is running in
+  // normal mode, so return kNormal.
+  // TODO(crbug.com/1348590): Change this to a DCHECK when the features are
+  // enabled and removed.
+  if (!initialized_) {
+    return EfficiencyMode::kNormal;
+  }
+
   bool high_efficiency_enabled = local_state_->GetBoolean(
       performance_manager::user_tuning::prefs::kHighEfficiencyModeEnabled);
-  bool battery_saver_enabled = local_state_->GetBoolean(
-      performance_manager::user_tuning::prefs::kBatterySaverModeEnabled);
+
+  performance_manager::user_tuning::UserPerformanceTuningManager*
+      user_performance_tuning_manager = performance_manager::user_tuning::
+          UserPerformanceTuningManager::GetInstance();
+  bool battery_saver_enabled =
+      user_performance_tuning_manager->IsBatterySaverActive();
 
   if (high_efficiency_enabled && battery_saver_enabled) {
     return EfficiencyMode::kBoth;
@@ -69,4 +112,4 @@ MetricsProvider::EfficiencyMode MetricsProvider::ComputeCurrentMode() const {
   return EfficiencyMode::kNormal;
 }
 
-}  // namespace performance_manager
+}  // namespace performance_manager

components/performance_manager/public/metrics/metrics_provider.h → chrome/browser/performance_manager/metrics/metrics_provider.h

@@ -2,21 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_
-#define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_
+#ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_METRICS_METRICS_PROVIDER_H_
+#define CHROME_BROWSER_PERFORMANCE_MANAGER_METRICS_METRICS_PROVIDER_H_
 
 #include "base/memory/raw_ptr.h"
+#include "chrome/browser/performance_manager/user_tuning/user_performance_tuning_manager.h"
 #include "components/metrics/metrics_provider.h"
-
 #include "components/prefs/pref_change_registrar.h"
 
+class ChromeMetricsServiceClient;
+class PerformanceManagerMetricsProviderTest;
 class PrefService;
 
 namespace performance_manager {
 
 // A metrics provider to add some performance manager related metrics to the UMA
 // protos on each upload.
-class MetricsProvider : public metrics::MetricsProvider {
+class MetricsProvider : public ::metrics::MetricsProvider,
+                        public performance_manager::user_tuning::
+                            UserPerformanceTuningManager::Observer {
  public:
   enum class EfficiencyMode {
     // No efficiency mode for the entire upload window
@@ -33,23 +37,43 @@ class MetricsProvider : public metrics::MetricsProvider {
     kMaxValue = kMixed
   };
 
-  explicit MetricsProvider(PrefService* local_state);
+  static MetricsProvider* GetInstance();
+
   ~MetricsProvider() override;
 
+  void Initialize();
+
   // metrics::MetricsProvider:
   // This is only called from UMA code but is public for testing.
   void ProvideCurrentSessionData(
-      metrics::ChromeUserMetricsExtension* uma_proto) override;
+      ::metrics::ChromeUserMetricsExtension* uma_proto) override;
 
  private:
-  void OnEfficiencyModeChanged();
+  friend class ::ChromeMetricsServiceClient;
+  friend class ::PerformanceManagerMetricsProviderTest;
+
+  explicit MetricsProvider(PrefService* local_state);
+
+  // UserPerformanceTuningManager::Observer:
+  void OnBatterySaverModeChanged(bool is_active) override;
+  void OnExternalPowerConnectedChanged(
+      bool external_power_connected) override{};
+  void OnBatteryThresholdReached() override{};
+  void OnMemoryThresholdReached() override{};
+  void OnTabCountThresholdReached() override{};
+  void OnJankThresholdReached() override{};
+
+  void OnTuningModesChanged();
   EfficiencyMode ComputeCurrentMode() const;
 
   PrefChangeRegistrar pref_change_registrar_;
   const raw_ptr<PrefService> local_state_;
   EfficiencyMode current_mode_ = EfficiencyMode::kNormal;
+
+  bool initialized_ = false;
+  ;
 };
 
 }  // namespace performance_manager
 
-#endif  // COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_
+#endif  // CHROME_BROWSER_PERFORMANCE_MANAGER_METRICS_METRICS_PROVIDER_H_