Issue 358553003: Service Worker: add layout tests for behavior of controller after unregister

Issue 358553003: Service Worker: add layout tests for behavior of controller after unregister (Closed)

Created:
6 years, 6 months ago by falken

Modified:
6 years, 6 months ago

Reviewers:
michaeln

CC:
blink-reviews, jsbell+serviceworker_chromium.org, tzik, serviceworker-reviews, nhiroki, falken, kinuko+serviceworker, horo+watch_chromium.org, alecflett+watch_chromium.org

Base URL:
svn://svn.chromium.org/blink/trunk

Project:
blink

Visibility:
Public.

More Reviews

Description

Service Worker: add layout tests for behavior of controller after unregister Unregister doesn't affect existing control over a document, but prevents the worker from controlling on future document loads. We already implement this behavior but no layout test existed yet. BUG=388095 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177054

Patch Set 1 #

Total comments: 10

Patch Set 2 : feedback #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+73 lines, -0 lines)			Patch
A	LayoutTests/http/tests/serviceworker/resources/simple.txt	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
A	LayoutTests/http/tests/serviceworker/resources/simple-intercept-worker.js	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
A	LayoutTests/http/tests/serviceworker/resources/unregister-controller-page.html	View	1	1 chunk	+16 lines, -0 lines	0 comments	Download
A	LayoutTests/http/tests/serviceworker/unregister-controller.html	View	1	1 chunk	+52 lines, -0 lines	0 comments	Download

Messages

Total messages: 12 (0 generated)

Expand Messages | Collapse Messages

falken

Just adds a layout test. I figure it's useful to assert this end-to-end behavior in ...

6 years, 6 months ago (2014-06-25 09:47:10 UTC) #1

michaeln

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/serviceworker/unregister-controller.html File LayoutTests/http/tests/serviceworker/unregister-controller.html (right): https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/serviceworker/unregister-controller.html#newcode19 LayoutTests/http/tests/serviceworker/unregister-controller.html:19: with_iframe(scope, t.step_func(function(frame) { how is 'scope' used by with_iframe? ...

6 years, 6 months ago (2014-06-26 00:41:15 UTC) #2

falken

Thanks. I've revised the test. https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/serviceworker/unregister-controller.html File LayoutTests/http/tests/serviceworker/unregister-controller.html (right): https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/serviceworker/unregister-controller.html#newcode19 LayoutTests/http/tests/serviceworker/unregister-controller.html:19: with_iframe(scope, t.step_func(function(frame) { On ...

6 years, 6 months ago (2014-06-26 03:03:16 UTC) #3

Thanks. I've revised the test.

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/unregister-controller.html (right):

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/unregister-controller.html:19:
with_iframe(scope, t.step_func(function(frame) {
On 2014/06/26 00:41:15, michaeln wrote:
> how is 'scope' used by with_iframe?

with_iframe makes an iframe with src=scope. The idea is just to create a
document that's in-scope, even if there's no resource at that URL. In this case,
we've registered the worker then loaded the document, so the document will have
body 'hello world'.

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/unregister-controller.html:31: var xhr =
new w.XMLHttpRequest();
On 2014/06/26 00:41:15, michaeln wrote:
> On my first reading, i didn't notice you were new'ing a w.XMLHttpRequest().
Are
> you sure about which context this request *will* be started in and also about
> which context it *should* be started in?
> 

Yeah I agree this is confusing though it seems to work as intended; the request
is done from the context of the iframe. I rearranged the tests so the the XHR
request is done from a function in the iframe's document.

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/unregister-controller.html:34:
assert_equals(xhr.response, 'hello, world', 'controller should intercept
requests');
On 2014/06/26 00:41:15, michaeln wrote:
> hello world is generated by the script, right? maybe have it generate
> 'hello world generated by script' ?

Done.

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/unregister-controller.html:44:
async_test(function(t) {
On 2014/06/26 00:41:15, michaeln wrote:
> does this next async_test() get queued up and wait for completion of the first
> test, or do they run concurrently? just curious?

They run concurrently. That's why we have to be careful to use different scopes
in these tests.

https://codereview.chromium.org/358553003/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/unregister-controller.html:61:
assert_equals(xhr.response, '<!DOCTYPE html>\n<title>Simple</title>\nHere\'s a
simple html file.\n', 'requests should not be intercepted');
On 2014/06/26 00:41:15, michaeln wrote:
> a simple text file would be even simpler (and make for shorter lines script)
> 'simple contents of file' ?

Done.