1181039 - "Harness status: OK" + failing tests when running "getregistration.https.html" test

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Noemí Freire (:noemi)]

Checked with 7/6 master build

Test run such as |./mach web-platform-tests _mozilla/service-workers/service-worker/getregistration.https.html|

Result:
* Harness status: OK
* Found 5 tests
* 3 Pass
* 2 Fail:
** Register then getRegistration	
*** assert_unreached: unexpected rejection: assert_equals: getRegistration should resolve with registration expected object "[object ServiceWorkerRegistration]" but got object "[object ServiceWorkerRegistration]" Reached unreachable code
unreached_rejection/<@https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/test-helpers.sub.js:42:7 Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1363:20 Test.prototype.step_func/<@https://web-platform.test:8443/resources/testharness.js:1387:1 Promise*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/getregistration.https.html:20:5 Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1363:20 async_test@https://web-platform.test:8443/resources/testharness.js:513:13 @https://web-platform.test:8443/_mozilla/service-workers/service-worker/getregistration.https.html:17:1 
** Register then getRegistration with a URL having a fragment	
*** assert_unreached: unexpected rejection: assert_equals: getRegistration should resolve with registration expected object "[object ServiceWorkerRegistration]" but got object "[object ServiceWorkerRegistration]" Reached unreachable code
unreached_rejection/<@https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/test-helpers.sub.js:42:7 Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1363:20 Test.prototype.step_func/<@https://web-platform.test:8443/resources/testharness.js:1387:1 Promise*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/getregistration.https.html:38:5 Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1363:20 async_test@https://web-platform.test:8443/resources/testharness.js:513:13 @https://web-platform.test:8443/_mozilla/service-workers/service-worker/getregistration.https.html:34:1 
* Traces: https://pastebin.mozilla.org/8838631

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1181039 - ServiceWorkerRegistration must be compared by scope. r?bkelly

Bug 1181039 - ServiceWorkerRegistration must be compared by scope. r?bkelly


The spec for getRegistration() and getRegistrations() states:

    "Resolve promise with a ServiceWorkerRegistration object, setting its
    service worker client to service worker client, which represents
    registration."

indicating a new ServiceWorkerRegistration is created per call. The existing
Blink test depends on Blink's implementation detail which caches the
registration.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Matt, just want to confirm that my interpretation is valid.

Comment 3

[Matt Falkenhagen]

Right, the Blink test is actually incorrect. The spec has a [NewObject] attribute so we must always return a new object here.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8649940 [details]
MozReview Request: Bug 1181039 - ServiceWorkerRegistration must be compared by scope. r?bkelly

https://reviewboard.mozilla.org/r/16511/#review14927

Ship It!

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=39ad0593de92

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/8dfdf95523d51a96b4bdf9ea86830aa7f34c25ba
changeset:  8dfdf95523d51a96b4bdf9ea86830aa7f34c25ba
user:       Nikhil Marathe <[e-mail]>
date:       Wed Aug 19 08:55:01 2015 -0700
description:
Bug 1181039 - ServiceWorkerRegistration must be compared by scope. r=bkelly


The spec for getRegistration() and getRegistrations() states:

    "Resolve promise with a ServiceWorkerRegistration object, setting its
    service worker client to service worker client, which represents
    registration."

indicating a new ServiceWorkerRegistration is created per call. The existing
Blink test depends on Blink's implementation detail which caches the
registration.

Comment 7

[Noemí Freire (:noemi)]

Patch provided by Nikhil so assigning this bug to him. Please feel free to change it if needed. Thanks!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8dfdf95523d5

Comment 9

[Noemí Freire (:noemi)]

Hi,

just checked on m-c (ba43a48d3c52 revision) and the tests successfully run. Thanks for fixing it!.

Summary

Harness status: OK

Found 5 tests
5 Pass
Details
Result	Test Name
Pass	getRegistration	
Pass	Register then getRegistration	
Pass	Register then getRegistration with a URL having a fragment	
Pass	getRegistration with a cross origin URL	
Pass	Register then Unregister then getRegistration

Comment 10

[Matt Falkenhagen]

I'm very sorry, apparently the spec *does* require JS equality. I misunderstood the [NewObject] vs [SameObject] annotation. See https://code.google.com/p/chromium/issues/detail?id=523904#c9

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Matt Falkenhagen from comment #10)
> I'm very sorry, apparently the spec *does* require JS equality. I
> misunderstood the [NewObject] vs [SameObject] annotation. See
> https://code.google.com/p/chromium/issues/detail?id=523904#c9

I think its still an open spec issue:

  https://github.com/slightlyoff/ServiceWorker/issues/416

But the consensus did seem to be for the same registration object.

Nikhil, do you remember all the tests we changed recently for this?

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

I wrote bug 1201127 to fix our various APIs to return the same registration objects.

Comment 13

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Ben Kelly [:bkelly] from comment #11)
> (In reply to Matt Falkenhagen from comment #10)
> > I'm very sorry, apparently the spec *does* require JS equality. I
> > misunderstood the [NewObject] vs [SameObject] annotation. See
> > https://code.google.com/p/chromium/issues/detail?id=523904#c9
> 
> I think its still an open spec issue:
> 
>   https://github.com/slightlyoff/ServiceWorker/issues/416
> 
> But the consensus did seem to be for the same registration object.
> 
> Nikhil, do you remember all the tests we changed recently for this?

answered in Bug 1201127