Skip to content

Commit

Permalink
Move BatteryLevelProvider from c/b/metrics to base/power_monitor
Browse files Browse the repository at this point in the history
This is the first step in the implementation of UserPerformanceTuningManager
described in go/uv-performance-bsm-manager.

Bug: 1348590
Change-Id: I5fe5aba6d2679bda8d0e6078d074415f395c2e59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3791716
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031706}
  • Loading branch information
Anthony Vallee-Dubois authored and Chromium LUCI CQ committed Aug 5, 2022
1 parent 6272a11 commit eebe0fe
Show file tree
Hide file tree
Showing 14 changed files with 136 additions and 109 deletions.
14 changes: 14 additions & 0 deletions base/BUILD.gn
Expand Up @@ -535,6 +535,8 @@ mixed_component("base") {
"pending_task.h",
"pickle.cc",
"pickle.h",
"power_monitor/battery_level_provider.cc",
"power_monitor/battery_level_provider.h",
"power_monitor/moving_average.cc",
"power_monitor/moving_average.h",
"power_monitor/power_monitor.cc",
Expand Down Expand Up @@ -1499,6 +1501,7 @@ mixed_component("base") {
":logging_buildflags",
":orderfile_buildflags",
":parsing_buildflags",
":power_monitor_buildflags",
":profiler_buildflags",
":sanitizer_buildflags",
":synchronization_buildflags",
Expand Down Expand Up @@ -2059,6 +2062,7 @@ mixed_component("base") {
"files/file_enumerator_win.cc",
"memory/platform_shared_memory_mapper_win.cc",
"memory/platform_shared_memory_region_win.cc",
"power_monitor/battery_level_provider_win.cc",
"power_monitor/power_monitor_device_source_win.cc",
"power_monitor/speed_limit_observer_win.cc",
"power_monitor/speed_limit_observer_win.h",
Expand Down Expand Up @@ -2116,6 +2120,7 @@ mixed_component("base") {
"memory/platform_shared_memory_region_mac.cc",
"message_loop/message_pump_kqueue.cc",
"message_loop/message_pump_kqueue.h",
"power_monitor/battery_level_provider_mac.mm",
"power_monitor/power_monitor_device_source_mac.mm",
"power_monitor/thermal_state_observer_mac.h",
"power_monitor/thermal_state_observer_mac.mm",
Expand Down Expand Up @@ -2634,6 +2639,14 @@ buildflag_header("profiler_buildflags") {
]
}

buildflag_header("power_monitor_buildflags") {
header = "power_monitor_buildflags.h"
header_dir = "base/power_monitor"
_has_battery_provider_impl = is_win || is_mac

flags = [ "HAS_BATTERY_LEVEL_PROVIDER_IMPL=$_has_battery_provider_impl" ]
}

# This is the subset of files from base that should not be used with a dynamic
# library. Note that this library cannot depend on base because base depends on
# base_static.
Expand Down Expand Up @@ -3189,6 +3202,7 @@ test("base_unittests") {
"parameter_pack_unittest.cc",
"path_service_unittest.cc",
"pickle_unittest.cc",
"power_monitor/battery_level_provider_unittest.cc",
"power_monitor/moving_average_unittest.cc",
"power_monitor/power_monitor_device_source_unittest.cc",
"power_monitor/power_monitor_unittest.cc",
Expand Down
Expand Up @@ -2,11 +2,13 @@
// 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"
#include "base/power_monitor/battery_level_provider.h"

#include "base/ranges/algorithm.h"

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

namespace base {

BatteryLevelProvider::BatteryState BatteryLevelProvider::MakeBatteryState(
const std::vector<BatteryDetails>& battery_details) {
Expand Down Expand Up @@ -35,4 +37,6 @@ BatteryLevelProvider::BatteryState BatteryLevelProvider::MakeBatteryState(
return state;
}

#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
} // namespace base

#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)
Expand Up @@ -2,27 +2,27 @@
// 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_
#ifndef BASE_POWER_MONITOR_BATTERY_LEVEL_PROVIDER_H_
#define BASE_POWER_MONITOR_BATTERY_LEVEL_PROVIDER_H_

#include <stdint.h>
#include <memory>
#include <vector>

#include "base/callback.h"
#include "base/power_monitor/power_monitor_buildflags.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

#define HAS_BATTERY_LEVEL_PROVIDER_IMPL() \
(BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC))
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
namespace base {

// BatteryLevelProvider provides an interface for querying battery state.
// A platform specific implementation is obtained with
// BatteryLevelProvider::Create().
class BatteryLevelProvider {
class BASE_EXPORT BatteryLevelProvider {
public:
// The three possible units of data returned by OS battery query functions,
// kMWh and kMAh are self-explanatory and the desired state of things, while
Expand Down Expand Up @@ -96,6 +96,8 @@ class BatteryLevelProvider {
const std::vector<BatteryDetails>& battery_details);
};

#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
} // namespace base

#endif // CHROME_BROWSER_METRICS_POWER_BATTERY_LEVEL_PROVIDER_H_
#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#endif // BASE_POWER_MONITOR_BATTERY_LEVEL_PROVIDER_H_
Expand Up @@ -2,7 +2,7 @@
// 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"
#include "base/power_monitor/battery_level_provider.h"

#import <Foundation/Foundation.h>
#include <IOKit/IOKitLib.h>
Expand All @@ -12,6 +12,7 @@
#include "base/mac/scoped_cftyperef.h"
#include "base/mac/scoped_ioobject.h"

namespace base {
namespace {

// Returns the value corresponding to |key| in the dictionary |description|.
Expand Down Expand Up @@ -131,3 +132,5 @@ void GetBatteryState(
.full_charged_capacity = static_cast<uint64_t>(max_capacity.value()),
.charge_unit = BatteryLevelUnit::kMAh}});
}

} // namespace base
Expand Up @@ -2,12 +2,13 @@
// 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"
#include "base/power_monitor/battery_level_provider.h"

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#include "testing/gtest/include/gtest/gtest.h"

namespace base {
namespace {

class FakeBatteryLevelProvider : public BatteryLevelProvider {
Expand Down Expand Up @@ -89,4 +90,6 @@ TEST(BatteryLevelProviderTest, MultipleBatteriesDischarging) {
EXPECT_NE(base::TimeTicks(), state.capture_time);
}

#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
} // namespace base

#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)
Expand Up @@ -2,7 +2,7 @@
// 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"
#include "base/power_monitor/battery_level_provider.h"

#define INITGUID
#include <windows.h> // Must be in front of other Windows header files.
Expand All @@ -22,6 +22,7 @@
#include "base/win/scoped_devinfo.h"
#include "base/win/scoped_handle.h"

namespace base {
namespace {

// Returns a handle to the battery interface identified by |interface_data|, or
Expand Down Expand Up @@ -63,7 +64,7 @@ base::win::ScopedHandle GetBatteryHandle(
// no battery present in this interface or nullopt on retrieval error.
// See
// https://docs.microsoft.com/en-us/windows/win32/power/ioctl-battery-query-tag
absl::optional<uint64_t> GetBatteryTag(HANDLE battery) {
absl::optional<ULONG> GetBatteryTag(HANDLE battery) {
ULONG battery_tag = 0;
ULONG wait = 0;
DWORD bytes_returned = 0;
Expand All @@ -88,9 +89,8 @@ absl::optional<uint64_t> GetBatteryTag(HANDLE battery) {
// Returns BATTERY_INFORMATION structure containing battery information, given
// battery handle and tag, or nullopt if the request failed. Battery handle and
// tag are obtained with GetBatteryHandle() and GetBatteryTag(), respectively.
absl::optional<BATTERY_INFORMATION> GetBatteryInformation(
HANDLE battery,
uint64_t battery_tag) {
absl::optional<BATTERY_INFORMATION> GetBatteryInformation(HANDLE battery,
ULONG battery_tag) {
BATTERY_QUERY_INFORMATION query_information = {};
query_information.BatteryTag = battery_tag;
query_information.InformationLevel = BatteryInformation;
Expand All @@ -109,7 +109,7 @@ absl::optional<BATTERY_INFORMATION> GetBatteryInformation(
// handle and tag, or nullopt if the request failed. Battery handle and tag are
// obtained with GetBatteryHandle() and GetBatteryTag(), respectively.
absl::optional<BATTERY_STATUS> GetBatteryStatus(HANDLE battery,
uint64_t battery_tag) {
ULONG battery_tag) {
BATTERY_WAIT_STATUS wait_status = {};
wait_status.BatteryTag = battery_tag;
BATTERY_STATUS battery_status;
Expand Down Expand Up @@ -153,8 +153,9 @@ class BatteryLevelProviderWin : public BatteryLevelProvider {
// TaskRunner used to run blocking `GetBatteryStateImpl()` queries, sequenced
// to avoid the performance cost of concurrent calls.
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_{
base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})};
base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(),
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})};

base::WeakPtrFactory<BatteryLevelProviderWin> weak_ptr_factory_{this};
};
Expand Down Expand Up @@ -186,7 +187,7 @@ BatteryLevelProviderWin::GetBatteryStateImpl() {
// https://docs.microsoft.com/en-us/windows/win32/power/enumerating-battery-devices
// Limit search to 8 batteries max. A system may have several battery slots
// and each slot may hold an actual battery.
for (int device_index = 0; device_index < 8; ++device_index) {
for (DWORD device_index = 0; device_index < 8; ++device_index) {
SP_DEVICE_INTERFACE_DATA interface_data = {};
interface_data.cbSize = sizeof(interface_data);

Expand All @@ -206,7 +207,7 @@ BatteryLevelProviderWin::GetBatteryStateImpl() {
if (!battery.IsValid())
return absl::nullopt;

absl::optional<uint64_t> battery_tag = GetBatteryTag(battery.Get());
absl::optional<ULONG> battery_tag = GetBatteryTag(battery.Get());
if (!battery_tag.has_value()) {
return absl::nullopt;
} else if (battery_tag.value() == BATTERY_TAG_INVALID) {
Expand Down Expand Up @@ -238,3 +239,5 @@ BatteryLevelProviderWin::GetBatteryStateImpl() {

return MakeBatteryState(battery_details_list);
}

} // namespace base
4 changes: 0 additions & 4 deletions chrome/browser/BUILD.gn
Expand Up @@ -3932,8 +3932,6 @@ static_library("browser") {
"metrics/first_web_contents_profiler_base.cc",
"metrics/first_web_contents_profiler_base.h",
"metrics/incognito_observer_desktop.cc",
"metrics/power/battery_level_provider.cc",
"metrics/power/battery_level_provider.h",
"metrics/power/power_metrics.cc",
"metrics/power/power_metrics.h",
"metrics/power/power_metrics_constants.cc",
Expand Down Expand Up @@ -5835,7 +5833,6 @@ static_library("browser") {
"metrics/google_update_metrics_provider_win.h",
"metrics/jumplist_metrics_win.cc",
"metrics/jumplist_metrics_win.h",
"metrics/power/battery_level_provider_win.cc",
"metrics/tab_stats/tab_stats_tracker_delegate_win.cc",
"metrics/tab_stats/tab_stats_tracker_win.cc",
"net/chrome_mojo_proxy_resolver_win.cc",
Expand Down Expand Up @@ -6121,7 +6118,6 @@ 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/battery_level_provider_mac.mm",
"metrics/power/coalition_resource_usage_provider_mac.h",
"metrics/power/coalition_resource_usage_provider_mac.mm",
"metrics/power/power_metrics_provider_mac.h",
Expand Down
Expand Up @@ -31,7 +31,6 @@
#include "chrome/browser/enterprise/browser_management/management_service_factory.h"
#include "chrome/browser/google/google_brand.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/browser/metrics/power/battery_level_provider.h"
#include "chrome/browser/metrics/power/power_metrics_reporter.h"
#include "chrome/browser/metrics/power/process_monitor.h"
#include "chrome/browser/metrics/process_memory_metrics_emitter.h"
Expand Down
17 changes: 9 additions & 8 deletions chrome/browser/metrics/power/power_metrics.cc
Expand Up @@ -16,12 +16,12 @@

namespace {

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)
constexpr const char* kBatteryDischargeRateHistogramName =
"Power.BatteryDischargeRate2";
constexpr const char* kBatteryDischargeModeHistogramName =
"Power.BatteryDischargeMode2";
#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#if BUILDFLAG(IS_MAC)
// Reports `proportion` of a time used to a histogram in permyriad (1/100 %).
Expand Down Expand Up @@ -63,11 +63,12 @@ void ReportAggregatedProcessMetricsHistograms(
}
}

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)
BatteryDischarge GetBatteryDischargeDuringInterval(
const absl::optional<BatteryLevelProvider::BatteryState>&
const absl::optional<base::BatteryLevelProvider::BatteryState>&
previous_battery_state,
const absl::optional<BatteryLevelProvider::BatteryState>& new_battery_state,
const absl::optional<base::BatteryLevelProvider::BatteryState>&
new_battery_state,
base::TimeDelta interval_duration) {
if (!previous_battery_state.has_value() || !new_battery_state.has_value()) {
return {BatteryDischargeMode::kRetrievalError, absl::nullopt};
Expand All @@ -88,9 +89,9 @@ BatteryDischarge GetBatteryDischargeDuringInterval(
return {BatteryDischargeMode::kMultipleBatteries, absl::nullopt};
}
if ((previous_battery_state->charge_unit ==
BatteryLevelProvider::BatteryLevelUnit::kRelative) ||
base::BatteryLevelProvider::BatteryLevelUnit::kRelative) ||
(new_battery_state->charge_unit ==
BatteryLevelProvider::BatteryLevelUnit::kRelative)) {
base::BatteryLevelProvider::BatteryLevelUnit::kRelative)) {
return {BatteryDischargeMode::kInsufficientResolution, absl::nullopt};
}

Expand Down Expand Up @@ -152,7 +153,7 @@ void ReportBatteryHistograms(base::TimeDelta interval_duration,
}
}
}
#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#if BUILDFLAG(IS_MAC)
void ReportShortIntervalHistograms(
Expand Down
11 changes: 6 additions & 5 deletions chrome/browser/metrics/power/power_metrics.h
Expand Up @@ -7,9 +7,9 @@

#include <vector>

#include "base/power_monitor/battery_level_provider.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "chrome/browser/metrics/power/battery_level_provider.h"
#include "chrome/browser/metrics/power/process_monitor.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

Expand Down Expand Up @@ -46,21 +46,22 @@ struct BatteryDischarge {
absl::optional<int64_t> rate;
};

#if HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#if BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)
// Computes and returns the battery discharge mode and rate during the interval.
// If the discharge rate isn't valid, the returned rate is nullopt and the
// reason is indicated per BatteryDischargeMode.
BatteryDischarge GetBatteryDischargeDuringInterval(
const absl::optional<BatteryLevelProvider::BatteryState>&
const absl::optional<base::BatteryLevelProvider::BatteryState>&
previous_battery_state,
const absl::optional<BatteryLevelProvider::BatteryState>& new_battery_state,
const absl::optional<base::BatteryLevelProvider::BatteryState>&
new_battery_state,
base::TimeDelta interval_duration);

// Report battery metrics to histograms with |suffixes|.
void ReportBatteryHistograms(base::TimeDelta interval_duration,
BatteryDischarge battery_discharge,
const std::vector<const char*>& suffixes);
#endif // HAS_BATTERY_LEVEL_PROVIDER_IMPL()
#endif // BUILDFLAG(HAS_BATTERY_LEVEL_PROVIDER_IMPL)

#if BUILDFLAG(IS_MAC)
void ReportShortIntervalHistograms(
Expand Down

0 comments on commit eebe0fe

Please sign in to comment.