Skip to content

Commit

Permalink
fix(service-worker): clean up caches from old SW versions (#26319)
Browse files Browse the repository at this point in the history
Since the SW immediately takes over all clients, it is safe to delete
caches used by older (e.g. beta) `@angular/service-worker` versions to
avoid running into browser storage quota limitations.

PR Close #26319
  • Loading branch information
gkalpak authored and mhevery committed Oct 16, 2018
1 parent 41de0e0 commit 2326b9c
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 6 deletions.
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

0 comments on commit 2326b9c

Please sign in to comment.