Effects: stabilize rAF logic & align timeout logic with it

rAF logic was introduced almost three years ago relative to this commit,
as a primary method for scheduling animation (see gh-1578 pull).

With it there was two substantial changes - one was explicitly mentioned
and the other was not.

First, if browser window was hidden aka `document.hidden === true`
it would immediately execute all scheduled animation without waiting
for time pass i.e. tick time become `0` instead of 13 ms of a default value.

Which created possibility for circular executions in case if `complete`
method executed the same animation (see gh-3434 issue).

And the second one - since then there was two ways of scheduling animation:
with `setInterval` and `requestAnimationFrame`, but there was a
difference in their execution.

In case of `setInterval` it waited default `jQuery.fx.interval` value before
actually starting the new tick, not counting the first step which wasn't
set to be executed through tick method (aka `jQuery.fx.tick`).

Whereas `requestAnimationFrame` first scheduled the call and executed
the `step` method right after that, counting the first call of
`jQuery.fx.timer`, `tick` was happening twice in one frame.

But since tests explicitly disabled rAF method i.e.
`requestAnimationFrame = null` and checking only `setInterval` logic,
since it's impossible to do it otherwise - we missed that change.

Faulty logic also was presented with `cancelAnimationFrame`, which couldn't
clear any timers since `raf` scheduler didn't define new `timerId` value.

Because that change was so subtle, apparently no user noticed it proving
that both `cancelAnimationFrame` and `clearInterval` code paths are redundant.

Since `cancelAnimationFrame` didn't work properly and rAF is and was a primary
used code path, plus the same approach is used in other popular animation libs.

Therefore those code paths were removed.

These changes also replace two different functions which schedule the animation
with one, which checks what type of logic should be used and executes it
appropriatley, but for secondary path it now uses `setTimeout` making it more
consistent with rAF path.

Since ticks are happening globally we also don't require to listen
`visibilitychange` event.

It also changes the way how first call is scheduled so execution of
animation will not happen twice in one frame.

No new tests were not introduced, since now `setTimeout` logic should be
equivalent to the rAF one, but one test was changed since now we actually
execute animation at the first tick.

Fixes gh-3434
Closes gh-3559

diff --git a/src/effects.js b/src/effects.js
index 8fa2e56e6e611d464c8605c89617344846a56d17..879b36135f99813f1028e684e3f1d06764ee0f5f 100644 (file)
--- a/src/effects.js
+++ b/src/effects.js
@@ -23,13 +23,18 @@ define( [
 "use strict";
 
 var
-       fxNow, timerId,
+       fxNow, inProgress,
        rfxtypes = /^(?:toggle|show|hide)$/,
        rrun = /queueHooks$/;
 
-function raf() {
-       if ( timerId ) {
-               window.requestAnimationFrame( raf );
+function schedule() {
+       if ( inProgress ) {
+               if ( document.hidden === false && window.requestAnimationFrame ) {
+                       window.requestAnimationFrame( schedule );
+               } else {
+                       window.setTimeout( schedule, jQuery.fx.interval );
+               }
+
                jQuery.fx.tick();
        }
 }
@@ -458,8 +463,8 @@ jQuery.speed = function( speed, easing, fn ) {
                easing: fn && easing || easing && !jQuery.isFunction( easing ) && easing
        };
 
-       // Go to the end state if fx are off or if document is hidden
-       if ( jQuery.fx.off || document.hidden ) {
+       // Go to the end state if fx are off
+       if ( jQuery.fx.off ) {
                opt.duration = 0;
 
        } else {
@@ -664,36 +669,22 @@ jQuery.fx.tick = function() {
 };
 
 jQuery.fx.timer = function( timer ) {
-       var i = jQuery.timers.push( timer ) - 1,
-               timers = jQuery.timers;
-
-       if ( timer() ) {
-               jQuery.fx.start();
-
-       // If the timer finished immediately, safely remove it (allowing for external removal)
-       // Use a superfluous post-decrement for better compressibility w.r.t. jQuery.fx.tick above
-       } else if ( timers[ i ] === timer ) {
-               timers.splice( i--, 1 );
-       }
+       jQuery.timers.push( timer );
+       jQuery.fx.start();
 };
 
 jQuery.fx.interval = 13;
 jQuery.fx.start = function() {
-       if ( !timerId ) {
-               timerId = window.requestAnimationFrame ?
-                       window.requestAnimationFrame( raf ) :
-                       window.setInterval( jQuery.fx.tick, jQuery.fx.interval );
+       if ( inProgress ) {
+               return;
        }
+
+       inProgress = true;
+       schedule();
 };
 
 jQuery.fx.stop = function() {
-       if ( window.cancelAnimationFrame ) {
-               window.cancelAnimationFrame( timerId );
-       } else {
-               window.clearInterval( timerId );
-       }
-
-       timerId = null;
+       inProgress = null;
 };
 
 jQuery.fx.speeds = {

diff --git a/test/unit/effects.js b/test/unit/effects.js
index d64a639a18fafdeda6d2b4bbdfaf3f79bb1dd3b4..54c7f7995d74fa4e9c3565cba53c761ea7be787f 100644 (file)
--- a/test/unit/effects.js
+++ b/test/unit/effects.js
@@ -1847,12 +1847,12 @@ QUnit.test( "non-px animation handles non-numeric start (#11971)", function( ass
 } );
 
 QUnit.test( "Animation callbacks (#11797)", function( assert ) {
-       assert.expect( 16 );
+       assert.expect( 15 );
 
        var prog = 0,
                targets = jQuery( "#foo" ).children(),
                done = false,
-               expectedProgress = 0;
+               expectedProgress = 1;
 
        targets.eq( 0 ).animate( {}, {
                duration: 1,
@@ -1910,14 +1910,7 @@ QUnit.test( "Animation callbacks (#11797)", function( assert ) {
                        assert.ok( true, "async: start" );
                },
                progress: function( anim, percent ) {
-
-                       // occasionally the progress handler is called twice in first frame.... *shrug*
-                       if ( percent === 0 && expectedProgress === 1 ) {
-                               return;
-                       }
                        assert.equal( percent, expectedProgress, "async: progress " + expectedProgress );
-
-                       // once at 0, once at 1
                        expectedProgress++;
                },
                done: function() {