[Reland] Make canceling Timers fast.

third_party/WebKit/Source/platform/Timer.cpp

Index: third_party/WebKit/Source/platform/Timer.cpp
diff --git a/third_party/WebKit/Source/platform/Timer.cpp b/third_party/WebKit/Source/platform/Timer.cpp
index 8ed079f3f31502703bc065e987c83d3562cddd48..58f59f0f5def11cedf1975d647b8700581b058bf 100644
--- a/third_party/WebKit/Source/platform/Timer.cpp
+++ b/third_party/WebKit/Source/platform/Timer.cpp
@@ -43,11 +43,11 @@ namespace blink {
 TimerBase::TimerBase(WebTaskRunner* webTaskRunner)
     : m_nextFireTime(0)
     , m_repeatInterval(0)
-    , m_cancellableTimerTask(nullptr)
     , m_webTaskRunner(webTaskRunner->clone())
 #if DCHECK_IS_ON()
     , m_thread(currentThread())
 #endif
+    , m_weakPtrFactory(this)
 {
     ASSERT(m_webTaskRunner);
 }
@@ -72,9 +72,7 @@ void TimerBase::stop()
 
     m_repeatInterval = 0;
     m_nextFireTime = 0;
-    if (m_cancellableTimerTask)
-        m_cancellableTimerTask->cancel();
-    m_cancellableTimerTask = nullptr;
+    m_weakPtrFactory.revokeAll();
 }
 
 double TimerBase::nextFireInterval() const
@@ -111,12 +109,12 @@ void TimerBase::setNextFireTime(double now, double delay)
 
     if (m_nextFireTime != newTime) {
         m_nextFireTime = newTime;
-        if (m_cancellableTimerTask)
-            m_cancellableTimerTask->cancel();
-        m_cancellableTimerTask = new CancellableTimerTask(this);
+
+        // Cancel any previously posted task.
+        m_weakPtrFactory.revokeAll();
 
         double delayMs = 1000.0 * (newTime - now);
-        timerTaskRunner()->postDelayedTask(m_location, m_cancellableTimerTask, delayMs);
+        timerTaskRunner()->postDelayedTask(m_location, base::Bind(&TimerBase::runInternal, m_weakPtrFactory.createWeakPtr()), delayMs);
     }
 }
 
@@ -126,6 +124,8 @@ void TimerBase::runInternal()
     if (!canFire())
         return;
 
+    m_weakPtrFactory.revokeAll();

[haraken, 2016/09/09 02:34:07]

Maybe do we want to move this to above line 124?

However, I'm not sure if we really need to call revokeAll() here. Given that
it's guaranteed that only one timer is scheduled in the task queue (can we add
DCHECK about this fact?), there should be no active weak pointers when
runInternal() is fired.

[alex clarke (OOO till 29th), 2016/09/09 09:40:58]

We do need to call it here to match the previous behavior of TimerBase::isActive

[alex clarke (OOO till 29th), 2016/09/12 11:07:18]

It looks moving this above caused the bot failures.  Lets try with it below. 
Considering it's about to get swept away maybe the result of TimerBase::isActive
doesn't matter?

+
     TRACE_EVENT0("blink", "TimerBase::run");
 #if DCHECK_IS_ON()
     DCHECK_EQ(m_thread, currentThread()) << "Timer posted by " << m_location.function_name() << " " << m_location.file_name() << " was run on a different thread";