[battery] Don't assume that coalition_info_resource_usage will work if it worked before.

It was wrong to assume that coalition_info_resource_usage will return
a value if it returned a value in a previous call. It can fail if the
kernel can't allocate memory
(https://github.com/apple/darwin-xnu/blob/main/osfmk/kern/coalition.c#L736).

This CL makes the code resilient to that.

Bug: 1298733
Change-Id: I3f6700dd6911eef06b9d2666e715a3a72e9ddcb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3473471
Commit-Queue: Francois Pierre Doray <fdoray@chromium.org>
Auto-Submit: Francois Pierre Doray <fdoray@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/main@{#973002}

chrome/browser/metrics/power/coalition_resource_usage_provider_mac.mm

@@ -114,28 +114,28 @@ void CoalitionResourceUsageProvider::EndIntervals(
   std::unique_ptr<coalition_resource_usage> sample =
       GetCoalitionResourceUsage(coalition_id_.value());
 
-  // `coalition_id_` being non-nullptr (see above) indicates that
+  // `sample`, `short_interval_begin_sample_` or `long_interval_begin_sample_`
-  // GetCoalitionResourceUsage() succeeded from Init(). It should succeed again
+  // can be nullptr if the kernel couldn't allocate memory.
-  // unless this is a test.
-  DCHECK(sample || ignore_not_alone_for_testing_);
 
   if (sample) {
     DCHECK(!short_interval_begin_time_.is_null());
-    DCHECK(short_interval_begin_sample_);
     DCHECK(!long_interval_begin_time_.is_null());
-    DCHECK(long_interval_begin_sample_);
 
-    *short_interval_resource_usage_rate =
+    if (short_interval_begin_sample_) {
-        power_metrics::GetCoalitionResourceUsageRate(
+      *short_interval_resource_usage_rate =
-            *short_interval_begin_sample_, *sample,
+          power_metrics::GetCoalitionResourceUsageRate(
-            now - short_interval_begin_time_, timebase_,
+              *short_interval_begin_sample_, *sample,
-            energy_impact_coefficients_);
+              now - short_interval_begin_time_, timebase_,
+              energy_impact_coefficients_);
+    }
 
-    *long_interval_resource_usage_rate =
+    if (long_interval_begin_sample_) {
-        power_metrics::GetCoalitionResourceUsageRate(
+      *long_interval_resource_usage_rate =
-            *long_interval_begin_sample_, *sample,
+          power_metrics::GetCoalitionResourceUsageRate(
-            now - long_interval_begin_time_, timebase_,
+              *long_interval_begin_sample_, *sample,
-            energy_impact_coefficients_);
+              now - long_interval_begin_time_, timebase_,
+              energy_impact_coefficients_);
+    }
   }
 
   short_interval_begin_sample_.reset();

chrome/browser/metrics/power/coalition_resource_usage_provider_mac_unittest.mm

@@ -81,3 +81,60 @@ TEST(CoalitionResourceUsageProviderTest, StartAndEndIntervals) {
   EXPECT_EQ(short_rate->interrupt_wakeups_per_second, 2.8);
   EXPECT_EQ(long_rate->interrupt_wakeups_per_second, 0.96);
 }
+
+// Test that there is no crash if `GetCoalitionResourceUsage()` returns nullptr.
+// Regression test for crbug.com/1298733
+TEST(CoalitionResourceUsageProviderTest, CoalitionResourceUsageIsNull) {
+  base::test::SingleThreadTaskEnvironment task_environment(
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME);
+  TestCoalitionResourceUsageProvider provider;
+
+  absl::optional<power_metrics::CoalitionResourceUsageRate> short_rate;
+  absl::optional<power_metrics::CoalitionResourceUsageRate> long_rate;
+
+  // Begin long interval.
+  provider.SetCoalitionResourceUsage(
+      std::make_unique<coalition_resource_usage>());
+  provider.Init();
+
+  // Begin short interval. `GetCoalitionResourceUsage()` is nullptr.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.StartShortInterval();
+
+  // End both intervals and start long interval. `GetCoalitionResourceUsage()`
+  // is nullptr.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.EndIntervals(&short_rate, &long_rate);
+  EXPECT_FALSE(short_rate.has_value());
+  EXPECT_FALSE(long_rate.has_value());
+  short_rate.reset();
+  long_rate.reset();
+
+  // Begin short interval.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.SetCoalitionResourceUsage(
+      std::make_unique<coalition_resource_usage>());
+  provider.StartShortInterval();
+
+  // End both intervals and start long interval.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.SetCoalitionResourceUsage(
+      std::make_unique<coalition_resource_usage>());
+  provider.EndIntervals(&short_rate, &long_rate);
+  EXPECT_TRUE(short_rate.has_value());
+  EXPECT_FALSE(long_rate.has_value());
+  short_rate.reset();
+  long_rate.reset();
+
+  // Begin short interval. `GetCoalitionResourceUsage()` is nullptr.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.StartShortInterval();
+
+  // End both intervals and start long interval.
+  task_environment.FastForwardBy(base::Seconds(1));
+  provider.SetCoalitionResourceUsage(
+      std::make_unique<coalition_resource_usage>());
+  provider.EndIntervals(&short_rate, &long_rate);
+  EXPECT_FALSE(short_rate.has_value());
+  EXPECT_TRUE(long_rate.has_value());
+}

components/power_metrics/resource_coalition_mac.h

@@ -30,7 +30,8 @@ struct EnergyImpactCoefficients;
 absl::optional<uint64_t> GetProcessCoalitionId(base::ProcessId pid);
 
 // Returns resource usage data for the coalition identified by |coalition_id|,
-// or nullptr if not available.
+// or nullptr if not available (e.g. if `coalition_id` is invalid or if the
+// kernel can't allocate memory).
 std::unique_ptr<coalition_resource_usage> GetCoalitionResourceUsage(
     int64_t coalition_id);