Revert "Clean up UsePowerMonitorWithThreadController"

This reverts commit f66896faa0eae6b4b3fa65eb4fe70bc8a7b76e47.

Reason for revert: b/374173161. This breaks the suspend behavior on
CrOS.

Original change's description:
> Clean up UsePowerMonitorWithThreadController
>
> The feature has been enabled by default for a long time, removing it
> should be a no-op.
>
> Bug: 356234726
> Change-Id: I85739a627364102ec97b00c99fc46616e97825e1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5832196
> Reviewed-by: Maks Orlovich <[email protected]>
> Reviewed-by: Francois Pierre Doray <[email protected]>
> Commit-Queue: Anthony Vallée-Dubois <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1359479}

Bug: 356234726, 374173161
Change-Id: Icd591c4e6dd4881ce93e48254b28a4cd6bc869f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5960956
Commit-Queue: Jie Jiang <[email protected]>
Reviewed-by: Maks Orlovich <[email protected]>
Reviewed-by: Francois Pierre Doray <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1373928}


CrOS-Libchrome-Original-Commit: 353ef9d1248cbd62fdd7280d89750ac7d65f68bc

Author: Jie Jiang

base/features.cc

@@ -25,6 +25,7 @@
 #endif
 
 #if BUILDFLAG(IS_WIN)
+#include "base/task/sequence_manager/thread_controller_power_monitor.h"
 #include "base/threading/platform_thread_win.h"
 #endif
 
@@ -147,6 +148,8 @@ void Init(EmitThreadControllerProfilerMetadata
 #endif
 
 #if BUILDFLAG(IS_WIN)
+  sequence_manager::internal::ThreadControllerPowerMonitor::
+      InitializeFeatures();
   InitializePlatformThreadFeatures();
 #endif
 }

base/task/sequence_manager/thread_controller_power_monitor.cc

@@ -12,6 +12,19 @@ namespace base {
 namespace sequence_manager {
 namespace internal {
 
+namespace {
+
+// Activate the power management events that affect task scheduling.
+BASE_FEATURE(kUsePowerMonitorWithThreadController,
+             "UsePowerMonitorWithThreadController",
+             FEATURE_ENABLED_BY_DEFAULT);
+
+// TODO(crbug.com/40127966): Remove this when the experiment becomes the
+// default.
+bool g_use_thread_controller_power_monitor_ = false;
+
+}  // namespace
+
 ThreadControllerPowerMonitor::ThreadControllerPowerMonitor() = default;
 
 ThreadControllerPowerMonitor::~ThreadControllerPowerMonitor() {
@@ -35,7 +48,27 @@ bool ThreadControllerPowerMonitor::IsProcessInPowerSuspendState() {
   return is_power_suspended_;
 }
 
+// static
+void ThreadControllerPowerMonitor::InitializeFeatures() {
+  DCHECK(!g_use_thread_controller_power_monitor_);
+  g_use_thread_controller_power_monitor_ =
+      FeatureList::IsEnabled(kUsePowerMonitorWithThreadController);
+}
+
+// static
+void ThreadControllerPowerMonitor::OverrideUsePowerMonitorForTesting(
+    bool use_power_monitor) {
+  g_use_thread_controller_power_monitor_ = use_power_monitor;
+}
+
+// static
+void ThreadControllerPowerMonitor::ResetForTesting() {
+  g_use_thread_controller_power_monitor_ = false;
+}
+
 void ThreadControllerPowerMonitor::OnSuspend() {
+  if (!g_use_thread_controller_power_monitor_)
+    return;
   DCHECK(!is_power_suspended_);
 
   TRACE_EVENT_BEGIN("base", "ThreadController::Suspended",
@@ -45,6 +78,9 @@ void ThreadControllerPowerMonitor::OnSuspend() {
 }
 
 void ThreadControllerPowerMonitor::OnResume() {
+  if (!g_use_thread_controller_power_monitor_)
+    return;
+
   // It is possible a suspend was already happening before the observer was
   // added to the power monitor. Ignoring the resume notification in that case.
   if (is_power_suspended_) {

base/task/sequence_manager/thread_controller_power_monitor.h

@@ -31,6 +31,12 @@ class BASE_EXPORT ThreadControllerPowerMonitor : public PowerSuspendObserver {
   // notifications.
   bool IsProcessInPowerSuspendState();
 
+  // Initializes features for this class. See `base::features::Init()`.
+  static void InitializeFeatures();
+
+  static void OverrideUsePowerMonitorForTesting(bool use_power_monitor);
+  static void ResetForTesting();
+
   // base::PowerSuspendObserver:
   void OnSuspend() override;
   void OnResume() override;

base/task/sequence_manager/thread_controller_power_monitor_unittest.cc

@@ -21,10 +21,13 @@ class ThreadControllerPowerMonitorTest : public testing::Test {
   void SetUp() override {
     thread_controller_power_monitor_ =
         std::make_unique<ThreadControllerPowerMonitor>();
+    internal::ThreadControllerPowerMonitor::OverrideUsePowerMonitorForTesting(
+        true);
   }
 
   void TearDown() override {
     thread_controller_power_monitor_.reset();
+    internal::ThreadControllerPowerMonitor::ResetForTesting();
   }
 
  protected:

base/task/sequence_manager/thread_controller_with_message_pump_impl_unittest.cc

@@ -234,6 +234,12 @@ class ThreadControllerWithMessagePumpTestBase : public testing::Test {
     thread_controller_.SetSequencedTaskSource(&task_source_);
   }
 
+  void SetUp() override {
+    ThreadControllerPowerMonitor::OverrideUsePowerMonitorForTesting(true);
+  }
+
+  void TearDown() override { ThreadControllerPowerMonitor::ResetForTesting(); }
+
   TimeTicks FromNow(TimeDelta delta) { return clock_.NowTicks() + delta; }
 
  protected:

base/test/power_monitor_test.cc

@@ -108,20 +108,6 @@ ScopedPowerMonitorTestSource::ScopedPowerMonitorTestSource() {
 }
 
 ScopedPowerMonitorTestSource::~ScopedPowerMonitorTestSource() {
-  if (is_suspended_) {
-    // Generate a resume here because there are global, leaky disk cache
-    // threads that lives throughout a long portion of some test targets.
-    // This thread is created the first time the disk cache is created, which
-    // instantiates a disk cache backend, which in turn creates a CacheThread as
-    // a global LazyInstance<>::Lazy. This thread gets its own
-    // ThreadControllerPowerMonitor, so if this test case doesn't simulate a
-    // resume, the next test will hit a DCHECK when it tries to generate its
-    // first suspend event. The "correct" solution to this problem would be to
-    // refactor the offending disk caches to support passing a thread through to
-    // the disk cache backend in tests, and avoid having a global/leaky instance
-    // around but this is a significant undertaking.
-    GenerateResumeEvent();
-  }
   base::PowerMonitor::GetInstance()->ShutdownForTesting();
 }
 
@@ -136,13 +122,11 @@ ScopedPowerMonitorTestSource::GetBatteryPowerStatus() const {
 }
 
 void ScopedPowerMonitorTestSource::Suspend() {
-  is_suspended_ = true;
   power_monitor_test_source_->Suspend();
 }
 
 void ScopedPowerMonitorTestSource::Resume() {
   power_monitor_test_source_->Resume();
-  is_suspended_ = false;
 }
 
 void ScopedPowerMonitorTestSource::SetBatteryPowerStatus(
@@ -151,13 +135,11 @@ void ScopedPowerMonitorTestSource::SetBatteryPowerStatus(
 }
 
 void ScopedPowerMonitorTestSource::GenerateSuspendEvent() {
-  is_suspended_ = true;
   power_monitor_test_source_->GenerateSuspendEvent();
 }
 
 void ScopedPowerMonitorTestSource::GenerateResumeEvent() {
   power_monitor_test_source_->GenerateResumeEvent();
-  is_suspended_ = false;
 }
 
 void ScopedPowerMonitorTestSource::GeneratePowerStateEvent(

base/test/power_monitor_test.h

@@ -58,7 +58,6 @@ class ScopedPowerMonitorTestSource {
   // Owned by PowerMonitor.
   raw_ptr<PowerMonitorTestSource, DanglingUntriaged>
       power_monitor_test_source_ = nullptr;
-  bool is_suspended_ = false;
 };
 
 class PowerMonitorTestObserver : public PowerSuspendObserver,

base/timer/wall_clock_timer_unittest.cc

@@ -249,12 +249,6 @@ TEST_F(WallClockTimerTest, NonStopTickClockWithLongPause) {
   task_environment_.FastForwardBy(past_time);
   fake_power_monitor_source_.Resume();
 
-  // The WallClockTimer restarts its task with a shorter timeout when the system
-  // resumes, so it needs to be taken out of the queue and executed. In this
-  // case, the time is already elapsed, so fast forwarding by 0 to pump the task
-  // without advancing time.
-  task_environment_.FastForwardBy(base::TimeDelta());
-
   ::testing::Mock::VerifyAndClearExpectations(&callback);
   EXPECT_FALSE(wall_clock_timer.IsRunning());
 }