<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[173132] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/173132">173132</a></dd>
<dt>Author</dt> <dd>barraclough@apple.com</dd>
<dt>Date</dt> <dd>2014-08-29 18:11:23 -0700 (Fri, 29 Aug 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageDOMTimercpp">trunk/Source/WebCore/page/DOMTimer.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (173131 => 173132)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+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
+
</ins><span class="cx"> 2014-08-29 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Improve showRenderTree() output.
</span></span></pre></div>
<a id="trunkSourceWebCorepageDOMTimercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/DOMTimer.cpp (173131 => 173132)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -55,12 +55,12 @@
</span><span class="cx"> {
</span><span class="cx"> return UserGestureIndicator::processingUserGesture()
</span><span class="cx"> && interval <= maxIntervalForUserGestureForwarding
</span><del>- && nestingLevel == 1; // Gestures should not be forwarded to nested timers.
</del><ins>+ && !nestingLevel; // Gestures should not be forwarded to nested timers.
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> DOMTimer::DOMTimer(ScriptExecutionContext* context, std::unique_ptr<ScheduledAction> action, int interval, bool singleShot)
</span><span class="cx"> : SuspendableTimer(context)
</span><del>- , m_nestingLevel(timerNestingLevel + 1)
</del><ins>+ , m_nestingLevel(timerNestingLevel)
</ins><span class="cx"> , m_action(WTF::move(action))
</span><span class="cx"> , m_originalInterval(interval)
</span><span class="cx"> , m_shouldForwardUserGesture(shouldForwardUserGesture(interval, m_nestingLevel))
</span><span class="lines">@@ -130,7 +130,8 @@
</span><span class="cx"> ASSERT(!document->frame()->timersPaused());
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><del>- timerNestingLevel = m_nestingLevel;
</del><ins>+ timerNestingLevel = std::min(m_nestingLevel + 1, maxTimerNestingLevel);
+
</ins><span class="cx"> ASSERT(!isSuspended());
</span><span class="cx"> ASSERT(!context->activeDOMObjectsAreSuspended());
</span><span class="cx"> UserGestureIndicator gestureIndicator(m_shouldForwardUserGesture ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
</span><span class="lines">@@ -141,11 +142,14 @@
</span><span class="cx">
</span><span class="cx"> // Simple case for non-one-shot timers.
</span><span class="cx"> if (isActive()) {
</span><del>- double minimumInterval = context->minimumTimerInterval();
- if (repeatInterval() && repeatInterval() < minimumInterval) {
</del><ins>+ if (m_nestingLevel < maxTimerNestingLevel) {
</ins><span class="cx"> m_nestingLevel++;
</span><del>- if (m_nestingLevel >= maxTimerNestingLevel)
- augmentRepeatInterval(minimumInterval - repeatInterval());
</del><ins>+
+ double minimumInterval = context->minimumTimerInterval();
+ if (repeatInterval() && repeatInterval() < minimumInterval) {
+ if (m_nestingLevel == maxTimerNestingLevel)
+ augmentRepeatInterval(minimumInterval - repeatInterval());
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_action->execute(context);
</span><span class="lines">@@ -201,6 +205,7 @@
</span><span class="cx">
</span><span class="cx"> void DOMTimer::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
</span><span class="cx"> {
</span><ins>+ ASSERT(m_nestingLevel <= maxTimerNestingLevel);
</ins><span class="cx"> if (m_nestingLevel < maxTimerNestingLevel)
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -218,6 +223,8 @@
</span><span class="cx">
</span><span class="cx"> double DOMTimer::intervalClampedToMinimum(int timeout, double minimumTimerInterval) const
</span><span class="cx"> {
</span><ins>+ ASSERT(m_nestingLevel <= maxTimerNestingLevel);
+
</ins><span class="cx"> double intervalMilliseconds = std::max(oneMillisecond, timeout * oneMillisecond);
</span><span class="cx">
</span><span class="cx"> if (intervalMilliseconds < minimumTimerInterval && m_nestingLevel >= maxTimerNestingLevel)
</span></span></pre>
</div>
</div>
</body>
</html>