From 4b08d452e4340529e6661854f93d787767fe10a2 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 5 Oct 2018 16:54:34 -0700 Subject: [PATCH 1/2] [scheduler] Eagerly schedule rAF at beginning of frame Eagerly schedule the next animation callback at the beginning of the frame. If the scheduler queue is not empty at the end of the frame, it will continue flushing inside that callback. If the queue *is* empty, then it will exit immediately. Posting the callback at the start of the frame ensures it's fired within the earliest possible frame. If we waited until the end of the frame to post the callback, we risk the browser skipping a frame and not firing the callback until the frame after that. --- packages/scheduler/src/Scheduler.js | 17 +++++++++++- .../src/__tests__/SchedulerDOM-test.js | 26 ++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/scheduler/src/Scheduler.js b/packages/scheduler/src/Scheduler.js index 42a5659beb16..995a465b9fde 100644 --- a/packages/scheduler/src/Scheduler.js +++ b/packages/scheduler/src/Scheduler.js @@ -605,7 +605,22 @@ if (typeof window !== 'undefined' && window._schedMock) { window.addEventListener('message', idleTick, false); var animationTick = function(rafTime) { - isAnimationFrameScheduled = false; + if (scheduledCallback !== null) { + // Eagerly schedule the next animation callback at the beginning of the + // frame. If the scheduler queue is not empty at the end of the frame, it + // will continue flushing inside that callback. If the queue *is* empty, + // then it will exit immediately. Posting the callback at the start of the + // frame ensures it's fired within the earliest possible frame. If we + // waited until the end of the frame to post the callback, we risk the + // browser skipping a frame and not firing the callback until the frame + // after that. + requestAnimationFrameWithTimeout(animationTick); + } else { + // No pending work. Exit. + isAnimationFrameScheduled = false; + return; + } + var nextFrameTime = rafTime - frameDeadline + activeFrameTime; if ( nextFrameTime < activeFrameTime && diff --git a/packages/scheduler/src/__tests__/SchedulerDOM-test.js b/packages/scheduler/src/__tests__/SchedulerDOM-test.js index b1253ffe9ddc..f90126333087 100644 --- a/packages/scheduler/src/__tests__/SchedulerDOM-test.js +++ b/packages/scheduler/src/__tests__/SchedulerDOM-test.js @@ -50,8 +50,9 @@ describe('SchedulerDOM', () => { function runRAFCallbacks() { startOfLatestFrame += frameSize; currentTime = startOfLatestFrame; - rAFCallbacks.forEach(cb => cb()); + const cbs = rAFCallbacks; rAFCallbacks = []; + cbs.forEach(cb => cb()); } function advanceOneFrame(config: FrameTimeoutConfigType = {}) { runRAFCallbacks(); @@ -59,8 +60,8 @@ describe('SchedulerDOM', () => { } let frameSize = 33; - let startOfLatestFrame = Date.now(); - let currentTime = Date.now(); + let startOfLatestFrame = 0; + let currentTime = 0; beforeEach(() => { // TODO pull this into helper method, reduce repetition. @@ -109,6 +110,25 @@ describe('SchedulerDOM', () => { expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number'); }); + it('inserts its rAF callback as early into the queue as possible', () => { + const {unstable_scheduleCallback: scheduleCallback} = Scheduler; + const log = []; + const useRAFCallback = () => { + log.push('userRAFCallback'); + }; + scheduleCallback(() => { + // Call rAF while idle work is being flushed. + requestAnimationFrame(useRAFCallback); + }); + advanceOneFrame({timeLeftInFrame: 1}); + // There should be two callbacks: the one scheduled by Scheduler at the + // beginning of the frame, and the one scheduled later during that frame. + expect(rAFCallbacks.length).toBe(2); + // The user callback should be the second callback. + rAFCallbacks[1](); + expect(log).toEqual(['userRAFCallback']); + }); + describe('with multiple callbacks', () => { it('accepts multiple callbacks and calls within frame when not blocked', () => { const {unstable_scheduleCallback: scheduleCallback} = Scheduler; From f92bbaa4d27c47e12b7d06963427fc8aab1330b9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 8 Oct 2018 17:14:57 -0700 Subject: [PATCH 2/2] Re-name scheduledCallback -> scheduledHostCallback --- packages/scheduler/src/Scheduler.js | 42 ++++++++++++++++------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/scheduler/src/Scheduler.js b/packages/scheduler/src/Scheduler.js index 995a465b9fde..d52babaa1c54 100644 --- a/packages/scheduler/src/Scheduler.js +++ b/packages/scheduler/src/Scheduler.js @@ -534,13 +534,13 @@ if (typeof window !== 'undefined' && window._schedMock) { } } - var scheduledCallback = null; - var isIdleScheduled = false; + var scheduledHostCallback = null; + var isMessageEventScheduled = false; var timeoutTime = -1; var isAnimationFrameScheduled = false; - var isPerformingIdleWork = false; + var isFlushingHostCallback = false; var frameDeadline = 0; // We start out assuming that we run at 30fps but then the heuristic tracking @@ -564,7 +564,12 @@ if (typeof window !== 'undefined' && window._schedMock) { return; } - isIdleScheduled = false; + isMessageEventScheduled = false; + + var prevScheduledCallback = scheduledHostCallback; + var prevTimeoutTime = timeoutTime; + scheduledHostCallback = null; + timeoutTime = -1; var currentTime = getCurrentTime(); @@ -572,7 +577,7 @@ if (typeof window !== 'undefined' && window._schedMock) { if (frameDeadline - currentTime <= 0) { // There's no time left in this idle period. Check if the callback has // a timeout and whether it's been exceeded. - if (timeoutTime !== -1 && timeoutTime <= currentTime) { + if (prevTimeoutTime !== -1 && prevTimeoutTime <= currentTime) { // Exceeded the timeout. Invoke the callback even though there's no // time left. didTimeout = true; @@ -584,19 +589,18 @@ if (typeof window !== 'undefined' && window._schedMock) { requestAnimationFrameWithTimeout(animationTick); } // Exit without invoking the callback. + scheduledHostCallback = prevScheduledCallback; + timeoutTime = prevTimeoutTime; return; } } - timeoutTime = -1; - var callback = scheduledCallback; - scheduledCallback = null; - if (callback !== null) { - isPerformingIdleWork = true; + if (prevScheduledCallback !== null) { + isFlushingHostCallback = true; try { - callback(didTimeout); + prevScheduledCallback(didTimeout); } finally { - isPerformingIdleWork = false; + isFlushingHostCallback = false; } } }; @@ -605,7 +609,7 @@ if (typeof window !== 'undefined' && window._schedMock) { window.addEventListener('message', idleTick, false); var animationTick = function(rafTime) { - if (scheduledCallback !== null) { + if (scheduledHostCallback !== null) { // Eagerly schedule the next animation callback at the beginning of the // frame. If the scheduler queue is not empty at the end of the frame, it // will continue flushing inside that callback. If the queue *is* empty, @@ -644,16 +648,16 @@ if (typeof window !== 'undefined' && window._schedMock) { previousFrameTime = nextFrameTime; } frameDeadline = rafTime + activeFrameTime; - if (!isIdleScheduled) { - isIdleScheduled = true; + if (!isMessageEventScheduled) { + isMessageEventScheduled = true; window.postMessage(messageKey, '*'); } }; requestHostCallback = function(callback, absoluteTimeout) { - scheduledCallback = callback; + scheduledHostCallback = callback; timeoutTime = absoluteTimeout; - if (isPerformingIdleWork || absoluteTimeout < 0) { + if (isFlushingHostCallback || absoluteTimeout < 0) { // Don't wait for the next frame. Continue working ASAP, in a new event. window.postMessage(messageKey, '*'); } else if (!isAnimationFrameScheduled) { @@ -667,8 +671,8 @@ if (typeof window !== 'undefined' && window._schedMock) { }; cancelHostCallback = function() { - scheduledCallback = null; - isIdleScheduled = false; + scheduledHostCallback = null; + isMessageEventScheduled = false; timeoutTime = -1; }; }