fix(service-worker): allow checking for updates when constantly polling the server (#40234)

Previously, the SW would wait to become idle before executing scheduled
tasks (including checks for newer app versions). It was considered idle
when it hadn't received any request for at least 5 seconds. As a result,
if the app performed polling (i.e. sent requests to the server) in a
shorter than 5 seconds interval, the SW would never detect and update to
a newer app version.
Related issue: #40207

This commit fixes this by adding a max delay to `IdleScheduler` to
ensure that no scheduled task will remain pending for longer than the
specified max delay.

PR Close #40234
This commit is contained in:
George Kalpakas 2021-01-08 13:59:35 +02:00 committed by atscott
parent 919b93a1f8
commit 5e0dbe314b
4 changed files with 66 additions and 6 deletions

View File

@ -25,7 +25,8 @@ type ClientAssignments = {
[id: string]: ManifestHash
};
const IDLE_THRESHOLD = 5000;
const IDLE_DELAY = 5000;
const MAX_IDLE_DELAY = 30000;
const SUPPORTED_CONFIG_VERSION = 1;
@ -167,7 +168,7 @@ export class Driver implements Debuggable, UpdateSource {
this.debugger = new DebugHandler(this, this.adapter);
// The IdleScheduler will execute idle tasks after a given delay.
this.idle = new IdleScheduler(this.adapter, IDLE_THRESHOLD, this.debugger);
this.idle = new IdleScheduler(this.adapter, IDLE_DELAY, MAX_IDLE_DELAY, this.debugger);
}
/**

View File

@ -25,8 +25,11 @@ export class IdleScheduler {
private emptyResolve: Function|null = null;
lastTrigger: number|null = null;
lastRun: number|null = null;
oldestScheduledAt: number|null = null;
constructor(private adapter: Adapter, private threshold: number, private debug: DebugLogger) {}
constructor(
private adapter: Adapter, private delay: number, private maxDelay: number,
private debug: DebugLogger) {}
async trigger(): Promise<void> {
this.lastTrigger = this.adapter.time;
@ -43,7 +46,12 @@ export class IdleScheduler {
};
this.scheduled = scheduled;
await this.adapter.timeout(this.threshold);
// Ensure that no task remains pending for longer than `this.maxDelay` ms.
const now = this.adapter.time;
const maxDelay = Math.max(0, (this.oldestScheduledAt ?? now) + this.maxDelay - now);
const delay = Math.min(maxDelay, this.delay);
await this.adapter.timeout(delay);
if (scheduled.cancel) {
return;
@ -75,15 +83,21 @@ export class IdleScheduler {
this.emptyResolve = null;
}
this.empty = Promise.resolve();
this.oldestScheduledAt = null;
}
schedule(desc: string, run: () => Promise<void>): void {
this.queue.push({desc, run});
if (this.emptyResolve === null) {
this.empty = new Promise(resolve => {
this.emptyResolve = resolve;
});
}
if (this.oldestScheduledAt === null) {
this.oldestScheduledAt = this.adapter.time;
}
}
get size(): number {

View File

@ -21,7 +21,7 @@ describe('IdleScheduler', () => {
beforeEach(() => {
scope = new SwTestHarnessBuilder().build();
idle = new IdleScheduler(scope, 1000, {
idle = new IdleScheduler(scope, 1000, 3000, {
log: (v, context) => console.error(v, context),
});
});
@ -137,5 +137,50 @@ describe('IdleScheduler', () => {
// The task should have executed.
expect(completed).toEqual(true);
});
it('executes tasks after max delay even with newer triggers', async () => {
// Set up a single idle task to set the completed flag to true when it runs.
let completed: boolean = false;
idle.schedule('work', async () => {
completed = true;
});
// Trigger the queue once. This trigger will start a timer for the idle timeout,
// but another `trigger()` will be called before that timeout passes.
const firstTrigger = idle.trigger();
// Advance the clock a little, but not enough to actually cause tasks to execute.
scope.advance(999);
expect(completed).toBe(false);
// Next, trigger the queue again.
const secondTrigger = idle.trigger();
// Advance the clock beyond the timeout for the first trigger, but not the second.
// This should cause the first trigger to resolve, but without running the task.
scope.advance(999);
await firstTrigger;
expect(completed).toBe(false);
// Next, trigger the queue again.
const thirdTrigger = idle.trigger();
// Advance the clock beyond the timeout for the second trigger, but not the third.
// This should cause the second trigger to resolve, but without running the task.
scope.advance(999);
await secondTrigger;
expect(completed).toBe(false);
// Next, trigger the queue again.
const forthTrigger = idle.trigger();
// Finally, advance the clock beyond `maxDelay` (3000) from the first trigger, but not beyond
// the timeout for the forth. This should cause the task to be executed nontheless.
scope.advance(3);
await Promise.all([thirdTrigger, forthTrigger]);
// The task should have executed.
expect(completed).toBe(true);
});
});
})();

View File

@ -38,7 +38,7 @@ describe('prefetch assets', () => {
let group: PrefetchAssetGroup;
let idle: IdleScheduler;
beforeEach(() => {
idle = new IdleScheduler(null!, 3000, {
idle = new IdleScheduler(null!, 3000, 30000, {
log: (v, ctx = '') => console.error(v, ctx),
});
group = new PrefetchAssetGroup(