Skip to content
This repository has been archived by the owner on Dec 9, 2021. It is now read-only.

Adds a new controlledBySW method #54

Merged
merged 6 commits into from Mar 28, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 25 additions & 0 deletions src/browser/sw-utils.js
Expand Up @@ -208,6 +208,31 @@ class SWUtils {
});
}

/**
* <p>Register a service worker with a unique scope and
* create an iframe that can be controlled by that service worker, then
* wait until the service worker takes control.</p>
*
* <p>Useful for testing behavior that assumes the page is already controlled
* by a service worker.</p>
*
* @param {String} swUrl The url to a service worker file to register
* @return {Promise.<HTMLElement>} Resolves with the iframe once the service
* worker has taken control.
*/
controlledBySW(swUrl) {
return this.activateSW(swUrl).then(iframe => new Promise(resolve => {
var iframeSW = iframe.contentWindow.navigator.serviceWorker;
if (iframeSW.controller) {
resolve(iframe);
} else {
iframeSW.addEventListener('controllerchange',
() => resolve(iframe),
{once: true});
}
}));
}

/**
* <p>Helper method that checks a cache with a specific name exists before
* retrieving all the cached responses inside of it.</p>
Expand Down
1 change: 1 addition & 0 deletions test/browser-tests/index.html
Expand Up @@ -47,6 +47,7 @@
<script src="/test/browser-tests/window-utils/get-iframe.js"></script>
<script src="/test/browser-tests/window-utils/install-sw.js"></script>
<script src="/test/browser-tests/window-utils/activate-sw.js"></script>
<script src="/test/browser-tests/window-utils/controlled-by-sw.js"></script>
<script src="/test/browser-tests/window-utils/get-all-cached-assets.js"></script>
<script src="/test/browser-tests/window-utils/run-sw-mocha-tests.js"></script>

Expand Down
64 changes: 64 additions & 0 deletions test/browser-tests/window-utils/controlled-by-sw.js
@@ -0,0 +1,64 @@
/*
Copyright 2016 Google Inc. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// This is a test and we want descriptions to be useful, if this
// breaks the max-length, it's ok.

/* eslint-disable max-len, no-unused-expressions */
/* eslint-env browser, mocha */

'use strict';

describe('Test swUtils.controlledBySW()', function() {
const SERVICE_WORKER_PATH = '/test/browser-tests/window-utils/serviceworkers';

it('should reject when called with no arguments', function(done) {
return window.goog.swUtils.controlledBySW()
.then(() => done(new Error('Should have rejected')))
.catch(() => done());
});

it('should reject when called with an array argument', function(done) {
return window.goog.swUtils.controlledBySW([])
.then(() => done(new Error('Should have rejected')))
.catch(() => done());
});

it('should reject called with an object argument', function(done) {
return window.goog.swUtils.controlledBySW({})
.then(() => done(new Error('Should have rejected')))
.catch(() => done());
});

it('should reject when used with an invalid service worker path', function(done) {
return window.goog.swUtils.controlledBySW(SERVICE_WORKER_PATH + '/sw-doesnt-exist.js')
.then(() => done(new Error('Should have rejected')))
.catch(() => done());
});

it('should reject when used with a service worker that fails to install', function(done) {
return window.goog.swUtils.controlledBySW(SERVICE_WORKER_PATH + '/sw-broken-install.js')
.then(() => done(new Error('Should have rejected')))
.catch(() => done());
});

it('should resolve once the service worker controls the iframe', function() {
return window.goog.swUtils.controlledBySW(SERVICE_WORKER_PATH + '/immediate-control.js')
.then(iframe => {
chai.expect(iframe.contentWindow.navigator.serviceWorker.controller).to.exist;
});
});
});
@@ -0,0 +1,2 @@
self.addEventListener('install', () => self.skipWaiting());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment here just along the lines of:

"This service worker does not use event.waitUntil() on purpose. This has ambiguity over implementation across browsers (i.e. whether it's needed to be used with skipWaiting and clients.claim()). This service worker is used to test controlledBySw() - PLEASE DO NOT CHANGE."

Without this I can see someone eventually sneaking in event.waitUntil() (i.e. myself) and then it's undermines the purpose of this new method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the ambiguity is, though? AFAIK there are no browsers that ever required an event.waitUntil() wrapper around self.skipWaiting() or self.clients.claim().

There are old examples that used event.waitUntil() (including many I wrote), but it turns out it was never actually necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I added a comment anyway!)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we've seen the difference in behavior - so it's not "required" it does affect the end behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL at the revised comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that works.

self.addEventListener('activate', () => self.clients.claim());