Move VTTRegion Timers to TaskRunnerTimer

third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp

Index: third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp
diff --git a/third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp b/third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp
index 772bca0fb86139c7c732c00ee58e64cb1ec37fb3..9c98a9a6eed42913758e9fc6a85195df88bea626 100644
--- a/third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp
+++ b/third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp
@@ -36,6 +36,7 @@
 #include "core/dom/DOMTokenList.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/dom/TaskRunnerHelper.h"
 #include "core/html/HTMLDivElement.h"
 #include "core/html/track/vtt/VTTParser.h"
 #include "core/html/track/vtt/VTTScanner.h"
@@ -91,8 +92,7 @@ VTTRegion::VTTRegion()
       m_viewportAnchor(FloatPoint(defaultAnchorPointX, defaultAnchorPointY)),
       m_scroll(defaultScroll),
       m_track(nullptr),
-      m_currentTop(0),
-      m_scrollTimer(this, &VTTRegion::scrollTimerFired) {}
+      m_currentTop(0) {}
 
 VTTRegion::~VTTRegion() {}
 
@@ -319,6 +319,7 @@ HTMLDivElement* VTTRegion::getDisplayTree(Document& document) {
   if (!m_regionDisplayTree) {
     m_regionDisplayTree = HTMLDivElement::create(document);
     prepareRegionDisplayTree();
+    prepareScrollTimer(document);
   }
 
   return m_regionDisplayTree;
@@ -357,7 +358,7 @@ void VTTRegion::displayLastVTTCueBox() {
   // css property to move elements. We should just scroll the text track cues on
   // the compositor with an animation.
 
-  if (m_scrollTimer.isActive())
+  if (!m_scrollTimer || m_scrollTimer->isActive())
     return;
 
   // If it's a scrolling region, add the scrolling class.
@@ -369,7 +370,7 @@ void VTTRegion::displayLastVTTCueBox() {
 
   // Find first cue that is not entirely displayed and scroll it upwards.
   for (Element* child = ElementTraversal::firstChild(*m_cueContainer);
-       child && !m_scrollTimer.isActive();
+       child && !m_scrollTimer->isActive();
        child = ElementTraversal::nextSibling(*child)) {
     ClientRect* clientRect = child->getBoundingClientRect();
     float childTop = clientRect->top();
@@ -439,19 +440,27 @@ void VTTRegion::prepareRegionDisplayTree() {
   m_regionDisplayTree->setShadowPseudoId(textTrackRegionShadowPseudoId());
 }
 
+void VTTRegion::prepareScrollTimer(Document& document) {

[fs, 2017/02/08 11:57:54]

Maybe just fold this into startTimer(), and possibly add a new method for
checking if the timer is active (that considers the possibility of a null timer
- if on-demand allocation is deemed necessary.) Could also fold stopTimer() into
it's only caller (to avoid pondering the contract there.)

+  DCHECK(!m_scrollTimer);
+  m_scrollTimer = WTF::makeUnique<TaskRunnerTimer<VTTRegion>>(
+      TaskRunnerHelper::get(TaskType::MediaElementEvent, &document), this,

[haraken, 2017/02/08 11:49:40]

foolip@ should know better but I guess this is not a speced task. Maybe
TaskType::UnspecedTimer?

https://w3c.github.io/webvtt/

[fs, 2017/02/08 11:57:54]

AFAIK, this timer is an implementation detail, so I guess "UnspecedTimer" would
be more appropriate.

[foolip, 2017/02/08 13:46:38]

Yep, I think so too. If that means that the &document isn't needed, I guess much
about this will be simplified?

If this timer could lead to any events being fired, and if it could be triggered
at a predictable time by scripts, then it'd be a different matter. That it's not
speced could always be a spec bug, but if it's unobservable then it just doesn't
matter.

+      &VTTRegion::scrollTimerFired);
+}
+
 void VTTRegion::startTimer() {
+  DCHECK(m_scrollTimer);
   DVLOG(VTT_LOG_LEVEL) << "startTimer";
 
-  if (m_scrollTimer.isActive())
+  if (m_scrollTimer->isActive())
     return;
 
   double duration = isScrollingRegion() ? scrollTime : 0;
-  m_scrollTimer.startOneShot(duration, BLINK_FROM_HERE);
+  m_scrollTimer->startOneShot(duration, BLINK_FROM_HERE);
 }
 
 void VTTRegion::stopTimer() {
   DVLOG(VTT_LOG_LEVEL) << "stopTimer";
-  m_scrollTimer.stop();
+  m_scrollTimer->stop();
 }
 
 void VTTRegion::scrollTimerFired(TimerBase*) {