Changeset 176282 in webkit

Timestamp:

Nov 18, 2014, 2:05:10 PM (12 years ago)

Author:

Chris Dumez

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/timer-unthrottle-on-scroll-expected.txt (added)
• LayoutTests/fast/dom/timer-unthrottle-on-scroll.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/DOMTimer.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-11-18  Chris Dumez  <cdumez@apple.com>
+
+        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  <litherum@gmail.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-11-18  Chris Dumez  <cdumez@apple.com>
+
+        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  <dbates@webkit.org>
 

trunk/Source/WebCore/page/DOMTimer.cpp

@@ -255 +255 @@
 {
     if (fireState.scriptMadeUserObservableChanges()) {
+        ASSERT(m_elementsCausingThrottling.isEmpty());
         if (m_throttleState != ShouldNotThrottle) {
             m_throttleState = ShouldNotThrottle;
-            ASSERT(m_elementsCausingThrottling.isEmpty());
             updateTimerIntervalIfNecessary();
         }
@@ -263 +263 @@
         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();
     }
 }
@@ -429 +433 @@
         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));
@@ -458 +455 @@
             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;