[idle] Use the time-qualified base::TimeDelta instead of a time-agnostic.

We use ints for capturing and passing around deltas of time. This CL
moves all of those uses to base::TimeDelta, which benefits us by
being clearer about units (e.g. seconds, mins, etc) and semantics
in tests.

Bug: 878979
Change-Id: I5ad1cfc105882b63af7a974a72576212c773cb4a
Reviewed-on: https://chromium-review.googlesource.com/c/1492991
Commit-Queue: Sam Goto <goto@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Sam Goto <goto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636529}

content/browser/idle/idle_manager.cc

@@ -27,11 +27,13 @@ class DefaultIdleProvider : public IdleManager::IdleTimeProvider {
   DefaultIdleProvider() = default;
   ~DefaultIdleProvider() override = default;
 
-  ui::IdleState CalculateIdleState(int idle_threshold) override {
-    return ui::CalculateIdleState(idle_threshold);
+  ui::IdleState CalculateIdleState(base::TimeDelta idle_threshold) override {
+    return ui::CalculateIdleState(idle_threshold.InSeconds());
   }
 
-  int CalculateIdleTime() override { return ui::CalculateIdleTime(); }
+  base::TimeDelta CalculateIdleTime() override {
+    return base::TimeDelta::FromSeconds(ui::CalculateIdleTime());
+  }
 
   bool CheckIdleStateIsLocked() override {
     return ui::CheckIdleStateIsLocked();
@@ -39,8 +41,8 @@ class DefaultIdleProvider : public IdleManager::IdleTimeProvider {
 };
 
 blink::mojom::IdleStatePtr IdleTimeToIdleState(bool locked,
-                                               int idle_time,
-                                               int idle_threshold) {
+                                               base::TimeDelta idle_time,
+                                               base::TimeDelta idle_threshold) {
   blink::mojom::UserIdleState user;
   if (idle_time >= idle_threshold)
     user = blink::mojom::UserIdleState::kIdle;
@@ -78,11 +80,11 @@ void IdleManager::CreateService(blink::mojom::IdleManagerRequest request,
   bindings_.AddBinding(this, std::move(request));
 }
 
-void IdleManager::AddMonitor(uint32_t threshold,
+void IdleManager::AddMonitor(base::TimeDelta threshold,
                              blink::mojom::IdleMonitorPtr monitor_ptr,
                              AddMonitorCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (threshold == 0) {
+  if (threshold.is_zero()) {
     mojo::ReportBadMessage("Invalid threshold");
     return;
   }
@@ -138,8 +140,9 @@ void IdleManager::StopPolling() {
   poll_timer_.Stop();
 }
 
-blink::mojom::IdleStatePtr IdleManager::CheckIdleState(int threshold) {
-  int idle_time = idle_time_provider_->CalculateIdleTime();
+blink::mojom::IdleStatePtr IdleManager::CheckIdleState(
+    base::TimeDelta threshold) {
+  base::TimeDelta idle_time = idle_time_provider_->CalculateIdleTime();
   bool locked = idle_time_provider_->CheckIdleStateIsLocked();
 
   return IdleTimeToIdleState(locked, idle_time, threshold);

content/browser/idle/idle_manager.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
+#include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "content/browser/idle/idle_monitor.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
@@ -37,8 +38,9 @@ class CONTENT_EXPORT IdleManager : public blink::mojom::IdleManager {
     // See ui/base/idle/idle.h for the semantics of these methods.
     // TODO(goto): should this be made private? Doesn't seem to be necessary
     // as part of a public interface.
-    virtual ui::IdleState CalculateIdleState(int idle_threshold) = 0;
-    virtual int CalculateIdleTime() = 0;
+    virtual ui::IdleState CalculateIdleState(
+        base::TimeDelta idle_threshold) = 0;
+    virtual base::TimeDelta CalculateIdleTime() = 0;
     virtual bool CheckIdleStateIsLocked() = 0;
 
    private:
@@ -53,7 +55,7 @@ class CONTENT_EXPORT IdleManager : public blink::mojom::IdleManager {
                      const url::Origin& origin);
 
   // blink.mojom.IdleManager:
-  void AddMonitor(uint32_t threshold,
+  void AddMonitor(base::TimeDelta threshold,
                   blink::mojom::IdleMonitorPtr monitor_ptr,
                   AddMonitorCallback callback) override;
 
@@ -84,7 +86,7 @@ class CONTENT_EXPORT IdleManager : public blink::mojom::IdleManager {
   // Callback for the async state query. Updates monitors as needed.
   void UpdateIdleStateCallback(int idle_time);
 
-  blink::mojom::IdleStatePtr CheckIdleState(int threshold);
+  blink::mojom::IdleStatePtr CheckIdleState(base::TimeDelta threshold);
 
   base::RepeatingTimer poll_timer_;
   std::unique_ptr<IdleTimeProvider> idle_time_provider_;

content/browser/idle/idle_manager_unittest.cc

@@ -4,10 +4,13 @@
 
 #include "content/browser/idle/idle_manager.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
+#include "base/time/time.h"
 #include "content/browser/permissions/permission_controller_impl.h"
 #include "content/public/browser/permission_controller.h"
 #include "content/public/browser/permission_type.h"
@@ -36,7 +39,7 @@ namespace content {
 
 namespace {
 
-const uint32_t kTresholdInSecs = 10;
+constexpr base::TimeDelta kTreshold = base::TimeDelta::FromSeconds(10);
 
 class MockIdleMonitor : public blink::mojom::IdleMonitor {
  public:
@@ -48,8 +51,8 @@ class MockIdleTimeProvider : public IdleManager::IdleTimeProvider {
   MockIdleTimeProvider() = default;
   ~MockIdleTimeProvider() override = default;
 
-  MOCK_METHOD1(CalculateIdleState, ui::IdleState(int));
-  MOCK_METHOD0(CalculateIdleTime, int());
+  MOCK_METHOD1(CalculateIdleState, ui::IdleState(base::TimeDelta));
+  MOCK_METHOD0(CalculateIdleTime, base::TimeDelta());
   MOCK_METHOD0(CheckIdleStateIsLocked, bool());
 
  private:
@@ -90,12 +93,13 @@ TEST_F(IdleManagerTest, AddMonitor) {
   }));
 
   // Initial state of the system.
-  EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(0));
+  EXPECT_CALL(*mock, CalculateIdleTime())
+      .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(0)));
   EXPECT_CALL(*mock, CheckIdleStateIsLocked())
       .WillRepeatedly(testing::Return(false));
 
   service_ptr->AddMonitor(
-      kTresholdInSecs, std::move(monitor_ptr),
+      kTreshold, std::move(monitor_ptr),
       base::BindOnce(
           [](base::OnceClosure callback, blink::mojom::IdleStatePtr state) {
             // The initial state of the status of the user is to be active.
@@ -128,10 +132,11 @@ TEST_F(IdleManagerTest, Idle) {
   {
     base::RunLoop loop;
     // Initial state of the system.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(0));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(0)));
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
           EXPECT_EQ(blink::mojom::UserIdleState::kActive, state->user);
           loop.Quit();
@@ -143,7 +148,8 @@ TEST_F(IdleManagerTest, Idle) {
   {
     base::RunLoop loop;
     // Simulates a user going idle.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(10));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(10)));
 
     // Expects Update to be notified about the change to idle.
     EXPECT_CALL(monitor, Update(_))
@@ -157,7 +163,8 @@ TEST_F(IdleManagerTest, Idle) {
   {
     base::RunLoop loop;
     // Simulates a user going active, calling a callback under the threshold.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(0));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(0)));
 
     // Expects Update to be notified about the change to active.
     // auto quit = loop.QuitClosure();
@@ -196,7 +203,7 @@ TEST_F(IdleManagerTest, UnlockingScreen) {
         .WillRepeatedly(testing::Return(true));
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
           EXPECT_EQ(blink::mojom::ScreenIdleState::kLocked, state->screen);
           loop.Quit();
@@ -248,7 +255,7 @@ TEST_F(IdleManagerTest, LockingScreen) {
         .WillRepeatedly(testing::Return(false));
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
           EXPECT_EQ(blink::mojom::ScreenIdleState::kUnlocked, state->screen);
           loop.Quit();
@@ -300,7 +307,7 @@ TEST_F(IdleManagerTest, LockingScreenThenIdle) {
         .WillRepeatedly(testing::Return(false));
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
           EXPECT_EQ(blink::mojom::UserIdleState::kActive, state->user);
           EXPECT_EQ(blink::mojom::ScreenIdleState::kUnlocked, state->screen);
@@ -332,7 +339,8 @@ TEST_F(IdleManagerTest, LockingScreenThenIdle) {
     base::RunLoop loop;
 
     // Simulates a user going idle, whilte the screen is still locked.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(10));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(10)));
     EXPECT_CALL(*mock, CheckIdleStateIsLocked())
         .WillRepeatedly(testing::Return(true));
 
@@ -370,12 +378,13 @@ TEST_F(IdleManagerTest, LockingScreenAfterIdle) {
     base::RunLoop loop;
 
     // Simulates a user going idle, but with the screen still unlocked.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(0));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(0)));
     EXPECT_CALL(*mock, CheckIdleStateIsLocked())
         .WillRepeatedly(testing::Return(false));
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
           EXPECT_EQ(blink::mojom::UserIdleState::kActive, state->user);
           EXPECT_EQ(blink::mojom::ScreenIdleState::kUnlocked, state->screen);
@@ -388,7 +397,8 @@ TEST_F(IdleManagerTest, LockingScreenAfterIdle) {
   {
     base::RunLoop loop;
     // Simulates a user going idle, but with the screen still unlocked.
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(10));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(10)));
     EXPECT_CALL(*mock, CheckIdleStateIsLocked())
         .WillRepeatedly(testing::Return(false));
 
@@ -408,7 +418,8 @@ TEST_F(IdleManagerTest, LockingScreenAfterIdle) {
     // Simulates the screeng getting locked by the system after the user goes
     // idle (e.g. screensaver kicks in first, throwing idleness, then getting
     // locked).
-    EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(10));
+    EXPECT_CALL(*mock, CalculateIdleTime())
+        .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(10)));
     EXPECT_CALL(*mock, CheckIdleStateIsLocked())
         .WillRepeatedly(testing::Return(true));
 
@@ -448,7 +459,7 @@ TEST_F(IdleManagerTest, RemoveMonitorStopsPolling) {
     base::RunLoop loop;
 
     service_ptr->AddMonitor(
-        kTresholdInSecs, std::move(monitor_ptr),
+        kTreshold, std::move(monitor_ptr),
         base::BindLambdaForTesting(
             [&](blink::mojom::IdleStatePtr state) { loop.Quit(); }));
 
@@ -486,12 +497,13 @@ TEST_F(IdleManagerTest, Threshold) {
   base::RunLoop loop;
 
   // Initial state of the system.
-  EXPECT_CALL(*mock, CalculateIdleTime()).WillRepeatedly(testing::Return(6));
+  EXPECT_CALL(*mock, CalculateIdleTime())
+      .WillRepeatedly(testing::Return(base::TimeDelta::FromSeconds(6)));
   EXPECT_CALL(*mock, CheckIdleStateIsLocked())
       .WillRepeatedly(testing::Return(false));
 
   service_ptr->AddMonitor(
-      5, std::move(monitor_ptr),
+      base::TimeDelta::FromSeconds(5), std::move(monitor_ptr),
       base::BindLambdaForTesting([&](blink::mojom::IdleStatePtr state) {
         EXPECT_EQ(blink::mojom::UserIdleState::kIdle, state->user);
         loop.Quit();

content/browser/idle/idle_monitor.cc

@@ -17,7 +17,7 @@ namespace content {
 
 IdleMonitor::IdleMonitor(blink::mojom::IdleMonitorPtr monitor,
                          blink::mojom::IdleStatePtr last_state,
-                         uint32_t threshold)
+                         base::TimeDelta threshold)
     : client_(std::move(monitor)),
       last_state_(std::move(last_state)),
       threshold_(threshold) {}

content/browser/idle/idle_monitor.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
+#include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/connection_error_callback.h"
@@ -25,14 +26,14 @@ class CONTENT_EXPORT IdleMonitor : public base::LinkNode<IdleMonitor> {
  public:
   IdleMonitor(blink::mojom::IdleMonitorPtr monitor,
               blink::mojom::IdleStatePtr last_state,
-              uint32_t threshold);
+              base::TimeDelta threshold);
   ~IdleMonitor();
 
   const blink::mojom::IdleState& last_state() const {
     return *last_state_.get();
   }
-  uint32_t threshold() const { return threshold_; }
-  void set_threshold(uint32_t threshold) { threshold_ = threshold; }
+  base::TimeDelta threshold() const { return threshold_; }
+  void set_threshold(base::TimeDelta threshold) { threshold_ = threshold; }
 
   void SetLastState(blink::mojom::IdleStatePtr state);
   void SetErrorHandler(base::OnceCallback<void(content::IdleMonitor*)> handler);
@@ -40,7 +41,7 @@ class CONTENT_EXPORT IdleMonitor : public base::LinkNode<IdleMonitor> {
  private:
   blink::mojom::IdleMonitorPtr client_;
   blink::mojom::IdleStatePtr last_state_;
-  uint32_t threshold_;
+  base::TimeDelta threshold_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

third_party/blink/public/platform/modules/idle/idle_manager.mojom

@@ -4,6 +4,8 @@
 
 module blink.mojom;
 
+import "mojo/public/mojom/base/time.mojom";
+
 // Implementation of the proposed "Idle Detection API".
 //
 // Proposal: https://github.com/inexorabletash/idle-detection
@@ -32,5 +34,6 @@ interface IdleManager {
   // initial state. It will be notified by calls to Update() per the threshold
   // registered for this instance. It can be unregistered by simply closing
   // the pipe.
-  AddMonitor(uint32 threshold, IdleMonitor monitor) => (IdleState state);
+  AddMonitor(mojo_base.mojom.TimeDelta threshold, IdleMonitor monitor)
+      => (IdleState state);
 };

third_party/blink/renderer/modules/idle/idle_manager.cc

@@ -36,6 +36,8 @@ ScriptPromise IdleManager::query(ScriptState* script_state,
     return ScriptPromise();
   }
 
+  base::TimeDelta threshold = base::TimeDelta::FromSeconds(threshold_seconds);
+
   // TODO: Permission check.
 
   if (!service_) {
@@ -51,12 +53,12 @@ ScriptPromise IdleManager::query(ScriptState* script_state,
 
   mojom::blink::IdleMonitorPtr monitor_ptr;
   IdleStatus* status =
-      IdleStatus::Create(ExecutionContext::From(script_state),
-                         threshold_seconds, mojo::MakeRequest(&monitor_ptr));
+      IdleStatus::Create(ExecutionContext::From(script_state), threshold,
+                         mojo::MakeRequest(&monitor_ptr));
 
   requests_.insert(resolver);
   service_->AddMonitor(
-      threshold_seconds, std::move(monitor_ptr),
+      threshold, std::move(monitor_ptr),
       WTF::Bind(&IdleManager::OnAddMonitor, WrapPersistent(this),
                 WrapPersistent(resolver), WrapPersistent(status)));
 

third_party/blink/renderer/modules/idle/idle_status.cc

@@ -18,7 +18,7 @@
 namespace blink {
 
 IdleStatus* IdleStatus::Create(ExecutionContext* context,
-                               uint32_t threshold,
+                               base::TimeDelta threshold,
                                mojom::blink::IdleMonitorRequest request) {
   auto* status =
       MakeGarbageCollected<IdleStatus>(context, threshold, std::move(request));
@@ -27,7 +27,7 @@ IdleStatus* IdleStatus::Create(ExecutionContext* context,
 }
 
 IdleStatus::IdleStatus(ExecutionContext* context,
-                       uint32_t threshold,
+                       base::TimeDelta threshold,
                        mojom::blink::IdleMonitorRequest request)
     : ContextLifecycleStateObserver(context),
       threshold_(threshold),

third_party/blink/renderer/modules/idle/idle_status.h

@@ -32,11 +32,11 @@ class IdleStatus final : public EventTargetWithInlineData,
   // to script until the monitor has been registered by the service and
   // returned an initial state.
   static IdleStatus* Create(ExecutionContext* context,
-                            uint32_t threshold,
+                            base::TimeDelta threshold,
                             mojom::blink::IdleMonitorRequest request);
 
   IdleStatus(ExecutionContext*,
-             uint32_t threshold,
+             base::TimeDelta threshold,
              mojom::blink::IdleMonitorRequest);
   ~IdleStatus() override;
   void Dispose();
@@ -76,7 +76,7 @@ class IdleStatus final : public EventTargetWithInlineData,
 
   Member<blink::IdleState> state_;
 
-  const uint32_t threshold_;
+  const base::TimeDelta threshold_;
 
   // Holds a pipe which the service uses to notify this object
   // when the idle state has changed.