Title: [173132] trunk/Source/WebCore
- Revision
- 173132
- Author
- [email protected]
- Date
- 2014-08-29 18:11:23 -0700 (Fri, 29 Aug 2014)
Log Message
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
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (173131 => 173132)
--- trunk/Source/WebCore/ChangeLog 2014-08-30 00:19:42 UTC (rev 173131)
+++ trunk/Source/WebCore/ChangeLog 2014-08-30 01:11:23 UTC (rev 173132)
@@ -1,3 +1,38 @@
+2014-08-29 Gavin Barraclough <[email protected]>
+
+ 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 <[email protected]>
Improve showRenderTree() output.
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (173131 => 173132)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-08-30 00:19:42 UTC (rev 173131)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-08-30 01:11:23 UTC (rev 173132)
@@ -55,12 +55,12 @@
{
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 @@
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 @@
// 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::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
{
+ ASSERT(m_nestingLevel <= maxTimerNestingLevel);
if (m_nestingLevel < maxTimerNestingLevel)
return;
@@ -218,6 +223,8 @@
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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
