[cleanup] Some cleanup in cc::DelayedUniqueNotifier

. Zero delay is not used anywhere. So remove support for zero delay.
. The notifier is only used in the compositor thread, so there is no
  need to use base::Lock. So this is removed, and replaced by a thread
  checker instead.

BUG=none

Change-Id: I1c998a011b9b28edab6b46ac9d772969bb4bb147
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391981
Commit-Queue: Khushal <khushalsagar@chromium.org>
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805669}

cc/base/delayed_unique_notifier.cc

@@ -4,6 +4,8 @@
 
 #include "cc/base/delayed_unique_notifier.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
@@ -18,13 +20,14 @@ DelayedUniqueNotifier::DelayedUniqueNotifier(
     : task_runner_(task_runner),
       closure_(std::move(closure)),
       delay_(delay),
-      notification_pending_(false) {}
+      notification_pending_(false) {
+  DCHECK(!delay_.is_zero());
+}
 
 DelayedUniqueNotifier::~DelayedUniqueNotifier() = default;
 
 void DelayedUniqueNotifier::Schedule() {
-  base::AutoLock hold(lock_);
-
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (notification_pending_) {
     next_notification_time_ = Now() + delay_;
     return;
@@ -40,12 +43,12 @@ void DelayedUniqueNotifier::Schedule() {
 }
 
 void DelayedUniqueNotifier::Cancel() {
-  base::AutoLock hold(lock_);
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   next_notification_time_ = base::TimeTicks();
 }
 
 void DelayedUniqueNotifier::Shutdown() {
-  base::AutoLock hold(lock_);
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
   // This function must destroy any weak ptrs since after being cancelled, this
   // class may be destroyed on another thread during compositor shutdown.
@@ -56,7 +59,7 @@ void DelayedUniqueNotifier::Shutdown() {
 }
 
 bool DelayedUniqueNotifier::HasPendingNotification() const {
-  base::AutoLock hold(lock_);
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   return notification_pending_ && !next_notification_time_.is_null();
 }
 
@@ -65,33 +68,29 @@ base::TimeTicks DelayedUniqueNotifier::Now() const {
 }
 
 void DelayedUniqueNotifier::NotifyIfTime() {
-  // Scope to release |lock_| before running the closure.
-  {
-    base::AutoLock hold(lock_);
-
-    // If next notifiaction time is not valid, then this schedule was canceled.
-    if (next_notification_time_.is_null()) {
-      notification_pending_ = false;
-      return;
-    }
-
-    // If the notification was rescheduled or arrived too early for any other
-    // reason, then post another task instead of running the callback.
-    base::TimeTicks now = Now();
-    if (next_notification_time_ > now) {
-      task_runner_->PostDelayedTask(
-          FROM_HERE,
-          base::BindOnce(&DelayedUniqueNotifier::NotifyIfTime,
-                         weak_ptr_factory_.GetWeakPtr()),
-          next_notification_time_ - now);
-      return;
-    }
-
-    // Note the order here is important since closure might schedule another
-    // run.
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  // If next notifiaction time is not valid, then this schedule was canceled.
+  if (next_notification_time_.is_null()) {
     notification_pending_ = false;
+    return;
   }
 
+  // If the notification was rescheduled or arrived too early for any other
+  // reason, then post another task instead of running the callback.
+  base::TimeTicks now = Now();
+  if (next_notification_time_ > now) {
+    task_runner_->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&DelayedUniqueNotifier::NotifyIfTime,
+                       weak_ptr_factory_.GetWeakPtr()),
+        next_notification_time_ - now);
+    return;
+  }
+
+  // Note the order here is important since closure might schedule another
+  // run.
+  notification_pending_ = false;
+
   closure_.Run();
 }
 

cc/base/delayed_unique_notifier.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
 #include "cc/base/base_export.h"
 
 namespace base {
@@ -48,8 +49,6 @@ class CC_BASE_EXPORT DelayedUniqueNotifier {
   // Returns true if a notification is currently scheduled to run.
   bool HasPendingNotification() const;
 
-  base::TimeDelta delay() const { return delay_; }
-
  protected:
   // Virtual for testing.
   virtual base::TimeTicks Now() const;
@@ -57,13 +56,12 @@ class CC_BASE_EXPORT DelayedUniqueNotifier {
  private:
   void NotifyIfTime();
 
+  THREAD_CHECKER(thread_checker_);
+
   base::SequencedTaskRunner* const task_runner_;
   const base::RepeatingClosure closure_;
   const base::TimeDelta delay_;
 
-  // Lock should be held before modifying |next_notification_time_| or
-  // |notification_pending_|.
-  mutable base::Lock lock_;
   base::TimeTicks next_notification_time_;
   bool notification_pending_;
 

cc/base/delayed_unique_notifier_unittest.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 #include "cc/base/delayed_unique_notifier.h"
+
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/containers/circular_deque.h"
@@ -52,41 +55,6 @@ class DelayedUniqueNotifierTest : public testing::Test {
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
 };
 
-TEST_F(DelayedUniqueNotifierTest, ZeroDelay) {
-  base::TimeDelta delay;  // Zero delay.
-  TestNotifier notifier(task_runner_.get(),
-                        base::BindRepeating(&DelayedUniqueNotifierTest::Notify,
-                                            base::Unretained(this)),
-                        delay);
-
-  EXPECT_EQ(0, NotificationCount());
-
-  // Basic schedule for |delay| from now.
-  base::TimeTicks schedule_time =
-      base::TimeTicks() + base::TimeDelta::FromMicroseconds(10);
-
-  notifier.SetNow(schedule_time);
-  notifier.Schedule();
-
-  base::circular_deque<base::TestPendingTask> tasks = TakePendingTasks();
-  ASSERT_EQ(1u, tasks.size());
-  EXPECT_EQ(base::TimeTicks() + delay, tasks[0].GetTimeToRun());
-
-  std::move(tasks[0].task).Run();
-  EXPECT_EQ(1, NotificationCount());
-
-  // 5 schedules should result in only one run.
-  for (int i = 0; i < 5; ++i)
-    notifier.Schedule();
-
-  tasks = TakePendingTasks();
-  ASSERT_EQ(1u, tasks.size());
-  EXPECT_EQ(base::TimeTicks() + delay, tasks[0].GetTimeToRun());
-
-  std::move(tasks[0].task).Run();
-  EXPECT_EQ(2, NotificationCount());
-}
-
 TEST_F(DelayedUniqueNotifierTest, SmallDelay) {
   base::TimeDelta delay = base::TimeDelta::FromMicroseconds(20);
   TestNotifier notifier(task_runner_.get(),

cc/trees/proxy_impl.cc

@@ -39,7 +39,8 @@ namespace cc {
 namespace {
 
 // Measured in seconds.
-const double kSmoothnessTakesPriorityExpirationDelay = 0.25;
+constexpr auto kSmoothnessTakesPriorityExpirationDelay =
+    base::TimeDelta::FromMilliseconds(250);
 
 }  // namespace
 
@@ -68,17 +69,12 @@ ProxyImpl::ProxyImpl(base::WeakPtr<ProxyMain> proxy_main_weak_ptr,
           task_runner_provider->ImplThreadTaskRunner(),
           base::BindRepeating(&ProxyImpl::RenewTreePriority,
                               base::Unretained(this)),
-          base::TimeDelta::FromSecondsD(
-              kSmoothnessTakesPriorityExpirationDelay)),
+          kSmoothnessTakesPriorityExpirationDelay),
       proxy_main_weak_ptr_(proxy_main_weak_ptr) {
   TRACE_EVENT0("cc", "ProxyImpl::ProxyImpl");
   DCHECK(IsImplThread());
   DCHECK(IsMainThreadBlocked());
 
-  // Double checking we set this correctly since double->int truncations are
-  // silent and have been done mistakenly: crbug.com/568120.
-  DCHECK(!smoothness_priority_expiration_notifier_.delay().is_zero());
-
   host_impl_ = layer_tree_host->CreateLayerTreeHostImpl(this);
   const LayerTreeSettings& settings = layer_tree_host->GetSettings();
   send_compositor_frame_ack_ = settings.send_compositor_frame_ack;