PowerManagerClient: Make fake use mock ticks

This change makes FakePowerManagerClient now use mock ticks for timers.
This means that timer expiration can be tested more strigently in tests
instead of earlier tests where only timer expiration was checked and not
the time when they expired.

BUG=962638
TEST=Unit tests.

Change-Id: Ic6b94e927dff2b55d9b622d77f663e377be7a4f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610224
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Auto-Submit: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659425}

chromeos/dbus/native_timer_unittest.cc

@@ -10,65 +10,52 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
+#include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "chromeos/dbus/power/power_manager_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
-namespace {
-
-// Returns true iff |Start| on |timer| succeeds and timer expiration occurs too.
-// The underlying fake power manager implementation expires the timer
-// immediately and the test doesn't sleep.
-bool CheckStartTimerAndExpiration(NativeTimer* timer,
-                                  base::TimeTicks absolute_expiration_ticks) {
-  base::RunLoop start_timer_loop;
-  base::RunLoop expiration_loop;
-  bool start_timer_result = false;
-  bool expiration_result = false;
-  timer->Start(
-      absolute_expiration_ticks,
-      base::BindOnce(
-          [](bool* result_out, base::OnceClosure quit_closure) {
-            *result_out = true;
-            std::move(quit_closure).Run();
-          },
-          &expiration_result, expiration_loop.QuitClosure()),
-      base::BindOnce(
-          [](bool* result_out, base::OnceClosure quit_closure, bool result) {
-            *result_out = result;
-            std::move(quit_closure).Run();
-          },
-          &start_timer_result, start_timer_loop.QuitClosure()));
-  start_timer_loop.Run();
-  if (!start_timer_result) {
-    return false;
-  }
-
-  // Run until timer expiration and check for the result.
-  expiration_loop.Run();
-  if (!expiration_result) {
-    return false;
-  }
-  return true;
-}
-
-}  // namespace
-
 class NativeTimerTest : public testing::Test {
  public:
   NativeTimerTest()
       : scoped_task_environment_(
-            base::test::ScopedTaskEnvironment::MainThreadType::IO) {}
+            base::test::ScopedTaskEnvironment::MainThreadType::IO_MOCK_TIME) {}
 
   ~NativeTimerTest() override = default;
 
   // testing::Test:
-  void SetUp() override { PowerManagerClient::InitializeFake(); }
+  void SetUp() override {
+    PowerManagerClient::InitializeFake();
+    FakePowerManagerClient::Get()->set_tick_clock(
+        scoped_task_environment_.GetMockTickClock());
+  }
 
   void TearDown() override { PowerManagerClient::Shutdown(); }
 
  protected:
+  // Returns true iff |Start| on |timer| succeeds and timer expiration occurs
+  // too. The underlying fake power manager implementation expires the timer
+  bool CheckStartTimerAndExpiration(NativeTimer* timer, base::TimeDelta delay) {
+    base::RunLoop start_timer_loop;
+    base::RunLoop expiration_loop;
+    bool start_timer_result = false;
+    bool expiration_result = false;
+    timer->Start(
+        scoped_task_environment_.GetMockTickClock()->NowTicks() + delay,
+        base::BindOnce([](bool* result_out) { *result_out = true; },
+                       &expiration_result),
+        base::BindOnce(
+            [](bool* result_out, bool result) { *result_out = result; },
+            &start_timer_result));
+
+    // Both starting the timer and timer firing should succeed.
+    scoped_task_environment_.FastForwardBy(delay);
+    if (!start_timer_result)
+      return false;
+    return expiration_result;
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
 
  private:
@@ -84,7 +71,7 @@ TEST_F(NativeTimerTest, CheckCreateFailure) {
 
   // Starting the timer should fail as timer creation failed.
   EXPECT_FALSE(CheckStartTimerAndExpiration(
-      &timer, base::TimeTicks() + base::TimeDelta::FromMilliseconds(1000)));
+      &timer, base::TimeDelta::FromMilliseconds(1000)));
 }
 
 TEST_F(NativeTimerTest, CheckCreateAndStartTimer) {
@@ -95,12 +82,12 @@ TEST_F(NativeTimerTest, CheckCreateAndStartTimer) {
 
   // Start timer and check if starting the timer and its expiration succeeded.
   EXPECT_TRUE(CheckStartTimerAndExpiration(
-      &timer, base::TimeTicks() + base::TimeDelta::FromMilliseconds(1000)));
+      &timer, base::TimeDelta::FromMilliseconds(1000)));
 
   // Start another timer and check if starting the timer and its expiration
   // succeeded.
   EXPECT_TRUE(CheckStartTimerAndExpiration(
-      &timer, base::TimeTicks() + base::TimeDelta::FromMilliseconds(1000)));
+      &timer, base::TimeDelta::FromMilliseconds(1000)));
 }
 
 }  // namespace chromeos

chromeos/dbus/power/fake_power_manager_client.cc

@@ -53,6 +53,18 @@ power_manager::BacklightBrightnessChange_Cause RequestCauseToChangeCause(
   return power_manager::BacklightBrightnessChange_Cause_USER_REQUEST;
 }
 
+// Copied from Chrome's //base/time/time_now_posix.cc.
+// Returns count of |clk_id| in the form of a time delta. Returns an empty
+// time delta if |clk_id| isn't present on the system.
+base::TimeDelta ClockNow(clockid_t clk_id) {
+  struct timespec ts;
+  if (clock_gettime(clk_id, &ts) != 0) {
+    NOTREACHED() << "clock_gettime(" << clk_id << ") failed.";
+    return base::TimeDelta();
+  }
+  return base::TimeDelta::FromTimeSpec(ts);
+}
+
 }  // namespace
 
 // static
@@ -62,7 +74,7 @@ FakePowerManagerClient* FakePowerManagerClient::Get() {
 }
 
 FakePowerManagerClient::FakePowerManagerClient()
-    : props_(power_manager::PowerSupplyProperties()) {
+    : props_(power_manager::PowerSupplyProperties()), tick_clock_(nullptr) {
   CHECK(!g_instance);
   g_instance = this;
 
@@ -323,8 +335,13 @@ void FakePowerManagerClient::StartArcTimer(
   // Post task to write to |clock_id|'s expiration fd. This will simulate the
   // timer expiring to the caller. Ignore delaying by
   // |absolute_expiration_time| for test purposes.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&ArcTimerExpirationCallback, it->second.get()));
+  base::TimeTicks current_ticks = GetCurrentBootTime();
+  base::TimeDelta task_delay;
+  if (absolute_expiration_time > current_ticks)
+    task_delay = absolute_expiration_time - current_ticks;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, base::BindOnce(&ArcTimerExpirationCallback, it->second.get()),
+      task_delay);
 }
 
 void FakePowerManagerClient::DeleteArcTimers(const std::string& tag,
@@ -459,4 +476,11 @@ bool FakePowerManagerClient::ApplyPendingScreenBrightnessChange() {
   return true;
 }
 
+// Returns time ticks from boot including time ticks spent during sleeping.
+base::TimeTicks FakePowerManagerClient::GetCurrentBootTime() {
+  if (tick_clock_)
+    return tick_clock_->NowTicks();
+  return base::TimeTicks() + ClockNow(CLOCK_BOOTTIME);
+}
+
 }  // namespace chromeos

chromeos/dbus/power/fake_power_manager_client.h

@@ -19,6 +19,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/optional.h"
+#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/dbus/power_manager/backlight.pb.h"
@@ -175,6 +176,9 @@ class COMPONENT_EXPORT(DBUS_POWER) FakePowerManagerClient
   // return false if there are no pending brightness changes.
   bool ApplyPendingScreenBrightnessChange();
 
+  // Returns time ticks from boot including time ticks spent during sleeping.
+  base::TimeTicks GetCurrentBootTime();
+
   // Sets the screen brightness percent to be returned.
   // The nullopt |percent| means an error. In case of success,
   // |percent| must be in the range of [0, 100].
@@ -186,6 +190,11 @@ class COMPONENT_EXPORT(DBUS_POWER) FakePowerManagerClient
     keyboard_brightness_percent_ = percent;
   }
 
+  // Sets |tick_clock| to |tick_clock_|.
+  void set_tick_clock(const base::TickClock* tick_clock) {
+    tick_clock_ = tick_clock;
+  }
+
  private:
   // Notifies |observers_| that |props_| has been updated.
   void NotifyObservers();
@@ -279,6 +288,9 @@ class COMPONENT_EXPORT(DBUS_POWER) FakePowerManagerClient
   // If non-empty, called by NotifyUserActivity().
   base::RepeatingClosure user_activity_callback_;
 
+  // Clock to use to calculate time ticks. Used for ArcTimer related APIs.
+  const base::TickClock* tick_clock_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<FakePowerManagerClient> weak_ptr_factory_{this};

chromeos/services/assistant/platform/power_manager_provider_impl.cc

@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/platform_thread.h"
+#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "services/device/public/mojom/constants.mojom.h"
 #include "services/device/public/mojom/wake_lock_provider.mojom.h"
@@ -35,11 +36,6 @@ base::TimeDelta ClockNow(clockid_t clk_id) {
   return base::TimeDelta::FromTimeSpec(ts);
 }
 
-// Returns time ticks from boot including time ticks spent during sleeping.
-base::TimeTicks GetCurrentBootTime() {
-  return base::TimeTicks() + ClockNow(CLOCK_BOOTTIME);
-}
-
 }  // namespace
 
 PowerManagerProviderImpl::PowerManagerProviderImpl(
@@ -97,6 +93,12 @@ void PowerManagerProviderImpl::ReleaseWakeLock() {
                      weak_factory_.GetWeakPtr()));
 }
 
+base::TimeTicks PowerManagerProviderImpl::GetCurrentBootTime() {
+  if (tick_clock_)
+    return tick_clock_->NowTicks();
+  return base::TimeTicks() + ClockNow(CLOCK_BOOTTIME);
+}
+
 void PowerManagerProviderImpl::AddWakeAlarmOnMainThread(
     AlarmId id,
     base::TimeTicks absolute_expiration_time,

chromeos/services/assistant/platform/power_manager_provider_impl.h

@@ -54,10 +54,18 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) PowerManagerProviderImpl
   void AcquireWakeLock() override;
   void ReleaseWakeLock() override;
 
+  void set_tick_clock_for_testing(const base::TickClock* tick_clock) {
+    DCHECK(tick_clock);
+    tick_clock_ = tick_clock;
+  }
+
  private:
   using CallbackAndTimer =
       std::pair<assistant_client::Callback0, std::unique_ptr<NativeTimer>>;
 
+  // Returns time ticks from boot including time ticks during sleeping.
+  base::TimeTicks GetCurrentBootTime();
+
   // Creates a native timer by calling |NativeTimer::Create|. Runs on
   // |main_thread_task_runner_|.
   void AddWakeAlarmOnMainThread(AlarmId id,
@@ -102,6 +110,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) PowerManagerProviderImpl
   // order to use Chrome APIs safely.
   const scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner_;
 
+  // Clock to use to calculate time ticks. Set and used only for testing.
+  const base::TickClock* tick_clock_ = nullptr;
+
   base::WeakPtrFactory<PowerManagerProviderImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PowerManagerProviderImpl);

chromeos/services/assistant/platform/power_manager_provider_impl_unittest.cc

@@ -28,16 +28,20 @@ class PowerManagerProviderImplTest : public testing::Test {
  public:
   PowerManagerProviderImplTest()
       : scoped_task_environment_(
-            base::test::ScopedTaskEnvironment::MainThreadType::IO),
+            base::test::ScopedTaskEnvironment::MainThreadType::IO_MOCK_TIME),
         wake_lock_provider_(
             connector_factory_.RegisterInstance(device::mojom::kServiceName)) {}
   ~PowerManagerProviderImplTest() override = default;
 
   void SetUp() override {
     chromeos::PowerManagerClient::InitializeFake();
+    FakePowerManagerClient::Get()->set_tick_clock(
+        scoped_task_environment_.GetMockTickClock());
     power_manager_provider_impl_ = std::make_unique<PowerManagerProviderImpl>(
         connector_factory_.GetDefaultConnector(),
         scoped_task_environment_.GetMainThreadTaskRunner());
+    power_manager_provider_impl_->set_tick_clock_for_testing(
+        scoped_task_environment_.GetMockTickClock());
   }
 
   void TearDown() override {
@@ -87,21 +91,20 @@ class PowerManagerProviderImplTest : public testing::Test {
   bool CheckAddWakeAlarmAndExpiration(uint64_t relative_time_ms,
                                       uint64_t max_delay_ms) {
     // Schedule wake alarm and check if valid id is returned and timer callback
-    // is fired. Fake power manager ensures that the timer is notified instantly
-    // in tests and the test doesn't sleep.
-    base::RunLoop run_loop;
+    // is fired.
     bool result = false;
-    assistant_client::Callback0 wake_alarm_expiration_cb([&]() {
-      result = true;
-      run_loop.QuitClosure().Run();
-    });
+    assistant_client::Callback0 wake_alarm_expiration_cb(
+        [&result]() { result = true; });
     assistant_client::PowerManagerProvider::AlarmId id =
         power_manager_provider_impl_->AddWakeAlarm(
             relative_time_ms, max_delay_ms,
             std::move(wake_alarm_expiration_cb));
-    run_loop.Run();
-    // Assistant client requires wake alarm ids to be > 0.
-    return id > 0UL && result;
+    scoped_task_environment_.FastForwardBy(
+        base::TimeDelta::FromMilliseconds(relative_time_ms));
+
+    if (id <= 0UL)
+      return false;
+    return result;
   }
 
  private: