Specify active service worker inheritance #2809

jungkees · 2017-07-04T12:23:47Z

This adds steps to make clients that have creator browsing contexts to
inherit the active service worker from their creators. While they have
an initial inherited active service worker, if one exists, navigation
matching still always wins. So, if the navigation goes through HTTP
fetch, the inherited active service worker is replaced by the matched
result. Also, if an iframe has a sandbox attribute set without
allow-same-origin token, the inherited active service worker is set to
null.

Related issue: w3c/ServiceWorker#765.

💥 Error: Wattsi server error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 7:57 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.

🔗 Related URL

Parsing MDN data...
Parsing...

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

This adds steps to make clients that have creator browsing contexts to inherit the active service worker from their creators. While they have an initial inherited active service worker, if one exists, navigation matching still always wins. So, if the navigation goes through HTTP fetch, the inherited active service worker is replaced by the matched result. Also, if an iframe has a sandbox attribute set without allow-same-origin token, the inherited active service worker is set to null. Related issue: w3c/ServiceWorker#765.

jungkees · 2017-07-04T12:45:09Z

I plan to revisit Add tests for service worker inheritance web-platform-tests/wpt#4610 for testing.
I'll address necessary changes for the environment's execution ready flag in a separate PR when we have more clear picture here.
This change includes:
- Inherit creator active service worker when creating a new browsing context.
- Transfer the creation URL and active service worker of the reserved environment gotten through the http fetch to the (reused) initial about:blank client.
- Nullify the client's active service worker when the sandboxed origin browsing context flag is set.

Note: I just came up with an idea (though I'm not sure it's better or feasible). For the initial about:blank client navigation, can we set the request's reserved client to the initial client (the environment settings object created during the creation of the browsing context) directly instead of creating and setting a new environment? (See https://html.spec.whatwg.org/#process-a-navigate-fetch step 4 and 5.) I'll think about that option too.

jungkees · 2017-07-04T12:48:29Z

It seems I can't set the reviewers. (Has the policy been changed?)

@annevk, @jakearchibald, @domenic, I'd appreciate your review.

/cc @wanderview @mattto

annevk · 2017-07-05T08:24:38Z

No, the reviewers feature is broken likely due to the size of the source resource. We've raised it with GitHub, but nothing thus far.

annevk

In general I'm a little hesitant to move forward with this, since all these algorithms need refactoring first. Once we make it clearer how documents and global objects are allocated it would be much easier to plug in service workers.

I wonder if it would not be better to wait with this PR until that is done. I am planning to actively work on that, for what it's worth.

annevk · 2017-07-05T08:26:27Z

source

+      <ol>
+       <li>
+        <p>If the new <code>Document</code> is not the <code>about:blank</code>
+        <code>Document</code>, then:</p>


I don't understand this conditional.

I was trying to exclude the case where the navigate was done against the (non-initial) about:blank Document. (This is basically the part that overrides the active service worker and the creation URL of the "old" environment settings object with the new values gotten from the navigate fetch.)

Thinking on your comment, I noticed that the about:blank document should have a unique origin. So, I think this condition isn't needed.

Addressed this in the follow-up commit.

annevk · 2017-07-05T08:28:18Z

source

+
+        <ol>
+         <li><p>Let <var>settingsObject</var> be <var>browsingContext</var>'s <span>active
+         document</span>'s <span>environment settings object</span>.</p></li>


So this is the "old" document?

Though I guess for the settings object that doesn't matter...

Right. This should be the old one because the new one isn't installed yet. This is just to get the handle to the settings object anyway as you pointed.

annevk · 2017-07-05T08:29:22Z

source

+         <li><p>Set <var>settingsObject</var>'s <span
+         data-x="concept-environment-creation-url">creation URL</span> to
+         <var>reservedEnvironment</var>'s <span data-x="concept-environment-creation-url">creation
+         URL</span>.</p></li>


Should this be done either way? Is this something that's wrong today?

We reuse the "old" environment settings object for the new Document in this case, but don't have any steps to change the creation URL and the active service worker. Currently without adding this, the resulting (old) environment settings object won't get a controller even though the navigate fetch matched and delivered one.

jungkees · 2017-07-05T11:07:16Z

I wonder if it would not be better to wait with this PR until that is done. I am planning to actively work on that, for what it's worth.

@annevk, I think I'm pretty clear about the behavior I'm trying to add in this PR: 1) a child/auxiliary client inherits its parent's SW 2) in the initial client reuse case, we should set the initial client's SW and the creation URL to the values we got from the navigate fetch 3) a sandboxed iframe without allow-same-origin won't get a controller.

But I guess it'll still take time to land this PR including the effort for testing. So, I'm fine with your plan. Just keep me posted in the related PRs.

domenic · 2017-07-05T12:01:51Z

FWIW I'd rather land it earlier than wait; I think we need to not hold up ongoing integration work on straightening out our story here.

I don't feel super-capable of reviewing this, so unless there's something specific I could help with I'll defer to @annevk...

jungkees · 2017-07-06T01:11:23Z

source

@@ -82208,7 +82201,7 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
       </li>

       <li><p><span>Set up a window environment settings object</span> with <var>realm execution
-       context</var> and <var>reservedEnvironment</var>, if present.</p></li>


Removed "if present" as reservedEnvironment should always be present in this step.

annevk · 2017-07-06T11:49:39Z

@domenic I appreciate the drive to just keep moving, but I think we'll end up with another #2687. As long as we haven't properly defined the lifetime of documents and in particular when they are created and how, we're not helping anyone by adding more text to the existing algorithms that have lots of known issues.

jungkees · 2018-07-03T01:36:23Z

We need a spec for the service worker inheritance. I think we can still work on this PR.

I wrote a glitch to test the current behavior: https://sw-inheritance.glitch.me/.

For the initial about:blank iframe
- Edge and Chrome return null.
- Firefox inherits the parent's controller.
For the sandboxed iframe
- All throw when accessing navigator.serviceWorker.controller.
For the sandboxed iframe with allow-same-origin
- All return null.

The expected behavior the PR suggests is inherit / null (and throw as-is) / inherit, respectively.

/cc SW folks: @jakearchibald @wanderview @aliams @slightlyoff @mattto @cdumez (sorry I haven't tested with Safari. Please add.)

wanderview · 2018-07-03T02:23:30Z

The expected behavior the PR suggests is inherit / null (and throw as-is) / inherit, respectively.

I think thats correct.

The firefox behavior for the sandboxed iframe with allow-same-origin can probably be fixed by changing this line:

https://searchfox.org/mozilla-central/rev/97d488a17a848ce3bebbfc83dc916cf20b88451c/docshell/base/nsDocShell.cpp#13815

To check (mSandboxFlags & SANDBOXED_ORIGIN) instead of any mSandboxFlags value.

cc @asutherland @mrbkap

wanderview · 2018-07-03T02:28:18Z

The expected behavior the PR suggests is inherit / null (and throw as-is) / inherit, respectively.

Safari TP 59 matches the expectations.

Note, though, the sandbox error is more about accessing the cross-origin iframe's contentWindow. It might be better to change the test so that the child frame postMessage()'s the controller scriptURL to the parent.

jungkees · 2018-07-03T04:22:30Z

Note, though, the sandbox error is more about accessing the cross-origin iframe's contentWindow. It might be better to change the test so that the child frame postMessage()'s the controller scriptURL to the parent.

That's right. But we shouldn't make the iframe navigate. Maybe should I use srcdoc that postMessage() to the parent? In that case, the inherited controller should hold, right?

asutherland · 2018-07-03T15:48:24Z

Firefox bug 1279406 tracks the sandboxed iframe with "allow-same-origin" inheritance and I've updated it to link to @wanderview's comment and suggested fix. Thanks, Ben!

wanderview · 2018-07-03T16:15:29Z

But we shouldn't make the iframe navigate.

Oh yeah, it would be hard to make a sandboxed about:blank post message anything.

Another way to observe it would be to use clients.matchAll() from the SW and look at the list of controlled clients.

jungkees · 2018-07-03T18:25:05Z

Another way to observe it would be to use clients.matchAll() from the SW and look at the list of controlled clients.

Right. postMessage() from sandboxed iframe won't work. So, this would be a way to go.

annevk · 2021-04-28T13:17:42Z

@antosart this is an interesting case for policy containers.

jungkees mentioned this pull request Jul 4, 2017

Specify active service worker inheritance #2080

Closed

annevk reviewed Jul 5, 2017

View reviewed changes

Remove unnecessary conditional

3d8a6d1

jungkees commented Jul 6, 2017

View reviewed changes

jungkees mentioned this pull request Nov 3, 2017

Create F2F agenda - 7 November 2017 w3c/ServiceWorker#1206

Open

Base automatically changed from master to main January 15, 2021 07:57

domenic force-pushed the main branch from 0947339 to 1b5099f Compare July 27, 2021 17:07

This comment was marked as spam.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify active service worker inheritance #2809

Specify active service worker inheritance #2809

jungkees commented Jul 4, 2017 •

edited by pr-preview bot

Loading

jungkees commented Jul 4, 2017

jungkees commented Jul 4, 2017

annevk commented Jul 5, 2017

annevk left a comment

annevk Jul 5, 2017

jungkees Jul 5, 2017

jungkees Jul 6, 2017

annevk Jul 5, 2017

annevk Jul 5, 2017

jungkees Jul 5, 2017

annevk Jul 5, 2017

jungkees Jul 5, 2017

jungkees commented Jul 5, 2017

domenic commented Jul 5, 2017

jungkees Jul 6, 2017

annevk commented Jul 6, 2017

jungkees commented Jul 3, 2018 •

edited

Loading

wanderview commented Jul 3, 2018

wanderview commented Jul 3, 2018

jungkees commented Jul 3, 2018

asutherland commented Jul 3, 2018

wanderview commented Jul 3, 2018

jungkees commented Jul 3, 2018

annevk commented Apr 28, 2021

This comment was marked as spam.

Specify active service worker inheritance #2809

Are you sure you want to change the base?

Specify active service worker inheritance #2809

Conversation

jungkees commented Jul 4, 2017 • edited by pr-preview bot Loading

💥 Error: Wattsi server error 💥

jungkees commented Jul 4, 2017

jungkees commented Jul 4, 2017

annevk commented Jul 5, 2017

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees commented Jul 5, 2017

domenic commented Jul 5, 2017

Choose a reason for hiding this comment

annevk commented Jul 6, 2017

jungkees commented Jul 3, 2018 • edited Loading

wanderview commented Jul 3, 2018

wanderview commented Jul 3, 2018

jungkees commented Jul 3, 2018

asutherland commented Jul 3, 2018

wanderview commented Jul 3, 2018

jungkees commented Jul 3, 2018

annevk commented Apr 28, 2021

This comment was marked as spam.

jungkees commented Jul 4, 2017 •

edited by pr-preview bot

Loading

jungkees commented Jul 3, 2018 •

edited

Loading