Closed Bug 1431814 Opened 6 years ago Closed 6 years ago

blob URL iframes should inherit parent's controlling service worker

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

While fixing blob URL workers to inherit their parent's service worker controller in bug 1231211 I noticed that iframes don't do the inheritance either.  Filing this as a follow-up bug since I'm not sure where to do it yet.  Also, we should add a WPT test for this.
Depends on: 1231211
See Also: 1231211
This fixes it in local testing.  I need to add a WPT for it.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Priority: -- → P2
Comment on attachment 8944062 [details] [diff] [review]
P1 Allow the controller to be inheritted for other local URLs like blob: and data:. r=asuth

This fixes service worker controller inheritance for blob: URLs.  The fix is very similar to what we did in bug 1426979.  Basically we just have to ignore the LoadInfo for "local" URL schemes so we can properly inherit.  Local URLs are defined in the spec as:

A local scheme is a scheme that is "about", "blob", "data", or "filesystem". 
https://fetch.spec.whatwg.org/#local-scheme

I don't think we have filesystem: URLs, so I just made this work for about, blob, and data.  I explicitly limit about: to things in the spec as I don't know that we want to inherit for other firefox specific about: pages.

Note, this change also isn't strictly necessary for data URL frames.  It seems that worked before anyway.  Possibly because they don't have LoadInfo objects or something.  I decided to include them here for completeness, though, since the local URL definition in the spec includes them.
Attachment #8944062 - Flags: review?(bugmail)
Comment on attachment 8944481 [details] [diff] [review]
P2 Add blob-url-inherits-controller.https.html wpt test. r=asuth

Andrew, this adds a WPT test that checks for controller inheritence for blob URL frames and workers.  Currently we fail the worker test because we don't expose navigator.serviceWorker on worker globals yet.

Note, I had thoughts of adding some cross-origin checks as well, so I wrote the test to be a bit extensible.  I decided to punt on that for now, though.  This initial start on the test at least verifies we inherit properly for the typical blob case.
Attachment #8944481 - Flags: review?(bugmail)
Attachment #8944062 - Flags: review?(bugmail) → review+
Comment on attachment 8944481 [details] [diff] [review]
P2 Add blob-url-inherits-controller.https.html wpt test. r=asuth

Review of attachment 8944481 [details] [diff] [review]:
-----------------------------------------------------------------

Great test.

::: testing/web-platform/tests/service-workers/service-worker/resources/blob-url-inherit-controller-frame.html
@@ +2,5 @@
> +<html>
> +<script>
> +function getBlobText(opts) {
> +  if (opts.child === 'iframe') {
> +    return '<script>' +

No action needed, but don't forget template literals (`blah`) that support newlines exist.  (And if you want the byproduct to be newline free, a tagged literal function like "norm = x => x.join('').replace(/\n */g, ' ')" can accomplish that for norm`stuff\n   blah`.  Although obviously if you want the interpolation to still work, something more is needed.)
Attachment #8944481 - Flags: review?(bugmail) → review+
Addressed review feedback about strings.  Also, I added checks for data URLs.  We don't inherit in that case because they get an opaque origin.
Attachment #8944481 - Attachment is obsolete: true
Attachment #8944786 - Flags: review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28e035f10d0a
P1 Allow the controller to be inheritted for other local URLs like blob: and data:. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/4fe6b13848c8
P2 Add local-url-inherit-controller.https.html wpt test. r=asuth
https://hg.mozilla.org/mozilla-central/rev/28e035f10d0a
https://hg.mozilla.org/mozilla-central/rev/4fe6b13848c8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: