Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(animationFrameScheduler): some tasks are never flushed and sometimes it breaks completely #7444

Open
wants to merge 4 commits into
base: 7.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 40 additions & 0 deletions spec/schedulers/AnimationFrameScheduler-spec.ts
Expand Up @@ -141,6 +141,46 @@ describe('Scheduler.animationFrame', () => {
}
});

it('should schedule next frame actions from a delayed one', (done) => {
animationFrame.schedule(() => {
animationFrame.schedule(() => { done(); });
}, 1);
});

it('should schedule 2 actions for a subsequent frame', (done) => {
let runFirst = false;
animationFrame.schedule(() => {
animationFrame.schedule(() => { runFirst = true; });
animationFrame.schedule(() => {
if (runFirst) {
done();
} else {
done(new Error('First action did not run'));
}
});
});
});

it('should handle delayed action without affecting next frame actions', (done) => {
let runDelayed = false;
let runFirst = false;
animationFrame.schedule(() => {
animationFrame.schedule(() => { runFirst = true; });
animationFrame.schedule(() => {
if (!runDelayed) {
done(new Error('Delayed action did not run'));
} else if (!runFirst) {
done(new Error('First action did not run'));
} else {
done();
}
});

// This action will execute before the next frame because the delay is less than the one of the frame
animationFrame.schedule(() => { runDelayed = true; }, 1);
});
});

it('should not execute rescheduled actions when flushing', (done) => {
let flushCount = 0;
let scheduledIndices: number[] = [];
Expand Down
10 changes: 8 additions & 2 deletions spec/schedulers/AsapScheduler-spec.ts
Expand Up @@ -66,7 +66,7 @@ describe('Scheduler.asap', () => {
const sandbox = sinon.createSandbox();
const fakeTimer = sandbox.useFakeTimers();
// callThrough is missing from the declarations installed by the typings tool in stable
const stubSetInterval = (<any> sandbox.stub(global, 'setInterval')).callThrough();
const stubSetInterval = (<any> sandbox.stub(fakeTimer, 'setInterval')).callThrough();
const period = 50;
const state = { index: 0, period };
type State = typeof state;
Expand All @@ -92,7 +92,7 @@ describe('Scheduler.asap', () => {
const sandbox = sinon.createSandbox();
const fakeTimer = sandbox.useFakeTimers();
// callThrough is missing from the declarations installed by the typings tool in stable
const stubSetInterval = (<any> sandbox.stub(global, 'setInterval')).callThrough();
const stubSetInterval = (<any> sandbox.stub(fakeTimer, 'setInterval')).callThrough();
const period = 50;
const state = { index: 0, period };
type State = typeof state;
Expand Down Expand Up @@ -148,6 +148,12 @@ describe('Scheduler.asap', () => {
}, 0, 0);
});

it('should schedule asap actions from a delayed one', (done) => {
asap.schedule(() => {
asap.schedule(() => { done(); });
}, 1);
});

it('should cancel the setImmediate if all scheduled actions unsubscribe before it executes', (done) => {
let asapExec1 = false;
let asapExec2 = false;
Expand Down
2 changes: 1 addition & 1 deletion src/internal/scheduler/AnimationFrameAction.ts
Expand Up @@ -33,7 +33,7 @@ export class AnimationFrameAction<T> extends AsyncAction<T> {
// cancel the requested animation frame and set the scheduled flag to
// undefined so the next AnimationFrameAction will request its own.
const { actions } = scheduler;
if (id != null && actions[actions.length - 1]?.id !== id) {
if (id != null && id === scheduler._scheduled && actions[actions.length - 1]?.id !== id) {
animationFrameProvider.cancelAnimationFrame(id as number);
scheduler._scheduled = undefined;
}
Expand Down
9 changes: 7 additions & 2 deletions src/internal/scheduler/AnimationFrameScheduler.ts
Expand Up @@ -13,8 +13,13 @@ export class AnimationFrameScheduler extends AsyncScheduler {
// are removed from the actions array and that can shift actions that are
// scheduled to be executed in a subsequent flush into positions at which
// they are executed within the current flush.
const flushId = this._scheduled;
this._scheduled = undefined;
let flushId;
if (action) {
flushId = action.id;
} else {
flushId = this._scheduled;
this._scheduled = undefined;
}

const { actions } = this;
let error: any;
Expand Down