Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(202)

Issue 2899343003: Upstream service worker "statechange" tests to WPT (Closed)

Created:
3 years, 7 months ago by mike3
Modified:
3 years, 6 months ago
Reviewers:
falken
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, kenjibaheux+watch_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, blink-reviews-w3ctests_chromium.org, nhiroki, kinuko+serviceworker, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, tzik
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Upstream service worker "statechange" tests to WPT **state** This test exists in equivalent forms in both the Web Platform Tests project and in the Chromium project. Remove the Chromium version of the test. **synced-state** This test exists in both WPT and the Chromium project tree. The WPT version is slightly weaker because it accounts for a condition that is not possible according to the latest version of the Service Workers specification [1]. > # Install > > [...] > > 16. Run the Update Registration State algorithm passing registration, > "waiting" and registration’s installing worker as the arguments. > 17. Run the Update Registration State algorithm passing registration, > "installing" and null as the arguments. > 18. Run the Update Worker State algorithm passing registration’s > waiting worker and installed as the arguments. > 19. Invoke Finish Job with job. > 20. Wait for all the tasks queued by Update Worker State invoked in > this algorithm have executed. > 21. Invoke Try Activate with registration. Due to step 20, worker activation does not occur until all prior tasks (which includes microtasks such as the Promise handler microtask created in the test's body) have executed. Remove the conditional logic from the WPT version. Ensure de-registration takes place regardless of test outcome. Ensure failure does not cause test timeout. Add a 'use strict' directive. Remove the Chromium version of the test. [1] https://w3c.github.io/ServiceWorker/#installation-algorithm retrieved on 17.05.24 BUG=688116 R=falken@chromium.org Review-Url: https://codereview.chromium.org/2899343003 Cr-Commit-Position: refs/heads/master@{#475404} Committed: https://chromium.googlesource.com/chromium/src/+/04cdcb87fc9391957c0b08aec388f881e0d0d94c

Patch Set 1 #

Total comments: 10

Patch Set 2 : Introduce additional assertions from Chromium #

Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -191 lines) Patch
M third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/synced-state.https.html View 1 1 chunk +77 lines, -48 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/state.html View 1 chunk +0 lines, -69 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html View 1 chunk +0 lines, -74 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
mike3
Hi Matt--this one may be a little tricky to review, so I've generated two sets ...
3 years, 7 months ago (2017-05-24 21:38:58 UTC) #1
falken
Looks pretty good but I think we dropped some asserts, no? https://codereview.chromium.org/2899343003/diff/1/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/synced-state.https.html File third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/synced-state.https.html (right): ...
3 years, 6 months ago (2017-05-29 04:06:26 UTC) #2
mike3
> Looks pretty good but I think we dropped some asserts, no? You're right, I ...
3 years, 6 months ago (2017-05-29 15:30:49 UTC) #3
falken
thanks, lgtm
3 years, 6 months ago (2017-05-30 00:55:03 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2899343003/20001
3 years, 6 months ago (2017-05-30 00:55:17 UTC) #6
commit-bot: I haz the power
3 years, 6 months ago (2017-05-30 02:18:31 UTC) #9
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/04cdcb87fc9391957c0b08aec388...

Powered by Google App Engine
This is Rietveld 408576698