1455078 - move service worker secure context checks to a webidl [Func] annotation

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently we do some manual checking for secure context in ServiceWorkerContainer::Register().  We should really move this to a webidl [Func] annotation so that the ServiceWorkerContainer is hidden instead of rejecting promises.

Note, we will need this before we can do bug 1131324 because the current code is very window-specific.

Comment 1

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Implement [SecureContext] for ServiceWorkerContainer

The spec mandates that ServiceWorkerContainer only be visible in a secure context,
yet currently it is (almost) always visible, but rejects calls to register() in
non-secure contexts. This commit moves the context check to a [Func] function, thus
implementing the behavior exactly as specified.

This commit uses the same mechanism used by [SecureContext] bindings instead of the
current ad hoc implementation.

Comment 2

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Refactor test to run in code in page

This test runs code in the frame script, which has system privileges. Even
though it trips the security tests as it is, it's not clear whether that
should be the case or not. By running the code in the page itself, the
expected behavior is governed by the service workers spec and is thus well
defined.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8999902 [details]
Bug 1455078: Refactor test to run in code in page

Andrew Sutherland [:asuth] has approved the revision.

Comment 4

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Test exposure of navigator.serviceWorker to data: iframes

Documents whose creation URL is a data: URL are not considered secure contexts,
therefore they should not have access to navigator.serviceWorker. Another WPT test,
local-url-inherit-controller.https.html, currently assumes the this interface is
available and uses it to test that data: frames do not inherit the parent's
controller; this is wrong, but before we can remove that test, we'd better test
explicitly that such frames cannot access the service worker interface and
therefore cannot have a controller.

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8999902 [details]
Bug 1455078: Refactor test to run in code in page

Alexandre Poirot [:ochameau] has approved the revision.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c2f920f1a2d
Refactor test to run in code in page r=ochameau,asuth

Comment 7

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/3c2f920f1a2d

Comment 8

[Yaron Tausky [:ytausky]]

Sorry for the confusion, but that was only one of several patches required to resolve this bug. Is there a way to mark a patch as partial?

Comment 9

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Enable service worker testing in test

This preference must be enabled since the test creates an insecure context, in
which navigator.serviceWorker is undefined.

Comment 10

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Remove non-conformant ServiceWorker test case

This test case would fail on a conforming implementation of the spec, since
navigator.serviceWorker is undefined in frames originating from data: URLs,
thus navigator.serviceWorker.controller doesn't exist.

Comment 11

[Yaron Tausky [:ytausky]]

Attachment: Bug 1455078: Make ServiceWorker test check interface availability with lowered privileges

The test as it is currently written checks whether navigator.serviceWorker
exists with system privileges. By using eval(), this commit makes it
perform the check with the content's privileges.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8998628 [details]
Bug 1455078: Implement [SecureContext] for ServiceWorkerContainer

Andrew Sutherland [:asuth] has approved the revision.

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9001914 [details]
Bug 1455078: Make ServiceWorker test check interface availability with lowered privileges

Andrew Sutherland [:asuth] has approved the revision.

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9001911 [details]
Bug 1455078: Enable service worker testing in test

Andrew Sutherland [:asuth] has approved the revision.

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9001912 [details]
Bug 1455078: Remove non-conformant ServiceWorker test case

Andrew Sutherland [:asuth] has approved the revision.

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9001552 [details]
Bug 1455078: Test exposure of navigator.serviceWorker to data: iframes

Andrew Sutherland [:asuth] has approved the revision.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e8edebf7500
Test exposure of navigator.serviceWorker to data: iframes r=asuth

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fc9afb46004
Remove non-conformant ServiceWorker test case r=asuth

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f29d9aef185
Enable service worker testing in test r=asuth

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e44b02e94ca
Make ServiceWorker test check interface availability with lowered privileges r=asuth

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7472e52f6d12
Implement [SecureContext] for ServiceWorkerContainer r=asuth

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12623 for changes under testing/web-platform/tests

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 24

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5e8edebf7500
https://hg.mozilla.org/mozilla-central/rev/6fc9afb46004
https://hg.mozilla.org/mozilla-central/rev/3f29d9aef185
https://hg.mozilla.org/mozilla-central/rev/9e44b02e94ca
https://hg.mozilla.org/mozilla-central/rev/7472e52f6d12

Comment 25

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 26

[Yaron Tausky [:ytausky]]

All the patches landed, this bug can be closed now.