- Revision
- 176282
- Author
- [email protected]
- Date
- 2014-11-18 14:05:10 -0800 (Tue, 18 Nov 2014)
Log Message
DOMTimers sometimes don't get unthrottled on scrolling
https://bugs.webkit.org/show_bug.cgi?id=138838
Reviewed by Antti Koivisto.
Source/WebCore:
DOMTimers sometimes didn't get unthrottled on scrolling. This is
because we stopped listening for viewport changes every time the
timer fires, but we didn't listen for those changes again if we
decided the timer should stay throttled after the timer's action
was executed.
We should listen for viewport changes again after executing the
timer's action, if there are elements outside the viewport causing
the DOMTimer to be throttled, even if the throttleState hasn't
changed (i.e the timer stays throttled).
Test: fast/dom/timer-unthrottle-on-scroll.html
* page/DOMTimer.cpp:
(WebCore::DOMTimer::updateThrottlingStateIfNecessary):
(WebCore::DOMTimer::updateTimerIntervalIfNecessary):
(WebCore::DOMTimer::updateThrottlingStateAfterViewportChange):
LayoutTests:
Add a layout test to verify that DOMTimers get unthrottled on scroll if
the element whose style they are changing is inside the viewport after
the scroll.
* fast/dom/timer-unthrottle-on-scroll-expected.txt: Added.
* fast/dom/timer-unthrottle-on-scroll.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (176281 => 176282)
--- trunk/LayoutTests/ChangeLog 2014-11-18 21:28:10 UTC (rev 176281)
+++ trunk/LayoutTests/ChangeLog 2014-11-18 22:05:10 UTC (rev 176282)
@@ -1,3 +1,17 @@
+2014-11-18 Chris Dumez <[email protected]>
+
+ DOMTimers sometimes don't get unthrottled on scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=138838
+
+ Reviewed by Antti Koivisto.
+
+ Add a layout test to verify that DOMTimers get unthrottled on scroll if
+ the element whose style they are changing is inside the viewport after
+ the scroll.
+
+ * fast/dom/timer-unthrottle-on-scroll-expected.txt: Added.
+ * fast/dom/timer-unthrottle-on-scroll.html: Added.
+
2014-11-18 Myles C. Maxfield <[email protected]>
Use underlining metrics from the font file
Added: trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll-expected.txt (0 => 176282)
--- trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll-expected.txt 2014-11-18 22:05:10 UTC (rev 176282)
@@ -0,0 +1,19 @@
+Tests that a repeating timer changing the style of an element outside viewport gets unthrottled when scrolling the element into view.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.isTimerThrottled(timeoutId) is false
+5th iteration, timer should be throttled as the element is outside the viewport.
+PASS wasThrottled is true
+Timer should still be throttled before scrolling.
+PASS internals.isTimerThrottled(timeoutId) is true
+Scrolling element into view.
+Timer should no longer be throttled
+PASS internals.isTimerThrottled(timeoutId) is false
+6th iteration, timer should still be unthrottled.
+PASS internals.isTimerThrottled(timeoutId) is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll.html (0 => 176282)
--- trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll.html (rev 0)
+++ trunk/LayoutTests/fast/dom/timer-unthrottle-on-scroll.html 2014-11-18 22:05:10 UTC (rev 176282)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+<div style="position: relative; width: 1600px; height: 2400px; background-color: green;">
+ <div style="position:absolute; left: 600px; top: 800px;"> <input id="textbox" type="text"></div>
+</div>
+<script>
+description("Tests that a repeating timer changing the style of an element outside viewport gets unthrottled when scrolling the element into view.");
+jsTestIsAsync = true;
+
+var testElement = document.getElementById('textbox');
+var iterationCount = 0;
+var wasThrottled = false;
+var wasThrottledBeforeScroll = false;
+var timeoutId;
+
+function scrollElementIntoView()
+{
+ debug("5th iteration, timer should be throttled as the element is outside the viewport.");
+ shouldBeTrue("wasThrottled");
+
+ debug("Timer should still be throttled before scrolling.");
+ shouldBeTrue("internals.isTimerThrottled(timeoutId)");
+
+ debug("Scrolling element into view.");
+ window.internals.scrollElementToRect(testElement, 0, 0, 300, 300);
+
+ debug("Timer should no longer be throttled");
+ shouldBeFalse("internals.isTimerThrottled(timeoutId)");
+}
+
+function timerCallback()
+{
+ ++iterationCount;
+ // Change the style of the element while it is outside viewport.
+ testElement.style["opacity"] = "" + (iterationCount / 10.);
+
+ // 5 iterations should be sufficient to throttle the timer.
+ if (iterationCount == 5) {
+ // Do not use shouldBeTrue() because it would cause a DOM tree mutation
+ // and unthrottle the timer.
+ wasThrottled = internals.isTimerThrottled(timeoutId);
+ setTimeout(scrollElementIntoView, 0);
+ } else if (iterationCount == 6) {
+ debug("6th iteration, timer should still be unthrottled.");
+ shouldBeFalse("internals.isTimerThrottled(timeoutId)");
+
+ clearInterval(timeoutId);
+ finishJSTest();
+ }
+}
+
+timeoutId = setInterval(timerCallback, 0);
+shouldBeFalse("internals.isTimerThrottled(timeoutId)");
+</script>
+<script src=""
+</body>
Modified: trunk/Source/WebCore/ChangeLog (176281 => 176282)
--- trunk/Source/WebCore/ChangeLog 2014-11-18 21:28:10 UTC (rev 176281)
+++ trunk/Source/WebCore/ChangeLog 2014-11-18 22:05:10 UTC (rev 176282)
@@ -1,3 +1,28 @@
+2014-11-18 Chris Dumez <[email protected]>
+
+ DOMTimers sometimes don't get unthrottled on scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=138838
+
+ Reviewed by Antti Koivisto.
+
+ DOMTimers sometimes didn't get unthrottled on scrolling. This is
+ because we stopped listening for viewport changes every time the
+ timer fires, but we didn't listen for those changes again if we
+ decided the timer should stay throttled after the timer's action
+ was executed.
+
+ We should listen for viewport changes again after executing the
+ timer's action, if there are elements outside the viewport causing
+ the DOMTimer to be throttled, even if the throttleState hasn't
+ changed (i.e the timer stays throttled).
+
+ Test: fast/dom/timer-unthrottle-on-scroll.html
+
+ * page/DOMTimer.cpp:
+ (WebCore::DOMTimer::updateThrottlingStateIfNecessary):
+ (WebCore::DOMTimer::updateTimerIntervalIfNecessary):
+ (WebCore::DOMTimer::updateThrottlingStateAfterViewportChange):
+
2014-11-18 Daniel Bates <[email protected]>
[iOS] Make WebCore build and link with public SDK
Modified: trunk/Source/WebCore/page/DOMTimer.cpp (176281 => 176282)
--- trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-18 21:28:10 UTC (rev 176281)
+++ trunk/Source/WebCore/page/DOMTimer.cpp 2014-11-18 22:05:10 UTC (rev 176282)
@@ -254,17 +254,21 @@
void DOMTimer::updateThrottlingStateIfNecessary(const DOMTimerFireState& fireState)
{
if (fireState.scriptMadeUserObservableChanges()) {
+ ASSERT(m_elementsCausingThrottling.isEmpty());
if (m_throttleState != ShouldNotThrottle) {
m_throttleState = ShouldNotThrottle;
- ASSERT(m_elementsCausingThrottling.isEmpty());
updateTimerIntervalIfNecessary();
}
} else if (fireState.scriptMadeNonUserObservableChanges()) {
if (m_throttleState != ShouldThrottle) {
m_throttleState = ShouldThrottle;
- fireState.elementsChangedOutsideViewport(m_elementsCausingThrottling);
updateTimerIntervalIfNecessary();
}
+ // Update our vector of Elements causing throttling and register
+ // for viewport changes if the vector is not empty.
+ fireState.elementsChangedOutsideViewport(m_elementsCausingThrottling);
+ if (isIntervalDependentOnViewport())
+ registerForViewportChanges();
}
}
@@ -428,13 +432,6 @@
if (WTF::areEssentiallyEqual(previousInterval, m_currentTimerInterval, oneMillisecond))
return;
- // Timer was throttled / unthrottled, make sure we register / unregister
- // from the FrameView if the timer's interval is dependent on viewport.
- if (isIntervalDependentOnViewport())
- registerForViewportChanges();
- else if (m_throttleState == ShouldNotThrottle)
- unregisterForViewportChanges();
-
if (repeatInterval()) {
ASSERT(WTF::areEssentiallyEqual(repeatInterval(), previousInterval, oneMillisecond));
LOG(DOMTimers, "%p - Updating DOMTimer's repeat interval from %g ms to %g ms due to throttling.", this, previousInterval * 1000., m_currentTimerInterval * 1000.);
@@ -457,6 +454,7 @@
if (element->isInsideViewport(&visibleRect)) {
LOG(DOMTimers, "%p - Script is changing style of an element that is now inside the viewport, unthrottling the timer.", this);
m_throttleState = ShouldNotThrottle;
+ unregisterForViewportChanges();
updateTimerIntervalIfNecessary();
break;
}
