Move VTTRegion Timers to TaskRunnerTimer

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

 /*
  * Copyright (C) 2013 Google 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 18 matching lines @@
  */
 
 #include "core/html/track/vtt/VTTRegion.h"
 
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/ClientRect.h"
 #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"
 #include "wtf/MathExtras.h"
 #include "wtf/text/StringBuilder.h"
 
 #define VTT_LOG_LEVEL 3
 
 namespace blink {
 
@@ skipping 35 matching lines @@
 }
 
 VTTRegion::VTTRegion()
     : m_id(emptyString),
       m_width(defaultWidth),
       m_heightInLines(defaultHeightInLines),
       m_regionAnchor(FloatPoint(defaultAnchorPointX, defaultAnchorPointY)),
       m_viewportAnchor(FloatPoint(defaultAnchorPointX, defaultAnchorPointY)),
       m_scroll(defaultScroll),
       m_track(nullptr),
-      m_currentTop(0),
-      m_scrollTimer(this, &VTTRegion::scrollTimerFired) {}
+      m_currentTop(0) {}
 
 VTTRegion::~VTTRegion() {}
 
 void VTTRegion::setTrack(TextTrack* track) {
   m_track = track;
 }
 
 void VTTRegion::setId(const String& id) {
   m_id = id;
 }
@@ skipping 206 matching lines @@
   DEFINE_STATIC_LOCAL(const AtomicString, trackRegionShadowPseudoId,
                       ("-webkit-media-text-track-region"));
 
   return trackRegionShadowPseudoId;
 }
 
 HTMLDivElement* VTTRegion::getDisplayTree(Document& document) {
   if (!m_regionDisplayTree) {
     m_regionDisplayTree = HTMLDivElement::create(document);
     prepareRegionDisplayTree();
+    prepareScrollTimer(document);
   }
 
   return m_regionDisplayTree;
 }
 
 void VTTRegion::willRemoveVTTCueBox(VTTCueBox* box) {
   DVLOG(VTT_LOG_LEVEL) << "willRemoveVTTCueBox";
   DCHECK(m_cueContainer->contains(box));
 
   double boxHeight = box->getBoundingClientRect()->bottom() -
@@ skipping 18 matching lines @@
 }
 
 void VTTRegion::displayLastVTTCueBox() {
   DVLOG(VTT_LOG_LEVEL) << "displayLastVTTCueBox";
   DCHECK(m_cueContainer);
 
   // FIXME: This should not be causing recalc styles in a loop to set the "top"
   // 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.
   if (isScrollingRegion())
     m_cueContainer->classList().add(textTrackCueContainerScrollingClass(),
                                     ASSERT_NO_EXCEPTION);
 
   float regionBottom = m_regionDisplayTree->getBoundingClientRect()->bottom();
 
   // 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();
     float childBottom = clientRect->bottom();
 
     if (regionBottom >= childBottom)
       continue;
 
     float height = childBottom - childTop;
 
@@ skipping 49 matching lines @@
   m_cueContainer->setInlineStyleProperty(CSSPropertyTop, 0.0,
                                          CSSPrimitiveValue::UnitType::Pixels);
 
   m_cueContainer->setShadowPseudoId(textTrackCueContainerShadowPseudoId());
   m_regionDisplayTree->appendChild(m_cueContainer);
 
   // 7.5 Every WebVTT region object is initialised with the following CSS
   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*) {
   DVLOG(VTT_LOG_LEVEL) << "scrollTimerFired";
 
   stopTimer();
   displayLastVTTCueBox();
 }
 
 DEFINE_TRACE(VTTRegion) {
   visitor->trace(m_cueContainer);
   visitor->trace(m_regionDisplayTree);
   visitor->trace(m_track);
 }
 
 }  // namespace blink