From 5e0dbe314bc76ba927e1ced8dcebbab9f31c3be6 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 8 Jan 2021 13:59:35 +0200 Subject: [PATCH] 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 --- packages/service-worker/worker/src/driver.ts | 5 +- packages/service-worker/worker/src/idle.ts | 18 ++++++- .../service-worker/worker/test/idle_spec.ts | 47 ++++++++++++++++++- .../worker/test/prefetch_spec.ts | 2 +- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index 9f37d38c54..9838186ccf 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -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); } /** diff --git a/packages/service-worker/worker/src/idle.ts b/packages/service-worker/worker/src/idle.ts index e84e1bc33a..36bae8d1c0 100644 --- a/packages/service-worker/worker/src/idle.ts +++ b/packages/service-worker/worker/src/idle.ts @@ -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 { 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 { 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 { diff --git a/packages/service-worker/worker/test/idle_spec.ts b/packages/service-worker/worker/test/idle_spec.ts index de863d7384..3d223c6ce3 100644 --- a/packages/service-worker/worker/test/idle_spec.ts +++ b/packages/service-worker/worker/test/idle_spec.ts @@ -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); + }); }); })(); diff --git a/packages/service-worker/worker/test/prefetch_spec.ts b/packages/service-worker/worker/test/prefetch_spec.ts index da8ec1ad02..df8970c2eb 100644 --- a/packages/service-worker/worker/test/prefetch_spec.ts +++ b/packages/service-worker/worker/test/prefetch_spec.ts @@ -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(