Skip to content

Commit

Permalink
Fire controlling event more than once (#2817) (#2857)
Browse files Browse the repository at this point in the history
* Fire controlling event more than once (#2817)

Dispatches the `controlling` event every time the documentation 
specifies that it should.

* Test tweak

* One more fix

Co-authored-by: Jeff Posnick <[email protected]>
  • Loading branch information
rockwalrus and jeffposnick authored Jul 1, 2021
1 parent 06a561a commit 0b5d9d8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 3 deletions.
2 changes: 1 addition & 1 deletion packages/workbox-window/src/Workbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class Workbox extends WorkboxEventTarget {

this._registration.addEventListener('updatefound', this._onUpdateFound);
navigator.serviceWorker.addEventListener(
'controllerchange', this._onControllerChange, {once: true});
'controllerchange', this._onControllerChange);

return this._registration;
}
Expand Down
4 changes: 3 additions & 1 deletion test/workbox-window/integration/test-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,14 @@ describe(`[workbox-window] Workbox`, function() {
expect(installedSpyArgs.length, 'installedSpy').to.eql(2);
expect(waitingSpyArgs.length, 'waitingSpy').to.eql(0);
expect(activatedSpyArgs.length, 'activatedSpy').to.eql(2);
expect(controllingSpyArgs.length, 'controllingSpy').to.eql(1);
expect(controllingSpyArgs.length, 'controllingSpy').to.eql(2);

expect(installedSpyArgs[0][0].isExternal).to.eql(false);
expect(activatedSpyArgs[0][0].isExternal).to.eql(false);
expect(controllingSpyArgs[0][0].isExternal).to.eql(false);
expect(installedSpyArgs[1][0].isExternal).to.eql(true);
expect(activatedSpyArgs[1][0].isExternal).to.eql(true);
expect(controllingSpyArgs[1][0].isExternal).to.eql(true);
});
});
});
55 changes: 54 additions & 1 deletion test/workbox-window/window/test-Workbox.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ describe(`[workbox-window] Workbox`, function() {
// the first time a page registers a SW). This case is tested in the
// integration tests.

expect(controlling1Spy.callCount).to.equal(1);
expect(controlling1Spy.callCount).to.equal(2);
assertMatchesWorkboxEvent(controlling1Spy.args[0][0], {
isExternal: false,
isUpdate: true,
Expand All @@ -855,6 +855,14 @@ describe(`[workbox-window] Workbox`, function() {
target: wb1,
type: 'controlling',
});
assertMatchesWorkboxEvent(controlling1Spy.args[1][0], {
isExternal: true,
isUpdate: true,
originalEvent: {type: 'controllerchange'},
sw: await wb1.getSW(),
target: wb1,
type: 'controlling',
});

// This will be an "external" event, due to wb3's SW taking control.
// wb2's SW never controls, because it's stuck in waiting.
Expand All @@ -874,6 +882,51 @@ describe(`[workbox-window] Workbox`, function() {
type: 'controlling',
});
});

it(`runs every time the registered SW is updated`, async function() {
const scriptURL = uniq('sw-skip-waiting.js.njk');
const wb1 = new Workbox(scriptURL);
const controlling1Spy = sandbox.spy();
wb1.addEventListener('controlling', controlling1Spy);
await wb1.register();
await nextEvent(wb1, 'controlling');

await updateVersion('2.0.0', scriptURL);

wb1.update();
await nextEvent(wb1, 'controlling');

await updateVersion('3.0.0', scriptURL);

wb1.update();
await nextEvent(wb1, 'controlling');

expect(controlling1Spy.callCount).to.equal(3);
assertMatchesWorkboxEvent(controlling1Spy.args[0][0], {
isExternal: false,
isUpdate: true,
originalEvent: {type: 'controllerchange'},
sw: await wb1.getSW(),
target: wb1,
type: 'controlling',
});
assertMatchesWorkboxEvent(controlling1Spy.args[1][0], {
isExternal: true,
isUpdate: true,
originalEvent: {type: 'controllerchange'},
sw: await wb1.getSW(),
target: wb1,
type: 'controlling',
});
assertMatchesWorkboxEvent(controlling1Spy.args[2][0], {
isExternal: true,
isUpdate: true,
originalEvent: {type: 'controllerchange'},
sw: await wb1.getSW(),
target: wb1,
type: 'controlling',
});
});
});

describe(`redundant`, function() {
Expand Down

0 comments on commit 0b5d9d8

Please sign in to comment.