Fix service worker gatherer by waiting for active state #1864
Conversation
|
Do we have a fairly solid repro for the bug? I'm trying it with heavier throttling on both caltrainschedule and fuzecaresystem and i can't repro the bug. |
|
In my test case you can see the issue at end. First it's installing and the audit goes right through it, now it waits for activated. I throttled my connection with charles and could reproduce it quite often. One time I didn't have a service worker, the other time I did. |
|
@paulirish here is a url that I could reproduce it with |
|
Referencing #1815 |
|
Can we get this merged in soon? |
lighthouse-core/gather/driver.js
Outdated
| const versionUpdatedListener = data => { | ||
| // find a service worker with runningStatus that looks like active | ||
| // on slow connections the serviceworker might still be installing | ||
| const nonRedundantServiceWorkers = data.versions.filter(sw => { |
There was a problem hiding this comment.
suggest nonRedundantServiceWorkers -> activateCandidates
lighthouse-core/gather/driver.js
Outdated
| return sw.status === 'activated'; | ||
| }); | ||
|
|
||
| const hasActiveServiceWorker = nonRedundantServiceWorkers.length && activeServiceWorker; |
There was a problem hiding this comment.
Do you need this boolean? Isn't activeServiceWorker enough?
There was a problem hiding this comment.
when reading the code over again it seems like you are right
lighthouse-core/gather/driver.js
Outdated
| return sw.status === 'activated'; | ||
| }); | ||
|
|
||
| const hasActiveServiceWorker = activeServiceWorker; |
There was a problem hiding this comment.
nit: let's remove save the return value of find() as hasActiveServiceWorker.
|
LGTM 👍 |
* Wait for active serviceworker * Fix test cases * Review fixes * Review fix
Fixes #1815 where we don't wait until the service worker is active.
Happens mostly on slow connections
not sure if I should mock and create a test for this?