Skip to content
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

Where to put client / context #318

Closed
annevk opened this issue Jun 13, 2014 · 24 comments
Closed

Where to put client / context #318

annevk opened this issue Jun 13, 2014 · 24 comments

Comments

@annevk
Copy link
Member

annevk commented Jun 13, 2014

Should they be on FetchEvent or Request?

FetchEvent makes sense since as far as I can tell they only make sense in service workers.

Request makes sense since conceptually they are tightly coupled with the request (and actually part of underlying request).

(Somewhat related: @jakearchibald has proposed context for ease of setting headers, but that would have to be a distinct feature. Maybe usageContext, see #279.)

@jakearchibald
Copy link
Contributor

As far as I know, they're only useful on Request for prioritisation, but that gets messy when they're pulled out of a cache, and event.default is a better option to preserve prioritisation of the default request.

FetchEvent makes more sense for info that wouldn't go into the cache.

@annevk
Copy link
Member Author

annevk commented Jul 2, 2014

Well, @mikewest and I discussed setting headers and some other things based on the context (when you hit the network, thus after SWs). Which seems like something that would need to be preserved.

@jakearchibald
Copy link
Contributor

Ohh, which headers are those? If it's for things like "Accept", I'd rather they were set on request construction such as:

new Request('img.webp', {type: 'image'});

or

Request.image('img.webp', opts);

@annevk
Copy link
Member Author

annevk commented Jul 2, 2014

Well, fetch() has its own context. It would be for non-fetch() contexts. And yes, it might be Accept and such.

@slightlyoff
Copy link
Contributor

I'd like to have context on Request objects. It'd be good to have metadata about the requesting context (e.g., what sort of client is requesting it, prerender, iframe, etc.) but that can be on the fetch event. Want to lock this down SRTL.

See also #210

@annevk
Copy link
Member Author

annevk commented Jul 29, 2014

@slightlyoff prerender and iframe are contexts...

@jakearchibald
Copy link
Contributor

Request contains details of how communication with the server will be managed. Things like context client & respondWith are related to the event that created the request.

@annevk
Copy link
Member Author

annevk commented Jul 29, 2014

Well context can influence priority. Not sure what the future will bring. My main reason for putting these on Request would be that the underlying model also has them there.

@jakearchibald
Copy link
Contributor

context can influence priority, but would you want that priority to be retained if the request is stored in a cache and used again some time later?

@annevk
Copy link
Member Author

annevk commented Aug 5, 2014

How would the priority change? Having said that, there might be things you lose when you store it in the cache, but not sure if context is alone in that.

@KenjiBaheux
Copy link
Collaborator

Removing "impacts MVP" as client and context are out of scope for the MVP.

@dmurph
Copy link
Collaborator

dmurph commented Oct 1, 2014

Adding MVP, as it's blocking an MVP feature. See #497

@annevk
Copy link
Member Author

annevk commented Oct 3, 2014

#502 suggests putting this on Request makes more sense so it can be reused by fetch() going forward.

@sicking
Copy link

sicking commented Oct 3, 2014

There's a few things that the UA can do if we know how a response will be used. Some of which has been discussed here, and some which hasn't.

Network prioritization

First of all we can make better default network prioritization decisions based on this. Though this is a little bit easier if we know what element is causing a network request rather than just what type. This way we can do things like and prioritize scripts/stylesheets that are blocking over ones that are not, and prioritize visible images over ones that are "below the fold". The latter one is a dynamic property which might change as reflows and scrolling happens. Fortunately HTTP/2 supports adjusting priorities dynamically.

But I think this is something that we can track using internal state in the Request object most of the time. But associated a Request and an Element sounds useful to enable at some point. Though obviously this is hard to do inside a SW given that we can't expose Element references.

For things that originate from within the service worker this seems a bit harder since I suspect that often times the SW will be just fetching resources ahead of time and thus all requests should have a low default priority. Though once a partial Response is returned as a result to a "fetch" event, we can adjust priorities as needed.

Caching Pre-processed data

Simply having the information "this is likely going to be used as a script" would enable the platform to pre-process the resource in order to make using the resource faster. For example we could precompile JS, or convert an image into a format that can be sent directly to the GPU.

This preprocessed data can then transparently be cached along with the network response whenever the page stores the response in the Cache API.

Caching only pre-processed data

Even better, if we knew "this resource will only be used as a script" we could then drop the actual byte stream and only keep around the preprocessed data. This would mainly save us from having to write both the byte stream as well as the preprocessed data to disk.

If we made it possible to provide more than a hint, and instead provide a promise "this will only be used as an image" when the Request is created, then we could forward that information to the Response object. I.e. the Response could indicate "this may only be used as an image", and any attempts to use the Response for something else would result in an error.

@annevk
Copy link
Member Author

annevk commented Oct 4, 2014

If we allow context to be set, it would have to be as a guaranteed promise, as CSP decisions are made based on its value. The Response object would most likely have to be opaque in that case, though once drawn on <img> or used by <script> some such I suppose it could still be considered same-origin at that point (for reading image data, logging errors, and friends).

This does seem to suggest to me that we should tie context to Request. Not sure about client, but likely too? @jakearchibald?

@KenjiBaheux
Copy link
Collaborator

We managed to solve the MVP issue without exposing context (client was and still is out of scope).

@annevk
Copy link
Member Author

annevk commented Oct 13, 2014

whatwg/fetch@3440a0a puts context on the Request object. I don't allow it to be set yet though as that is way more complicated. Just exposing it.

@jakearchibald
Copy link
Contributor

Fair enough to context on Request (I have no strong opinion). If we move client onto Request we have to allow ServiceWorkerClient on Window too. I'm happy for it to stay on FetchEvent

@annevk
Copy link
Member Author

annevk commented Oct 13, 2014

I would expect that if we move client as well we would make it return null in document and worker environments (or a pointer to the global object, but that seems bad).

@KenjiBaheux KenjiBaheux modified the milestones: Version 2, Version 1 Oct 15, 2014
@KenjiBaheux
Copy link
Collaborator

None of these are in scope for v1

@jungkees
Copy link
Collaborator

I think the client for fetch event in service worker carries more information (a service worker client) than what fetch spec specifies as a client (a environment settings object). Agreed on having it on FetchEvent. Also agreed on using context defined in Request. Can we tag it decided?

@annevk
Copy link
Member Author

annevk commented Oct 22, 2014

@jungkees, @jakearchibald is merging the concepts of service worker client and environment settings object.

@annevk
Copy link
Member Author

annevk commented Oct 22, 2014

(Having said that, for now client would only be useful inside service workers. I'm not entirely clear on whether that will always be the case. Hard to predict.)

@annevk
Copy link
Member Author

annevk commented Dec 12, 2014

We resolved context. This is superseded by #575.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants