DOMTimer::m_nestingLevel is prone to overflow
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Aug 2014 01:11:23 +0000 (01:11 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Aug 2014 01:11:23 +0000 (01:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136399

Reviewed by Alexey Proskuryakov.

Since this would happen after the 2 billionth timer fire this is unlikely,
and consequences aren't severe (breaks throttling).

This change has the following consequences.

    - m_nestingLevel saturates to its max value.
    - unnested timers are indicated by a nesting level of 0.
    - repeat timers update m_nestingLevel on every fire,
      not just those that should have been throttled.

The last point is subtle, but ultimately should be inconsequential. Timers
whose requested timeout is less that the minimum interval will saturate quickly
anyway; timers with an original interval greater than the minimum previously
wouldn't have incremented m_nestingLevel, but doing so now doesn't hurt since
they won't be throttled when they hit the threshold. This simplifies things
conceptually a little & reduces the test performed on each timer fire.

* page/DOMTimer.cpp:
(WebCore::shouldForwardUserGesture):
    - unnested timers are indicated by a nesting level of 0
(WebCore::DOMTimer::DOMTimer):
    - don't increment nesting level on construction
(WebCore::DOMTimer::fired):
    - saturating increments
(WebCore::DOMTimer::adjustMinimumTimerInterval):
(WebCore::DOMTimer::intervalClampedToMinimum):
    - added ASSERTs

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

Source/WebCore/ChangeLog
Source/WebCore/page/DOMTimer.cpp

index 50caec4..f4fc199 100644 (file)
@@ -1,3 +1,38 @@
+2014-08-29  Gavin Barraclough  <barraclough@apple.com>
+
+        DOMTimer::m_nestingLevel is prone to overflow
+        https://bugs.webkit.org/show_bug.cgi?id=136399
+
+        Reviewed by Alexey Proskuryakov.
+
+        Since this would happen after the 2 billionth timer fire this is unlikely,
+        and consequences aren't severe (breaks throttling).
+
+        This change has the following consequences.
+
+            - m_nestingLevel saturates to its max value.
+            - unnested timers are indicated by a nesting level of 0.
+            - repeat timers update m_nestingLevel on every fire,
+              not just those that should have been throttled.
+
+        The last point is subtle, but ultimately should be inconsequential. Timers
+        whose requested timeout is less that the minimum interval will saturate quickly
+        anyway; timers with an original interval greater than the minimum previously
+        wouldn't have incremented m_nestingLevel, but doing so now doesn't hurt since
+        they won't be throttled when they hit the threshold. This simplifies things
+        conceptually a little & reduces the test performed on each timer fire.
+
+        * page/DOMTimer.cpp:
+        (WebCore::shouldForwardUserGesture):
+            - unnested timers are indicated by a nesting level of 0
+        (WebCore::DOMTimer::DOMTimer):
+            - don't increment nesting level on construction
+        (WebCore::DOMTimer::fired):
+            - saturating increments
+        (WebCore::DOMTimer::adjustMinimumTimerInterval):
+        (WebCore::DOMTimer::intervalClampedToMinimum):
+            - added ASSERTs
+
 2014-08-29  Zalan Bujtas  <zalan@apple.com>
 
         Improve showRenderTree() output.
index 348ef48..844360b 100644 (file)
@@ -55,12 +55,12 @@ static inline bool shouldForwardUserGesture(int interval, int nestingLevel)
 {
     return UserGestureIndicator::processingUserGesture()
         && interval <= maxIntervalForUserGestureForwarding
-        && nestingLevel == 1; // Gestures should not be forwarded to nested timers.
+        && !nestingLevel; // Gestures should not be forwarded to nested timers.
 }
 
 DOMTimer::DOMTimer(ScriptExecutionContext* context, std::unique_ptr<ScheduledAction> action, int interval, bool singleShot)
     : SuspendableTimer(context)
-    , m_nestingLevel(timerNestingLevel + 1)
+    , m_nestingLevel(timerNestingLevel)
     , m_action(WTF::move(action))
     , m_originalInterval(interval)
     , m_shouldForwardUserGesture(shouldForwardUserGesture(interval, m_nestingLevel))
@@ -130,7 +130,8 @@ void DOMTimer::fired()
         ASSERT(!document->frame()->timersPaused());
     }
 #endif
-    timerNestingLevel = m_nestingLevel;
+    timerNestingLevel = std::min(m_nestingLevel + 1, maxTimerNestingLevel);
+
     ASSERT(!isSuspended());
     ASSERT(!context->activeDOMObjectsAreSuspended());
     UserGestureIndicator gestureIndicator(m_shouldForwardUserGesture ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
@@ -141,11 +142,14 @@ void DOMTimer::fired()
 
     // Simple case for non-one-shot timers.
     if (isActive()) {
-        double minimumInterval = context->minimumTimerInterval();
-        if (repeatInterval() && repeatInterval() < minimumInterval) {
+        if (m_nestingLevel < maxTimerNestingLevel) {
             m_nestingLevel++;
-            if (m_nestingLevel >= maxTimerNestingLevel)
-                augmentRepeatInterval(minimumInterval - repeatInterval());
+
+            double minimumInterval = context->minimumTimerInterval();
+            if (repeatInterval() && repeatInterval() < minimumInterval) {
+                if (m_nestingLevel == maxTimerNestingLevel)
+                    augmentRepeatInterval(minimumInterval - repeatInterval());
+            }
         }
 
         m_action->execute(context);
@@ -201,6 +205,7 @@ void DOMTimer::didStop()
 
 void DOMTimer::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
 {
+    ASSERT(m_nestingLevel <= maxTimerNestingLevel);
     if (m_nestingLevel < maxTimerNestingLevel)
         return;
 
@@ -218,6 +223,8 @@ void DOMTimer::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
 
 double DOMTimer::intervalClampedToMinimum(int timeout, double minimumTimerInterval) const
 {
+    ASSERT(m_nestingLevel <= maxTimerNestingLevel);
+
     double intervalMilliseconds = std::max(oneMillisecond, timeout * oneMillisecond);
 
     if (intervalMilliseconds < minimumTimerInterval && m_nestingLevel >= maxTimerNestingLevel)