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

Issue 1731823003: Service Worker: Add worker client test for Clients.get(id) (Closed)

Created:
4 years, 10 months ago by jungkees
Modified:
4 years, 10 months ago
Reviewers:
falken, zino, nhiroki
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, kenjibaheux+watch_chromium.org, tzik, serviceworker-reviews, nhiroki, falken, kinuko+serviceworker, blink-reviews, horo+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Service Worker: Add worker client test for Clients.get(id) As a follow-up of https://codereview.chromium.org/1439333002, this CL adds a layout test to cover Clients.get(id) for worker clients. Upon verifying the test result on the chromium waterfall, we plan to make the flag stable. Spec discussion: https://github.com/slightlyoff/ServiceWorker/issues/723#issuecomment-151326898 Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#clients-get-method Intent to implement and ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/XlBHOIOCjUk BUG=552310 Committed: https://crrev.com/21165961cd08e211f8efbe37f937769fb6131f3f Cr-Commit-Position: refs/heads/master@{#377480}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address comments #

Messages

Total messages: 12 (6 generated)
jungkees
nhiroki@, I uploaded a snapshot having added a layout test for a worker client. I ...
4 years, 10 months ago (2016-02-24 13:00:39 UTC) #3
nhiroki
lgtm https://codereview.chromium.org/1731823003/diff/1/third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html File third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html (right): https://codereview.chromium.org/1731823003/diff/1/third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html#newcode30 third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html:30: return new Promise(function(resolve, reject) { nit: |reject| can ...
4 years, 10 months ago (2016-02-24 15:30:43 UTC) #4
jungkees
Thanks for the review. Addressed the comments. https://codereview.chromium.org/1731823003/diff/1/third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html File third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html (right): https://codereview.chromium.org/1731823003/diff/1/third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html#newcode30 third_party/WebKit/LayoutTests/http/tests/serviceworker/clients-get-client-types.html:30: return new ...
4 years, 10 months ago (2016-02-25 01:06:23 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1731823003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1731823003/20001
4 years, 10 months ago (2016-02-25 01:11:00 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 10 months ago (2016-02-25 02:59:13 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2016-02-25 03:00:57 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/21165961cd08e211f8efbe37f937769fb6131f3f
Cr-Commit-Position: refs/heads/master@{#377480}

Powered by Google App Engine
This is Rietveld 408576698