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(service-worker): clean up caches from old SW versions #26319

Closed
wants to merge 1 commit into from
Closed
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
32 changes: 29 additions & 3 deletions packages/service-worker/worker/src/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,22 @@ export class Driver implements Debuggable, UpdateSource {
// The activate event is triggered when this version of the service worker is
// first activated.
this.scope.addEventListener('activate', (event) => {
// As above, it's safe to take over from existing clients immediately, since
// the new SW version will continue to serve the old application.
event !.waitUntil(this.scope.clients.claim());
event !.waitUntil((async() => {
// As above, it's safe to take over from existing clients immediately, since the new SW
// version will continue to serve the old application.
await this.scope.clients.claim();

// Once all clients have been taken over, we can delete caches used by old versions of
// `@angular/service-worker`, which are no longer needed. This can happen in the background.
this.idle.schedule('activate: cleanup-old-sw-caches', async() => {
try {
await this.cleanupOldSwCaches();
} catch (err) {
// Nothing to do - cleanup failed. Just log it.
this.debugger.log(err, 'cleanupOldSwCaches @ activate: cleanup-old-sw-caches');
}
});
})());

// Rather than wait for the first fetch event, which may not arrive until
// the next time the application is loaded, the SW takes advantage of the
Expand Down Expand Up @@ -872,6 +885,19 @@ export class Driver implements Debuggable, UpdateSource {
await this.sync();
}

/**
* Delete caches that were used by older versions of `@angular/service-worker` to avoid running
* into storage quota limitations imposed by browsers.
* (Since at this point the SW has claimed all clients, it is safe to remove those caches.)
*/
async cleanupOldSwCaches(): Promise<void> {
const cacheNames = await this.scope.caches.keys();
const oldSwCacheNames =
cacheNames.filter(name => /^ngsw:(?:active|staged|manifest:.+)$/.test(name));

await Promise.all(oldSwCacheNames.map(name => this.scope.caches.delete(name)));
}

/**
* Determine if a specific version of the given resource is cached anywhere within the SW,
* and fetch it if so.
Expand Down
85 changes: 83 additions & 2 deletions packages/service-worker/worker/test/happy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,56 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
driver = new Driver(scope, scope, new CacheDatabase(scope, scope));
});

async_it('activates without waiting', async() => {
const skippedWaiting = await scope.startup(true);
expect(skippedWaiting).toBe(true);
});

async_it('claims all clients, after activation', async() => {
const claimSpy = spyOn(scope.clients, 'claim');

await scope.startup(true);
expect(claimSpy).toHaveBeenCalledTimes(1);
});

async_it('cleans up old `@angular/service-worker` caches, after activation', async() => {
const claimSpy = spyOn(scope.clients, 'claim');
const cleanupOldSwCachesSpy = spyOn(driver, 'cleanupOldSwCaches');

// Automatically advance time to trigger idle tasks as they are added.
scope.autoAdvanceTime = true;
await scope.startup(true);
await scope.resolveSelfMessages();
scope.autoAdvanceTime = false;

expect(cleanupOldSwCachesSpy).toHaveBeenCalledTimes(1);
expect(claimSpy).toHaveBeenCalledBefore(cleanupOldSwCachesSpy);
});

async_it(
'does not blow up if cleaning up old `@angular/service-worker` caches fails', async() => {
spyOn(driver, 'cleanupOldSwCaches').and.callFake(() => Promise.reject('Ooops'));

// Automatically advance time to trigger idle tasks as they are added.
scope.autoAdvanceTime = true;
await scope.startup(true);
await scope.resolveSelfMessages();
scope.autoAdvanceTime = false;

server.clearRequests();

expect(driver.state).toBe(DriverReadyState.NORMAL);
expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo');
server.assertNoOtherRequests();
});

async_it('initializes prefetched content correctly, after activation', async() => {
expect(await scope.startup(true)).toEqual(true);
// Automatically advance time to trigger idle tasks as they are added.
scope.autoAdvanceTime = true;
await scope.startup(true);
await scope.resolveSelfMessages();
await driver.initialized;
scope.autoAdvanceTime = false;

server.assertSawRequestFor('ngsw.json');
server.assertSawRequestFor('/foo.txt');
server.assertSawRequestFor('/bar.txt');
Expand Down Expand Up @@ -825,6 +871,41 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
});
});

describe('cleanupOldSwCaches()', () => {
async_it('should delete the correct caches', async() => {
const oldSwCacheNames = ['ngsw:active', 'ngsw:staged', 'ngsw:manifest:a1b2c3:super:duper'];
const otherCacheNames = [
'ngsuu:active',
'not:ngsw:active',
'ngsw:staged:not',
'NgSw:StAgEd',
'ngsw:manifest',
];
const allCacheNames = oldSwCacheNames.concat(otherCacheNames);

await Promise.all(allCacheNames.map(name => scope.caches.open(name)));
expect(await scope.caches.keys()).toEqual(allCacheNames);

await driver.cleanupOldSwCaches();
expect(await scope.caches.keys()).toEqual(otherCacheNames);
});

async_it('should delete other caches even if deleting one of them fails', async() => {
const oldSwCacheNames = ['ngsw:active', 'ngsw:staged', 'ngsw:manifest:a1b2c3:super:duper'];
const deleteSpy = spyOn(scope.caches, 'delete')
.and.callFake(
(cacheName: string) =>
Promise.reject(`Failed to delete cache '${cacheName}'.`));

await Promise.all(oldSwCacheNames.map(name => scope.caches.open(name)));
const error = await driver.cleanupOldSwCaches().catch(err => err);

expect(error).toBe('Failed to delete cache \'ngsw:active\'.');
expect(deleteSpy).toHaveBeenCalledTimes(3);
oldSwCacheNames.forEach(name => expect(deleteSpy).toHaveBeenCalledWith(name));
});
});

describe('bugs', () => {
async_it('does not crash with bad index hash', async() => {
scope = new SwTestHarnessBuilder().withServerState(brokenServer).build();
Expand Down
9 changes: 8 additions & 1 deletion packages/service-worker/worker/testing/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context
private skippedWaiting = true;

private selfMessageQueue: any[] = [];
autoAdvanceTime = false;
// TODO(issue/24571): remove '!'.
unregistered !: boolean;
readonly notifications: {title: string, options: Object}[] = [];
Expand Down Expand Up @@ -228,14 +229,20 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context
}

timeout(ms: number): Promise<void> {
return new Promise(resolve => {
const promise = new Promise<void>(resolve => {
this.timers.push({
at: this.time + ms,
duration: ms,
fn: resolve,
fired: false,
});
});

if (this.autoAdvanceTime) {
this.advance(ms);
}

return promise;
}

advance(by: number): void {
Expand Down