ServiceWorker: Expose registration within ServiceWorkerGlobalScope [3/3]

ServiceWorker: Expose registration within ServiceWorkerGlobalScope [3/3]

This CL adds a test to verify registration attribute on ServiceWorkerGlobalScope.

[1] Blink: https://codereview.chromium.org/811793002/
[2] Chromium: https://codereview.chromium.org/849163002/
[3] Blink: THIS PATCH


BUG=437677
TEST=run-webkit-tests http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189012

[nhiroki, 2015-01-19 01:51:51]

Patchset #1 (id:1) has been deleted

[nhiroki, 2015-01-19 01:59:21]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2015-01-19 01:59:48]

nhiroki@chromium.org changed reviewers:
+ falken@chromium.org, kinuko@chromium.org

[nhiroki, 2015-01-19 01:59:49]

Ptal, thanks!

[falken, 2015-01-19 08:45:04]

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html
(right):

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:21:
'Service Worker should respond to fetch');
let's remove the frame and then call unregister_and_done. Looks like there's no
need do to that in another .then()?

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/test-helpers.js (right):

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/test-helpers.js:66: return new
URL(url, location).toString().replace(/#.*$/, '');
nit: self.location to be clear it's global

[nhiroki, 2015-01-20 11:00:11]

Patchset #2 (id:60001) has been deleted

[nhiroki, 2015-01-20 11:10:07]

Thank you. Updated.

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html
(right):

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:21:
'Service Worker should respond to fetch');
On 2015/01/19 08:45:04, falken wrote:
> let's remove the frame and then call unregister_and_done. Looks like there's
no
> need do to that in another .then()?

Done.

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/test-helpers.js (right):

https://codereview.chromium.org/822593003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/test-helpers.js:66: return new
URL(url, location).toString().replace(/#.*$/, '');
On 2015/01/19 08:45:04, falken wrote:
> nit: self.location to be clear it's global

Done.

[kinuko, 2015-01-26 03:15:53]

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js:36:
// FIXME: Verify that registration.active is set.
When are we going to fix this?

[nhiroki, 2015-01-26 04:00:39]

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js:36:
// FIXME: Verify that registration.active is set.
On 2015/01/26 03:15:53, kinuko wrote:
> When are we going to fix this?

Following CLs will fix this:
[1] https://codereview.chromium.org/855383006/
[2] https://codereview.chromium.org/861743002/

(I'll resume to work on these CLs soon)

[nhiroki, 2015-01-27 03:55:28]

Ping?

[kinuko, 2015-01-27 03:57:33]

On 2015/01/27 03:55:28, nhiroki (slow) wrote:
> Ping?

lgtm (can we have a bug number in the FIXME comment for easier tracking just in
case?)

[falken, 2015-01-27 04:08:16]

lgtm with nits

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:10:
var scope = "resources/scope/registration-attribute";
nit: single-quotes for javascript strings

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:12:
return navigator.serviceWorker.register(script, {scope: scope})
nit: we typically do unregister_and_register() in the beginning of the test

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js:46:
e.respondWith(new Response('Hello, world!'));
nit: instead of "Hello, world", respond with 'PASS' is more idiomatic. Or else
something descriptive like "This response came from the Service Worker."

[nhiroki, 2015-01-27 07:51:26]

Thank you!

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:10:
var scope = "resources/scope/registration-attribute";
On 2015/01/27 04:08:15, falken wrote:
> nit: single-quotes for javascript strings

Done.

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/registration-attribute.html:12:
return navigator.serviceWorker.register(script, {scope: scope})
On 2015/01/27 04:08:15, falken wrote:
> nit: we typically do unregister_and_register() in the beginning of the test

Done.

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js
(right):

https://codereview.chromium.org/822593003/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/registration-attribute-worker.js:46:
e.respondWith(new Response('Hello, world!'));
On 2015/01/27 04:08:15, falken wrote:
> nit: instead of "Hello, world", respond with 'PASS' is more idiomatic. Or else
> something descriptive like "This response came from the Service Worker."

Done.

[nhiroki, 2015-01-27 07:51:32]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2015-01-27 07:51:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/822593003/100001

[commit-bot: I haz the power, 2015-01-27 09:16:25]

Committed patchset #3 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189012