ServiceWorker: Add a test to check if worker is deleted after reloading iframe following unregister

ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
Committed: https://crrev.com/6a23a81226c806c8a34291b177ee058dd45b6527
Cr-Commit-Position: refs/heads/master@{#386599}

[shimazu (google), 2016-04-07 07:10:13]

shimazu@google.com changed reviewers:
+ nhiroki@chromium.org

[shimazu (google), 2016-04-07 07:10:14]

PTAL:)

[nhiroki, 2016-04-07 10:27:59]

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:65:
.then(function(r) {
Can you make indents consistent with other tests?

do_something()           // The base line
  .then(function() {     // +2 spaces
      return promise_a;  // +6 spaces
    })                   // +4 spaces
  .then(function() {
      return 
    })

See: http://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:78:
frame.onload = function() { resolve(); };
|frame.onload = resolve;| may work.

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:92:
}).then(function (state) {
- There is an extra space before |(state)|.
- Can you remove |state| argument because it's not used?

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:94:
'installing version goes to active after activated');
To identify the worker, how about keeping the installing version around line 87
and then comparing it with |registration.active| here?

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:94:
'installing version goes to active after activated');
Generally, an assertion message should have the word 'should':

  "Messages should describe the desired behavior and use the word 'should'; this
aids clarity."

From http://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style:

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:96:
'installing SW comes to active after activated');
There are some variants that represent service workers ('installing version',
'installing SW', 'active'). Can you make them consistent?

[shimazu (google), 2016-04-08 02:09:20]

Thanks, I incorporate with the reviews.

And, I found, on firefox, the |registration.active| in l.99 is different with
|registration.installing| in l.89, and this test fails. This test can help to
track an issue of the different behavior.

PTAL again:)

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:65:
.then(function(r) {
On 2016/04/07 10:27:58, nhiroki wrote:
> Can you make indents consistent with other tests?
> 
> do_something()           // The base line
>   .then(function() {     // +2 spaces
>       return promise_a;  // +6 spaces
>     })                   // +4 spaces
>   .then(function() {
>       return 
>     })
> 
> See:
http://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style

Done.

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:78:
frame.onload = function() { resolve(); };
On 2016/04/07 10:27:58, nhiroki wrote:
> |frame.onload = resolve;| may work.

Done.

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:92:
}).then(function (state) {
On 2016/04/07 10:27:58, nhiroki wrote:
> - There is an extra space before |(state)|.
> - Can you remove |state| argument because it's not used?

Done.

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:94:
'installing version goes to active after activated');
On 2016/04/07 10:27:59, nhiroki wrote:
> Generally, an assertion message should have the word 'should':
> 
>   "Messages should describe the desired behavior and use the word 'should';
this
> aids clarity."
> 
> From
http://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style:

Done.

https://codereview.chromium.org/1865103003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:96:
'installing SW comes to active after activated');
On 2016/04/07 10:27:58, nhiroki wrote:
> There are some variants that represent service workers ('installing version',
> 'installing SW', 'active'). Can you make them consistent?

Done.

[falken, 2016-04-08 02:27:17]

falken@chromium.org changed reviewers:
+ falken@chromium.org

[falken, 2016-04-08 02:27:18]

Drive by comment inline.

> And, I found, on firefox, the |registration.active| in l.99 is different with
> |registration.installing| in l.89, and this test fails. This test can help to
> track an issue of the different behavior.

Good find. See https://bugs.chromium.org/p/chromium/issues/detail?id=459457 and
https://github.com/slightlyoff/ServiceWorker/issues/622 and
https://github.com/slightlyoff/ServiceWorker/issues/416 for some background, not
sure what Firefox ended up doing.

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:106:
'should renew the SW script.');
This description doesn't seem to capture what the test is doing. It should
mention something about reloading the last iframe using an uninstalling
registration causes the registration to be deleted.

[falken, 2016-04-08 02:29:02]

Also, "worker is updated after reloading" is a bit misleading; it sounds like
update() or Soft Update. The test is testing registration deletion, and then
calling register().

[nhiroki, 2016-04-08 03:24:40]

On 2016/04/08 02:27:18, falken wrote:
> Drive by comment inline.
> 
> > And, I found, on firefox, the |registration.active| in l.99 is different
with
> > |registration.installing| in l.89, and this test fails. This test can help
to
> > track an issue of the different behavior.
> 
> Good find. See https://bugs.chromium.org/p/chromium/issues/detail?id=459457
and
> https://github.com/slightlyoff/ServiceWorker/issues/622 and
> https://github.com/slightlyoff/ServiceWorker/issues/416 for some background,
not
> sure what Firefox ended up doing.

registration-service-worker-attributes.html could check the object equality
instead of script url.

[nhiroki, 2016-04-08 03:46:26]

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:92:
'active sw should be null after reloading');
The purpose of these assertions is to check that the unregistered registration
is not resurrected after reloading. To explicitly confirm that, we may also want
to check the equality between new and old registrations like this:

  new_registration = r;
  assert_not_equals(new_registration, registration);
  assert_not_equals(new_registration.installing, null);
  assert_equals(new_registration.active, null);

[shimazu (google), 2016-04-08 04:32:00]

On 2016/04/08 03:24:40, nhiroki wrote:
> On 2016/04/08 02:27:18, falken wrote:
> > Drive by comment inline.
> > 
> > > And, I found, on firefox, the |registration.active| in l.99 is different
> with
> > > |registration.installing| in l.89, and this test fails. This test can help
> to
> > > track an issue of the different behavior.
> > 
> > Good find. See https://bugs.chromium.org/p/chromium/issues/detail?id=459457
> and
> > https://github.com/slightlyoff/ServiceWorker/issues/622 and
> > https://github.com/slightlyoff/ServiceWorker/issues/416 for some background,
> not
> > sure what Firefox ended up doing.
> 
> registration-service-worker-attributes.html could check the object equality
> instead of script url.

I checked the equality of a series of |registration.installing|, |.waiting| and
|.active| by registration-service-worker-attributes.html with a small fix. It
shows chromium keeps the equality but firefox doesn't. This seems the reason of
failure of my new test.
Do you have any idea to check the equality of service workers?

[shimazu (google), 2016-04-11 03:28:45]

This test is simplified for checking the difference of "registration" instead of
"service worker". I separated a test to confirm the equality of "service worker"
object in javascript side, and I'll add it into synced-state.html and
registration-service-worker-attributes.html by another CL.

PTAL:)

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:92:
'active sw should be null after reloading');
On 2016/04/08 03:46:26, nhiroki wrote:
> The purpose of these assertions is to check that the unregistered registration
> is not resurrected after reloading. To explicitly confirm that, we may also
want
> to check the equality between new and old registrations like this:
> 
>   new_registration = r;
>   assert_not_equals(new_registration, registration);
>   assert_not_equals(new_registration.installing, null);
>   assert_equals(new_registration.active, null);

Done.

https://codereview.chromium.org/1865103003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:106:
'should renew the SW script.');
On 2016/04/08 02:27:18, falken wrote:
> This description doesn't seem to capture what the test is doing. It should
> mention something about reloading the last iframe using an uninstalling
> registration causes the registration to be deleted.

Done.

[nhiroki, 2016-04-11 05:33:13]

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:61:
'with-old-registration-in-use';
I'm wondering if we could minimize this test.

What we want to test is that reloading the last controlled iframe appropriately
releases the unregistered-but-still-used registration. Is this right? To check
it, we could change the test like this:

1. register
2. create a controlled iframe
3. unregister
4. reload the iframe
5. confirm that the iframe is not controlled (ie. frame's controller attribute
is null)
6. (optional) check if getRegistration() returns 'undefined'

WDYT?

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:91:
navigator.serviceWorker.getRegistration(scope)
To reduce the nest level, you can return the promise here:

.then(function(new_reg)) {
    assert_not_equals(new_reg, old_reg);
    return nav.sW.getRegistration(scope);  // return here.
  })
.then(function(r) {
    assert_not_equals(r, old_reg);
    ...
  })

[shimazu (google), 2016-04-11 06:58:15]

Fixed these things. \PTAL/

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:61:
'with-old-registration-in-use';
On 2016/04/11 05:33:13, nhiroki wrote:
> I'm wondering if we could minimize this test.
> 
> What we want to test is that reloading the last controlled iframe
appropriately
> releases the unregistered-but-still-used registration. Is this right? To check
> it, we could change the test like this:
> 
> 1. register
> 2. create a controlled iframe
> 3. unregister
> 4. reload the iframe
> 5. confirm that the iframe is not controlled (ie. frame's controller attribute
> is null)
> 6. (optional) check if getRegistration() returns 'undefined'
> 
> WDYT?

I checked if getRegistration returns not null before reregistration after
reloading on firefox, and it was true.
Then, I changed the test to check if frame is not controlled by SW after
reloading.

https://codereview.chromium.org/1865103003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:91:
navigator.serviceWorker.getRegistration(scope)
On 2016/04/11 05:33:13, nhiroki wrote:
> To reduce the nest level, you can return the promise here:
> 
> .then(function(new_reg)) {
>     assert_not_equals(new_reg, old_reg);
>     return nav.sW.getRegistration(scope);  // return here.
>   })
> .then(function(r) {
>     assert_not_equals(r, old_reg);
>     ...
>   })

Thanks! Done.

[nhiroki, 2016-04-11 09:51:36]

Almost there!

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:63:
var new_registration;
unused

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:65:
var service_worker;
unused

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:87:
'provided from service worker');
"controlled by" would be more common phrase than "provided from" in SW.

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:97:
'service worker should unregister the service worker successfully');
I think the main action of this test is 'reloading' (not 'unregistration'), so
how about make it the subject? For example...

  'Reloading the last controlled iframe after unregistration should delete the
registration'

[shimazu (google), 2016-04-12 03:42:30]

Thanks, PTAL!

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:63:
var new_registration;
On 2016/04/11 09:51:36, nhiroki wrote:
> unused

Done.

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:65:
var service_worker;
On 2016/04/11 09:51:36, nhiroki wrote:
> unused

Done.

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:87:
'provided from service worker');
On 2016/04/11 09:51:36, nhiroki wrote:
> "controlled by" would be more common phrase than "provided from" in SW.

Done.

https://codereview.chromium.org/1865103003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:97:
'service worker should unregister the service worker successfully');
On 2016/04/11 09:51:36, nhiroki wrote:
> I think the main action of this test is 'reloading' (not 'unregistration'), so
> how about make it the subject? For example...
> 
>   'Reloading the last controlled iframe after unregistration should delete the
> registration'

Done.

[nhiroki, 2016-04-12 04:19:10]

LGTM

As falken@ mentioned, 'worker is updated after reloading' sounds a bit
confusing. Can you revise the subject before pushing this into the CQ?

https://codereview.chromium.org/1865103003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:61:
'with-old-registration-in-use';
This scope name looks outdated. Can you update this?

[shimazu (google), 2016-04-12 05:08:19]

Description was changed from

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
==========

to

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
==========

[shimazu (google), 2016-04-12 05:22:29]

https://codereview.chromium.org/1865103003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html
(right):

https://codereview.chromium.org/1865103003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/unregister-then-register.html:61:
'with-old-registration-in-use';
On 2016/04/12 04:19:10, nhiroki wrote:
> This scope name looks outdated. Can you update this?

Done.

[shimazu (google), 2016-04-12 05:23:19]

The CQ bit was checked by shimazu@google.com

[shimazu (google), 2016-04-12 05:23:19]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1865103003/#ps160001
(title: "Update the scope to match the latest test")

[commit-bot: I haz the power, 2016-04-12 05:23:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865103003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865103003/160001

[commit-bot: I haz the power, 2016-04-12 06:25:27]

Description was changed from

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
==========

to

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
==========

[commit-bot: I haz the power, 2016-04-12 06:25:29]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-04-12 06:27:11]

Description was changed from

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032
==========

to

==========
ServiceWorker: Add a test to check if worker is updated after reloading

After unregistration of a service worker, reloading an iframe provided
by the SW should release that worker. This patch is a test for checking
this behavior.

And, I expected the result of this new test is different between
chromium and firefox based on crbug.com/594032, but both two
environments passed the test. I'm not sure the difference between reload
button and frame.contentWindow.reload().

BUG=594032

Committed: https://crrev.com/6a23a81226c806c8a34291b177ee058dd45b6527
Cr-Commit-Position: refs/heads/master@{#386599}
==========

[commit-bot: I haz the power, 2016-04-12 06:27:12]

Patchset 9 (id:??) landed as
https://crrev.com/6a23a81226c806c8a34291b177ee058dd45b6527
Cr-Commit-Position: refs/heads/master@{#386599}

[falken, 2016-04-12 11:58:21]

On 2016/04/12 04:19:10, nhiroki wrote:
> LGTM
> 
> As falken@ mentioned, 'worker is updated after reloading' sounds a bit
> confusing. Can you revise the subject before pushing this into the CQ?
> 

For future reference, you have to change the first line of the description, not
the subject field. The subject field has no effect in what's committed (yep,
confusing...).

Thanks for the patch!

[falken, 2016-04-12 23:39:22]

I think this test might not assert the desired behavior. Reload of a page never
causes the "all clients are unloaded" state that triggers Clear
Registration/Activate. So we'd expect the same thing to happen for reloading an
iframe.

Can we additionally check whether reloading an iframe triggers Activate for
Chrome and Firefox, and file a spec bug about this?

[nhiroki, 2016-04-13 00:56:24]

On 2016/04/12 23:39:22, falken wrote:
> I think this test might not assert the desired behavior. Reload of a page
never
> causes the "all clients are unloaded" state that triggers Clear
> Registration/Activate. So we'd expect the same thing to happen for reloading
an
> iframe.

Hmm... I don't fully understand why reloading an iframe does not cause "all
clients are unloaded" state yet. I'd like to confirm my understanding, so let me
share my interpretation of the specs...

  1. Reloading an iframe by Location.reload() triggers a navigation [1]
  2. Navigation triggers "prompt to unload the document" at the step 9 [2]
  3. Unloading triggers "Handle Service Worker Client Unload" [3]

  (According to the paragraph "When a user requests that ..." on reload()
algorithm [1], reload of a page by UI seems also to trigger a navigation/unload)

Is this interpretation missing something? For example, are there any
early-return cases w/o unloading a client in the above sequences?

[1] https://html.spec.whatwg.org/multipage/browsers.html#dom-location-reload
[2] https://html.spec.whatwg.org/multipage/browsers.html#navigate
[3]
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#on-client-un...

> Can we additionally check whether reloading an iframe triggers Activate for
> Chrome and Firefox, and file a spec bug about this?

[falken, 2016-04-13 03:09:14]

On 2016/04/13 00:56:24, nhiroki wrote:
> On 2016/04/12 23:39:22, falken wrote:
> > I think this test might not assert the desired behavior. Reload of a page
> never
> > causes the "all clients are unloaded" state that triggers Clear
> > Registration/Activate. So we'd expect the same thing to happen for reloading
> an
> > iframe.
> 
> Hmm... I don't fully understand why reloading an iframe does not cause "all
> clients are unloaded" state yet. I'd like to confirm my understanding, so let
me
> share my interpretation of the specs...
> 
>   1. Reloading an iframe by Location.reload() triggers a navigation [1]
>   2. Navigation triggers "prompt to unload the document" at the step 9 [2]
>   3. Unloading triggers "Handle Service Worker Client Unload" [3]
> 
>   (According to the paragraph "When a user requests that ..." on reload()
> algorithm [1], reload of a page by UI seems also to trigger a
navigation/unload)
> 
> Is this interpretation missing something? For example, are there any
> early-return cases w/o unloading a client in the above sequences?
> 
> [1] https://html.spec.whatwg.org/multipage/browsers.html#dom-location-reload
> [2] https://html.spec.whatwg.org/multipage/browsers.html#navigate
> [3]
>
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#on-client-un...
> 

I think it's possible the spec doesn't say what's intended, but there's clear
consensus that reload of a page by UI should not activate the worker (verified
at the F2F). The reason is that the new client is created before the old client
is unloaded, so there's never a time when "all clients are unloaded". So I'd be
surprised that iframe reload doesn't work the same way.

I think the easiest thing is just raise a spec issue and see what the intended
behavior is, but first I wanted to make sure the behavior we're observing is the
same with Activation (rather than specific to Clear Registration).

> > Can we additionally check whether reloading an iframe triggers Activate for
> > Chrome and Firefox, and file a spec bug about this?

[nhiroki, 2016-04-13 04:04:43]

On 2016/04/13 03:09:14, falken wrote:
> On 2016/04/13 00:56:24, nhiroki wrote:
> > On 2016/04/12 23:39:22, falken wrote:
> > > I think this test might not assert the desired behavior. Reload of a page
> > never
> > > causes the "all clients are unloaded" state that triggers Clear
> > > Registration/Activate. So we'd expect the same thing to happen for
reloading
> > an
> > > iframe.
> > 
> > Hmm... I don't fully understand why reloading an iframe does not cause "all
> > clients are unloaded" state yet. I'd like to confirm my understanding, so
let
> me
> > share my interpretation of the specs...
> > 
> >   1. Reloading an iframe by Location.reload() triggers a navigation [1]
> >   2. Navigation triggers "prompt to unload the document" at the step 9 [2]
> >   3. Unloading triggers "Handle Service Worker Client Unload" [3]
> > 
> >   (According to the paragraph "When a user requests that ..." on reload()
> > algorithm [1], reload of a page by UI seems also to trigger a
> navigation/unload)
> > 
> > Is this interpretation missing something? For example, are there any
> > early-return cases w/o unloading a client in the above sequences?
> > 
> > [1] https://html.spec.whatwg.org/multipage/browsers.html#dom-location-reload
> > [2] https://html.spec.whatwg.org/multipage/browsers.html#navigate
> > [3]
> >
>
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#on-client-un...
> > 
> 
> I think it's possible the spec doesn't say what's intended, but there's clear
> consensus that reload of a page by UI should not activate the worker (verified
> at the F2F). The reason is that the new client is created before the old
client
> is unloaded, so there's never a time when "all clients are unloaded". So I'd
be
> surprised that iframe reload doesn't work the same way.
> 
> I think the easiest thing is just raise a spec issue and see what the intended
> behavior is, but first I wanted to make sure the behavior we're observing is
the
> same with Activation (rather than specific to Clear Registration).

I understand it. Thank you for your explanation :)

> > > Can we additionally check whether reloading an iframe triggers Activate
for
> > > Chrome and Firefox, and file a spec bug about this?

shimazu-san, can you check this?

[nhiroki, 2016-04-13 04:12:15]

On 2016/04/13 04:04:43, nhiroki wrote:
> On 2016/04/13 03:09:14, falken wrote:
> > On 2016/04/13 00:56:24, nhiroki wrote:
> > > On 2016/04/12 23:39:22, falken wrote:
> > > > I think this test might not assert the desired behavior. Reload of a
page
> > > never
> > > > causes the "all clients are unloaded" state that triggers Clear
> > > > Registration/Activate. So we'd expect the same thing to happen for
> reloading
> > > an
> > > > iframe.
> > > 
> > > Hmm... I don't fully understand why reloading an iframe does not cause
"all
> > > clients are unloaded" state yet. I'd like to confirm my understanding, so
> let
> > me
> > > share my interpretation of the specs...
> > > 
> > >   1. Reloading an iframe by Location.reload() triggers a navigation [1]
> > >   2. Navigation triggers "prompt to unload the document" at the step 9 [2]
> > >   3. Unloading triggers "Handle Service Worker Client Unload" [3]
> > > 
> > >   (According to the paragraph "When a user requests that ..." on reload()
> > > algorithm [1], reload of a page by UI seems also to trigger a
> > navigation/unload)
> > > 
> > > Is this interpretation missing something? For example, are there any
> > > early-return cases w/o unloading a client in the above sequences?
> > > 
> > > [1]
https://html.spec.whatwg.org/multipage/browsers.html#dom-location-reload
> > > [2] https://html.spec.whatwg.org/multipage/browsers.html#navigate
> > > [3]
> > >
> >
>
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#on-client-un...
> > > 
> > 
> > I think it's possible the spec doesn't say what's intended, but there's
clear
> > consensus that reload of a page by UI should not activate the worker
(verified
> > at the F2F). The reason is that the new client is created before the old
> client
> > is unloaded, so there's never a time when "all clients are unloaded". So I'd
> be
> > surprised that iframe reload doesn't work the same way.
> > 
> > I think the easiest thing is just raise a spec issue and see what the
intended
> > behavior is, but first I wanted to make sure the behavior we're observing is
> the
> > same with Activation (rather than specific to Clear Registration).
> 
> I understand it. Thank you for your explanation :)
> 
> > > > Can we additionally check whether reloading an iframe triggers Activate
> for
> > > > Chrome and Firefox, and file a spec bug about this?
> 
> shimazu-san, can you check this?

We maybe need to move the test added by this CL to chromium/ directory because
it does not verify the desired (spec'ed) behavior but the current chromium
behavior.