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

[ServiceWorker] Reject update() from within installing workers #16983

Merged
merged 1 commit into from May 29, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 23, 2019

The service worker spec mandates that update() fails immediately if
called from within service workers that are currently installing
(i.e. if update() is called in the |install|-event-handler).

This change implements this behavior​ and updates the WPT to match the
spec.

Bug: 895845
Change-Id: Ie07563e5ac8260554ef0947be4cb020124cfeddf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626600
Commit-Queue: Yannic Bonenberger <contact@yannic-bonenberger.com>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663695}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1626600 branch 3 times, most recently from 937815e to 80b0211 Compare May 28, 2019 09:04
The service worker spec mandates that update() fails immediately if
called from within service workers that are currently installing
(i.e. if update() is called in the |install|-event-handler).

This change implements this behavior​ and updates the WPT to match the
spec.

Bug: 895845
Change-Id: Ie07563e5ac8260554ef0947be4cb020124cfeddf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626600
Commit-Queue: Yannic Bonenberger <contact@yannic-bonenberger.com>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663695}
@Hexcles
Copy link
Member

Hexcles commented May 29, 2019

The flakiness is pre-existing:

Running the tests without the patch showed the same flaky tests as running them with the patch.

Here’s the log for Chrome:
https://tools.taskcluster.net/groups/HhU663d0QWmdEzYl-RMVnQ/tasks/cQYZOxAbRoCY09u9DASa3w/runs/0/logs/public%2Flogs%2Flive.log#L45120
Firefox was killed with a timeout, but the logs look similar:
https://tools.taskcluster.net/groups/HhU663d0QWmdEzYl-RMVnQ/tasks/W0NQ9kORTfGXM9wXRrXWSA/runs/0/logs/public%2Flogs%2Flive.log

I'm force-merging it.

@Hexcles Hexcles merged commit cf1859b into master May 29, 2019
@Hexcles Hexcles deleted the chromium-export-cl-1626600 branch May 29, 2019 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants