Timer's nextFireInterval() / repeatInterval() should return Seconds
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Apr 2017 17:09:09 +0000 (17:09 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Apr 2017 17:09:09 +0000 (17:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170639

Reviewed by Simon Fraser.

Timer's nextFireInterval() / repeatInterval() should return Seconds, not double.

Source/WebCore:

* loader/NavigationScheduler.cpp:
* page/DOMTimer.cpp:
(WebCore::DOMTimer::updateTimerIntervalIfNecessary):
* page/SuspendableTimer.cpp:
(WebCore::SuspendableTimer::suspend):
(WebCore::SuspendableTimer::repeatInterval):
* page/SuspendableTimer.h:
* page/animation/AnimationBase.cpp:
(WebCore::AnimationBase::timeToNextService):
(WebCore::AnimationBase::getTimeToNextEvent):
(WebCore::AnimationBase::goIntoEndingOrLoopingState):
* page/animation/AnimationBase.h:
* page/animation/CSSAnimationController.cpp:
(WebCore::CSSAnimationControllerPrivate::updateAnimations):
(WebCore::CSSAnimationControllerPrivate::updateAnimationTimerForRenderer):
(WebCore::CSSAnimationControllerPrivate::updateAnimationTimer):
(WebCore::CSSAnimationControllerPrivate::animationFrameCallbackFired):
* page/animation/CSSAnimationControllerPrivate.h:
* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::timeToNextService):
* page/animation/CompositeAnimation.h:
* page/animation/ImplicitAnimation.cpp:
(WebCore::ImplicitAnimation::timeToNextService):
* page/animation/ImplicitAnimation.h:
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::timeToNextService):
* page/animation/KeyframeAnimation.h:
* platform/ThreadTimers.cpp:
(WebCore::ThreadTimers::sharedTimerFiredInternal):
* platform/Timer.cpp:
(WebCore::TimerBase::nextFireInterval):
* platform/Timer.h:
(WebCore::TimerBase::repeatInterval):
* platform/graphics/ca/TileController.cpp:
(WebCore::TileController::setIsInWindow):
(WebCore::TileController::scheduleTileRevalidation):
* platform/graphics/ca/TileController.h:
* platform/graphics/ca/TileGrid.cpp:
(WebCore::TileGrid::revalidateTiles):

Source/WebKit2:

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::layerVolatilityTimerFired):
(WebKit::WebPage::markLayersVolatile):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215143 268f45cc-cd09-0410-ab3c-d52691b4dbfc

23 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/NavigationScheduler.cpp
Source/WebCore/page/DOMTimer.cpp
Source/WebCore/page/SuspendableTimer.cpp
Source/WebCore/page/SuspendableTimer.h
Source/WebCore/page/animation/AnimationBase.cpp
Source/WebCore/page/animation/AnimationBase.h
Source/WebCore/page/animation/CSSAnimationController.cpp
Source/WebCore/page/animation/CSSAnimationControllerPrivate.h
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/page/animation/CompositeAnimation.h
Source/WebCore/page/animation/ImplicitAnimation.cpp
Source/WebCore/page/animation/ImplicitAnimation.h
Source/WebCore/page/animation/KeyframeAnimation.cpp
Source/WebCore/page/animation/KeyframeAnimation.h
Source/WebCore/platform/ThreadTimers.cpp
Source/WebCore/platform/Timer.cpp
Source/WebCore/platform/Timer.h
Source/WebCore/platform/graphics/ca/TileController.cpp
Source/WebCore/platform/graphics/ca/TileController.h
Source/WebCore/platform/graphics/ca/TileGrid.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/WebPage.cpp

index f9655b3..b6a6c2a 100644 (file)
@@ -1,3 +1,52 @@
+2017-04-08  Chris Dumez  <cdumez@apple.com>
+
+        Timer's nextFireInterval() / repeatInterval() should return Seconds
+        https://bugs.webkit.org/show_bug.cgi?id=170639
+
+        Reviewed by Simon Fraser.
+
+        Timer's nextFireInterval() / repeatInterval() should return Seconds, not double.
+
+        * loader/NavigationScheduler.cpp:
+        * page/DOMTimer.cpp:
+        (WebCore::DOMTimer::updateTimerIntervalIfNecessary):
+        * page/SuspendableTimer.cpp:
+        (WebCore::SuspendableTimer::suspend):
+        (WebCore::SuspendableTimer::repeatInterval):
+        * page/SuspendableTimer.h:
+        * page/animation/AnimationBase.cpp:
+        (WebCore::AnimationBase::timeToNextService):
+        (WebCore::AnimationBase::getTimeToNextEvent):
+        (WebCore::AnimationBase::goIntoEndingOrLoopingState):
+        * page/animation/AnimationBase.h:
+        * page/animation/CSSAnimationController.cpp:
+        (WebCore::CSSAnimationControllerPrivate::updateAnimations):
+        (WebCore::CSSAnimationControllerPrivate::updateAnimationTimerForRenderer):
+        (WebCore::CSSAnimationControllerPrivate::updateAnimationTimer):
+        (WebCore::CSSAnimationControllerPrivate::animationFrameCallbackFired):
+        * page/animation/CSSAnimationControllerPrivate.h:
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::timeToNextService):
+        * page/animation/CompositeAnimation.h:
+        * page/animation/ImplicitAnimation.cpp:
+        (WebCore::ImplicitAnimation::timeToNextService):
+        * page/animation/ImplicitAnimation.h:
+        * page/animation/KeyframeAnimation.cpp:
+        (WebCore::KeyframeAnimation::timeToNextService):
+        * page/animation/KeyframeAnimation.h:
+        * platform/ThreadTimers.cpp:
+        (WebCore::ThreadTimers::sharedTimerFiredInternal):
+        * platform/Timer.cpp:
+        (WebCore::TimerBase::nextFireInterval):
+        * platform/Timer.h:
+        (WebCore::TimerBase::repeatInterval):
+        * platform/graphics/ca/TileController.cpp:
+        (WebCore::TileController::setIsInWindow):
+        (WebCore::TileController::scheduleTileRevalidation):
+        * platform/graphics/ca/TileController.h:
+        * platform/graphics/ca/TileGrid.cpp:
+        (WebCore::TileGrid::revalidateTiles):
+
 2017-04-08  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         Unreviewed Mac cmake buildfix after r215051, just for fun.
index 0f0290c..a391851 100644 (file)
@@ -135,7 +135,7 @@ protected:
         m_haveToldClient = true;
 
         UserGestureIndicator gestureIndicator(userGestureToForward());
-        frame.loader().clientRedirected(m_url, delay(), currentTime() + timer.nextFireInterval(), lockBackForwardList());
+        frame.loader().clientRedirected(m_url, delay(), currentTime() + timer.nextFireInterval().value(), lockBackForwardList());
     }
 
     void didStopTimer(Frame& frame, bool newLoadInProgress) override
@@ -278,7 +278,7 @@ public:
         m_haveToldClient = true;
 
         UserGestureIndicator gestureIndicator(userGestureToForward());
-        frame.loader().clientRedirected(m_submission->requestURL(), delay(), currentTime() + timer.nextFireInterval(), lockBackForwardList());
+        frame.loader().clientRedirected(m_submission->requestURL(), delay(), currentTime() + timer.nextFireInterval().value(), lockBackForwardList());
     }
 
     void didStopTimer(Frame& frame, bool newLoadInProgress) override
index 48876ec..b3b6953 100644 (file)
@@ -400,7 +400,7 @@ void DOMTimer::updateTimerIntervalIfNecessary()
         return;
 
     if (repeatInterval()) {
-        ASSERT(repeatIntervalSeconds() == previousInterval);
+        ASSERT(repeatInterval() == previousInterval);
         LOG(DOMTimers, "%p - Updating DOMTimer's repeat interval from %.2f ms to %.2f ms due to throttling.", this, previousInterval.milliseconds(), m_currentTimerInterval.milliseconds());
         augmentRepeatInterval(m_currentTimerInterval - previousInterval);
     } else {
index fa0849d..2876286 100644 (file)
@@ -66,7 +66,7 @@ void SuspendableTimer::suspend(ReasonForSuspension)
     m_savedIsActive = TimerBase::isActive();
     if (m_savedIsActive) {
         m_savedNextFireInterval = TimerBase::nextUnalignedFireInterval();
-        m_savedRepeatInterval = TimerBase::repeatIntervalSeconds();
+        m_savedRepeatInterval = TimerBase::repeatInterval();
         TimerBase::stop();
     }
 }
@@ -119,13 +119,13 @@ void SuspendableTimer::startOneShot(Seconds interval)
     }
 }
 
-double SuspendableTimer::repeatInterval() const
+Seconds SuspendableTimer::repeatInterval() const
 {
     if (!m_suspended)
         return TimerBase::repeatInterval();
     if (m_savedIsActive)
-        return m_savedRepeatInterval.value();
-    return 0;
+        return m_savedRepeatInterval;
+    return 0_s;
 }
 
 void SuspendableTimer::augmentFireInterval(Seconds delta)
index f0671d9..b079e56 100644 (file)
@@ -48,7 +48,7 @@ public:
     void startRepeating(double repeatInterval) { startRepeating(Seconds { repeatInterval }); }
     void startOneShot(double interval) { startOneShot(Seconds { interval }); }
 
-    double repeatInterval() const;
+    Seconds repeatInterval() const;
     void augmentFireInterval(double delta) { augmentFireInterval(Seconds { delta }); }
     void augmentRepeatInterval(double delta) { augmentRepeatInterval(Seconds { delta }); }
 
@@ -59,9 +59,6 @@ public:
     void startRepeating(std::chrono::milliseconds repeatInterval) { startRepeating(msToSeconds(repeatInterval)); }
     void startOneShot(std::chrono::milliseconds interval) { startOneShot(msToSeconds(interval)); }
 
-    std::chrono::milliseconds repeatIntervalMS() const { return secondsToMS(repeatInterval()); }
-    Seconds repeatIntervalSeconds() const { return Seconds { repeatInterval() }; }
-
     void augmentFireInterval(Seconds delta);
     void augmentRepeatInterval(Seconds delta);
 
index e177f0d..a04e17b 100644 (file)
@@ -564,12 +564,12 @@ void AnimationBase::updatePlayState(EAnimPlayState playState)
     updateStateMachine(pause ?  AnimationStateInput::PlayStatePaused : AnimationStateInput::PlayStateRunning, -1);
 }
 
-double AnimationBase::timeToNextService()
+std::optional<Seconds> AnimationBase::timeToNextService()
 {
-    // Returns the time at which next service is required. -1 means no service is required. 0 means 
+    // Returns the time at which next service is required. std::nullopt means no service is required. 0 means
     // service is required now, and > 0 means service is required that many seconds in the future.
     if (paused() || isNew() || postActive() || fillingForwards())
-        return -1;
+        return std::nullopt;
     
     if (m_animationState == AnimationState::StartWaitTimer) {
 #if ENABLE(CSS_ANIMATIONS_LEVEL_2)
@@ -578,19 +578,19 @@ double AnimationBase::timeToNextService()
                 float currentScrollPosition = m_object->view().frameView().scrollPositionForFixedPosition().y().toFloat();
                 auto& scrollTrigger = downcast<ScrollAnimationTrigger>(*m_animation->trigger());
                 if (currentScrollPosition >= scrollTrigger.startValue().value() && (!scrollTrigger.hasEndValue() || currentScrollPosition <= scrollTrigger.endValue().value()))
-                    return 0;
+                    return 0_s;
             }
-            return -1;
+            return std::nullopt;
         }
 #endif
         double timeFromNow = m_animation->delay() - (beginAnimationUpdateTime() - m_requestedStartTime);
-        return std::max(timeFromNow, 0.0);
+        return std::max(Seconds { timeFromNow }, 0_s);
     }
     
     fireAnimationEventsIfNeeded();
         
     // In all other cases, we need service right away.
-    return 0;
+    return 0_s;
 }
 
 // Compute the fractional time, taking into account direction.
@@ -675,7 +675,7 @@ double AnimationBase::progress(double scale, double offset, const TimingFunction
     return 0;
 }
 
-void AnimationBase::getTimeToNextEvent(double& time, bool& isLooping) const
+void AnimationBase::getTimeToNextEvent(Seconds& time, bool& isLooping) const
 {
     // Decide when the end or loop event needs to fire
     const double elapsedDuration = std::max(beginAnimationUpdateTime() - m_startTime, 0.0);
@@ -696,12 +696,12 @@ void AnimationBase::getTimeToNextEvent(double& time, bool& isLooping) const
         isLooping = false;
     }
     
-    time = durationLeft;
+    time = Seconds { durationLeft };
 }
 
 void AnimationBase::goIntoEndingOrLoopingState()
 {
-    double t;
+    Seconds t;
     bool isLooping;
     getTimeToNextEvent(t, isLooping);
     LOG(Animations, "%p AnimationState %s -> %s", this, nameForState(m_animationState), isLooping ? "Looping" : "Ending");
index 2dd0c30..7690c91 100644 (file)
@@ -129,7 +129,7 @@ public:
 
     bool isAccelerated() const { return m_isAccelerated; }
 
-    virtual double timeToNextService();
+    virtual std::optional<Seconds> timeToNextService();
 
     double progress(double scale = 1, double offset = 0, const TimingFunction* = nullptr) const;
 
@@ -232,7 +232,7 @@ protected:
 
     static void setNeedsStyleRecalc(Element*);
     
-    void getTimeToNextEvent(double& time, bool& isLooping) const;
+    void getTimeToNextEvent(Seconds& time, bool& isLooping) const;
 
     double fractionalTime(double scale, double elapsedTime, double offset) const;
 
index 0a7e51f..a53cce7 100644 (file)
@@ -123,19 +123,19 @@ bool CSSAnimationControllerPrivate::clear(RenderElement& renderer)
     return animation->isSuspended();
 }
 
-double CSSAnimationControllerPrivate::updateAnimations(SetChanged callSetChanged/* = DoNotCallSetChanged*/)
+std::optional<Seconds> CSSAnimationControllerPrivate::updateAnimations(SetChanged callSetChanged/* = DoNotCallSetChanged*/)
 {
     AnimationPrivateUpdateBlock updateBlock(*this);
-    double timeToNextService = -1;
+    std::optional<Seconds> timeToNextService;
     bool calledSetChanged = false;
 
     for (auto& compositeAnimation : m_compositeAnimations) {
         CompositeAnimation& animation = *compositeAnimation.value;
         if (!animation.isSuspended() && animation.hasAnimations()) {
-            double t = animation.timeToNextService();
-            if (t != -1 && (t < timeToNextService || timeToNextService == -1))
-                timeToNextService = t;
-            if (!timeToNextService) {
+            std::optional<Seconds> t = animation.timeToNextService();
+            if (t && (!timeToNextService || t.value() < timeToNextService.value()))
+                timeToNextService = t.value();
+            if (timeToNextService && timeToNextService.value() == 0_s) {
                 if (callSetChanged != CallSetChanged)
                     break;
                 Element* element = compositeAnimation.key->element();
@@ -155,44 +155,47 @@ double CSSAnimationControllerPrivate::updateAnimations(SetChanged callSetChanged
 
 void CSSAnimationControllerPrivate::updateAnimationTimerForRenderer(RenderElement& renderer)
 {
-    double timeToNextService = 0;
+    std::optional<Seconds> timeToNextService;
 
     const CompositeAnimation* compositeAnimation = m_compositeAnimations.get(&renderer);
     if (!compositeAnimation->isSuspended() && compositeAnimation->hasAnimations())
         timeToNextService = compositeAnimation->timeToNextService();
 
-    if (m_animationTimer.isActive() && (m_animationTimer.repeatInterval() || m_animationTimer.nextFireInterval() <= timeToNextService))
+    if (!timeToNextService)
         return;
 
-    m_animationTimer.startOneShot(timeToNextService);
+    if (m_animationTimer.isActive() && (m_animationTimer.repeatInterval() || m_animationTimer.nextFireInterval() <= timeToNextService.value()))
+        return;
+
+    m_animationTimer.startOneShot(timeToNextService.value());
 }
 
 void CSSAnimationControllerPrivate::updateAnimationTimer(SetChanged callSetChanged/* = DoNotCallSetChanged*/)
 {
-    double timeToNextService = updateAnimations(callSetChanged);
+    std::optional<Seconds> timeToNextService = updateAnimations(callSetChanged);
 
-    LOG(Animations, "updateAnimationTimer: timeToNextService is %.2f", timeToNextService);
+    LOG(Animations, "updateAnimationTimer: timeToNextService is %.2f", timeToNextService.value_or(Seconds { -1 }).value());
 
-    // If we want service immediately, we start a repeating timer to reduce the overhead of starting
+    // If we don't need service, we want to make sure the timer is no longer running
     if (!timeToNextService) {
+        if (m_animationTimer.isActive())
+            m_animationTimer.stop();
+        return;
+    }
+
+    // If we want service immediately, we start a repeating timer to reduce the overhead of starting
+    if (!timeToNextService.value()) {
         auto* page = m_frame.page();
         bool shouldThrottle = page && page->isLowPowerModeEnabled();
         Seconds delay = shouldThrottle ? animationTimerThrottledDelay : animationTimerDelay;
 
-        if (!m_animationTimer.isActive() || m_animationTimer.repeatInterval() != delay.value())
+        if (!m_animationTimer.isActive() || m_animationTimer.repeatInterval() != delay)
             m_animationTimer.startRepeating(delay);
         return;
     }
 
-    // If we don't need service, we want to make sure the timer is no longer running
-    if (timeToNextService < 0) {
-        if (m_animationTimer.isActive())
-            m_animationTimer.stop();
-        return;
-    }
-
     // Otherwise, we want to start a one-shot timer so we get here again
-    m_animationTimer.startOneShot(timeToNextService);
+    m_animationTimer.startOneShot(timeToNextService.value());
 }
 
 void CSSAnimationControllerPrivate::updateStyleIfNeededDispatcherFired()
@@ -247,9 +250,9 @@ void CSSAnimationControllerPrivate::addElementChangeToDispatch(Element& element)
 
 void CSSAnimationControllerPrivate::animationFrameCallbackFired()
 {
-    double timeToNextService = updateAnimations(CallSetChanged);
+    std::optional<Seconds> timeToNextService = updateAnimations(CallSetChanged);
 
-    if (timeToNextService >= 0)
+    if (timeToNextService)
         m_frame.document()->view()->scheduleAnimation();
 }
 
index 4c48ad6..955a887 100644 (file)
@@ -49,7 +49,7 @@ public:
     ~CSSAnimationControllerPrivate();
 
     // Returns the time until the next animation needs to be serviced, or -1 if there are none.
-    double updateAnimations(SetChanged callSetChanged = DoNotCallSetChanged);
+    std::optional<Seconds> updateAnimations(SetChanged callSetChanged = DoNotCallSetChanged);
     void updateAnimationTimer(SetChanged callSetChanged = DoNotCallSetChanged);
 
     CompositeAnimation& ensureCompositeAnimation(RenderElement&);
index d1684ee..daddda4 100644 (file)
@@ -366,33 +366,33 @@ std::unique_ptr<RenderStyle> CompositeAnimation::getAnimatedStyle() const
     return resultStyle;
 }
 
-double CompositeAnimation::timeToNextService() const
+std::optional<Seconds> CompositeAnimation::timeToNextService() const
 {
-    // Returns the time at which next service is required. -1 means no service is required. 0 means 
+    // Returns the time at which next service is required. std::nullopt means no service is required. 0 means
     // service is required now, and > 0 means service is required that many seconds in the future.
-    double minT = -1;
+    std::optional<Seconds> minT;
     
     if (!m_transitions.isEmpty()) {
         for (auto& transition : m_transitions.values()) {
-            double t = transition->timeToNextService();
-            if (t == -1)
+            std::optional<Seconds> t = transition->timeToNextService();
+            if (!t)
                 continue;
-            if (t < minT || minT == -1)
-                minT = t;
-            if (minT == 0)
-                return 0;
+            if (!minT || t.value() < minT.value())
+                minT = t.value();
+            if (minT.value() == 0_s)
+                return 0_s;
         }
     }
     if (!m_keyframeAnimations.isEmpty()) {
         m_keyframeAnimations.checkConsistency();
         for (auto& animation : m_keyframeAnimations.values()) {
-            double t = animation->timeToNextService();
-            if (t == -1)
+            std::optional<Seconds> t = animation->timeToNextService();
+            if (!t)
                 continue;
-            if (t < minT || minT == -1)
-                minT = t;
-            if (minT == 0)
-                return 0;
+            if (!minT || t.value() < minT.value())
+                minT = t.value();
+            if (minT.value() == 0_s)
+                return 0_s;
         }
     }
 
index f2b7798..5342c7d 100644 (file)
@@ -59,7 +59,7 @@ public:
     std::unique_ptr<RenderStyle> getAnimatedStyle() const;
     bool computeExtentOfTransformAnimation(LayoutRect&) const;
 
-    double timeToNextService() const;
+    std::optional<Seconds> timeToNextService() const;
     
     CSSAnimationControllerPrivate& animationController() const { return m_animationController; }
 
index 924eb4b..13976af 100644 (file)
@@ -315,17 +315,17 @@ void ImplicitAnimation::checkForMatchingBackdropFilterFunctionLists()
 }
 #endif
 
-double ImplicitAnimation::timeToNextService()
+std::optional<Seconds> ImplicitAnimation::timeToNextService()
 {
-    double t = AnimationBase::timeToNextService();
-    if (t != 0 || preActive())
+    std::optional<Seconds> t = AnimationBase::timeToNextService();
+    if (!t || t.value() != 0_s || preActive())
         return t;
-        
+
     // A return value of 0 means we need service. But if this is an accelerated animation we 
     // only need service at the end of the transition.
     if (CSSPropertyAnimation::animationOfPropertyIsAccelerated(m_animatingProperty) && isAccelerated()) {
         bool isLooping;
-        getTimeToNextEvent(t, isLooping);
+        getTimeToNextEvent(t.value(), isLooping);
     }
     return t;
 }
index 9447417..8e17195 100644 (file)
@@ -70,7 +70,7 @@ public:
 
     void blendPropertyValueInStyle(CSSPropertyID, RenderStyle*);
 
-    double timeToNextService() override;
+    std::optional<Seconds> timeToNextService() override;
     
     bool active() const { return m_active; }
     void setActive(bool b) { m_active = b; }
index 7e4da6d..4fff11c 100644 (file)
@@ -490,14 +490,14 @@ void KeyframeAnimation::checkForMatchingBackdropFilterFunctionLists()
 }
 #endif
 
-double KeyframeAnimation::timeToNextService()
+std::optional<Seconds> KeyframeAnimation::timeToNextService()
 {
-    double t = AnimationBase::timeToNextService();
-    if (t != 0 || preActive())
+    std::optional<Seconds> t = AnimationBase::timeToNextService();
+    if (!t || t.value() != 0_s || preActive())
         return t;
-        
+
     // A return value of 0 means we need service. But if we only have accelerated animations we 
-    // only need service at the end of the transition
+    // only need service at the end of the transition.
     bool acceleratedPropertiesOnly = true;
     
     for (auto propertyID : m_keyframes.properties()) {
@@ -509,7 +509,7 @@ double KeyframeAnimation::timeToNextService()
 
     if (acceleratedPropertiesOnly) {
         bool isLooping;
-        getTimeToNextEvent(t, isLooping);
+        getTimeToNextEvent(t.value(), isLooping);
     }
 
     return t;
index 0ab982b..4e000a1 100644 (file)
@@ -60,7 +60,7 @@ public:
     void setUnanimatedStyle(std::unique_ptr<RenderStyle> style) { m_unanimatedStyle = WTFMove(style); }
     RenderStyle* unanimatedStyle() const { return m_unanimatedStyle.get(); }
 
-    double timeToNextService() override;
+    std::optional<Seconds> timeToNextService() override;
 
 protected:
     void onAnimationStart(double elapsedTime) override;
index bc3a818..f665534 100644 (file)
@@ -111,7 +111,7 @@ void ThreadTimers::sharedTimerFiredInternal()
         timer->m_unalignedNextFireTime = MonotonicTime { };
         timer->heapDeleteMin();
 
-        Seconds interval = timer->repeatIntervalSeconds();
+        Seconds interval = timer->repeatInterval();
         timer->setNextFireTime(interval ? fireTime + interval : MonotonicTime { });
 
         // Once the timer has been fired, it may be deleted, so do nothing else with it after this point.
index 18fa9e5..fad4e5a 100644 (file)
@@ -226,13 +226,13 @@ void TimerBase::stop()
     ASSERT(!inHeap());
 }
 
-double TimerBase::nextFireInterval() const
+Seconds TimerBase::nextFireInterval() const
 {
     ASSERT(isActive());
     MonotonicTime current = MonotonicTime::now();
     if (m_nextFireTime < current)
-        return 0;
-    return (m_nextFireTime - current).value();
+        return 0_s;
+    return m_nextFireTime - current;
 }
 
 inline void TimerBase::checkHeapIndex() const
index 36733da..96c4f10 100644 (file)
@@ -70,11 +70,9 @@ public:
     WEBCORE_EXPORT void stop();
     bool isActive() const;
 
-    double nextFireInterval() const; // FIXME: Should return Seconds.
+    Seconds nextFireInterval() const;
     Seconds nextUnalignedFireInterval() const;
-    double repeatInterval() const { return m_repeatInterval.value(); } // FIXME: Should return Seconds.
-    Seconds repeatIntervalSeconds() const { return m_repeatInterval; } // FIXME: Remove once repeatInterval() returns Seconds.
-    std::chrono::milliseconds repeatIntervalMS() const { return secondsToMS(repeatInterval()); }
+    Seconds repeatInterval() const { return m_repeatInterval; }
 
     void augmentFireInterval(Seconds delta) { setNextFireTime(m_nextFireTime + delta); }
     void augmentFireInterval(std::chrono::milliseconds delta) { augmentFireInterval(msToSeconds(delta)); }
index cd092f6..01ea9af 100644 (file)
@@ -285,7 +285,7 @@ void TileController::setIsInWindow(bool isInWindow)
     if (m_isInWindow)
         setNeedsRevalidateTiles();
     else {
-        const double tileRevalidationTimeout = 4;
+        const Seconds tileRevalidationTimeout = 4_s;
         scheduleTileRevalidation(tileRevalidationTimeout);
     }
 }
@@ -480,7 +480,7 @@ void TileController::adjustTileCoverageRect(FloatRect& coverageRect, const Float
 #endif
 }
 
-void TileController::scheduleTileRevalidation(double interval)
+void TileController::scheduleTileRevalidation(Seconds interval)
 {
     if (m_tileRevalidationTimer.isActive() && m_tileRevalidationTimer.nextFireInterval() < interval)
         return;
index 2778950..f00c6bb 100644 (file)
@@ -36,6 +36,7 @@
 #include <wtf/Deque.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/RetainPtr.h>
+#include <wtf/Seconds.h>
 
 namespace WebCore {
 
@@ -141,7 +142,7 @@ public:
 private:
     TileGrid& tileGrid() { return *m_tileGrid; }
 
-    void scheduleTileRevalidation(double interval);
+    void scheduleTileRevalidation(Seconds interval);
 
     float topContentInset() const { return m_topContentInset; }
 
index 1d2d807..8a90c2b 100644 (file)
@@ -365,7 +365,7 @@ void TileGrid::revalidateTiles(TileValidationPolicy validationPolicy)
     TileCohort currCohort = nextTileCohort();
     unsigned tilesInCohort = 0;
 
-    double minimumRevalidationTimerDuration = std::numeric_limits<double>::max();
+    Seconds minimumRevalidationTimerDuration = Seconds::infinity();
     bool needsTileRevalidation = false;
     
     auto tileSize = m_controller.tileSize();
@@ -404,7 +404,7 @@ void TileGrid::revalidateTiles(TileValidationPolicy validationPolicy)
                         continue;
                     double timeUntilCohortExpires = cohort.timeUntilExpiration();
                     if (timeUntilCohortExpires > 0) {
-                        minimumRevalidationTimerDuration = std::min(minimumRevalidationTimerDuration, timeUntilCohortExpires);
+                        minimumRevalidationTimerDuration = std::min(minimumRevalidationTimerDuration, Seconds { timeUntilCohortExpires });
                         needsTileRevalidation = true;
                     } else
                         tileLayer->removeFromSuperlayer();
index e6cc5af..7333b09 100644 (file)
@@ -1,3 +1,16 @@
+2017-04-08  Chris Dumez  <cdumez@apple.com>
+
+        Timer's nextFireInterval() / repeatInterval() should return Seconds
+        https://bugs.webkit.org/show_bug.cgi?id=170639
+
+        Reviewed by Simon Fraser.
+
+        Timer's nextFireInterval() / repeatInterval() should return Seconds, not double.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::layerVolatilityTimerFired):
+        (WebKit::WebPage::markLayersVolatile):
+
 2017-04-07  Alex Christensen  <achristensen@webkit.org>
 
         Modernize WebPage.h
index e30db1b..74306f3 100644 (file)
@@ -254,8 +254,8 @@ using namespace WebCore;
 namespace WebKit {
 
 static const double pageScrollHysteresisSeconds = 0.3;
-static const std::chrono::milliseconds initialLayerVolatilityTimerInterval { 20 };
-static const std::chrono::seconds maximumLayerVolatilityTimerInterval { 2 };
+static const Seconds initialLayerVolatilityTimerInterval { 20_ms };
+static const Seconds maximumLayerVolatilityTimerInterval { 2_s };
 
 #define RELEASE_LOG_IF_ALLOWED(...) RELEASE_LOG_IF(isAlwaysOnLoggingAllowed(), Layers, __VA_ARGS__)
 #define RELEASE_LOG_ERROR_IF_ALLOWED(...) RELEASE_LOG_ERROR_IF(isAlwaysOnLoggingAllowed(), Layers, __VA_ARGS__)
@@ -2164,7 +2164,7 @@ void WebPage::callVolatilityCompletionHandlers()
 
 void WebPage::layerVolatilityTimerFired()
 {
-    auto newInterval = 2 * m_layerVolatilityTimer.repeatIntervalMS();
+    Seconds newInterval = m_layerVolatilityTimer.repeatInterval() * 2.;
     bool didSucceed = markLayersVolatileImmediatelyIfPossible();
     if (didSucceed || newInterval > maximumLayerVolatilityTimerInterval) {
         m_layerVolatilityTimer.stop();
@@ -2173,7 +2173,7 @@ void WebPage::layerVolatilityTimerFired()
         return;
     }
 
-    RELEASE_LOG_ERROR_IF_ALLOWED("%p - WebPage - Failed to mark all layers as volatile, will retry in %lld ms", this, static_cast<long long>(newInterval.count()));
+    RELEASE_LOG_ERROR_IF_ALLOWED("%p - WebPage - Failed to mark all layers as volatile, will retry in %g ms", this, newInterval.value() * 1000);
     m_layerVolatilityTimer.startRepeating(newInterval);
 }
 
@@ -2204,7 +2204,7 @@ void WebPage::markLayersVolatile(std::function<void ()> completionHandler)
         return;
     }
 
-    RELEASE_LOG_IF_ALLOWED("%p - Failed to mark all layers as volatile, will retry in %lld ms", this, initialLayerVolatilityTimerInterval.count());
+    RELEASE_LOG_IF_ALLOWED("%p - Failed to mark all layers as volatile, will retry in %g ms", this, initialLayerVolatilityTimerInterval.value() * 1000);
     m_layerVolatilityTimer.startRepeating(initialLayerVolatilityTimerInterval);
 }