Refactor Timer classes in preparation for landing FrameTimers.

third_party/WebKit/Source/platform/Timer.h

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 26 matching lines @@
 #include "wtf/Threading.h"
 #include "wtf/Vector.h"
 
 namespace blink {
 
 // Time intervals are all in seconds.
 
 class PLATFORM_EXPORT TimerBase {
     WTF_MAKE_NONCOPYABLE(TimerBase);
 public:
-    TimerBase();
     explicit TimerBase(WebTaskRunner*);
     virtual ~TimerBase();
 
     void start(double nextFireInterval, double repeatInterval, const WebTraceLocation&);
 
     void startRepeating(double repeatInterval, const WebTraceLocation& caller)
     {
         start(repeatInterval, repeatInterval, caller);
     }
     void startOneShot(double interval, const WebTraceLocation& caller)
@@ skipping 12 matching lines @@
         double now = timerMonotonicallyIncreasingTime();
         setNextFireTime(now, std::max(m_nextFireTime - now + delta, 0.0));
         m_repeatInterval += delta;
     }
 
     struct PLATFORM_EXPORT Comparator {
         bool operator()(const TimerBase* a, const TimerBase* b) const;
     };
 
 protected:
-    static WebTaskRunner* UnthrottledWebTaskRunner();
+    static WebTaskRunner* getTimerTaskRunner();
+    static WebTaskRunner* getDefaultTaskRunner();
 
 private:
     virtual void fired() = 0;
 
     virtual WebTaskRunner* timerTaskRunner() const;
 
     NO_LAZY_SWEEP_SANITIZE_ADDRESS
     virtual bool canFire() const { return true; }
 
     double timerMonotonicallyIncreasingTime() const;
@@ skipping 57 matching lines @@
 template<typename T>
 class TimerIsObjectAliveTrait<T, true> {
 public:
     static bool isHeapObjectAlive(T* objectPointer)
     {
         return !ThreadHeap::willObjectBeLazilySwept(objectPointer);
     }
 };
 
 template <typename TimerFiredClass>
-class Timer : public TimerBase {
+class TaskRunnerTimer : public TimerBase {
 public:
-    using TimerFiredFunction = void (TimerFiredClass::*)(Timer<TimerFiredClass>*);
+    using TimerFiredFunction = void (TimerFiredClass::*)(TimerBase*);
 
-    // TODO(dcheng): Consider removing this overload once all timers are using the
-    // appropriate task runner. https://crbug.com/624694
-    Timer(TimerFiredClass* o, TimerFiredFunction f)
-        : m_object(o), m_function(f)
-    {
-    }
-
-    Timer(TimerFiredClass* o, TimerFiredFunction f, WebTaskRunner* webTaskRunner)
+    TaskRunnerTimer(WebTaskRunner* webTaskRunner, TimerFiredClass* o, TimerFiredFunction f)
         : TimerBase(webTaskRunner), m_object(o), m_function(f)
     {
     }
 
-    ~Timer() override { }
+    ~TaskRunnerTimer() override { }
 
 protected:
     void fired() override
     {
         (m_object->*m_function)(this);
     }
 
     NO_LAZY_SWEEP_SANITIZE_ADDRESS
     bool canFire() const override
     {
         // Oilpan: if a timer fires while Oilpan heaps are being lazily
         // swept, it is not safe to proceed if the object is about to
         // be swept (and this timer will be stopped while doing so.)
         return TimerIsObjectAliveTrait<TimerFiredClass>::isHeapObjectAlive(m_object);
     }
 
 private:
     // FIXME: Oilpan: TimerBase should be moved to the heap and m_object should be traced.
     // This raw pointer is safe as long as Timer<X> is held by the X itself (That's the case
     // in the current code base).
     GC_PLUGIN_IGNORE("363031")
     TimerFiredClass* m_object;
     TimerFiredFunction m_function;
 };
 
+// TODO(dcheng): Consider removing this overload once all timers are using the
+// appropriate task runner. https://crbug.com/624694
+template <typename TimerFiredClass>
+class Timer : public TaskRunnerTimer<TimerFiredClass> {
+public:
+    using TimerFiredFunction = typename TaskRunnerTimer<TimerFiredClass>::TimerFiredFunction;
+
+    ~Timer() override { }
+
+    Timer(TimerFiredClass* timerFiredClass, TimerFiredFunction timerFiredFunction)
+        : TaskRunnerTimer<TimerFiredClass>(TimerBase::getTimerTaskRunner(), timerFiredClass, timerFiredFunction)

[haraken, 2016/07/28 14:44:46]

Doesn't this mean that all Timers start using the timer task runner? The timer
task runner should be limited to SuspendableTimers and a couple of other timers
(at least until we figure out what timers should  use what task runners).

[Sami, 2016/07/28 16:38:55]

All timers already use the timer task runner, unless I missed something :)

[haraken, 2016/07/28 17:38:15]

Oh, I was misunderstanding. You're right.

- Only DOMTimer is in frameScheduler()->timerTaskRunner().
- Other timers are in currentThread()->scheduler()->timerTaskRunner(). If I'm
not misunderstanding our plan, currentThread()->scheduler() is going to be
removed
(https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj...).

[dcheng, 2016/07/29 02:23:18]

Right, per Sami, the default for Timer right now is to already use the timer
task queue, so this doesn't change anything for them.

+    {
+    }
+};
+
+
 // This subclass of Timer posts its tasks on the current thread's default task runner.
 // Tasks posted on there are not throttled when the tab is in the background.
 template <typename TimerFiredClass>
-class UnthrottledTimer : public Timer<TimerFiredClass> {
+class UnthrottledThreadTimer : public TaskRunnerTimer<TimerFiredClass> {

[haraken, 2016/07/28 14:44:46]

Maybe can we keep the current name as is? UnthrottledTimer sounds nicer to me.

[dcheng, 2016/07/29 02:23:18]

I think this might be confusing in combination with UnthrottledTimer for frame
though, right? Since we want to discourage this, I think it's OK to give the
discouraged versions longer names. I think that I will actually be removing this
in a followup, and moving all uses of this to frame's unthrottled task runner.
Does that make sense?

[haraken, 2016/07/29 08:35:27]

Is your plan to replace UnthrottledThreadTimer with
TaskRunnerHelper::getUnthrottledTaskRunner()->postTask()? Then it makes sense.

Maybe it would be helpful to prefix deprecated classes with "Deprecated" or
"Legacy". At least add a comment that this timer is deprecated.

[dcheng, 2016/07/29 08:44:40]

Yes, though I would probably just use a subclass of the not-yet-landed
FrameTimer since there are cancellation semantics involved.

Since we don't have any alternatives for code to use yet, I'll add the comment
about deprecation once we land some FrameTimers that associate with a frame
scheduler. Does that sound good? 

 public:
-    using TimerFiredFunction = void (TimerFiredClass::*)(Timer<TimerFiredClass>*);
+    using TimerFiredFunction = typename TaskRunnerTimer<TimerFiredClass>::TimerFiredFunction;
 
-    ~UnthrottledTimer() override { }
+    ~UnthrottledThreadTimer() override { }
 
-    UnthrottledTimer(TimerFiredClass* timerFiredClass, TimerFiredFunction timerFiredFunction)
-        : Timer<TimerFiredClass>(timerFiredClass, timerFiredFunction, TimerBase::UnthrottledWebTaskRunner())
+    UnthrottledThreadTimer(TimerFiredClass* timerFiredClass, TimerFiredFunction timerFiredFunction)
+        : TaskRunnerTimer<TimerFiredClass>(TimerBase::getDefaultTaskRunner(), timerFiredClass, timerFiredFunction)
     {
     }
 };
 
 NO_LAZY_SWEEP_SANITIZE_ADDRESS
 inline bool TimerBase::isActive() const
 {
     ASSERT(m_thread == currentThread());
     return m_cancellableTimerTask;
 }
 
 } // namespace blink
 
 #endif // Timer_h