-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
consider Client behavior for windows where initial about:blank is replaced with a loaded document #1091
Comments
Talking with @bzbarsky, it may be very hard to determine if an initial about:blank will indeed be followed up another document. And I don't think we can just ignore about:blank completely because programmatically constructed iframes are supposed to inherit their parent's service worker controller, etc. We need to represent them as Client objects. |
In this iframe /
Following the navigation, the client will be the same as the |
@jakearchibald, I'm kind of confused by #1091 (comment). I don't recall this being the outcome from the f2f meeting. I thought we said we were going to do:
If you don't want the existing about:blank to be "reserved" since its already execution ready, we could also do:
So in:
I guess I would always expect It also seems to make sense to make I can go either way about whether @smaug---- also was wondering what we do if replacement load is for a download. The example he gave was:
In this case, if someone clicked the link I would expect:
If it was |
Although, I still wonder how devs can use this API if we don't provide a consistent place for them to find "the client that will exist if this navigation succeeds". If we don't provide the about:blank initial window as the reserved client, then devs have to do some complicated logic to determine they are in this weird replacement case. |
I read through the meeting minutes and it seems we did not really come to conclusion here. I need to do something for our implementation, though, so I'd like to make a sketch proposal we can discuss at the next meeting. Hopefully I will have something implemented behind a pref so we can try it out. My idea is to be more explicit about the replacement case. Instead of putting the about:blank client ID in the I took the term "initial" here from the "initial about:blank document" used in the HTML spec. I also considered So for this code snippet:
The FetchEvent would have:
After frame.html loads it has the same client id as the initial about:blank iframe. |
By the way, about:blank replacement windows are in theory already exposed, even without reserved Client ID. Consider this page: <iframe src="frame.html" id="frame"></iframe>
<script>
var f = document.getElementById('frame');
f.contentWindow.foo = 'foo'
</script> here the Consider if "frame.html" is delayed in loading. Perhaps a service worker purposefully does this. In this case:
I'm intending to write a test for this, but I'm unsure if I should upstream it to WPT yet or not. What do people think? |
IMO, we might want to put a 'do not merget yet' tag on the test until we come to a conclusion. Or, you can just share the test through the firefox code search if that's more convenient? |
I'll post the test once I have it written. Its revealing more problems for me that I'm trying to work through. One thing that I have noticed is that service workers may have exposed some browser optimizations that were not observable before. Consider loading a simply With service workers, though, we can do this:
In this case our optimization to avoid creating the initial about:blank is exposed because step (2) won't find a Client. At least with our current Clients implementation anyway. I'm trying to make this work without forcing a de-opt in the general case. I'm trying to make our code create the initial about:blank on demand when |
@wanderview, I think I should catch up a bit but in #1091 (comment):
clientId for the snippet should be the about:blank client. clientId in my mental model is the request's client's id. When the src attribute of the iframe's set, foo.html's nested browsing context is navigated instead of the top-level browsing context. See https://html.spec.whatwg.org/#otherwise-steps-for-iframe-or-frame-elements step 4. That is, the request passed to fetch will use the iframe's settings object as the client. I'm a bit concerned about introducing initialClientId. It seems it goes too much complex for corner cases. Coming back to the definition we originally thought:
For normal case (i.e. navigate from non-initial document), they seem to be apparent. For replacement case, I think they should be:
As this is the case where the existing global (and the environment settings object) are legitimately reused, the reserved client being removed and the client being the final client for the resource sounds good to me. FYI, whatwg/html#2080 is the related PR that I should revisit now. |
FYI. I tested the snippet in #1091 (comment) on Chrome. It doesn't seem to reuse the existing window of about:blank document. The state set to the global object is lost after the navigation. |
Yes, I did some compat testing and @bzbarsky pointed out the only real use where browsers maintain compat is this:
What do you think the various properties on FetchEvent should be for this situation? |
That's right. I confirmed by testing Chrome reuses the global in the case shown in your previous comment.
To my understanding, the declaratively-defined-iframe case and the snippet in #1091 (comment) go through the same flow according to the spec. I think the properties on FetchEvent should be the same. That is:
So, devs will see two different clients (about:blank and reserved env) exist during the fetch, which is what's happening indeed. And when the resulting document is installed, the about:blank client is reused and the reserved client is abandoned. (If the navigate found a matching active service worker, that property should be overwritten to the client.) |
Do you mean a new reserved client that is different from the about:blank initial document client? Which client does the final document end up with? Either way you are suggesting that the client ID for a global change during its lifetime. That seems like a bad invariant to break. And all of this is observable to the service worker. Consider this: <!doctype html>
<!-- top.html -->
<html>
<body>
<!-- declaratively defined iframe -->
<iframe src="nested.html" id="nested"></iframe>
<script>
let frame = document.getElementById("nested");
frame.contentWindow.navigator.serviceWorker.postMessage(evt => {
// do stuff and post back to the service worker
evt.source.postMessage(someMsg);
});
</script>
</body>
</html> A clients.matchAll() can get and postMessage() to the initial about:blank. Of course the final window is also accessible as well. I think the service worker could get confused if the MessageEvent.source.id changes over time for the same global. I understand this is complex, but this is the cost of exposing the browser's underlying window/global model via a primitive API like Clients. The about:blank replacement complexity is already there. The primitive needs to reflect that to be honest about what is really happening. |
I agree. Just want to make sure it doesn't get more complex than needed.
Yes.
The about:blank client.
The client IDs aren't changing. In the fetch event, devs will see both the client and the reserved client, but the reserved client will be gone away when the navigate completes in this case. I mean only the about:blank client with its initial ID will be alive. I think it isn't odd as that's what happens. In this case, the reserved client is just used during the main resource fetch for matching a service worker, queueing postMessages, etc. I think the messages sent to the reserved client should be requeued to the about:blank client when the document is installed. |
How can the service worker tell whats going to happen here? Does it have to execute code later to test to see which Client ended up being the final window? This does not seem simpler to me.
I'm sorry, but this seems a lot more complicated than just providing the correct Client in the first place. As an implementer I'd like to keep the Client behavior as close to actual window creation behavior as possible. It reduces special cases in an already complicated part of the system. I'm not a web developer, but it seems to me that they would probably like to have a property they can reference that they can be certain will in fact become the final Client. I'd really like to just put the initial Client ID in the |
Or maybe this all suggests we should avoid exposing reserved Clients at all. Its pretty complicated and perhaps not worth the hassle. @jakearchibald, can you refresh my memory as to why we needed reserved Client ID? Can we get the same thing another way without actually referencing not-quite-there-yet Clients? |
Here is my test case for initial about:blank handling in today's service worker spec: None of this requires reserved client ID, etc. |
@wanderview , thanks for sharing the tests! Both Firefox and Chrome respond with 'failure: could not find about:blank client' for all the cases. As you pointed earlier, the about:blank window either isn't created yet or not exposed to matchAll in some reason at least.
In my memory, the major use case was to open up an ability to manage caches per-client basis. A reservedClientId and a targetClientId can give a clue to create and delete caches, respectively. Another use case was messaging. I don't recall anything else. Let's start with what we agree on first. The final client should be the about:blank client. Right? That client is created when the nested browsing context is created and later reused for the fetched document. In #1091 (comment), you said clientId should be the top-level client (foo.html in the example.) But I think it should be that of the about:blank client. The reason is the fetch request's client is the about:blank client. Do you agree? Beyond that, what reservedClientId should represent here and whether we'll introduce initialClientId are the further discussion points. /cc @jakearchibald |
Correct, but I think this behavior is what we've agreed on in the spec so far, right? This test passes in firefox with the patches I have in progress at: https://bugzilla.mozilla.org/show_bug.cgi?id=1293277
Agreed.
Honestly I'm not sure any more. I thought it would have used the client for the global of the script that made the change, but maybe this has changed recently with @domenic's changes. In any case, I agree it should be some existing Client which we determined to have "caused" the navigation to start.
Right. And the main reason I see for not just putting an initial about:blank client in I guess if we made real reserved clients not queue message events, then this problem would go away. |
Yes. I think so.
IIRC, it wasn't changed.
I think the initial about:blank case isn't different from other cases. Setting src (both declarartively or from script) triggers a navigate of the nested browsing context. The nested browsing context's active document's client being the request's client seems reasonable.
I'd like to clarify if we were on the same page about this. I didn't mean to put an initial about:blank client in reservedClientId. It was about whether to expose the reserved environment made anyway for the http fetch of the navigate request (because all the normal navigate fetch would expose the reserved environment.) It seems we're back to the f2f decision considering the about:blank is to be the final client and exposing the reserved client is too complex. For this, devs should be able to detect it's the initial about:blank case. I now better understand why you proposed initialClientId. I was thinking about using clientId instead of adding a new property like: clients.get(fetchEvent.clientId).then(client => { /* client.url == 'about:blank' && it's a navigate of nested browsing context */ }); Hmm.. I think we still can't distinguish it from a non-initial about:blank case if the session history has other documents in the list. E.g. [document1, document2, about:blank]. So, should we add initialClientId? |
Finally caught up with this, sorry for not pitching in sooner. In the case of:
From reading https://html.spec.whatwg.org/multipage/iframe-embed-object.html#process-the-iframe-attributes, it seems I don't like using
|
In summary:
Btw, I can't get the replacement thing to happen in any browser other than Firefox.
If other browsers are replacing the global when navigating We maybe need to stress in the tests that it's more important that the client IDs reflect what the browser does, rather than simply pass the tests. As in, |
We talked about this in IRC, but for those following along at home, browsers do have some compat here. Its narrowly confined to the particular case where an iframe is statically defined in the document and then an inline script modifies the global before it loads. Cases where you try to dynamically create the iframe have a variety of compat issues:
See the scrollback in this IRC conversation with @bzbarsky for some more discussion: http://logs.glob.uno/?c=mozilla%23content&s=14+Jun+2017&e=14+Jun+2017#c447305 |
I think we need to figure out our |
More details from IRC: As @jungkees said, one of the main use-cases is messaging. The developer should be able to send a message to the resulting page as part of a request of mode navigation. We handle this with reserved clients by buffering messages until the client is created & the port is started (either explicitly by calling However, this won't work with iframes (or Even if we make initial & reserved different things, we need to solve this for the initial case. |
https://html.spec.whatwg.org/multipage/parsing.html#the-end We currently open the client message queue after For initial windows, we could open the client message queue after the load event (that would fit with Firefox's behaviour right @wanderview?) or expect developers to do it manually using |
Note that the Firefox behavior is not HTML-spec-compliant (nor is anyone's else's, in various different and incompatible ways), and if/when browsers update to follow the current spec that approach will break. So you either need to come up with a design here that works with the HTML spec behavior or get the HTML spec changed in some way or something. |
@bzbarsky which bit breaks the spec? Opening the queue on the load event? Yeah, I'm happy to ditch that idea and expect developers to open the queue via other means if they want it to work for initial |
@jakearchibald, would this apply only to Changing |
@wanderview yeah, I'm only talking about |
I've updated web-platform-tests/wpt#6343 will the new names. I guess the next step is to add the iframe edge cases. Is there any point adding |
Does this really require manual tests? I have a |
Chromium's test runner seems to allow |
@annevk, I'm trying to find a way during the Handle Fetch to see whether the navigate request is from the initial about:blank client. We want to set .resultingClientId to the initial client's id in this case. (Please read from #1091 (comment) for background.) I couldn't come up with any good way other than thinking about something similar to https://html.spec.whatwg.org/#initialise-the-document-object step 1:
But I don't think that's a proper way as it'd reference the request's client's browsing context from a task in the SW's event loop. Would that be okay? Adding a state to an environment settings object to tell it's tied to the initial about:blank Document seems to be an option. But I know we don't want to add one if that's avoidable. Do you have any idea? |
If you have a handle on an environment settings object you can get to the "corresponding" document, no? I do think we want to add state there about initial about:blank as currently the definition is a little lacking, but that still requires a lot of refactoring to happen first (which I'm working on, but it's taking a long time). |
Yes, but without a state on the environment settings object, I should still cross the process boundary to check if that document is the only entry in the client's browsing context's session history. Also, after reading whatwg/html#2774 (comment), I'm concerned about accessing such a state defined in the environment settings object from within parallel steps. We should be able to make sure that the state hasn't been mutated. When the navigate flow initiated from the initial about:blank client got to Handle Fetch, isn't there any possibility the browsing context's session history had changed? I believe the navigate algorithm has guards against multiple navigate attempts, but want to clarify we won't have a race condition. |
Yeah, if you're in parallel you can't access environment settings objects either. You'd need to propagate that state or message pass it across. (We really need "threads" rather than "in parallel" to describe these things better.) |
I think introducing the thread concept wouldn't solve this particular case where we want to reference the environment settings object's item concurrently from the main thread and the SW thread. (I absolutely agree to the idea of spec'ing the thread concept to solve the ambiguity around designating parallel execution context.)
I think if we add something like the environment settings object's initial client flag, we wouldn't have a problem accessing the flag from Handle Fetch as https://html.spec.whatwg.org/#navigate step 3, 4, and 6 don't allow two navigate instances to run concurrently. |
F2F:
Implementation priority is Action: Look at worklets – can a single client fetch create multiple clients. Pretty sure we decided they were subresources. |
#1245 will track the work decided in this thread. |
We had decided (and I forgot) to change the name of FetchEvent.targetClientId to .replacesClientId to clarify the meaning that this client is a to be replaced client: #1091 (comment). Related issue: #1245.
We had decided to change the name of FetchEvent.targetClientId to .replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the name of the request's target client id item to replaces client id. Related issue: w3c/ServiceWorker#1245. SW PR: w3c/ServiceWorker#1333.
We had decided to change the name of FetchEvent.targetClientId to .replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the reference to the request's target client id to request's replaces client id. Related issue: w3c/ServiceWorker#1245. SW PR: w3c/ServiceWorker#1333. Fetch PR: whatwg/fetch#774.
We had decided to change the name of FetchEvent's targetClientId to replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the name of request's target client id to replaces client id. See also: * w3c/ServiceWorker#1245 * w3c/ServiceWorker#1333 * whatwg/html#3788
We had decided to change the name of FetchEvent's targetClientId to replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the reference to the request's target client id to request's replaces client id. See also: * w3c/ServiceWorker#1245 * w3c/ServiceWorker#1333 * whatwg/fetch#774
We had decided (and I forgot) to change the name of FetchEvent.targetClientId to .replacesClientId to clarify the meaning that this client is a to-be-replaced client: #1091 (comment). Fetch PR: whatwg/fetch#774. HTML PR: whatwg/html#3788. Related issue: #1245.
We had decided to change the name of FetchEvent's targetClientId to replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the reference to the request's target client id to request's replaces client id. See also: * w3c/ServiceWorker#1245 * w3c/ServiceWorker#1333 * whatwg/fetch#774
We had decided to change the name of FetchEvent's targetClientId to replacesClientId to clarify the meaning that this client is a to be replaced client: w3c/ServiceWorker#1091 (comment). Accordingly, this changes the reference to the request's target client id to request's replaces client id. See also: * w3c/ServiceWorker#1245 * w3c/ServiceWorker#1333 * whatwg/fetch#774
Our current plan is to expose the environment creation URL as
Client.url
when the environment is marked execution ready. This is a bit confusing for the case where the environment has multiple documents; like when an initial about:blank is replaced with a loaded document. See:whatwg/html#2456
My read of the spec and my initial implementation so far expose the about:blank URL briefly as
Client.url
. In addition, the environment is marked execution ready twice. Once as about:blank and once as the final loaded URL. The second time its marked execution ready this basically amounts to updating the creation URL.Is this intended behavior for the clients API? I suspect perhaps the intent was not to mark the Client as non-reserved until after the final loaded document is execution ready.
The text was updated successfully, but these errors were encountered: