[battery]: Implement battery level provider.

This CL extracts the logic of querying battery state from
PowerDrainRecorder into a new BatteryLevelProvider, to eventually
reuse on other platforms.
BatteryLevelProvider is platform specific. This CL implements mac only.
Other platforms will be implemented in follow-up CLs.

Bug: 1153193
Change-Id: I56bca3b851428483395e5edf6279087cd293e7e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574438
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840826}

chrome/browser/BUILD.gn

@@ -815,6 +815,7 @@ static_library("browser") {
     "metrics/network_quality_estimator_provider_impl.h",
     "metrics/oom/out_of_memory_reporter.cc",
     "metrics/oom/out_of_memory_reporter.h",
+    "metrics/power/battery_level_provider.h",
     "metrics/process_memory_metrics_emitter.cc",
     "metrics/process_memory_metrics_emitter.h",
     "metrics/renderer_uptime_tracker.cc",
@@ -4958,8 +4959,9 @@ static_library("browser") {
       "media_galleries/mac/mtp_device_delegate_impl_mac.mm",
       "memory_details_mac.cc",
       "metrics/chrome_browser_main_extra_parts_metrics_mac.mm",
-      "metrics/power_metrics_provider_mac.h",
-      "metrics/power_metrics_provider_mac.mm",
+      "metrics/power/battery_level_provider_mac.mm",
+      "metrics/power/power_metrics_provider_mac.h",
+      "metrics/power/power_metrics_provider_mac.mm",
       "notifications/alert_dispatcher_mac.h",
       "notifications/notification_platform_bridge_mac.h",
       "notifications/notification_platform_bridge_mac.mm",

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -183,7 +183,7 @@
 #endif  // !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
 
 #if defined(OS_MAC)
-#include "chrome/browser/metrics/power_metrics_provider_mac.h"
+#include "chrome/browser/metrics/power/power_metrics_provider_mac.h"
 #endif
 
 namespace {

chrome/browser/metrics/power/OWNERS

+etiennep@chromium.org
+fdoray@chromium.org
+olivierli@chromium.org

chrome/browser/metrics/power/battery_level_provider.h

+// Copyright 2020 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_METRICS_POWER_BATTERY_LEVEL_PROVIDER_H_
+#define CHROME_BROWSER_METRICS_POWER_BATTERY_LEVEL_PROVIDER_H_
+
+#include "base/callback.h"
+#include "base/optional.h"
+
+// BatteryLevelProvider provides an interface for querrying battery state.
+// A platform specific implementation is obtained with
+// BatteryLevelProvider::Create().
+class BatteryLevelProvider {
+ public:
+  // Represents the state of the battery at a certain point in time.
+  struct BatteryState {
+    // A fraction of the maximal battery capacity of the system, in the range
+    // [0.00, 1.00].
+    double charge_level = 0;
+
+    // True if the system is running on battery power, false if the system is
+    // drawing power from an external power source.
+    bool on_battery = false;
+
+    // The time at which the battery state capture took place.
+    base::TimeTicks capture_time;
+  };
+
+  // Creates a platform specific BatteryLevelProvider able to retrieve battery
+  // state.
+  static std::unique_ptr<BatteryLevelProvider> Create();
+
+  virtual ~BatteryLevelProvider() = default;
+
+  BatteryLevelProvider(const BatteryLevelProvider& other) = delete;
+  BatteryLevelProvider& operator=(const BatteryLevelProvider& other) = delete;
+
+  // Returns the current battery state, or nullopt if no battery is present or
+  // querying battery information failed.
+  virtual base::Optional<BatteryState> GetBatteryState() = 0;
+
+ protected:
+  BatteryLevelProvider() = default;
+};
+
+#endif  // CHROME_BROWSER_METRICS_POWER_BATTERY_LEVEL_PROVIDER_H_

chrome/browser/metrics/power/battery_level_provider_mac.mm

+// Copyright 2020 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/metrics/power/battery_level_provider.h"
+
+#import <Foundation/Foundation.h>
+#include <IOKit/IOKitLib.h>
+#include <IOKit/ps/IOPSKeys.h>
+
+#include "base/mac/foundation_util.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/mac/scoped_ioobject.h"
+
+namespace {
+
+// Returns the value corresponding to |key| in the dictionary |description|.
+// Returns |default_value| if the dictionary does not contain |key|, the
+// corresponding value is nullptr or it could not be converted to SInt64.
+base::Optional<SInt64> GetValueAsSInt64(CFDictionaryRef description,
+                                        CFStringRef key) {
+  CFNumberRef number_ref =
+      base::mac::GetValueFromDictionary<CFNumberRef>(description, key);
+
+  SInt64 value;
+  if (number_ref && CFNumberGetValue(number_ref, kCFNumberSInt64Type, &value))
+    return value;
+
+  return base::nullopt;
+}
+
+base::Optional<bool> GetValueAsBoolean(CFDictionaryRef description,
+                                       CFStringRef key) {
+  CFBooleanRef boolean =
+      base::mac::GetValueFromDictionary<CFBooleanRef>(description, key);
+  if (!boolean)
+    return base::nullopt;
+  return CFBooleanGetValue(boolean);
+}
+
+}  // namespace
+
+class BatteryLevelProviderMac : public BatteryLevelProvider {
+ public:
+  BatteryLevelProviderMac() = default;
+  ~BatteryLevelProviderMac() override = default;
+
+  base::Optional<BatteryState> GetBatteryState() override;
+};
+
+std::unique_ptr<BatteryLevelProvider> BatteryLevelProvider::Create() {
+  return std::make_unique<BatteryLevelProviderMac>();
+}
+
+base::Optional<BatteryLevelProvider::BatteryState>
+BatteryLevelProviderMac::GetBatteryState() {
+  const base::TimeTicks capture_time = base::TimeTicks::Now();
+
+  // Retrieve the IOPMPowerSource service.
+  const base::mac::ScopedIOObject<io_service_t> service(
+      IOServiceGetMatchingService(kIOMasterPortDefault,
+                                  IOServiceMatching("IOPMPowerSource")));
+
+  // Gather a dictionary containing the power information.
+  base::ScopedCFTypeRef<CFMutableDictionaryRef> dict;
+  kern_return_t result = IORegistryEntryCreateCFProperties(
+      service.get(), dict.InitializeInto(), 0, 0);
+
+  // Retrieving dictionary failed. Cannot proceed.
+  if (result != KERN_SUCCESS)
+    return base::nullopt;
+
+  base::Optional<bool> external_connected =
+      GetValueAsBoolean(dict, CFSTR("ExternalConnected"));
+  // Value was not available.
+  if (!external_connected.has_value())
+    return base::nullopt;
+
+  CFStringRef capacity_key;
+  CFStringRef max_capacity_key;
+
+  // Use the correct key depending on macOS version.
+  if (@available(macOS 10.14.0, *)) {
+    capacity_key = CFSTR("AppleRawCurrentCapacity");
+    max_capacity_key = CFSTR("AppleRawMaxCapacity");
+  } else {
+    capacity_key = CFSTR("CurrentCapacity");
+    max_capacity_key = CFSTR("RawMaxCapacity");
+  }
+
+  // Extract the information from the dictionary.
+  base::Optional<SInt64> current_capacity =
+      GetValueAsSInt64(dict, capacity_key);
+  base::Optional<SInt64> max_capacity =
+      GetValueAsSInt64(dict, max_capacity_key);
+
+  // If any of the values were not available.
+  if (!current_capacity.has_value() || !max_capacity.has_value())
+    return base::nullopt;
+
+  // Avoid invalid division.
+  if (*max_capacity == 0)
+    return base::nullopt;
+
+  // |ratio| is the result of dividing |current_capacity| by |max_capacity|.
+  double charge_level = static_cast<double>(current_capacity.value()) /
+                        static_cast<double>(max_capacity.value());
+
+  return BatteryState{charge_level, !(*external_connected), capture_time};
+}

chrome/browser/metrics/power_metrics_provider_mac.h → chrome/browser/metrics/power/power_metrics_provider_mac.h

@@ -2,14 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_METRICS_POWER_METRICS_PROVIDER_MAC_H_
-#define CHROME_BROWSER_METRICS_POWER_METRICS_PROVIDER_MAC_H_
+#ifndef CHROME_BROWSER_METRICS_POWER_POWER_METRICS_PROVIDER_MAC_H_
+#define CHROME_BROWSER_METRICS_POWER_POWER_METRICS_PROVIDER_MAC_H_
 
 #include "components/metrics/metrics_provider.h"
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/time/time.h"
+#include "chrome/browser/metrics/power/battery_level_provider.h"
 
 // Records battery power drain in a histogram. To use, repeatedly call
 // RecordBatteryDischarge() at regular intervals. The implementation tries to
@@ -39,42 +40,6 @@
 //   };
 //
 class PowerDrainRecorder {
- private:
-  // Represents the state of the battery at a certain point in time.
-  class BatteryState {
-   public:
-    // Use this constructor to extract the actual battery information using
-    // system functions.
-    BatteryState();
-
-    // Use this constructor to control the value of all the members.
-    BatteryState(int capacity, bool on_battery, base::TimeTicks creation_time);
-
-    // Returns the battery capacity at the time of capture.
-    int capacity() const { return capacity_; }
-
-    // Returns true if the system ran on battery power at the time of capture.
-    bool on_battery() const { return on_battery_; }
-
-    // Explicitly mark the state object as on battery on not.
-    void SetIsOnBattery(bool on_battery);
-
-    // Return the time at which the object was created.
-    base::TimeTicks creation_time() const { return creation_time_; }
-
-   private:
-    // A portion of the maximal battery capacity of the system.  Units are
-    // hundredths of a percent. ie: 99.8742% capacity --> 998742.
-    int capacity_ = 0;
-
-    // For the purpose of this class not being on battery power is considered
-    // equivalent to charging.
-    bool on_battery_ = false;
-
-    // The time at which the battery state capture took place.
-    base::TimeTicks creation_time_;
-  };
-
  public:
   // |recording_interval| is the time that is supposed to elapse between calls
   // to RecordBatteryDischarge().
@@ -90,24 +55,16 @@ class PowerDrainRecorder {
 
   // Replace the function used to get BatteryState values. Use only for testing
   // to not depend on actual system information.
-  void SetGetBatteryStateCallBackForTesting(
-      base::RepeatingCallback<BatteryState()>);
+  void SetBatteryLevelProviderForTesting(
+      std::unique_ptr<BatteryLevelProvider> provider);
 
  private:
-  // Simply creates a BatteryState() object and returns it. Stand-in that can
-  // replaced in tests.
-  BatteryState GetCurrentBatteryState();
-
-  // Use this callback to get the current battery state. Override in tests to
-  // provide test values.
-  base::RepeatingCallback<BatteryState()> battery_state_callback_ =
-      base::BindRepeating(&PowerDrainRecorder::GetCurrentBatteryState,
-                          base::Unretained(this));
+  // Used to get the current battery state.
+  std::unique_ptr<BatteryLevelProvider> battery_level_provider_ =
+      BatteryLevelProvider::Create();
 
-  // Default battery state value that makes sure it will not be used towards the
-  // first discharge calculation that will happen at some point in the future.
-  BatteryState previous_battery_state_ =
-      BatteryState(0, false, base::TimeTicks{});
+  // Latest battery state provided by |battery_level_provider_|.
+  base::Optional<BatteryLevelProvider::BatteryState> battery_state_;
 
   // Time that should elapse between calls to RecordBatteryDischarge.
   const base::TimeDelta recording_interval_;
@@ -144,4 +101,4 @@ class PowerMetricsProvider : public metrics::MetricsProvider {
   scoped_refptr<Impl> impl_;
 };
 
-#endif  // CHROME_BROWSER_METRICS_POWER_METRICS_PROVIDER_MAC_H_
+#endif  // CHROME_BROWSER_METRICS_POWER_POWER_METRICS_PROVIDER_MAC_H_

chrome/browser/metrics/power_metrics_provider_mac.mm → chrome/browser/metrics/power/power_metrics_provider_mac.mm

@@ -2,15 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/metrics/power_metrics_provider_mac.h"
+#include "chrome/browser/metrics/power/power_metrics_provider_mac.h"
+
+#include <utility>
 
 #import <Foundation/Foundation.h>
-#include <IOKit/IOKitLib.h>
 #include <libkern/OSByteOrder.h>
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/mac/foundation_util.h"
 #include "base/mac/scoped_ioobject.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
@@ -33,24 +33,6 @@ constexpr base::TimeDelta kStartupPowerMetricsCollectionInterval =
 constexpr base::TimeDelta kPostStartupPowerMetricsCollectionInterval =
     base::TimeDelta::FromSeconds(60);
 
-// Returns the value corresponding to |key| in the dictionary |description|.
-// Returns an unpopulated value if the dictionary does not contain |key|, the
-// corresponding value is nullptr or it could not be converted to SInt64.
-base::Optional<SInt64> GetValueAsSInt64(CFDictionaryRef description,
-                                        CFStringRef key) {
-  base::Optional<SInt64> value;
-
-  CFNumberRef number =
-      base::mac::GetValueFromDictionary<CFNumberRef>(description, key);
-
-  SInt64 v;
-  if (number && CFNumberGetValue(number, kCFNumberSInt64Type, &v)) {
-    value.emplace(v);
-  }
-
-  return value;
-}
-
 // This API is undocumented. It can read hardware sensors including
 // temperature, voltage, and power. A useful tool for discovering new keys is
 // <https://github.com/theopolis/smc-fuzzer>. The following definitions are
@@ -215,100 +197,31 @@ ThermalStateUMA ThermalStateToUmaEnumValue(NSProcessInfoThermalState state) {
 
 }  // namespace
 
-PowerDrainRecorder::BatteryState::BatteryState(int capacity,
-                                               bool on_battery,
-                                               base::TimeTicks creation_time)
-    : capacity_(capacity),
-      on_battery_(on_battery),
-      creation_time_(creation_time) {}
-
-PowerDrainRecorder::BatteryState::BatteryState()
-    : on_battery_(base::PowerMonitor::IsOnBatteryPower()),
-      creation_time_(base::TimeTicks::Now()) {
-  // Retrieve the IOPMPowerSource service.
-  const base::mac::ScopedIOObject<io_service_t> service(
-      IOServiceGetMatchingService(kIOMasterPortDefault,
-                                  IOServiceMatching("IOPMPowerSource")));
-
-  // Gather a dictionary containing the power information.
-  base::ScopedCFTypeRef<CFMutableDictionaryRef> dict;
-  kern_return_t result = IORegistryEntryCreateCFProperties(
-      service.get(), dict.InitializeInto(), 0, 0);
-
-  // Retrieving dictionary failed. Cannot proceed.
-  if (result != KERN_SUCCESS) {
-    // Mark this recording as not on battery so it does not get used in
-    // calculations.
-    on_battery_ = false;
-    return;
-  }
-
-  CFStringRef capacity_key;
-  CFStringRef max_capacity_key;
-
-  // Use the correct key depending on macOS version.
-  if (@available(macOS 10.14.0, *)) {
-    capacity_key = CFSTR("AppleRawCurrentCapacity");
-    max_capacity_key = CFSTR("AppleRawMaxCapacity");
-  } else {
-    capacity_key = CFSTR("CurrentCapacity");
-    max_capacity_key = CFSTR("RawMaxCapacity");
-  }
-
-  // Extract the information from the dictionary.
-  base::Optional<SInt64> current_capacity =
-      GetValueAsSInt64(dict, capacity_key);
-  base::Optional<SInt64> max_capacity =
-      GetValueAsSInt64(dict, max_capacity_key);
-
-  // If any of the values were not available.
-  if (!current_capacity.has_value() || !max_capacity.has_value()) {
-    // Mark this recording as not on battery so it does not get used in
-    // calculations.
-    on_battery_ = false;
-    return;
-  }
-
-  double ratio = static_cast<double>(current_capacity.value()) /
-                 static_cast<double>(max_capacity.value());
-
-  // |ratio| is a the result of dividing |current_capacity| by |max_capacity|.
-  // |current_capacity| is smaller or equal to |max_capacity| so the ratio is
-  // in range [0.00, 1.00]. The difference between two of these values is what
-  // will be stored in a histogram. This histogram will have a bucket of size
-  // 1000 so the value is multiplied by 10000 to bring it in the range of
-  // [0, 10000].
-  constexpr int kScalingFactor = 10000;
-  capacity_ = kScalingFactor * ratio;
-}
-
-void PowerDrainRecorder::BatteryState::SetIsOnBattery(bool on_battery) {
-  on_battery_ = on_battery;
-}
-
 PowerDrainRecorder::PowerDrainRecorder(base::TimeDelta recording_interval)
     : recording_interval_(recording_interval) {}
 PowerDrainRecorder::~PowerDrainRecorder() = default;
 
 void PowerDrainRecorder::RecordBatteryDischarge() {
-  BatteryState current_state = battery_state_callback_.Run();
+  base::Optional<BatteryLevelProvider::BatteryState> previous_battery_state =
+      std::exchange(battery_state_, battery_level_provider_->GetBatteryState());
 
-  bool should_record = true;
+  // Missing battery values.
+  if (!battery_state_.has_value() || !previous_battery_state.has_value())
+    return;
 
   // Not discharging.
-  if (!current_state.on_battery() || !previous_battery_state_.on_battery()) {
-    should_record = false;
-  }
+  if (!battery_state_->on_battery || !previous_battery_state->on_battery)
+    return;
 
-  if (current_state.capacity() > previous_battery_state_.capacity()) {
-    // Capacity went up since last measurement. It's suspected the computer
+  if (battery_state_->charge_level > previous_battery_state->charge_level) {
+    // Charge level went up since last measurement. It's suspected the computer
     // was charged for less than the collection interval. Consider this time
     // slice as not even "on battery".
-    should_record = false;
+    return;
   }
 
   const base::TimeDelta time_since_last_record =
-      current_state.creation_time() - previous_battery_state_.creation_time();
+      battery_state_->capture_time - previous_battery_state->capture_time;
 
   // Ratio by which the time elapsed can deviate from |recording_interval|
   // without invalidating this sample.
@@ -320,38 +233,35 @@ void PowerDrainRecorder::RecordBatteryDischarge() {
       (recording_interval_ * kTolerablePositiveDrift)) {
     // Too much time passed since the last record. Either the task took
     // too long to get executed or system sleep took place.
-    should_record = false;
+    return;
   }
 
   if (time_since_last_record <
       (recording_interval_ * kTolerableNegativeDrift)) {
     // The recording task executed too early after the previous one, possibly
     // because the previous task took too long to execute.
-    should_record = false;
-  }
-
-  if (should_record) {
-    // Use this to normalize all measurements to to recording period.
-    const double time_elapsed_ratio =
-        recording_interval_.InSeconds() / time_since_last_record.InSecondsF();
-
-    int discharge = time_elapsed_ratio * (previous_battery_state_.capacity() -
-                                          current_state.capacity());
-    base::UmaHistogramCounts1000("Power.Mac.BatteryDischarge", discharge);
+    return;
   }
 
-  // Update the battery state.
-  previous_battery_state_ = current_state;
-}
+  // |time_elapsed_ratio| is used to normalize discharge rate measurements to
+  // the expected recording period.
+  const double time_elapsed_ratio =
+      recording_interval_.InSeconds() / time_since_last_record.InSecondsF();
 
-void PowerDrainRecorder::SetGetBatteryStateCallBackForTesting(
-    base::RepeatingCallback<BatteryState()> callback) {
-  battery_state_callback_ = callback;
+  // The charge_level is a ratio in range [0.00, 1.00]. The difference between
+  // two of these values is what will be stored in a histogram. This histogram
+  // will have a bucket of size 1000 so the value is multiplied by 10000 to
+  // bring it in the range of [0, 10000].
+  constexpr int kScalingFactor = 10000;
+  int discharge =
+      kScalingFactor * time_elapsed_ratio *
+      (previous_battery_state->charge_level - battery_state_->charge_level);
+  base::UmaHistogramCounts1000("Power.Mac.BatteryDischarge", discharge);
 }
 
-// static
-PowerDrainRecorder::BatteryState PowerDrainRecorder::GetCurrentBatteryState() {
-  return PowerDrainRecorder::BatteryState{};
+void PowerDrainRecorder::SetBatteryLevelProviderForTesting(
+    std::unique_ptr<BatteryLevelProvider> provider) {
+  battery_level_provider_ = std::move(provider);
 }
 
 class PowerMetricsProvider::Impl : public base::RefCountedThreadSafe<Impl> {

chrome/test/BUILD.gn

@@ -1692,7 +1692,8 @@ if (!is_android) {
     }
 
     if (is_mac) {
-      sources += [ "../browser/metrics/power_metrics_provider_mac_unittest.cc" ]
+      sources +=
+          [ "../browser/metrics/power/power_metrics_provider_mac_unittest.cc" ]
     }
 
     if (is_chromeos_ash) {