Title: [186195] trunk
Revision
186195
Author
[email protected]
Date
2015-07-01 14:52:07 -0700 (Wed, 01 Jul 2015)

Log Message

Regression(183998): Disqus comments take a very long time to load
https://bugs.webkit.org/show_bug.cgi?id=146522
<rdar://problem/21590601>

Reviewed by Simon Fraser.

Source/WebCore:

Stop throttling requestAnimationFrame() in iframes that are not visible
due to them being zero-sized or display:none. Those are usually utility
iframes and throttling them is risky.

Se still throttle requestAnimationFrame() in iframes that would be
visible in theory but are currently not noticeable because they are
outside the viewport.

Test:
- fast/animation/request-animation-frame-throttle-subframe.html
- fast/animation/request-animation-frame-throttle-subframe-display-none.html
- fast/animation/request-animation-frame-throttle-subframe-zero-size.html

* page/FrameView.cpp:
(WebCore::FrameView::updateScriptedAnimationsAndTimersThrottlingState):

LayoutTests:

* fast/animation/request-animation-frame-throttle-subframe-display-none-expected.txt:
* fast/animation/request-animation-frame-throttle-subframe-display-none.html:
Update layout test as we changed behavior here. We no longer throttle display:none
iframes.

* fast/animation/request-animation-frame-throttle-subframe-zero-size-expected.txt: Added.
* fast/animation/request-animation-frame-throttle-subframe-zero-size.html: Added.
Add layout test to check that we don't throttle RaF in iframes that are zero-sized.

* fast/animation/resources/requestAnimationFrame-frame-2.html:
* fast/animation/resources/requestAnimationFrame-frame.html:
Update use of requestAnimationFrame() to be more representative of real world usage.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (186194 => 186195)


--- trunk/LayoutTests/ChangeLog	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/LayoutTests/ChangeLog	2015-07-01 21:52:07 UTC (rev 186195)
@@ -1,3 +1,24 @@
+2015-07-01  Chris Dumez  <[email protected]>
+
+        Regression(183998): Disqus comments take a very long time to load
+        https://bugs.webkit.org/show_bug.cgi?id=146522
+        <rdar://problem/21590601>
+
+        Reviewed by Simon Fraser.
+
+        * fast/animation/request-animation-frame-throttle-subframe-display-none-expected.txt:
+        * fast/animation/request-animation-frame-throttle-subframe-display-none.html:
+        Update layout test as we changed behavior here. We no longer throttle display:none
+        iframes.
+
+        * fast/animation/request-animation-frame-throttle-subframe-zero-size-expected.txt: Added.
+        * fast/animation/request-animation-frame-throttle-subframe-zero-size.html: Added.
+        Add layout test to check that we don't throttle RaF in iframes that are zero-sized.
+
+        * fast/animation/resources/requestAnimationFrame-frame-2.html:
+        * fast/animation/resources/requestAnimationFrame-frame.html:
+        Update use of requestAnimationFrame() to be more representative of real world usage.
+
 2015-07-01  Simon Fraser  <[email protected]>
 
         Mark compositing/masks/compositing-clip-path-origin.html as being image-flakey.

Modified: trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none-expected.txt (186194 => 186195)


--- trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none-expected.txt	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none-expected.txt	2015-07-01 21:52:07 UTC (rev 186195)
@@ -1,18 +1,9 @@
-Tests that requestAnimationFrame is throttled in subframes that are display:none
+Tests that requestAnimationFrame is not throttled in subframes that are display:none
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Frame is initially visibile so requestAnimationFrame should not be throttled
-PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() became false
 PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() is false
-Hiding subframe.
-testFrame.style.display = 'none';
-PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() became true
-PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() is true
-Show subframe again
-testFrame.style.display = 'block';
-PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() became false
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none.html (186194 => 186195)


--- trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none.html	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none.html	2015-07-01 21:52:07 UTC (rev 186195)
@@ -3,42 +3,21 @@
 <body>
 <script src=""
 <script>
-description("Tests that requestAnimationFrame is throttled in subframes that are display:none");
+description("Tests that requestAnimationFrame is not throttled in subframes that are display:none");
 window.jsTestIsAsync = true;
 
-function checkSubframeThrottled()
+function runTest()
 {
-    shouldBeTrue("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()");
+    testFrame = document.getElementById("testFrame");
 
-    debug("Show subframe again");
-    evalAndLog("testFrame.style.display = 'block';");
+    // Force layout.
+    document.body.offsetTop;
 
-    shouldBecomeEqual("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()", "false", finishJSTest);
-}
-
-function hideFrame()
-{
     shouldBeFalse("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()");
-
-    debug("Hiding subframe.");
-    evalAndLog("testFrame.style.display = 'none';");
-
-    shouldBecomeEqual("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()", "true", checkSubframeThrottled);
+    finishJSTest();
 }
-
-function runTest()
-{
-    testFrame = document.getElementById("testFrame");
-    debug("Frame is initially visibile so requestAnimationFrame should not be throttled");
-    shouldBecomeEqual("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()", "false", hideFrame);
-}
-
-var i = 0;
-requestAnimationFrame(function() {
-    i++;
-});
 </script>
-<iframe id="testFrame" src="" _onload_="runTest()"></iframe>
+<iframe id="testFrame" style="position: absolute; top: 800px; display: none;" src="" _onload_="runTest()"></iframe>
 <script src=""
 </body>
 </html>

Added: trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size-expected.txt (0 => 186195)


--- trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size-expected.txt	2015-07-01 21:52:07 UTC (rev 186195)
@@ -0,0 +1,10 @@
+Tests that requestAnimationFrame is not throttled in zero-sized subframes
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS testFrame.contentWindow.internals.isRequestAnimationFrameThrottled() is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size.html (0 => 186195)


--- trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size.html	                        (rev 0)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size.html	2015-07-01 21:52:07 UTC (rev 186195)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that requestAnimationFrame is not throttled in zero-sized subframes");
+window.jsTestIsAsync = true;
+
+function runTest()
+{
+    testFrame = document.getElementById("testFrame");
+
+    // Force layout.
+    document.body.offsetTop;
+
+    shouldBeFalse("testFrame.contentWindow.internals.isRequestAnimationFrameThrottled()");
+    finishJSTest();
+}
+</script>
+<iframe id="testFrame" style="position: absolute; top: 800px; left: 0; height: 0;" src="" _onload_="runTest()"></iframe>
+<script src=""
+</body>
+</html>

Modified: trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame-2.html (186194 => 186195)


--- trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame-2.html	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame-2.html	2015-07-01 21:52:07 UTC (rev 186195)
@@ -3,9 +3,13 @@
 <body>
 <script>
 var i = 0;
-requestAnimationFrame(function() {
+function step()
+{
   i++;
-});
+  requestAnimationFrame(step);
+}
+
+requestAnimationFrame(step);
 </script>
 </body>
 </html>

Modified: trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame.html (186194 => 186195)


--- trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame.html	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/LayoutTests/fast/animation/resources/requestAnimationFrame-frame.html	2015-07-01 21:52:07 UTC (rev 186195)
@@ -3,10 +3,15 @@
 <body>
 <iframe id="grandChildFrame" src=""
 <script>
+
 var i = 0;
-requestAnimationFrame(function() {
+function step()
+{
   i++;
-});
+  requestAnimationFrame(step);
+}
+
+requestAnimationFrame(step);
 </script>
 </body>
 </html>

Modified: trunk/Source/WebCore/ChangeLog (186194 => 186195)


--- trunk/Source/WebCore/ChangeLog	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/Source/WebCore/ChangeLog	2015-07-01 21:52:07 UTC (rev 186195)
@@ -1,3 +1,27 @@
+2015-07-01  Chris Dumez  <[email protected]>
+
+        Regression(183998): Disqus comments take a very long time to load
+        https://bugs.webkit.org/show_bug.cgi?id=146522
+        <rdar://problem/21590601>
+
+        Reviewed by Simon Fraser.
+
+        Stop throttling requestAnimationFrame() in iframes that are not visible
+        due to them being zero-sized or display:none. Those are usually utility
+        iframes and throttling them is risky.
+
+        Se still throttle requestAnimationFrame() in iframes that would be
+        visible in theory but are currently not noticeable because they are
+        outside the viewport.
+
+        Test:
+        - fast/animation/request-animation-frame-throttle-subframe.html
+        - fast/animation/request-animation-frame-throttle-subframe-display-none.html
+        - fast/animation/request-animation-frame-throttle-subframe-zero-size.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateScriptedAnimationsAndTimersThrottlingState):
+
 2015-06-30  Ryosuke Niwa  <[email protected]>
 
         REGRESSION (r179168): Characters overlap after resizing the font on the copy-pasted Japanese text

Modified: trunk/Source/WebCore/page/FrameView.cpp (186194 => 186195)


--- trunk/Source/WebCore/page/FrameView.cpp	2015-07-01 21:34:58 UTC (rev 186194)
+++ trunk/Source/WebCore/page/FrameView.cpp	2015-07-01 21:52:07 UTC (rev 186195)
@@ -2171,9 +2171,8 @@
     if (!document)
         return;
 
-    // FIXME: This doesn't work for subframes of a "display: none" frame because
-    // they have a non-null ownerRenderer.
-    bool shouldThrottle = !frame().ownerRenderer() || visibleRect.isEmpty();
+    // We don't throttle zero-size or display:none frames because those are usually utility frames.
+    bool shouldThrottle = visibleRect.isEmpty() && !m_size.isEmpty() && frame().ownerRenderer();
 
 #if ENABLE(REQUEST_ANIMATION_FRAME)
     if (auto* scriptedAnimationController = document->scriptedAnimationController())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to