libchrome: Remove backward compatibility patch for metrics
BUG=b:329506713
TEST=CQ
Cq-Depend: chromium:5372947
Change-Id: Iac33f42275b8e962716d3fa944bf458ecafdb769
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/libchrome/+/5373065
Commit-Queue: Nathan Muggli <nmuggli@google.com>
Reviewed-by: Qijiang Yūki Ishii <fqj@google.com>
Tested-by: Nathan Muggli <nmuggli@google.com>
diff --git a/libchrome_tools/patches/backward-compatibility-0200-Revert-Return-nullopt-on-error-from-ProcessMetrics-C.patch b/libchrome_tools/patches/backward-compatibility-0200-Revert-Return-nullopt-on-error-from-ProcessMetrics-C.patch
deleted file mode 100644
index 24bf4eb..0000000
--- a/libchrome_tools/patches/backward-compatibility-0200-Revert-Return-nullopt-on-error-from-ProcessMetrics-C.patch
+++ /dev/null
@@ -1,458 +0,0 @@
-From 1793b7e0837efdf44ba08e9289ac0d24727a1947 Mon Sep 17 00:00:00 2001
-From: Nathan Muggli <nmuggli@google.com>
-Date: Wed, 13 Mar 2024 16:07:37 -0600
-Subject: [PATCH] Revert "Return nullopt on error from ProcessMetrics CPU
- measurements"
-
-This reverts commit 94912573bfad4a687bc86cbbcbafad7ec0dad19e.
----
- base/process/internal_linux.cc | 15 ++---
- base/process/internal_linux.h | 9 ---
- base/process/process_metrics.cc | 8 +--
- base/process/process_metrics.h | 15 ++---
- base/process/process_metrics_apple.cc | 14 ++---
- base/process/process_metrics_linux.cc | 56 ++++++++----------
- base/process/process_metrics_unittest.cc | 72 ++++++++++++++++--------
- ipc/ipc_cpu_perftest.cc | 6 +-
- 8 files changed, 90 insertions(+), 105 deletions(-)
-
-diff --git a/base/process/internal_linux.cc b/base/process/internal_linux.cc
-index c8f38dc28c..b9d3f0f851 100644
---- a/base/process/internal_linux.cc
-+++ b/base/process/internal_linux.cc
-@@ -14,7 +14,6 @@
- #include "base/files/file_util.h"
- #include "base/logging.h"
- #include "base/notreached.h"
--#include "base/numerics/safe_conversions.h"
- #include "base/strings/string_number_conversions.h"
- #include "base/strings/string_split.h"
- #include "base/strings/string_util.h"
-@@ -207,17 +206,10 @@ void ParseProcStat(const std::string& contents, ProcStatMap* output) {
- int64_t GetProcStatsFieldAsInt64(const std::vector<std::string>& proc_stats,
- ProcStatsFields field_num) {
- DCHECK_GE(field_num, VM_PPID);
-- return GetProcStatsFieldAsOptionalInt64(proc_stats, field_num).value_or(0);
--}
-+ CHECK_LT(static_cast<size_t>(field_num), proc_stats.size());
-
--std::optional<int64_t> GetProcStatsFieldAsOptionalInt64(
-- base::span<const std::string> proc_stats,
-- ProcStatsFields field_num) {
- int64_t value;
-- if (StringToInt64(proc_stats[size_t{field_num}], &value)) {
-- return value;
-- }
-- return std::nullopt;
-+ return StringToInt64(proc_stats[field_num], &value) ? value : 0;
- }
-
- size_t GetProcStatsFieldAsSizeT(const std::vector<std::string>& proc_stats,
-@@ -250,7 +242,8 @@ int64_t ReadProcSelfStatsAndGetFieldAsInt64(ProcStatsFields field_num) {
- return ReadStatFileAndGetFieldAsInt64(stat_file, field_num);
- }
-
--size_t ReadProcStatsAndGetFieldAsSizeT(pid_t pid, ProcStatsFields field_num) {
-+size_t ReadProcStatsAndGetFieldAsSizeT(pid_t pid,
-+ ProcStatsFields field_num) {
- std::string stats_data;
- if (!ReadProcStats(pid, &stats_data))
- return 0;
-diff --git a/base/process/internal_linux.h b/base/process/internal_linux.h
-index e7b70601bc..a2657a7bc3 100644
---- a/base/process/internal_linux.h
-+++ b/base/process/internal_linux.h
-@@ -11,11 +11,9 @@
- #include <stddef.h>
- #include <stdint.h>
- #include <unistd.h>
--#include <optional>
- #include <string>
- #include <vector>
-
--#include "base/containers/span.h"
- #include "base/files/dir_reader_posix.h"
- #include "base/files/file_path.h"
- #include "base/process/process_handle.h"
-@@ -102,13 +100,6 @@ enum ProcStatsFields {
- int64_t GetProcStatsFieldAsInt64(const std::vector<std::string>& proc_stats,
- ProcStatsFields field_num);
-
--// Reads the `field_num`th field from `proc_stats`. Asserts that `field_num` is
--// a valid index into `proc_stats`. Returns nullopt if the field doesn't contain
--// a valid integer.
--std::optional<int64_t> GetProcStatsFieldAsOptionalInt64(
-- base::span<const std::string> proc_stats,
-- ProcStatsFields field_num);
--
- // Same as GetProcStatsFieldAsInt64(), but for size_t values.
- size_t GetProcStatsFieldAsSizeT(const std::vector<std::string>& proc_stats,
- ProcStatsFields field_num);
-diff --git a/base/process/process_metrics.cc b/base/process/process_metrics.cc
-index e9d90dcad8..e59252c6e3 100644
---- a/base/process/process_metrics.cc
-+++ b/base/process/process_metrics.cc
-@@ -123,12 +123,8 @@ double ProcessMetrics::GetPlatformIndependentCPUUsage(
- return 100.0 * cpu_time_delta / time_delta;
- }
-
--std::optional<double> ProcessMetrics::GetPlatformIndependentCPUUsage() {
-- const std::optional<TimeDelta> cpu_usage = GetCumulativeCPUUsage();
-- if (!cpu_usage.has_value()) {
-- return std::nullopt;
-- }
-- return GetPlatformIndependentCPUUsage(cpu_usage.value());
-+double ProcessMetrics::GetPlatformIndependentCPUUsage() {
-+ return GetPlatformIndependentCPUUsage(GetCumulativeCPUUsage());
- }
- #endif
-
-diff --git a/base/process/process_metrics.h b/base/process/process_metrics.h
-index db4ea9d024..b9688fe6ed 100644
---- a/base/process/process_metrics.h
-+++ b/base/process/process_metrics.h
-@@ -12,7 +12,6 @@
- #include <stdint.h>
-
- #include <memory>
--#include <optional>
-
- #include "base/base_export.h"
- #include "base/gtest_prod_util.h"
-@@ -125,16 +124,14 @@ class BASE_EXPORT ProcessMetrics {
- [[nodiscard]] double GetPlatformIndependentCPUUsage(TimeDelta cumulative_cpu);
-
- // Same as the above, but automatically calls GetCumulativeCPUUsage() to
-- // determine the current cumulative CPU. Returns nullopt if
-- // GetCumulativeCPUUsage() fails.
-- [[nodiscard]] std::optional<double> GetPlatformIndependentCPUUsage();
-+ // determine the current cumulative CPU.
-+ [[nodiscard]] double GetPlatformIndependentCPUUsage();
-
- // Returns the cumulative CPU usage across all threads of the process since
-- // process start, or nullopt on error. In case of multi-core processors, a
-- // process can consume CPU at a rate higher than wall-clock time, e.g. two
-- // cores at full utilization will result in a time delta of 2 seconds/per 1
-- // wall-clock second.
-- [[nodiscard]] std::optional<TimeDelta> GetCumulativeCPUUsage();
-+ // process start. In case of multi-core processors, a process can consume CPU
-+ // at a rate higher than wall-clock time, e.g. two cores at full utilization
-+ // will result in a time delta of 2 seconds/per 1 wall-clock second.
-+ [[nodiscard]] TimeDelta GetCumulativeCPUUsage();
-
- #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID) || \
- BUILDFLAG(IS_AIX)
-diff --git a/base/process/process_metrics_apple.cc b/base/process/process_metrics_apple.cc
-index 685e0c4360..1a23c8a083 100644
---- a/base/process/process_metrics_apple.cc
-+++ b/base/process/process_metrics_apple.cc
-@@ -11,8 +11,6 @@
- #include <stdint.h>
- #include <sys/sysctl.h>
-
--#include <optional>
--
- #include "base/apple/mach_logging.h"
- #include "base/apple/scoped_mach_port.h"
- #include "base/logging.h"
-@@ -95,10 +93,10 @@ mach_port_t ProcessMetrics::TaskForHandle(ProcessHandle process_handle) const {
- return task;
- }
-
--std::optional<TimeDelta> ProcessMetrics::GetCumulativeCPUUsage() {
-+TimeDelta ProcessMetrics::GetCumulativeCPUUsage() {
- mach_port_t task = TaskForHandle(process_);
- if (task == MACH_PORT_NULL) {
-- return std::nullopt;
-+ return TimeDelta();
- }
-
- // Libtop explicitly loops over the threads (libtop_pinfo_update_cpu_usage()
-@@ -110,12 +108,12 @@ std::optional<TimeDelta> ProcessMetrics::GetCumulativeCPUUsage() {
- &thread_info_count);
- if (kr != KERN_SUCCESS) {
- // Most likely cause: |task| is a zombie.
-- return std::nullopt;
-+ return TimeDelta();
- }
-
- task_basic_info_64 task_info_data;
- if (!GetTaskInfo(task, &task_info_data)) {
-- return std::nullopt;
-+ return TimeDelta();
- }
-
- /* Set total_time. */
-@@ -139,10 +137,10 @@ std::optional<TimeDelta> ProcessMetrics::GetCumulativeCPUUsage() {
- // a lag before it shows up in the terminated thread times returned by
- // GetTaskInfo(). Make sure CPU usage doesn't appear to go backwards if
- // GetCumulativeCPUUsage() is called in the interval.
-- return std::optional(last_measured_cpu_);
-+ return last_measured_cpu_;
- }
- last_measured_cpu_ = measured_cpu;
-- return std::optional(measured_cpu);
-+ return measured_cpu;
- }
-
- int ProcessMetrics::GetPackageIdleWakeupsPerSecond() {
-diff --git a/base/process/process_metrics_linux.cc b/base/process/process_metrics_linux.cc
-index 0ed005ff09..5d11ac6638 100644
---- a/base/process/process_metrics_linux.cc
-+++ b/base/process/process_metrics_linux.cc
-@@ -17,7 +17,6 @@
- #include <optional>
- #include <utility>
-
--#include "base/containers/span.h"
- #include "base/cpu.h"
- #include "base/files/dir_reader_posix.h"
- #include "base/files/file_util.h"
-@@ -58,26 +57,24 @@ uint64_t ReadFileToUint64(const FilePath& file) {
- }
- #endif
-
--// Get the total CPU from a proc stat buffer. Return value is a TimeDelta
--// converted from a number of jiffies on success or nullopt if parsing failed.
--std::optional<TimeDelta> ParseTotalCPUTimeFromStats(
-- base::span<const std::string> proc_stats) {
-- const std::optional<int64_t> utime =
-- internal::GetProcStatsFieldAsOptionalInt64(proc_stats,
-- internal::VM_UTIME);
-- if (utime.value_or(-1) < 0) {
-- return std::nullopt;
-- }
-- const std::optional<int64_t> stime =
-- internal::GetProcStatsFieldAsOptionalInt64(proc_stats,
-- internal::VM_UTIME);
-- if (stime.value_or(-1) < 0) {
-- return std::nullopt;
-+// Get the total CPU from a proc stat buffer. Return value is number of jiffies
-+// on success or 0 if parsing failed.
-+int64_t ParseTotalCPUTimeFromStats(const std::vector<std::string>& proc_stats) {
-+ return internal::GetProcStatsFieldAsInt64(proc_stats, internal::VM_UTIME) +
-+ internal::GetProcStatsFieldAsInt64(proc_stats, internal::VM_STIME);
-+}
-+
-+// Get the total CPU of a single process. Return value is number of jiffies
-+// on success or -1 on error.
-+int64_t GetProcessCPU(pid_t pid) {
-+ std::string buffer;
-+ std::vector<std::string> proc_stats;
-+ if (!internal::ReadProcStats(pid, &buffer) ||
-+ !internal::ParseProcStats(buffer, &proc_stats)) {
-+ return -1;
- }
-- const TimeDelta cpu_time = internal::ClockTicksToTimeDelta(
-- base::ClampAdd(utime.value(), stime.value()));
-- CHECK(!cpu_time.is_negative());
-- return std::optional(cpu_time);
-+
-+ return ParseTotalCPUTimeFromStats(proc_stats);
- }
-
- } // namespace
-@@ -93,15 +90,8 @@ size_t ProcessMetrics::GetResidentSetSize() const {
- checked_cast<size_t>(getpagesize());
- }
-
--std::optional<TimeDelta> ProcessMetrics::GetCumulativeCPUUsage() {
-- std::string buffer;
-- std::vector<std::string> proc_stats;
-- if (!internal::ReadProcStats(process_, &buffer) ||
-- !internal::ParseProcStats(buffer, &proc_stats)) {
-- return std::nullopt;
-- }
--
-- return ParseTotalCPUTimeFromStats(proc_stats);
-+TimeDelta ProcessMetrics::GetCumulativeCPUUsage() {
-+ return internal::ClockTicksToTimeDelta(GetProcessCPU(process_));
- }
-
- bool ProcessMetrics::GetCumulativeCPUUsagePerThread(
-@@ -120,11 +110,9 @@ bool ProcessMetrics::GetCumulativeCPUUsagePerThread(
- return;
- }
-
-- const std::optional<TimeDelta> thread_time =
-- ParseTotalCPUTimeFromStats(proc_stats);
-- if (thread_time.has_value()) {
-- cpu_per_thread.emplace_back(tid, thread_time.value());
-- }
-+ TimeDelta thread_time = internal::ClockTicksToTimeDelta(
-+ ParseTotalCPUTimeFromStats(proc_stats));
-+ cpu_per_thread.emplace_back(tid, thread_time);
- });
-
- return !cpu_per_thread.empty();
-diff --git a/base/process/process_metrics_unittest.cc b/base/process/process_metrics_unittest.cc
-index 230d22499e..84baef0f11 100644
---- a/base/process/process_metrics_unittest.cc
-+++ b/base/process/process_metrics_unittest.cc
-@@ -8,7 +8,6 @@
- #include <stdint.h>
-
- #include <memory>
--#include <optional>
- #include <sstream>
- #include <string>
- #include <utility>
-@@ -37,7 +36,6 @@
- #include "build/blink_buildflags.h"
- #include "build/build_config.h"
- #include "build/chromeos_buildflags.h"
--#include "testing/gmock/include/gmock/gmock.h"
- #include "testing/gtest/include/gtest/gtest.h"
- #include "testing/multiprocess_func_list.h"
-
-@@ -66,12 +64,6 @@ namespace base::debug {
-
- namespace {
-
--using ::testing::AssertionFailure;
--using ::testing::AssertionResult;
--using ::testing::AssertionSuccess;
--using ::testing::Ge;
--using ::testing::Optional;
--
- #if ENABLE_CPU_TESTS
-
- void BusyWork(std::vector<std::string>* vec) {
-@@ -82,21 +74,19 @@ void BusyWork(std::vector<std::string>* vec) {
- }
- }
-
--// Tests that GetCumulativeCPUUsage() returns a valid result that's equal to or
--// greater than `prev_cpu_usage`, and returns the result. If
--// GetCumulativeCPUUsage() returns an error, records a failed expectation and
--// returns an empty TimeDelta so that each test doesn't need to check for
--// nullopt repeatedly.
- TimeDelta TestCumulativeCPU(ProcessMetrics* metrics, TimeDelta prev_cpu_usage) {
-- const std::optional<TimeDelta> current_cpu_usage =
-- metrics->GetCumulativeCPUUsage();
-- EXPECT_THAT(current_cpu_usage, Optional(Ge(prev_cpu_usage)));
-- EXPECT_THAT(metrics->GetPlatformIndependentCPUUsage(), Optional(Ge(0.0)));
-- return current_cpu_usage.value_or(TimeDelta());
-+ const TimeDelta current_cpu_usage = metrics->GetCumulativeCPUUsage();
-+ EXPECT_GE(current_cpu_usage, prev_cpu_usage);
-+ EXPECT_GE(metrics->GetPlatformIndependentCPUUsage(), 0.0);
-+ return current_cpu_usage;
- }
-
- #endif // ENABLE_CPU_TESTS
-
-+using ::testing::AssertionFailure;
-+using ::testing::AssertionResult;
-+using ::testing::AssertionSuccess;
-+
- // Helper to deal with Mac process launching complexity. On other platforms this
- // is just a thin wrapper around SpawnMultiProcessTestChild.
- class TestChildLauncher {
-@@ -634,7 +624,7 @@ TEST_F(SystemMetricsTest, TestNoNegativeCpuUsage) {
- std::unique_ptr<ProcessMetrics> metrics =
- ProcessMetrics::CreateCurrentProcessMetrics();
-
-- EXPECT_THAT(metrics->GetPlatformIndependentCPUUsage(), Optional(Ge(0.0)));
-+ EXPECT_GE(metrics->GetPlatformIndependentCPUUsage(), 0.0);
-
- Thread thread1("thread1");
- Thread thread2("thread2");
-@@ -697,15 +687,38 @@ TEST_F(SystemMetricsTest, MeasureChildCpuUsage) {
- #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_FUCHSIA)
- // Windows and Fuchsia return final measurements of a process after it exits.
- TestCumulativeCPU(metrics.get(), cpu_usage2);
-+#elif BUILDFLAG(IS_APPLE)
-+ // Mac and iOS return 0 on error. GetPlatformIndependentCPUUsage subtracts
-+ // from this to get a negative.
-+ EXPECT_EQ(metrics->GetCumulativeCPUUsage(), TimeDelta());
-+ EXPECT_LT(metrics->GetPlatformIndependentCPUUsage(), 0.0);
- #else
-- // All other platforms return an error.
-- EXPECT_EQ(metrics->GetCumulativeCPUUsage(), std::nullopt);
-- EXPECT_EQ(metrics->GetPlatformIndependentCPUUsage(), std::nullopt);
-+ // All other platforms return a negative time value to indicate an error.
-+ EXPECT_LT(metrics->GetCumulativeCPUUsage(), TimeDelta());
-+ EXPECT_LT(metrics->GetPlatformIndependentCPUUsage(), 0.0);
- #endif
- }
-
- #endif // !BUILDFLAG(IS_APPLE) || BUILDFLAG(USE_BLINK)
-
-+#if BUILDFLAG(IS_FUCHSIA)
-+
-+// Fuchsia CHECK's when measuring an invalid procses.
-+using SystemMetricsDeathTest = SystemMetricsTest;
-+TEST_F(SystemMetricsDeathTest, InvalidProcessCpuUsage) {
-+ std::unique_ptr<ProcessMetrics> metrics =
-+ ProcessMetrics::CreateProcessMetrics(kNullProcessHandle);
-+ EXPECT_CHECK_DEATH(
-+ { [[maybe_unused]] TimeDelta usage = metrics->GetCumulativeCPUUsage(); });
-+ EXPECT_CHECK_DEATH({
-+ [[maybe_unused]] double usage = metrics->GetPlatformIndependentCPUUsage();
-+ });
-+}
-+
-+#else // !BUILDFLAG(IS_FUCHSIA)
-+
-+// Windows, Mac and iOS also return 0 on error. All other platforms return a
-+// negative value to indicate an error.
- TEST_F(SystemMetricsTest, InvalidProcessCpuUsage) {
- #if BUILDFLAG(IS_MAC)
- std::unique_ptr<ProcessMetrics> metrics =
-@@ -714,9 +727,20 @@ TEST_F(SystemMetricsTest, InvalidProcessCpuUsage) {
- std::unique_ptr<ProcessMetrics> metrics =
- ProcessMetrics::CreateProcessMetrics(kNullProcessHandle);
- #endif
-- EXPECT_EQ(metrics->GetCumulativeCPUUsage(), std::nullopt);
-- EXPECT_EQ(metrics->GetPlatformIndependentCPUUsage(), std::nullopt);
-+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_APPLE)
-+ EXPECT_EQ(metrics->GetCumulativeCPUUsage(), TimeDelta());
-+#else
-+ EXPECT_LT(metrics->GetCumulativeCPUUsage(), TimeDelta());
-+#endif
-+
-+ // The first call always caches the result of GetCumulativeCPUUsage() and
-+ // returns 0, even if it's an error value. The second call returns the delta
-+ // with the cached result, which is 0 when the second GetCumulativeCPUUsage()
-+ // call returns the same error value.
-+ EXPECT_EQ(metrics->GetPlatformIndependentCPUUsage(), 0.0);
-+ EXPECT_EQ(metrics->GetPlatformIndependentCPUUsage(), 0.0);
- }
-+#endif // !BUILDFLAG(IS_FUCHSIA)
-
- #endif // ENABLE_CPU_TESTS
-
-diff --git a/ipc/ipc_cpu_perftest.cc b/ipc/ipc_cpu_perftest.cc
-index dacbdc987b..a58d6b401a 100644
---- a/ipc/ipc_cpu_perftest.cc
-+++ b/ipc/ipc_cpu_perftest.cc
-@@ -3,7 +3,6 @@
- // found in the LICENSE file.
-
- #include <memory>
--#include <optional>
- #include <string_view>
- #include <tuple>
-
-@@ -74,7 +73,7 @@ class PerfCpuLogger {
- process_metrics_(base::ProcessMetrics::CreateCurrentProcessMetrics()) {
- // Query the CPU usage once to start the recording interval.
- const double inital_cpu_usage =
-- process_metrics_->GetPlatformIndependentCPUUsage().value_or(-1.0);
-+ process_metrics_->GetPlatformIndependentCPUUsage();
- // This should have been the first call so the reported cpu usage should be
- // exactly zero.
- DCHECK_EQ(inital_cpu_usage, 0.0);
-@@ -84,8 +83,7 @@ class PerfCpuLogger {
- PerfCpuLogger& operator=(const PerfCpuLogger&) = delete;
-
- ~PerfCpuLogger() {
-- const double result =
-- process_metrics_->GetPlatformIndependentCPUUsage().value_or(-1.0);
-+ double result = process_metrics_->GetPlatformIndependentCPUUsage();
- base::LogPerfResult(test_name_.c_str(), result, "%");
- }
-
---
-2.44.0.278.ge034bb2e1d-goog
-
