Collaborator

Apologies in advance for the length of this issue.

A few weeks ago I was discussing the topic of the upcoming "PlzNavigate" feature with @naskooskov, @n8schloss, and @bmaurer.

The TL;DR of PlzNavigate is that navigation actions in Chromium will not be handled as they currently are -- sending them through a Renderer process which then routes them to the Browser process for eventual dispatch by the network stack -- and will instead be immediately routed to the Browser-side network stack, improving time-to-navigation in the common (non-SW-controlled) case. This is beneficial in the PlzNavigate world which is much more aggressively multi-process oriented. Saving the time to create processes is a big win, particularly on Android which "features" particularly slow native process creation.

In Chromium (and one assumes similarly architected browsers), this means that PlzNavigate-style request optimisation runs afoul of Service Worker handling of these requests. This isn't particularly satisfying as the SW may indeed choose to make a request for the top-level resource from the network. Indeed, waiting to issue these requests on Service Worker startup is being reported by large sites as a regression in the 10s or even hundred+ millisecond range. This is notable on sites which do not handle fetches for top-level documents but only want to use SWs for caching.

What if we could enable PlzNavigate and remove the hit generated by SW startup?

The idea in the following proposal is to allow a style of declarative navigation request decoration for these "preflight" navigation requests, allowing the Service Worker to use (or discard) the response. If no decoration is added and the site's SW decides to handle the request directly (e.g. with a e.respondWith(fetch(e.request))), nothing should break. Similarly, it's a goal to avoid sending the results of the "preflight" request to the document without the Service Worker's involvement.

To accomplish this, the proposal we sketched out on the whiteboard was to allow the onfetch event that corresponds to the navigation to have access to the original (preflight) response. To enable a savvy server-side to repurpose this preflight navigation to, e.g., send up-to-date data in a different format than HTML (imagine JSON or similar), we'd also allow the Service Worker to register a header to pass along with the preflight'd navigation request. All together, the strawman looks roughly like:

self.onactivate = (e) => {
  // If unset, preflight requests are sent without special marking
  e.setPreflightHeader("X-Site-Specific-Header", "thinger");

  // ...
}

self.onfetch = (e) => {
  if (e.preflightResponse) {
    // This is a navigation fetch which has already been issued.
    // If the `preflightResponse` isn't used, then everything proceeds as
    // if it hadn't been sent in the first place.
  }
}

Obviously the names are bike-sheddable. The goal however isn't to be super declarative about deciding what "routes" are handled in which style. Instead, it's to allow the maximum of flexibility for cooperating servers and clients to eliminate SW startup latency.

Thoughts?

/cc @jakearchibald @wanderview @jungkees @mkruisselbrink

Member
annevk commented Jun 28, 2016

I'd like to know how specific this is to Chrome. From a high-level perspective it seems like a very weird hack.

NekR commented Jun 28, 2016

@annevk I guess we need feedback from all implementers here, i.e. include Edge team to this conversation. Does anyone know who is working on SW in Microsoft?

Collaborator
jakearchibald commented Jun 28, 2016 edited

Going to tackle this in two replies, one addressing the problem & another addressing the proposed solution - because I don't think they match up right now.

@slightlyoff

Back when we started work on SW there were calls for higher level APIs, manifests and such, because SW may present performance issues in some cases, and your response to that was roughly "We shouldn't try and solve performance problems until we know we have them, and what shape & size they are". This is still a good plan.

I want us to present solid data here that shows the size & shape of the problem, and I want other vendors to verify that SW startup is the bottleneck here, and not something specific to Chrome.

There are multiple ways to solve this problem, and the way to pick the best solution is for us all to have good visibility into the issue.

Collaborator
jakearchibald commented Jun 28, 2016 edited

Without concrete data on the problem, it's difficult to assess solutions. Service worker startup is not free - but we need to assess when that cost significantly detracts from the benefits. It could be:

I have no fetch event. I'm paying the startup cost for no reason.

We solved this in #718, but Chrome hasn't landed it yet. This is the case for at least one of the large sites reporting this regression.

I have a fetch event but it doesn't respondWith. The result is blocked on SW startup, despite the SW having no impact on the result.

We have an existing solution that we can apply here: passive listeners. Although we may need to disable reading the request body in this case.

I have a fetch event, it sometimes calls respondWith. If it does, it sometimes fetches event.request.

I think this is the case the proposed solution is aiming for, but I really want to hear more about sites that use this pattern.

self.onactivate = (e) => {
  // If unset, preflight requests are sent without special marking
  e.setPreflightHeader("X-Site-Specific-Header", "thinger");

  // ...
}

This suggests the preflight will happen for each and every request (or just navigation requests for some reason?), which feels like a huge change. If I serve a 3gb video from the cache, what happens to the preflight? Is the user going to end up downloading the 3gb again, or will the stream be aborted? Either way, as a developer, I feel like I've just lost a lot of control.

self.onfetch = (e) => {
  if (e.preflightResponse) {
    // This is a navigation fetch which has already been issued.
    // If the `preflightResponse` isn't used, then everything proceeds as
    // if it hadn't been sent in the first place.
  }
}

If the fetch event is blocked on a preflight response, you've killed service worker as a means for creating offline-first experiences. One way around this is to make e.preflightResponse undefined if the fetch event can fire before we have the preflight response. This becomes unpredictable and another loss of control. But furthermore:

self.onfetch = event => {
  event.respondWith(
    event.preflightResponse || fetch(event.request)
  )
}

If preflightResponse is fired because the fetch event won the race, we execute fetch(event.request), making the request a second time. You could work around this by making preflightResponse a promise, but things are getting messy. You're still going to get the double request for existing service workers.

Additionally, using the response is still blocked on the SW, as the fetch event is always consulted. Is that ok? Again I want more concrete data.

The preflight is always going to happen, but I have to opt into using it. It seems really weird that the preflight isn't an opt-in, there isn't an opt-out, yet using the preflight is opt-in.

The goal however isn't to be super declarative about deciding what "routes" are handled in which style. Instead, it's to allow the maximum of flexibility for cooperating servers and clients to eliminate SW startup latency.

I see you put that bit in because I'm in favour of a route-based solution. 😄 But I don't see how routes are a worse solution, and your proposal doesn't feel awfully flexible, easy to reason about, or even address (what seems to be) the route of the problem.

A route-based solution should allow a developer to declare "for requests that look like this, do this", in a way that can at the very least started while the service worker boots up. This wins over the proposed preflight solution because:

  • It needn't be restricted to navigation requests
  • It's opt-in, removing the need for redundant requests & bandwidth usage
  • It can reach completion prior to service worker boot up
  • Whether the service worker is needed at all for a particular request can be determined in advance

But again, I think we need clear data before we do something like this. We shouldn't rubber-stamp a scenario-solving solution, especially while we're so vague on the scenario.

Collaborator
jakearchibald commented Jun 28, 2016 edited

A route-based sketch:

self.addEventListener('install', event => {
  event.waitUntil(
    // populate caches
  );

  event.declareRoute("/", new FallbackSources(
    new CachesSource(),
    new FetchSource(),
    new FetchEventSource()
  ));
});

This API is up for debate wayyyyyy beyond the naming, and I still think we need better data before we'd proceed with this. But here's how the above sketch could work:

installEvent.declareRoute(requestDescriptor, action, opts)

  • requestDescriptor - the requests this route this should handle. I'm doing a lot of hand-waving with this param right now, but it should be able to describe specific paths, path prefixes, path suffixes, origin (or any origin), method, maybe more. This could also be used in an API like request.matches(descriptor). Could also include ways to modify the request, such as appendHeaders.
  • action - where to get the response from.
  • opts.fireFetchEvent = "no" - ("yes"/"no"/"passive") - should the fetch event trigger after the action has resolved? The response, if any, will be available in fetchEvent.routeResponse. This allows JS to handle the response either directly or passively.

new FallbackSources(...sources)

Attempt each source in series until an acceptable response is found. Alternatives could be new AnySource(...sources) which races sources until one provides an acceptable response.

new CachesSource(opts)

Look for a match in the cache. opts can handle the request to match on, which would be the current request by default, but could be set to something static (for getting a fallback page from the cache). Other options would cover specific caches, and things like ignoreSearch.

new FetchSource(opts)

Try to get a response from the network. opts could include the request to fetch, which would be the current request by default. opts could also include things like timeouts and such.

new FetchEventSource()

Defer to the fetch event.

So the example above:

  1. Looks for a response in the cache
  2. Else attempts to request from the network
  3. Else fires the fetch event a usual

The whole thing can complete without the service worker starting up unless we hit step 3, and I'm pretty confident it could be polyfilled.

Collaborator

Can we tie this "preflight request" to the concept of "preload"? We already have a rel=preload concept in the specs I believe. The PlzNavigate effort to preload the site before the user finishes typing could be seen as the same mechanism. This would give us a consistent mechanism to hang this optimization on without directly tying it to browser-specific mechanisms.

This does some like it could be useful, but its complex enough that it would probably only be used by a minority of sites. Maybe its worth it if those sites are extremely high traffic?

Collaborator

@jakearchibald So, the following would allow service worker startup to happen simultaneous with the network request?

  event.declareRoute("/", new FetchSource(), { fireFetchEvent: 'yes' });
Collaborator

Yep!

Collaborator

But the load would be delayed by bad network connections. I guess maybe your AnySource() solution would allow you to handle this, but its not clear to me exactly how the pre-flight proposal would like to handle flaky network, either.

Collaborator

Yeah, AnySource would handle this, as would a FallbackSource that favoured the cache

Collaborator

I'd like to know how specific this is to Chrome. From a high-level perspective it seems like a very weird hack.

I don't think this is particularly chrome specific. Starting a worker thread, parsing js, interpreting, running the jit if necessary, etc are all overhead to letting the SW handle the fetch event. Coming up with a mechanism to allow this overhead to be performed in parallel with an initial network request would benefit all browsers.

I guess ultimately its a tradeoff between complexity and those overhead costs. Firefox might be slightly faster to start a service worker right now (I don't really know though), but we will likely end up with overhead similar to chrome once we fix our infrastructure to handle multiple content processes.

If we can come up with something that is not too complex, then it seems like a useful addition.

I do kind of agree with Jake, though, that maybe we should wait to implement these kinds of optimizations. Its still early days for people figuring out how they want to use service workers. If we implement this now we might miss out on a useful pattern people find they need or might make it more general purpose (complex) than is needed.

Collaborator

With the routing proposal...

self.addEventListener('install', event => {
  event.waitUntil(
    // populate caches
  );
});

self.addEventListener('fetch', event => {
  event.respondWith(
    caches.match(event.request).then(r => r || fetch(event.request))
  );
});

...could be replaced with....

self.addEventListener('install', event => {
  event.waitUntil(
    // populate caches
  );

  event.declareRoute("/", new FallbackSources(
    new CachesSource(),
    new FetchSource()
  ));
});

...and the SW wouldn't need to boot up for any fetch events.

My blog currently composes a streamed response. If the cost of service worker bootup was greater than its benefit, I could do:

self.addEventListener('install', event => {
  event.waitUntil(
    // populate caches
  );

  event.declareRoute({mode: "navigation"}, new AnySource(
    new FetchEventSource(),
    new FetchSource()
  ));
});

...allowing the browser to race the network and the fetch event for navigations.

Collaborator

A few things seems missing from the conversation.

Many sites we've partnered with have deployed nearly-no-op SWs to do tracking and monitoring. This defeats the no-onfetch optimisation. Sites like FB and Google Inbox have goals that they go to extraordinary lengths to meet but have difficulty achieving thanks to the complexity of their stacks and the # of folks adding features:

  1. Get UI to users as fast as possible
  2. Get fresh data into that UI, even though the UI is script-mediated (and, if not possible, show stale data)
  3. Monitor the performance of the overall experience end-to-end

Getting UI to users tends to involve "booting up" large amounts of JS, CSS, and associated context. This is true for FB, Inbox (in some modes), Docs (in some cases), and many others. In a no-SW world, the best strategy is to inline fresh content into the response document where the booted code can consume it and render it.

With SWs in the mix or server-rendering of snapshots (Inbox & Docs, but only in certain modes), things get more complicated. A service designer of one of these systems wants to be able to:

  • Get UI to the user reliably; that is to say, if there's a "header" or "shell" that can be served from cache with low variance, that's a good thing to do, even if the total latency may be slightly higher due to SW wakeup costs
  • Get requests for fresh data onto the network ASAP. Assuming you respond with a "shell" for example.com/ (which we might imagine coming from a declaratively routed cache as @jakearchibald suggests, but not necessarily so), it'd be helpful to get a request to the server for new data started as soon in the lifetime as possible.
  • Ensure that everything works sensibly when there's no SW in play

To handle the second of those, the proposal I've provided lets the server know a few things:

  1. A Service Worker is in play and therefore the app shell or "header" resources are already booting up
  2. That, as a result, it's safe to respond with just data, not any cruft or dressing associated with a full-page render. In sever-language, it allows the server to send only a "partial"
  3. That the response won't be dropped on the floor because the SW will be in play to handle (and redirect) it

It does this with minimal new infrastructure, enabling both streaming for "static" sites that want to handle HTML partials and not data, as well as allowing sites that deal in data (vs. inline HTML) to do so naturally.

Would love to hear from @bmaurer on the alternatives proposed here as well.

That's a pretty good summary for what we want to do. In the shorter term we're going to cache parts of the page markup and then make a request for the rest, in the much longer run we're going to cache most of the markup but will still need to make a request for page data. In both cases we're going to want to make a request to our server to get data as soon as we can and at the same time start getting code to the client window as soon as we can.

Service worker startup isn't free: in Chrome, our in-the-field logging is telling us that service worker startup adds about 200ms to page load time on desktop. It might be slightly better or worse in other browsers, but it's still going to be a significant cost to startup a process and initialize the service worker code. Our site is pretty optimized to deliver resources quickly and in the right order, so our concern is that this extra time might mean that service workers are a regression in some cases.

What we don't want to do however is have a race between network and cache without actually opening from the service worker. If we're going to build a version of the site that can be fully loaded from a service worker we will most likely still need to make a request each time the page loads to get the newest content, so simply loading the site from cache won't always be a win if we have to wait to make a request afterwards anyway.

The optimization that we need allows us to get the initial request out before the service worker starts up but still allows the fetch event to get and process a request if one was sent out.

Having the option to set a header or other fetch options here is important because even in the short term we aren't going to return the same kind of markup when a service worker is there vs when it isn't. Also, making this field nullable in the fetch event seems fine to me. It can be called possiblePreflightRequest or something like that and then you can wait for it just like a normal request if you want to use it.

Collaborator

@slightlyoff can you answer my questions about your proposal & show why it's better than declarative routing?

bmaurer commented Jul 3, 2016

To add a bit more color to Nate's comments --

First, I think it's worth considering the position a site is in as it first adopts service worker. SW is a huge change to how people write websites. Any complex site is going to take a while to get it's feet wet. One thing that Alex mentions is that some people might use SW to do things that don't involve intercepting the root document the user navigates to (eg, caching static resources). A site might gradually use sw for only parts of the root document. Maybe first they just cache their page header. To the extent that using SW causes a perf hit (as Nate mentions there's a startup cost at least in chrome) that makes it hard for a site to dip their feet into using SW. Even if SW eventually enables great wins sites tend to develop incrementally and want each increment to be better on metrics. So in my mind an ideal solution here would be that a service worker that does nothing, has no cost

Now let's think about a more advanced SW deployment, taking a site like FB as an example. FB would have two goals:

  1. Display the chrome of the app as fast as possible, start warming up the JIT on core JS frameworks, and render existing data in cache
  2. Get up to date data from the server as quickly as possible (eg an updated newsfeed)

I think many apps share this set of dual goals -- for example, an email application would want to show existing emails quickly but fetch updated emails. In order to accomplish (2) one needs to communicate to the site's server "this user is opening the app". This message is generally fairly light weight -- at a minimum it needs to identify the user. But it may also need to communicate a small amount of information about the state of the cache. For example, you might need to communicate the last cached newsfeed story. As a slightly more complicated case one could imagine a large class of apps (uber, postmates, etc) that want to communicate the location as quickly as possible.

On our mobile apps we go to great lengths to make this notification happen as quickly as possible. Early in the startup process we send out a UDP packet that contains an encrypted user identifier. (https://code.facebook.com/posts/1675399786008080/optimizing-facebook-for-ios-start-time/)

Jake's route based proposal makes sense to me here. It seems like a generalization of Alex's preflight request. I think to make Jake's proposal work you'd need a few things:

  1. The cost of having a route that uses the cache and then fetch should be nearly identical to a site which doesn't use SW at all.
  2. A sw should be able to have some state that is sent with a declaratively specified fetch. Eg, last cached feed story time.
  3. It might be nice if the declarative post can be a POST request that is incomplete. That way once the SW starts up it can add more data to the post but the server can start processing
Collaborator
slightlyoff commented Jul 3, 2016 edited

I'll try (although I though I had by clarifying the requirements):

  • First, the route based proposal doesn't appear to allow the server to decide how to overload the first response to serve only data (not UI + data).
  • Next, blocking on the SW is usually OK because now we're in a race. SW startup can be in the 10s to hundreds of ms in the worst cases, but isn't usually worse than that. You burn as much on DNS. As a result, it won't generally be a net loss to wait on the SW for mediating this interaction, particularly if you can shave off overhead of getting a new request out the door.
  • Lastly, the routing based proposal either has to defeat PlzNavigate entirely (if you only specify a FetchSource()) or, as I understand the example given, still wait on the renderer process for handling the CacheSource() entries (it's unclear we'd want the mapping logic for them in the browser process).

I think we should probably do declarative routing. We should probably go with something not dissimilar from your proposal, @jakearchibald, but I don't think it solves the issues @n8schloss or @bmaurer are raising nor does it really help our PlzNavigate integration. It's also quite a bit larger.

As a final thought, I think these can layer together quite nicely. Having the preflight come back as a readable stream that can be directed to the document (in cases where you might use a FetchSource() or a CacheSource()) doesn't seem to be at odds with the declarative proposal. It does mean you'd need to wake the SW to handle it, but that could also be turned into a declarative option I suppose. It's a new model and quite a lot of new API, though, so I have a preference for a minimal intervention.

Member
annevk commented Jul 4, 2016

I think that if we want to handle navigation differently (so sites can only do service workers for subresources or some such) it needs to be an opt-in primitive of sorts. We should not take total network control away from the site due to Chrome's PlzNavigate project.

Member
annevk commented Jul 4, 2016

It's also now more clear than ever that those pushing to make fetch opt-in were correct. 😕

Collaborator
jakearchibald commented Jul 4, 2016 edited

Trying to get a concrete picture of the problem here. Someone shout up if this is wrong.

The loading strategy is:

  • Load UI sans-content from the cache
  • JS loads content 'include' from network

Here's some sample code for this: https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/b43199815f451cadde7280aa9b8dea1f84a88387

And the problem is:

  • On initial navigation the service worker bootup time (200ms?) delays cached shell render and delays subsequent network request for content

It's still unclear to me how that delay compares to the benefits of rendering without the network. Even if it's only the shell that's rendered without the network, getting the shell & JS from the cache should come with a benefit vs the network.

But yes, as a site adopts SW, there may be particular navigation routes that simply defer to default browser behaviour.

Collaborator
jakearchibald commented Jul 4, 2016 edited

@n8schloss

What we don't want to do however is have a race between network and cache without actually opening from the service worker.

But what about a race between the fetch event and a default network response?

Collaborator

@bmaurer

A site might gradually use sw for only parts of the root document

This is why we need a routing approach. Making all-or-nothing behaviour changes doesn't fit into this gradual model.

So in my mind an ideal solution here would be that a service worker that does nothing, has no cost

Like I said in #920 (comment), the spec already caters for this, and impelementation is in progress.

The cost of having a route that uses the cache and then fetch should be nearly identical to a site which doesn't use SW at all.

Seems fair. It certainly shouldn't be slower than appcache.

A sw should be able to have some state that is sent with a declaratively specified fetch. Eg, last cached feed story time.

Do you need something beyond last-modified or ETag?

It might be nice if the declarative post can be a POST request that is incomplete. That way once the SW starts up it can add more data to the post but the server can start processing

This seems like a separate thing (it's the first time posting has come up). What problem is this solving? As in, when would you want to POST along with an initial navigation? Related: there is a rough plan for background posting/downloads.

Collaborator
jakearchibald commented Jul 4, 2016 edited

@slightlyoff

the route based proposal doesn't appear to allow the server to decide how to overload the first response

These could easily be options to FetchSource.

event.declareRoute({mode: 'navigation'}, new FetchSource({
  applyHeaders: {
    'X-Site-Specific-Header': "thinger"
  }
}), {fireFetchEvent: 'yes'});

I think that's equivalent to your proposal (I don't know for sure as you haven't described how your proposal works),

blocking on the SW is usually OK because now we're in a race

Are we sure that's the case? Based on the code example above, if we had routing, the following could happen while the SW was booting up:

  • Shell fetched from cache, giving a first render
  • JS fetched from cache
  • JS parsed & executed
  • Fetch called for data

If that takes longer than 200ms, then we've won. Not only that, but we rendered while that was happening.

Contrast this to your preflight proposal, where:

  • Render is blocked by SW startup, and potentially the network request too (you haven't explained how it works)
  • The fetch call is a separate request, so it won't have the preflight there, so I don't see how the problem is solved
  • The fetch call may also be blocked on the network, even if it doesn't need to be.

Lastly, the routing based proposal either has to defeat PlzNavigate entirely (if you only specify a FetchSource()) or, as I understand the example given, still wait on the renderer process for handling the CacheSource() entries (it's unclear we'd want the mapping logic for them in the browser process).

If you're racing FetchSource with other sources it won't be a problem. That's basically what your proposal is doing (I think), but it only does that.

Having the preflight come back as a readable stream that can be directed to the document (in cases where you might use a FetchSource() or a CacheSource()) doesn't seem to be at odds with the declarative proposal. It does mean you'd need to wake the SW to handle it, but that could also be turned into a declarative option I suppose.

We're already talking about hacking around your proposal. It's too high level, and too all-encompasing. Also, you still haven't answered my questions on how it works.

My questions are in #920 (comment). The outstanding questions are:

  • Is the preflight opt-in? If so, by what mechanism?
  • Does the preflight happen for every request, or just navigations?
  • What type is e.preflightResponse is it a Response?
  • What happens if the fetch event can fire before e.preflightResponse is ready?
  • If I do not use the preflight, what happens to it? Eg what if it's a 3gb video?
  • If my SW does fetch(event.request), is that an additional request? That will be the case for most existing SWs
  • If my SW does fetch(event.request.url), is that an additional request?
Collaborator

@annevk

It's also now more clear than ever that those pushing to make fetch opt-in were correct.

It's already opt-in. If you don't want it, don't add a fetch listener.

Member
annevk commented Jul 4, 2016

True, we made it opt-in ex post facto through a hack. It still looks weird compared to the other APIs exposed by service workers and makes it harder to add registration options specific to fetch, such as some kind of filtering mechanism.

Collaborator

Here's the current model for an "app shell" site that uses JS to populate content https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/b43199815f451cadde7280aa9b8dea1f84a88387

Here's what it would look like with the routing proposal https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/c8bdf11ada8a99bb506f369d7d040051ad4dda16 - this doesn't need the SW to boot up at all.

@slightlyoff, can you show how your solution would work in this case? (in addition to clarifying the behaviour of your proposal)

bmaurer commented Jul 4, 2016

The loading strategy is:

And the problem is:

  • On initial navigation the service worker bootup time (200ms?) delays cached shell render and delays subsequent network request for content

It's still unclear to me how that delay compares to the benefits of rendering without the network. Even if it's only the shell that's rendered without the network, getting the shell & JS from the cache should come with a benefit vs the network.

The problem is that even though we win on getting the shell displayed faster, we lose in terms of the amount of time that it takes for the server to start processing our request and to send more data. Keep in mind that today Facebook uses chunked encoding. Within 10 ms of a request touching our server we start sending back instructions on how to load the app shell. So for us the loading of the app shell is already happening in parallel with our server work. For many requests the critical path in displaying new content is the speed of computing that ranking. Even if SW is a win in being able to display the app shell more quickly that may not help us meet the business objective of displaying up to date newsfeed stories more quickly.

What we're looking for here is the best of both worlds -- being able to display the app shell without a network connection without increasing the delay between when a user clicks facebook.com and when our servers are able to start giving them new content.

Collaborator
jakearchibald commented Jul 4, 2016 edited

@bmaurer

Thanks! Is there any server rendering of dynamic content going on here, or is content added to the page using JS? If it's the former, a streaming solution seems a better fit. If it's the latter, could <link rel="preload"> (in combination with the routing) start warming up the server as the shell is booting up?

bmaurer commented Jul 4, 2016

This would be content to be added via JS. The problem is that rel=preload is going to get loaded way too late. In chrome at least that requires the renderer process to be started. Also, this doesn't give the SW a great chance to save state. Your idea of using etags is interesting but I think it might not capture the kinds of data one would want to send. For example, one may wish to send information about what JS/CSS files are in the cache so the server can render markup for an appropriate version of the site.

Ideally what I'd want here is something like "facebook.com/ has a route that will load a cached version of the app shell. In addition, it will send a request to facebook.com/newdata with the header CurState:XYZ where XYZ can be managed by the service worker as its cached state changes". The request to /newdata should have as little delay as possible.

Collaborator

@bmaurer hmm, so there's a problem with both my & @slightlyoff's proposals for this use-case. The preloaded response that ends up in the navigation's fetch event, is actually intended for a future subresource fetch.

Both designs put you in a situation where you'd have to pass the preloaded response into the global scope and hope it's still there when you need it. This is really hacky, and would break if a browser ever spun up multiple instances of the SW for parallelism (there's no plan to do this, but it'd be nice if we could at some point).

It feels like <link rel="preload"> has the right behaviour aside from happening too late. So the solution could be routing + a list of resources to preload, which would be satisfied by their own routes.

Something like https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/cb8328fee735414ced91ff38617b48f73cfb0a1f?

Collaborator

Something like https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/cb8328fee735414ced91ff38617b48f73cfb0a1f?

I think this is missing the current state token, though, right? The /page-data request needs to have some piece of state so the server can generate the appropriate update response.

In the above comments it seems they wanted to use a POST, although I guess a cookie could be used with your proposal here?

Collaborator

I captured @slightlyoff in the pub and got him to answer the questions on his proposal. Here they are (from memory):

Is the preflight opt-in? If so, by what mechanism?

Maybe/probably. No mechanism yet. Maybe it should only be for certain urls.

Does the preflight happen for every request, or just navigations?

Just navigations.

What type is e.preflightResponse is it a Response?
What happens if the fetch event can fire before e.preflightResponse is ready?

e.preflightResponse should be a promise, therefore the fetch event isn't delayed.

If I do not use the preflight, what happens to it? Eg what if it's a 3gb video?

Unsure.

If my SW does fetch(event.request), is that an additional request? That will be the case for most existing SWs
If my SW does fetch(event.request.url), is that an additional request?

It will make an extra request.

Collaborator

@wanderview

I think this is missing the current state token, though, right?

Oh good point. Updated code so the preload includes credentials. https://gist.github.com/jakearchibald/4927b34bb14c3681a3c1c1ce6ede5b09/dc1b9eccb825287cda46a8defed1254d660e2675

In the above comments it seems they wanted to use a POST, although I guess a cookie could be used with your proposal here?

Yeah, I'm not sure what the POST thing's for.

Collaborator

Yeah, I'm not sure what the POST thing's for.

Well, I think @bmaurer said they wanted the state token to be managed by the service worker directly. Right now there is no way to set a cookie in a service worker, although I seem to recall some API was under discussion there.

Collaborator

It seemed to me that facebook wanted to start getting user content prepared before SW boots up. The credentialed request in my example allows that. If the state token is managed by the SW, then you're back into a position where you have to wait for SW to boot up, which is no different than today.

Collaborator

Well @bmaurer can best clarify what he meant by:

the header CurState:XYZ where XYZ can be managed by the service worker as its cached state changes

But I read that as the last time the service worker changed something it would store the new XYZ header in some API that would then go out with the next preload. So it doesn't have to block the preload, but its still managed by the service worker.

Collaborator

I guess if the routes are mutable, then the SW could update the routes with the new header info.

Collaborator

Sorry for the slow response, have been on holiday. Thanks @jakearchibald for capturing (accurately) our pub conversation.

My largest concern about the declarative proposal here is that it's a high-level, open-ended design space which is something we've decided in the past not to attack without lots of evidence of need. At some level, I think these proposals are compatible. You can imagine the high-level thing being implemented in terms of the low-level sketch I outlined without much difficulty.

While I accept the critique that this may be scenario solving to some degree, we've had multiple customers ask for something that's effectively this and we know that multi-process browsers will be architecturally constrained in ways that make the PlzNavigate decision common.

The alternative that preserves our low-level bias in design is to simply block all navigations that have SWs on them on renderer startup and defeat PlzNavigate (and equivalents). I'm not sure this is tenable, so I'd like to see something small that's low-level in this area while we design the high-level route system.

Member
annevk commented Jul 19, 2016

I think the main problem with your sketch is that it's not opt-in and uses confusing terminology. I think by default a service worker should remain in control of all networking activity. It can relinquish some of that control back to the browser, but that should be opt-in. The terminology should make it clearer that this affects navigation only (only top-level presumably?).

Even then, I'd still like to hear from Apple, Microsoft, and Mozilla's e10s team what their thoughts are on this (and also Servo). Would love to have some insight as to how much this relates to Chrome architecture decisions vs overall browser architecture.

Collaborator
jakearchibald commented Jul 19, 2016 edited

@slightlyoff

My largest concern about the declarative proposal here is that it's a high-level, open-ended design space which is something we've decided in the past not to attack without lots of evidence of need

Both proposals are declarative. With your proposal you're declaring that an additional request should happen along with navigations, and it should have particular headers.

While I accept the critique that this may be scenario solving to some degree

It's trying to scenario-solve, but it's missing the mark. You're creating a preflight for the navigation request, but you're already hacking it to return different data.

I agree with @annevk that I'd like to hear more from other vendors on this, but if we're going to continue to throw around ideas we can't forget about the problems actually being presented:

The way things are today:

Without SW:

  1. Navigation request made - Facebook may return a generic response, but can start preparing user-specific parts
  2. Wait for renderer & response
  3. Page starts fetching user-specific bits

With SW:

  1. Wait for renderer & service worker
  2. Fire fetch event for navigation, return cached response, but also ping Facebook so it can start preparing user-specific parts
  3. Page starts fetching user-specific bits

If I'm reading @bmaurer correctly (correct me if the above lists are wrong), the problems are:

  • SW startup adds significant delay to a cached response
  • SW startup (and therefore renderer startup) adds significant delay to the Facebook ping

I don't think we should try and solve these with one function call. There's two parts there, avoiding SW startup for handling responses, and providing some kind of declarative preload (maybe multiple, maybe not to the same URL).

Speaking as a Mozilla SW dev focused on @bmaurer's use case (and with less wisdom than @wanderview), what if we hang a "session priming request" off the ServiceWorker registration?

General operation:

  • The ServiceWorker is able to set and update a small request off its registration that it can freely update with best-effort persistence but no transaction semantics or durability guarantees.
  • When a navigate request is triggered it necessarily gets run by your ServiceWorker registry to determine whether to intercept or not. If it matches, meaning it will be controlled by the ServiceWorker:
    • If there is no suppression timer, the session priming request is triggered as a "no-store" request, helping establish a network connection and delivering the needed cookie-ish identifier to the server so it can start doing database loads and whatever processing is needed in parallel with the browser's SW load. This starts a session clock suppression timer so every navigation request doesn't generate wasteful spam. Timer interval can be specified by the SW to best match the server backend with some sanity clamps by the browser, and/or dependent on the lifetime of the HTTP/2 connection.
    • The ServiceWorker is spun up like usual.
  • If the server has data it knows the ServiceWorker will want, it uses HTTP/2 Server Push to push the data into the network cache. The SW can fish the data out of there and it's the cache's problem to unify races between the SW requesting something and the server pushing it. The SW does not need to be aware of special types of requests like preflight requests.
Collaborator

Again I think this is scenario-solving to the extreme. I'd rather look at solutions that solve this issue for Facebook, but maybe some other sites too.

We already have <link rel="preload"> for preloading and we should lean on its semantics as much as possible. The missing part is a way to assign preloads to a navigation request before spinning up the service worker.

<link rel="preload"> already needs to define the life of its cache & how requests are matched to it, so I don't see why we should reinvent that.

The preloaded URL may be the same across every navigation on Facebook's origin, but I don't think that will be generally true.

Collaborator
jakearchibald commented Jul 20, 2016 edited

Here's a little menu of features. To those building sites: of the following, which is the minimum that solves the problems you have? Which features are more than you'll ever need?

  1. When the user navigates to my site I want to ping a URL with some sort of credential. I'm happy for the actual navigation request to go through the service worker and therefore be delayed by it starting up, but the ping should not depend on SW startup.
    1. I don't care about the response of the ping
    2. I want to handle the response of the ping, but I don't intend to use it in response to a fetch event within the next few seconds
    3. I want to handle the response of the ping, and I'm likely to use it in response to a fetch event in the next few seconds
  2. When the user navigates to my site, I want a network request for the navigation to race SW startup & response. If the SW manages to win, at least the network request served as a ping.
  3. When the user navigates to my site I want to perform a series of network preloads while the SW is starting up
    1. I want those preloads to bypass the service worker completely
    2. I want the request to bypass the SW, but I want the SW to process the response
    3. I want one of the above depending on the URL
  4. For particular requests (depending on URL, type, & headers), I want to declare the behaviour (serve from cache, serve from network) up front so the browser can handle particular requests/responses without starting the SW

Additionally, should 1 & 2 happen for all navigations, or just initial launches?

+@bmaurer @n8schloss

asutherland commented Jul 20, 2016 edited

Declarative routing is definitely more broadly useful.

From an implementation perspective, if we're talking about minimizing latency so that a site can know ASAP when a session is starting for its app shell (and to help the site avoid going crazy with push notifications and background syncs to mitigate the latency and avoid Flash Of Stale Content on startup)[1], then from Firefox's perspective AIUI, we want to hang the info about the request off the registration.

This is because we currently always have the registration around and can probably keep it that way at least on an a top/recent basis for small requests. Anything more than that currently involves us opening per-origin data which means doing a non-trivial amount of I/O that is either explicitly or implicitly serialized behind other I/O. We ask our quota manager to open the origin's directory, we open API specific SQLite databases for each origin, etc.

If we had declarative routing, we could probably just extract this information up out of the routes as an optimization. And I do believe it could respond faster in many other more generic cases than we could spin up a service worker and load its routing polyfill; just not as fast as a specialized thing hung off the registration. My main concern about the declarative routing is that it seems like a huge chunk of API surface that it's premature to standardize right now, and unrealistic to expect Firefox at least to get to in the short/medium-term based on our focus on non-polyfill-able APIs and our multiprocess priorities.

I think we could get to a more targeted mechanism hung off the registration sooner, but it's definitely possible as what amounts to a short-term hack it's not worth it and we should wait for declarative routing.

Re: <link rel="preload">, I'm not sure how much of its semantics are applicable to the app shell session priming startup case. It's explicitly about having an existing document context and that seems deeply baked in, but maybe I'm looking at the wrong thing? (http://w3c.github.io/preload/). Otherwise, I do agree it makes sense to try and leverage existing semantics specified by existing specs.

1: And I do think this is a very important use-case. I previously worked on Firefox OS apps, primarily email. Despite our offline/client-side bias, the name of the game was always minimizing the time to useful content. Since anything the server does is inherently parallel, it's a major win to notify the server before blocking on local device I/O which can exhibit variability from contention.

Collaborator

@asutherland it seems to me that all these things (declarative routing or a preflight) would be hung off a service worker rather than the registration. As in if V1 sets these things, but V2 doesn't, they're gone. That's closest to how the fetch event works today.

I think we could get to a more targeted mechanism hung off the registration sooner, but it's definitely possible as what amounts to a short-term hack it's not worth it and we should wait for declarative routing

That's what I'm aiming to discover through my previous comment. I'm not against a focused API, but I don't want to end up creating a doThatThingForFacebook() API that means we later need to ship a subtly different doThatThingForTwitter() API.

So to respond to @jakearchibald's comment above.

When the user navigates to my site I want to ping a URL with some sort of credential. I'm happy for the actual navigation request to go through the service worker and therefore be delayed by it starting up, but the ping should not depend on SW startup.

I'm not sure what you mean by ping, do you mean hit a url and be able to access it's response from the SW or something else?

In our use case and I suspect many others we're going to be caching most of the page shell and responses from our servers to service workers will return structured data. For Facebook if we measure the time that it takes to show newsfeed a lot of this time is blocked by fetching the data on our backend, and if we look at how long it takes to start service workers then the delay introduced by having them on the critical path introduces an overall regression. This kind of API is the best solution for this problem imo, the service worker stays in charge and still can construct responses, but hitting the backend isn't blocked on the service worker starting. I suspect that for most sites this will be a common use case, you'll want to cache most of your site but you still want to have content as fresh as possiable as quickly as possiable.

I think it's also okay for preflightRequest to be optionally there or not, this way if the SW is indeed started and it wants to add extra data to headers or make a request to a different endpoint all together it can. But if the SW is not started it only get's to add headers (and maybe change some other fetch options).

When the user navigates to my site, I want a network request for the navigation to race SW startup & response. If the SW manages to win, at least the network request served as a ping.

Nope, we ultimately want to reply with just the newsfeed content as structured data. We want the SW to be able to get this and then turn it into the webpage. We always want the service worker on the critical path in this case.

When the user navigates to my site I want to perform a series of network preloads while the SW is starting up

So right now we'd just want to do one preload before the sw starts and then be able to access its response in the fetch event.

For particular requests (depending on URL, type, & headers), I want to declare the behaviour (serve from cache, serve from network) up front so the browser can handle particular requests/responses without starting the SW

Nope, at least right now we're never going to respond just from cache and will always have a hybrid kind of approach where the service worker constructs a response from both cache and network.

I could see that others might find this useful though and I bet that down the line this would be useful for us as well, but this kind of declarative routing could also totally work with preflight request and it also doesn't solve the same issue that preflight request solves.

Collaborator

@n8schloss given you use chunked-encoding currently, how important is a streaming render to you (in terms of the streaming HTML parser)?

If you're downloading content client side then displaying it, you generally have to download it all before displaying any of it. Is this a problem?

Collaborator

I had a chat with @n8schloss & co and have a much better handle on the problem.

Their fetch event does something equivalent to this:

  1. Request page header from cache
  2. Request data from server
  3. Create readable stream & respond with it
  4. Pipe in the header
  5. Pipe in the data once it arrives

All my assumptions were based on the simpler shell model where the shell loads then it sets about getting data, which isn't the case here (also yay streams!).

My idea around reusing <link rel="preload"> semantics but allowing the request to start before the document does not work, since the preload cache is per document and this request is being used entirely in the service worker.

Making the preload available along with the navigation request works here, since it's being piped into the navigation response. However, this doesn't really work with the simpler shell model, where you're left with a response that you don't need yet. Best you can do is throw it into the global scope and hope for the best.

They also want to be able to set headers on the preload request outside of the SW lifecycle (such as "last post received timestamp", so setting it via the activate/install event is not the solution. Cookies aren't the prefered way of doing this, since there's no cookies API in SW, and having that data hit the server for other requests is undesirable.

Collaborator

FWIW, there's another very important web site that uses a similar serving pattern and would benefit from this - my blog.

jakearchibald added this to the Version 2 milestone Jul 25, 2016
delapuente commented Jul 26, 2016 edited

Aside from Facebook problem. Although @jakearchibald declarative syntax is tempting, I think it is too soon for this. @slightlyoff preflight solution seems to me a very valid options but I really think it should be opt-in so, during registration, a service worker could include a preflight list (networkRace) such as:

navigator.serviceWorker.register('sw.js', { scope: '/', networkRace: ['/ping', '/shell-assets'] });

List items would act as prefixes so any request to /shell-assets/* would be raced with the service worker. About the problem with the 3 GiB video, you could cancel the response in progress (I'm not talking about cancellable promises) by modifying the API:

self.onfetch = (e) => {
  if (isHeavyAsset(e.request) && e.networkResponse.inProgress) {
    e.networkResponse.abort();
    e.respondWith(handle(e.request));
  }
}

If for some race condition, you end sending another request to the network as in:

self.onfetch = event => {
  event.respondWith(
    event.preflightResponse || fetch(event.request)
  )
}

The second one will hit the http cache hopefully instead of reaching the actual network.

With the cancellable API the former example would be something like:

self.onfetch = event => {
  event.respondWith(
    (!event.networkResponse.inProgress && event.networkResponse.response) || fetch(event.request)
  )
}

Does it make sense?

Anyway, optimal solution would be to somehow foresee the need of the service worker and reduce the startup penalty. Perhaps a more low level solution for this is a better option.

Collaborator

F2F:

  • WebKit & MS think the preflight is a good idea
  • Not keen on the "preflight" naming
  • We need to name this, but not now
  • It's for GET navigation
  • The request the same as event.request plus a dict with an addHeaders property, allowing the setting of headers & the change of mode - but this is tricky when mode is navigate, which it is
  • It happens for everything in scope
  • This API should sit on the registration object
  • It should return a promise, which resolves once the rule is in place, or rejects if the options are invalid (not-allowed headers)
  • Once the rule is in place, the preflight will always be done, it's not optional from the browser's point of view, for consistency it should be done for every GET navigate
  • We can add url prefix in later
  • concurrentFetch?
Member
annevk commented Jul 31, 2016

Isn’t each GET bavigate going to be wasteful and encourage a more complicated pushState() navigation scheme for subsequent navigations? (Which would be problematic if we ever manage to tie other things to navigation, such as animations, apart from making sites more complicated.)

bmaurer commented Jul 31, 2016

Isn’t each GET bavigate going to be wasteful and encourage a more complicated pushState() navigation scheme for subsequent navigations? (Which would be problematic if we ever manage to tie other things to navigation, such as animations, apart from making sites more complicated.)

Realistically I think a complex application will still end up using pushState rather than doing fresh navigations on each page view. Keep in mind that the use case here is on URLs where getting the lastest server-side data is critical for the page to be "done", ie a news feed or other page that is expected to show real time information.

Even if you take out network latency there's a substantial price to be paid in terms of initializing client side JS (installing a bunch of prototype objects, etc). Apps may have features that cross page navigation (eg on Facebook chat tabs) where a fresh page view could be a disruptive experience (imagine you're half way through typing a message).

One thing that could mitigate this issue is to allow for a TTL for pre-flight requests -- if the page is in browser history and less than X seconds old not to issue a pre-flight.

Collaborator

@annevk

Isn’t each GET bavigate going to be wasteful and encourage a more complicated pushState() navigation scheme for subsequent navigations?

This feature will be opt-in, and includes the setting of some headers. A well built server could return an empty response for the preflight. Sure it's more wasteful than nothing, but it isn't much. I think there's potential for browser optimisation here too. If the concurrent fetch's response body isn't read by the time respondWith & waitUntil resolves, the browser can cancel it.

@bmaurer

Realistically I think a complex application will still end up using pushState rather than doing fresh navigations on each page view

I hope we can tackle this at some point. I hear developers going down this route because they want transitions, but it seems a shame that they have to reimplement the whole navigation state thing for that. At some point I hope we can look at navigation transitions again.

I know it isn't a complex app, but I wouldn't go pushState on my blog, but I would use this concurrent fetch feature. With pushState I'd lose all the performance I get through progressive rendering - although we could fix that by exposing the streaming parser to pages.

bmaurer commented Aug 1, 2016

I know it isn't a complex app, but I wouldn't go pushState on my blog, but I would use this concurrent fetch feature. With pushState I'd lose all the performance I get through progressive rendering - although we could fix that by exposing the streaming parser to pages.

Maybe you could solve this by making the pre-flight request a cacheable resource. That way if the user has visited that specific page on your blog within a timeframe you define the request would never hit your server.

Collaborator

I think I may already be doing that. But I'm not really worried about the cost of the preflight, since it's less than the cost of a regular navigation.

If I've cached a given article, I guess I could set a cookie with the last-modified time, so the server could shortcut the response. The cookie is better than the header in this case as it can be scoped to a particular article. Ideally last-modified would do the trick, but I guess it's possible for the resource to be in the cache API but not the HTTP cache.

ykahlon commented Aug 19, 2016

Google Docs is experimenting with service workers and we noticed that in the 95th percentile there is a regression of ~400ms and most of it is related to starting the SW.

This solution will completely eliminate the problem for us.

We actually need a simpler solution, we would like the SW to make the preflight request and only if the SW decides to make the exact same request, it will be handed to the SW (the SW doesn't need to know whether it was a preflight request).

If the SW does not make the request, the request that was made to the server will be ignored.

NekR commented Aug 19, 2016 edited

@ykahlon Well, I'm thinking about self.requests API which could potentially solve this problem too: #959 (comment)

NekR commented Aug 19, 2016 edited

(My kind-of a proposal for this case, a bit rethinked/reworked)

The requests

This API is supposed to solve this issue and the race issue for link=preload #959

Preflight

First, here are some scetches of the "pre-flight opt-in" which are based on what was writting here and decided on F2F meeting:

Registration, preflight is opt-in

Each preflight request will have Service-Worker-Preflight: something header.

navigator.serviceWorker.register('/sw.js', {
  preflight: {
    // Preflight interval. Is it really needed?
    // There was some mentions that HTTP cached would solve this.
    // Will `Vary: Service-Worker-Preflight` on response work fine here? CDN?
    ttl: 1000 * 60
  }
});
Pre-flight data
// Naming not final of course
self.registration.setPreflightProperties([
  headers: {
    'X-Status': 'Y'
  }
]).then(() => {
  console.log('Done');
});

This is what is related to preflight, following API is for general use cases which may solve Facebook/Google Docs issue with ServiceWorker.

API

(in TypeScript format, not spec format)

interface RequestsStorage {
  match(Request | string): Promise<Response>;
  putUntil(Request | string, Promise<Response>): void;
}

interface ServiceWorkerGlobalScope {
    readonly requests: RequestsStorage;
}
  • RequestsStorage#putUntil: Puts a request to the storage until Promise<Response> settled.
    For the case of preflight, UA need to hold the request in the store until fetch event ends
  • RequestsStorage#match: Macthes passed request agains stored ones. Requests should be differentiated by Service-Worker-Preflight header too. If there is no matching request in the store, then probably the promise should be rejected. For the simplicity, in the following example promise is fullfiled with an empty response when there is no macthing requests.

Example

The website. It wants to preflight requests and wants to handle them differently based on the requested page. It also sends Link preload headers for some pages.

Example: https://gist.github.com/NekR/d2d70f4e34d8829ea3c078456351ddce


This is how I think it may work. I hope it isn't very stupid :-)

UPD: Fix assets preloading example.

NekR commented Aug 20, 2016

Okay, I improved the example again and put it to a gist: https://gist.github.com/NekR/d2d70f4e34d8829ea3c078456351ddce

Some notes: I do always use self.requests and only then use self.caches to check for requests. This is because I defined that self.requests.putUntil() holds the requests until promise settles, which might be wrong and probably it may hold the request until SW is destroyed (or request manually removed). So in current situation, if I do self.caches check first, I potentially may have a race when after self.caches check request was removed from the self.requests and I missed it from both places.

Collaborator

@ykahlon

We actually need a simpler solution

But just to clarify, the idea above works for you too?

we would like the SW to make the preflight request and only if the SW decides to make the exact same request, it will be handed to the SW (the SW doesn't need to know whether it was a preflight request).

So does that mean your implementation would look something like:

self.addEventListener('fetch', event => {
  event.respondWith(
    event.preloadResponse || fetch(event.request)
  );
});

…where preloadResponse is a promise for a response fetched using event.request plus optional headers.

ykahlon commented Aug 22, 2016

@jakearchibald, The example above will work for us.

delapuente commented Aug 23, 2016 edited

I though this option would be per route. If this is the case and trying to anticipate other kind of scenarios, what about a NetworkRace type and something like:

navigator.serviceWorker.register('sw.js', {
  routes: {
    'some/route': new NetworkRace(),
    'some/other/route': new NetworkRace({ headers: ..., ttl: ... })
  }
});

Handling the fetch event, we can use something like:

self.onfetch = evt => {
  doThingsWith(evt.networkResponse);
};
delapuente commented Aug 23, 2016 edited

Aside, @NekR I don't get why we need the RequestStorage, could you clarify, please?

And, for clarification: suppose a preflight request is so fast that it answers completely before waking the service worker up? What happen then? Is that response used ignoring to trigger fetch to the sw? Or are we always waiting for the sw anyway? If the latter is the case, perhaps rather than preload or preflight, what we want is something like ping:

navigator.serviceWorker.register('sw.js', {
  routes: {
    'some/route': new PingNetwork(),
    'some/other/route': new PingNetwork({ headers: ..., ttl: ... })
  }
});
ykahlon commented Aug 23, 2016

@delapuente, I think that it is important that no matter what, the service worker will be the one that decides what to do with the request even if the browser already has the response ready.

Otherwise, the behavior will be non deterministic and in some cases the service worker may want to load from local storage or perform some other work.

@delapuente, at the last f2f I think we decided that preflight was pretty tangential to routing. In this case we aren't trying to define behaviors to bypass the service worker or to race the network and cache. We simply want the request to be sent out without incurring the full service worker startup cost. Then once the service worker has started it can get the request and presumably use the resulting stream and maybe mix stuff from that stream and cache. But it's still totally up to the service worker.

The preflight api as talked about at the f2f is the simplest, most performant and broadest solution that we could come up with for this. Routing is another beast altogether.

Yes, sure, I agree with @ykahlon on the service worker being the ultimate responsible for handling the request and with @n8schloss about not routing, this API proposal was shaped to allow adding routes at some point in the future. I'm fine with a declarative API too as proposed by @NekR.

The thing I don't understand is the need of the RequestStorage (perhaps I'm missing some scenario) and I would prefer to make the preflight by route (actually by prefix) to not reach the network in not critic requests.

NekR commented Aug 24, 2016

RequestStorage API isn't really supposed to be related to the case in this issue. I originally came with its idea in #959 issue. The reason why is to have a way to reuse existing requests. I'm not particularly sure how often this would happen in the wild, but the use case was:

  • Go to page A
  • Preload page B with /Link header
  • Preload page B's assets by Link header in B's response
  • If user clicks on the B link on the A page -- reuse existing preload requests, or load from cache, or load from network

This is why I got the idea about self.requests.match(). Next I thought that the problem in this issue could be also solved with such API. Ideally, I think that all requests which possibly may/need to bypass SW (for performance reasons) are ended up in self.requests storage. In the example on my proposal I showed that only preflight requests goes automatically (browser puts it) into self.requests, but ideally all Link header preloads of that preflight request should be put into self.requests by the browser. The reason of course, as you mentioned, is that preflight might be finished before SW starts up, so browser will be able to start preloading of other critical data before SW handles preflight request.

One real world example might be this, which I had in one of my previous projects: We had very fast newsfeed generation, but very slow generation/fetching of likes for the newsfeed. So we ended up with 2 requests: one for newsfeed on one for likes, to show content to the user as fast as possible.

In the case with preflight + preload with Link header that could be optimized while still serving app shell from SW.

Collaborator
jakearchibald commented Sep 8, 2016 edited

Right, let's sort out the API for this. How about:

self.addEventListener('install', event => {
  event.addNavigationPreload();
});

self.addEventListener('fetch', event => {
  event.respondWith(event.preloadResponse || fetch(event.request));
});
  • addNavigationPreload must be called while the service worker's state is installing.
  • This setting sits with the service worker, not the registration, so if the next version of the service worker does not call addNavigationPreload, there will be no preloads once that service worker is active.
  • The preload happens for fetches with mode "navigation" only, and happens for all in-scope fetches with mode "navigation".
  • The preload request is the same as the original, except for the addition of a Service-Worker: navigation-preload header.
  • preloadResponse is a promise for a response, or null if no preload was made.
  • If preloadResponse is unresolved by the time the fetch event has ended (including waitUntil etc), the browser may abort the request.
  • If preloadResponse's response body has not been used by the time the fetch event has ended (including waitUntil etc), the browser may abort the response.

How's that?

@n8schloss you mentioned wanting to set a header to a particular value, but the more I think about it the more it feels like cookies with a different name. If we had a cookie API in service worker, would that be enough here?

Hmm, I'm not totally sure that this would work for us. Being able to modify this after the SW is installed is important and I think a header makes much more sense than cookies.

Header VS Cookie

We're going to be serving totally different responses when there is a SW present vs when there isn't. Having a header means that we can ensure that this extra info only gets added when there's an SW, using a cookie means that it'll always be sent. This can lead to some weird things if a user uninstalls an SW without deleting cookies, if browser vendors ever want to build something that skips SWs when they are being unresponsive, if SW cleanup runs into issues, etc.
Having info that can and is only added via the SW for these requests is important, cookies would not provide this but headers do.

Modifying After Instillation

If we go the header route then we'll likely want to change the header without updating the SW. A use case for this is if we do a code push to update Facebook, but nothing in the Service Worker code changed. In that case we may not want to push an update to the Service Worker, but we'd still want to modify the header to match what the newest version of Facebook server code is expecting. Or we may want to update the header with information about how up to date our cache of the page chrome is, etc.
If we don't go the header route, then we'll likely need to turn off preflight in cases where the active SW and Facebook server revision are out of sync and we need to add information from within the service worker before sending out requests.

delapuente commented Sep 9, 2016 edited

Having a header means that we can ensure that this extra info only gets added when there's an SW, using a cookie means that it'll always be sent.

The header Service-Worker: navigation-preload is set in case of preload only. You can add extra information in a cookie and check the value only if the header is present.

@jakearchibald how about change the name preload with prefetch to align with resource hints?

Collaborator
jakearchibald commented Sep 9, 2016 edited

@n8schloss as @delapuente points out, you'd have the fixed Service-Worker: navigation-preload header to identify the preload, but you'd be able to use cookies for dynamic stuff like "last post date" etc etc. Does this work?

Collaborator

@delapuente

how about change the name preload with prefetch to align with resources hints?

Prefetch is at the browser's discretion, preload is a non-optional declarative fetch.

Since this concurrent request is not at the browser's discretion, "preload" seems like a better fit.

Did not know. Thank you.

jakearchibald changed the title from Eliminating SW startup latency for common case to Making a concurrent request for navigations Sep 13, 2016

Yes, with the "navigation-preload" header we can ensure that tell that something came from the Service Worker, but we still end up with a situation where we we're going to be sending down unneeded data on every request. There's data that only makes sense on preload from a service worker context so to me it makes sense that we'd just set and send them only from within the service worker.

Also, while this won't necessarily be a problem for us, cookies can be modified from both the window and via responses from the server. For developers who don't control their entire stack this could very well cause issues.

Cookies seem like a hack here, I think that being able to set data in a header makes more sense and brings preload's options more in line with what you can do with fetch today.

Collaborator

we still end up with a situation where we we're going to be sending down unneeded data on every request

I agree that's not ideal, but how much additional data are you planning on sending? And what will that data be?

Also, while this won't necessarily be a problem for us, cookies can be modified from both the window and via responses from the server. For developers who don't control their entire stack this could very well cause issues.

If we want this header to be updated outside of the service worker lifecycle it puts developers in a weird position, as the code for activating this feature can get out of sync with the fetch handling code. If they activate this feature and their fetch event doesn't cater for it, they can end up with two HTTP requests per navigation rather than one.

Cookies seem like a hack here

Cookies already exist for storing state and sending it to the server, creating another thing which does that feels like a hack.

Jake's cookie suggestion definitely makes a lot of sense to me. At least in Gecko, cookies are synchronously available just like SW registrations. They're also a known quantity, although things like limits do vary between browsers. And creating a less wacky API to manipulate them would be a great thing for everyone. It also avoids creating another separately managed per-TLD+1 hunk of (potentially) persistent memory. Sites get the same (memory) budget they had before with the same latency characteristics.

The one downside is that without some specialization, this would end up being a "it will work most of the time" thing. In addition to the SW-specific-cookies-not-deleted-on-unregister case @n8schloss pointed out, there's also the issue that cookies are subject to eviction due to browser-wide cookie limits and per-TLD+1 base domain limits. The required specialization seems like it would be a major hack/monkeypatch not particularly justified by this use-case. For example, we could treat a cookie with the path of the SW itself as magic so that it gets deleted on unregister and it is pinned so it doesn't get evicted by standard eviction logic.

The argument for leaving cookies as they are is that:

  • At least for Gecko, cookie eviction occurs based on last access and global eviction happens very infrequently (~every 30 days), so an impacted SW would likely be for a rarely visited site/path where arguably the SW itself might also be appropriate to nuke.
  • If the cookie needs to be more than a boolean indicator, the cookie should end up being set regularly anyways. Plus, the no-cookie-because-it-got-evicted case is also roughly equivalent to the first-install case.
n8schloss commented Sep 14, 2016 edited

I still think that being able to add a custom header is preferable to cookies. To sum up my argument for a custom header:

  • As @asutherland said, cookies will work most of the time but they can be very unreliable. Different browsers have different limits for cookies and will do different things when they hit such limits. I think that the cookie solution will introduce a number of edge cases for when cookies are missing that will need to be dealt with. We'll be able to figure this out, but for smaller developers I think this will introduce a bit of unnecessary overhead and make doing complex things with joining cache and response streams in service workers much harder. Ultimately I think that there's somewhat of a tradeoff here between a bit of extra work for vendors and work for web developers.
  • Cookies get sent always but much of this data is only relevant for service workers. Sending the cookies on every page is a waste and also due to the various browser cookie limits it may mean that we end up hitting such limits faster.
  • Even in @jakearchibald's proposal we end up adding a header. We might as well allow developers to customize its contents.
  • No cookie API exists for service workers right now. It's nice to talk in the abstract about how it'd be nice to have one down the road, but as it stands preload would be much less useful for us until something like that was made. Such an API seems tangential from this.
  • We already allow developers to add custom headers to requests objects that they are making as a result of a fetch event. If the goal of preload is to allow for similar requests earlier then to me it seems like we should try to get as much of the fetch functionality as is reasonable. (More developer control > less developer control)

Would there be issues with allowing custom data to be set on a X-Service-Worker-Preload header?

Collaborator
jakearchibald commented Sep 20, 2016 edited
  • We need to make sure this header doesn't get copied over on redirect
  • Some users are detecting the Service-Worker header and thinking it's a SW script request, so we should use a different header: Navigation-Preload: true by default, but can be set to any value
  • It'd be nice to have self. that refers to this service worker instance in a service worker global (self.own)
  • The header needs to be settable at any point
  • The setting should live with the lifecycle of the service worker
  • Need a way to clear it
  • What should happen if value is set to false? Is it confusing if we stringify it?
// this can be called at any point
registration.active.setNavigationPreload({
  value: 'true' // header value
}).then(() => console.log(`It's set!`));

registration.active.getNavigationPreload()
  .then(settings => console.log(settings.value));

self.addEventListener('fetch', event => {
  event.respondWith(event.navigationPreload || fetch(event.request));
});

@jakearchibald to propose API

I was thinking on @annevk proposal for fetchOptions. You're right on this affects fetch standard and it was clear to me when we make it accessible from clients. Perhaps this proposal is more complicated but what about providing a window.fetchPolicies namespace (exposed to Service Workers) and keep it separated from Service Workers.

// set or reconfigure
window.fetchPolicies.setNavigationPreload({ headerValue: 'true', scope: '/one-scope' });
window.fetchPolicies.setNavigationPreload({ headerValue: 'true', scope: '/other/scope' });

// clear
window.fetchPolicies.clearNavigationPreload({ scope: '/' });

// access to response.
self.onfetch = evt => {
  evt.respondWith(evt.navigationPreload || fetch(event.request));
};

With ergonomics in mind, navigation preload could be set at the same time you register a Service Worker.

navigator.serviceWorker.register('sw.js', { scope: '/one/scope/', navigationPreload: 'true' });

What do you think?

Collaborator

This is definately a service worker API, as it's useless without it.

We decided to tie the setting to the SW to reduce the chance of enabling the feature when you didn't have a SW that knows how to handle it.

The proposal we developed as a group (in my comment above) is also accessible from clients.

We decided to tie the setting to the SW to reduce the chance of enabling the feature when you didn't have a SW that knows how to handle it.

Sure. +1

Collaborator
jakearchibald commented Oct 3, 2016 edited

Soooooo our API design doesn't really work.

We decided that this setting should have a lifetime associated with the service worker itself, meaning that it must be set per service worker. This reduces the likelihood that it'd be set and forgotten about, leaving (some?) users with double requests on navigations, since the preload is never used.

This worked well with my pre-F2F proposal, but we also decided that the value for the header must be settable at any point. This is at odds with the above idea, because I'm pretty sure developers will want to treat the header as a piece of state that lives beyond the service worker.

Eg: Imagine I'm updating my service worker. I use navigation preloads, and I want to continue using them:

self.addEventListener('install', event => {
  event.waitUntil(
    new Promise(resolve => {
      if (!self.registration.active) {
        resolve();
        return;
      }

      resolve(self.registration.active.getNavigationPreload());
    }).then(settings => {
      settings = settings || {value: 'some-default'};
      return self.registration.installing.setNavigationPreload(settings);
    })
  );
});

The above is horrible, and doesn't even work. If the header value is updated at any point while the new service worker is waiting, its initial value will be out of date.

Developers could work around this by calling setNavigationPreload on all workers in a registration every time they use it (urgh), or call setNavigationPreload in the activate event, but by that time the current header setting has gone, so the developer would have to also retain it in indexedDB or whatever.

Feels like we've solved one footgun and created two more.

If allowing the header to be set at any time is a requirement (and Facebook says it is), I think we should just put this on the registration object.

Collaborator
jakearchibald commented Oct 3, 2016 edited

Here's a proposal for a registration-based API:

// enabling the feature.
self.addEventListener('activate', event => {
  event.waitUntil(
    self.registration.navigationPreload.enable()
  );
});

navigationPreload.enable can be called at any point, but the recommendation will be to call it in the activate event. That way it'll be enabled when there's a service worker in place that has a fetch event listener that knows what to do with it.

The preload will have the header Service-Worker-Navigation-Preload: true. We suggested Navigation-Preload at the F2F, but my feeling is this could get confused with link[rel=prerender].

// disabling the feature.
self.addEventListener('activate', event => {
  event.waitUntil(
    self.registration.navigationPreload.disable()
  );
});

Both enable and disable return a promise that resolves once the setting is updated on the registration.

If you ever launch a service worker that enables this feature, then later decide you don't need it, you're kinda doomed to forever disable it in your activate event. I don't see a way around that.

// access to response (same as previous proposals)
self.addEventListener('fetch', event => {
  event.respondWith(event.navigationPreload || fetch(event.request));
});

// setting the header
self.registration.navigationPreload.setHeaderValue("hello");

The header can be set at any point. Setting returns a promise that resolves once the registration is updated. Setting the header does not auto-enable the feature. This means page code can be updating the header without enabling the feature for a service worker that isn't ready for it.

Also:

self.registration.navigationPreload.getState(state => {
  console.log(state); // {enabled: false, headerValue: "true"}
});

Any takers?

n8schloss commented Oct 3, 2016 edited

Everything in the comment above makes sense to me. I suspect that most people will want to keep the header somewhat in line with their caches, and this isn't all that different from managing the sw cache (in terms of which lifecycle stage you'd want to do things in).

jakearchibald self-assigned this Oct 4, 2016
jakearchibald added a commit that referenced this issue Oct 4, 2016
jakearchibald API surface for #920
Still need to make the actual preload request happen, which will involves changes to the fetch API.
50c3cdf
Collaborator

I'd like us to reconsider event.preloadResponse instead of event.navigationPreload.

If, in future, we have some kind of routing system (klaxon), it'd be nice to provide a per-route way to define a preload and what it'd look like. This means preloads could happen for subresources, and maybe non-GET requests. It'd be great if we could reuse event.preloadResponse in that case, since it's the same feature.

@jakearchibald what are the advantages to allow .enable() to be called at any point? And why are you forced to disable preload in the activate event? Can I disable the feature during a fetch event?

Collaborator

@delapuente

what are the advantages to allow .enable() to be called at any point?

None really. By having the setting on the registration we can't really restrict when it can be called without creating a confusing API. We'd have to strongly recommend calling it during activate.

And why are you forced to disable preload in the activate event?

It can also be disabled at any point, it's just that activate is the best time to do it.

Can I disable the feature during a fetch event?

Yep, whenever you want.

Member
annevk commented Oct 6, 2016

Will patterns deployed for preloadResponse today be forward-compatible? How can we be sure?

NekR commented Oct 7, 2016

Just for my understanding, can anyone explain what is wrong with my self.requests proposal? I mean, you just moved without even saying "it doesn't work for us", which is perfectly fine because you are more experienced in it. I just want to understand what is wrong.

Collaborator

@NekR I see you mentioning it a few times, but I couldn't really find a concrete & up-to-date proposal. I see https://gist.github.com/NekR/d2d70f4e34d8829ea3c078456351ddce, but it doesn't really explain how it works.

I'm not against (in future) some kind of API to find out about in-flight requests in the fetch group, but using such a thing becomes really tricky. Timing is especially difficult as you're exposing a single request to multiple processes.

NekR commented Oct 8, 2016

@jakearchibald "the proposal" was this comment: #920 (comment) but yes, I probably had to make it in a separate repo.

but using such a thing becomes really tricky. Timing is especially difficult as you're exposing a single request to multiple processes.

I see. So it's just too complicated for a simple use case from this repo.

Collaborator

@NekR No need for a separate repo, I just saw other comments saying you'd updated bits and I didn't know if you'd updated that comment or if it was spread across multiple places. A single gist may have been better. Anyway, here's a review:

Triggering this feature at registration time is confusing as it isn't clear what it's tied to. If my url & scope are the same, but preflight has changed, will my service worker be refetched? Will preflights start happening immediately for the active worker, or is the setting tied to subsequent service workers? If I wanted to disable this feature how would I do that? Problem is the code calling register is likely cached, so I'd need to update my SW to update that script, then reload the page to change the setting?

It doesn't really mention what ttl does.

It doesn't really cover which requests are in requests, is it just <link rel="preload"> and this new preflight thing, or is it all fetches in the fetch group?

It doesn't cover what happens if multiple processes read a single response. Do additional reads fail? This is where most of the complexity happens, you'd need some sort of mutex for accessing a response body.

Collaborator
wanderview commented Oct 11, 2016 edited

Some thoughts from #whatwg IRC today:

In regards to the "preload header", it feels like we are close to a somewhat generic thing that could be separated from the service worker API. Perhaps we could consider:

  1. An API that matches Request objects using the Cache API matching algorithm.
  2. Stores Headers values. Each origin has a unique store.
  3. When an origin performs a fetch it matches the Request against its stored values and appends any resulting Headers.
  4. A service worker preload fetch would always have a ServiceWorkerPreload header. This would let Requests use a Vary:ServiceWorkerPreload header to restrict things to preloads.

Then you could do things like:

let headers = new Headers();
headers.append('X-Food', 'Candy Corn');
headers.append('X-Drink', 'Cider');

let request = new Request('/order.html');

autoHeaders.put(request, headers);

Or for a preload only header:


let headers = new Headers();
headers.append('X-Preload-Value', 'Boo');

let request = new Request('/order.html', {
  // spec service worker preload requests to include 'ServiceWorkerPreload' header
  headers: { Vary: 'ServiceWorkerPreload' }
});

autoHeaders.put(request, headers);

Note, this does not provide any substring matching. So it doesn't allow you to set headers for an entire scope or collection of URLs. If that is needed we could consider adding it. For Cache API, though, we considered it a de-opt and removed it previously. Maybe its more necessary for something like this, though.

Anyway, this is just an idea. I'm not objecting to the current spec proposal.

Collaborator
domenic commented Oct 24, 2016

In https://codereview.chromium.org/2416843002/ I was made aware that this proposal introduces a nullable promise type into the IDL. I guess navigationPreload is supposed to be specced as a Promise<Response>?.

We are actually hoping to outlaw that from Web IDL entirely: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25049

Can someone summarize why a nullable promise type is needed? I don't see any IDL in this issue, and I can't even see anyone proposing navigationPreload as a promise for Response, much less a nullable one, so maybe there is just some confusion going around.

NekR commented Oct 24, 2016

@domenic in short, during navigate fetch event there already could be an inflight request, or could not. So it either exists or not, and at a time of the fetch event response may not be available yet, hence a promise.

But honestly I thought that it's going to be:

if (event.navigationPreload) {
  event.navigationPreload.then(() => ...);
}

Not nullable promise. With nullable promise it would require checking that promise all the time.

Collaborator
domenic commented Oct 24, 2016

Maybe I was unclear. By nullable promise I mean a value that is sometimes null, instead of a promise. That appears to be what your example illustrates.

We should not introduce this new concept that is unused anywhere else on the platform. Everywhere else, if an attribute represents an asynchronous value, it's always accessed asynchronously---it's not sync-if-null, async-otherwise.

@domenic, that makes sense to me. It's important that we provide a sync way to check this though. There are times where someone might not want to call respondWith if the preload was not sent. I guess maybe we'll need something like event.hasNavigationPreload which is a bool and then event.navigationPreload which is always a Promise<?Request> (ie a promise that will sometimes resolve to a request and other times resolve to null)?

Collaborator
domenic commented Oct 25, 2016

That would work. But I'm not sure why you wouldn't just do

event.navigationPreload.then(res => {
  if (res) {
    event.respondWith(res);
  } else {
    // ...
  }
});

Comparing it with

if (event.hasNavigationPreload) {
  event.respondWith(event.navigationPreload);
} else {
  // ...
}

it seems like at most you'd lose a single microtask in the no-navigation-preload case, and they'd be equivalent in the yes-navigation-preload case.

NekR commented Oct 25, 2016

I agree that makes sense indeed especially for await.

That would work. But I'm not sure why you wouldn't just do

respondWith should be called in sync, AFAIK. Probably this could work instead:

const wait = event.navigationPreload.then(res => {
  if (res) {
    event.respondWith(res);
  } else {
    // ...
  }
});

event.waitUntil(wait);

@domenic, wouldn't the example you posted throw an error? As soon as you've done any async work you can no longer safely call event.respondWith because any subsequent call is in a different function. (See step 1 in section 4.5.4 on https://www.w3.org/TR/service-workers/#fetch-event-respondwith-method)

Collaborator
domenic commented Oct 25, 2016

Ah yeah, I wasn't aware of that, but @NekR's got my back there :)

@domenic, that example has the same issue :)

From section 4.5.4, "If the active function is not the callback of the event handler whose type is fetch, then: 1) Throw an "InvalidStateError" exception."

Collaborator
domenic commented Oct 25, 2016

Right, we'd presumably change the spec to allow it in the appropriate event.

NekR commented Oct 25, 2016 edited

@n8schloss I expected that it may not work, that's why I tagged it "probably". But not knowing that sections of the spec, it feels like that should work.

@domenic, I don't think that's a good idea because we still want the browser to be able to respond to the fetch event normally if respondWith wasn't called. Imo the developer really needs to decide if they are going to respond to a fetch event before any async work happens.

NekR commented Oct 25, 2016

@n8schloss That sounds reasonable too, but there are some moments when developer doesn't know that ahead of time. This is why event.respondWith(fetch(event.request)) is trying to as close as possible to not calling responseWith. What is the concern of doing it after checking navigationPreload?

Collaborator
domenic commented Oct 25, 2016

I'm very confused. Above you said we weren't talking about the fetch event. Now we are again?

Regardless, I'll leave this to those who are more involved. My only real concerns are not having a nullable promise type, and not overburdening the API because of some desire to make it sync, when in reality the worst case would be a single microtask of delay.

@domenic, we're talking about a field on the fetch event, but sure, that sounds reasonable. I'll let others chime in with what they think should be the resolution here :)

Collaborator
wanderview commented Oct 25, 2016 edited

@n8schloss Would you be comfortable with always calling respondWith() and using a pass-through fetch if necessary?

addEventListener('fetch', evt => {
  evt.respondWith(evt.navigationPreload().then(res => {
    return res || fetch(evt.request);
  });
});

I think people have the impression that not calling respondWith() is more efficient than a pass-through fetch. The API design, however, is built on the premise that these are effectively equivalent. I think we should just lean on that here and continue to make the pass-through better.

Also, I don't recall no-respondWith being any faster than pass-through fetch in my recent benchmarking.

@wanderview, our in the field data is still showing fetch from service workers as much slower than fetch from the window (at least for Chrome). In Chrome this will likely be the case until crbug.com/450471 is solved. In addition to the perf benefits, there are also UX benifits to not responding to fetch from the SW. Many browsers will not show the default error page when there's a network error given to respondWith. The browser network page is going to be much much more useful that anything a developer makes to indicate that something is wrong with the user's network setup (and I think it's somewhat unreasonable for most developers to have to make an error page for each error condition that could happen anyway).

That being said, for us, we're likely going to always enable preload, so this is not an issue for us right now. But by not having a way to get this info synchronously we're limiting the flexibility of the api and maybe making it hard for other developers and could be hurting things that we want to do in the future. When the browser is dispatching the fetch event it should already know if it sent a preload request or not, so it doesn't seem like a huge deal to make that info available to developers.

Collaborator

The error page issue is fixable by the browsers. We just need to stash the reason for the NetworkError on an internal attribute that content can't see. Then we can use the correct network error page if its passed back to a navigation respondWith().

I also feel that the perf issues should be fixable long term.

I guess I'm just worried about contorting the API for short term issues. If the problems can't be fixed, we could also add a sync boolean getter later.

That's fair, on the flip side I'd argue that the browser already needs to have this data so there's minimal overhead to exposing it to the developer. Since at least for a while developers will be able to extract value from having this info, why not just expose it?

I realize that at this point we're kind of nitpicking, so if implementing a sync way to get this data isn't trivial then I'm fine if we don't do it :)

Collaborator

@domenic #983 for the spec work. I'm happy for navigationPreload to be always-there, although it feels somewhat less convenient.

Collaborator
domenic commented Oct 28, 2016

@jakearchibald now that apparently navigationPreload is no longer returning a promise, it can be nullable with no problem if that's more convenient.

Collaborator

@jakearchibald Are we sure we will never want this for non-navigations? Should it just be called preload instead of navigationPreload?

Collaborator

@domenic I thought it was going to be a promise. What have I missed?

Collaborator
domenic commented Oct 28, 2016

@jakearchibald in #983 it's apparently going to be a NavigationPreloadManager instead.

Collaborator

That's probably an error on my part, will take a look tomorrow.

mattto commented Oct 31, 2016

In #983 there are two objects named navigationPreload:

  • ServiceWorkerRegistration.navigationPreload is a NavigationPreloadManager
  • FetchEvent.navigationPreload is a Promise<Response>.
Collaborator

Yeah, I'd forgotten to add FetchEvent#preloadResponse.

Also, I've called it preloadResponse, I agree with @wanderview, we don't need to make the naming navigation-only.

I've gone with @domenic's suggesting of making it resolve with undefined if it isn't a navigation or the feature hasn't been enabled.

At the risk of sounding very stupid (I do not know inner workings of browser and potentional security problems).

Wouldn't it be simpler to just define a way to unsubscribe some requests from service workers (e.g. what @jakearchibald proposed with .declareRoute and FetchSource) AND creating an API that could manipulate browser HTTP cache (not cache api).
The end result would be that developers would be able to set (from js) aggressive max-age + expires headers (which developers should already know) on critical-path requests (hence not waiting for them even if they are subject to occasional change) without waiting for service worker to startup or waiting for any other API.
The added benefit would be, that developers would get means to update long-living (static) requests/resources upon webpage facelift/redesign without need to use some kind of hack (?v=X).

@jmuransky, that's not solving the problem that we're trying to solve here. The idea here is that we still want the service worker code to run and be able to process the result of the network fetche. We just want said fetch to go out to the network earlier, before we've incurred the cost of starting the service worker. This is somewhat similar to the optimization talked about at https://code.facebook.com/posts/1675399786008080/optimizing-facebook-for-ios-start-time/.

Ok. My mistake then...

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Nov 8, 2016
horo-t Set "Service-Worker-Navigation-Preload" header for the navigation req…
…uests.


w3c/ServiceWorker#920 (comment)

BUG=649558

Review-Url: https://codereview.chromium.org/2473263003
Cr-Commit-Position: refs/heads/master@{#430559}
754e439
mattto commented Nov 10, 2016

Implementor feedback from Chrome: the ability to change navigation preload state (NPS) anywhere (enable/disable/setHeaderValue) is causing some complexities with how this thing is persisted in relation to the existing Registration record.

  1. I believe the ideal behavior for the NPS setters is to first persist to disk, then resolve the promise, or reject, e.g., an IO error. A much simpler design would be to resolve immediately and persist to disk as a best effort. How do others feel about this?
  2. (Assuming we do care about resolving after successful persistence above) I think most complexity is caused by the ability to set NPS before there is an installed worker: register().then(r => r.navigationPreload.enable()). In this case, for Chrome, nothing is yet on disk: a stored Registration is really just an installed worker: before a worker is installed, there is nothing stored. I believe Firefox is similar. It'd simplify things if the NPS setters reject if there is no active worker. What do others feel about this restriction?

Arguably the registration returned is a nascent one and won't have meaning until a worker successfully installs, so setters shouldn't be called yet.

mattto commented Nov 17, 2016

More specific feedback: Chrome's initial implementation will reject enable()/disable() if there is no active worker. We could change behavior later to allow it but it'd make the implementation a bit more complex. Related is w3c/push-api#221, the push spec's subscribe() waits for an active worker but this can cause a deadlock (neither implementation currently follows this: Chrome rejects if no active worker and Firefox resolves without waiting).

From a developer perspective it's much much much simpler if you are able to call enable()/disable() during the active event. It'd be a bit confusing to have to call enable() first thing after your SW runs once it's already been activated, or to have to make logic to deal with both the activated/unactivated cases.

mattto commented Nov 17, 2016

That's covered because in onactivate the worker is already active. This will restrict you from calling it in oninstall (in the case of a new registration. during updates it's callable because there's an active worker).

Collaborator

From a developer perspective it's much much much simpler if you are able to call enable()/disable() during the active event.

During the activate event the worker is already the active worker (at least with the terminology as used in the spec), so I'd expect that to still work

Collaborator

I think it's important for NPS promises to resolve once the data is successfully stored.

As for the timing, I was following the push API's lead here. I'm not against changing as long as we both change.

I don't fully understand the complexity in waiting. Assuming…

registration.navigationPreload.enable();

…waits for an active worker, how is this more complex than:

await registration.ready;
registration.navigationPreload.enable();

(assuming #770)

mattto commented Nov 20, 2016

I actually didn't really have waiting in mind when talking about complexity above. The complexity was about implementing resolve-after-store without waiting for active. This would likely require (for Chrome) initially storing the NPS outside the registration record if needed, then moving it into the record once that's stored (to avoid regressing the time taken to load a registration).

Implementation-wise, I think waiting is doable. The big disadvantage is the event.waitUntil(registration.navigationPreload.enable()) deadlock footgun in the install event. I guess developer tools can issue a warning in this case, but we'd need to do it for any API that waits for active.

It's still simplest to just reject when there is no active worker though. Since Chrome's push API has done this since inception, it's probably OK to keep doing it.

Collaborator

@mattto

The complexity was about implementing resolve-after-store without waiting for active

But store can only happen once there's an active worker. If the tab closes before a worker becomes active, nothing is stored, so I'm not sure why we'd need to store anything outside the registration.

It's still simplest to just reject when there is no active worker though. Since Chrome's push API has done this since inception, it's probably OK to keep doing it.

Hmm, yeah, background spec also does the same even though it's spec'd to "wait". @wanderview how do you feel about this change? I'm happy with it.

If we agree I'll submit PRs in push, sync & update NPS.

Collaborator

Works for me as long as we update the spec to match. Thanks.

mattto commented Nov 22, 2016

The complexity was about implementing resolve-after-store without waiting for active

But store can only happen once there's an active worker. If the tab closes before a worker becomes active, nothing is stored, so I'm not sure why we'd need to store anything outside the registration.

Ah yes, this is getting into implementation details that are probably not worth going into. The proposals to reject on non-active or wait for active are OK with me, with a preference for rejection. Again my complexity feedback was about trying to implement a solution that neither waits for active nor rejects on non-active, yet still has the properties that upon resolution:

  • the NPS has been stored, or
  • the NPS will be stored once the registration is stored (in case of no active worker yet)

For this solution, it’s not enough to just set the NPS in memory on our “IO” thread (where live service worker registrations are managed) and expect it to be stored once the registration is stored because of race conditions: the store operation may already be queued up or running on the “DB” thread. So I was thinking the DB thread should always write the newest NPS state: either in the registration if it exists or outside if not, and swap in outside NPS once the registration is stored. But I guess alternatively the DB thread could keep unstored NPS in memory and check that whenever storing a registration, so yes it could be possible. Still a bit clunky though.

Collaborator
jakearchibald commented Nov 22, 2016 edited

I'm going to update the PR so we reject if there's no active SW. I'll submit PRs for background sync and push to do the same.

Update: I'm going to tackle #965 then this

jakearchibald added a commit to w3c/push-api that referenced this issue Dec 2, 2016
jakearchibald Reject if there's no active service worker
This matches current implementations w3c/ServiceWorker#920 (comment)
9349b66
jakearchibald added a commit to jakearchibald/BackgroundSync that referenced this issue Dec 2, 2016
jakearchibald Reject if active worker is null. 296bfcc
jakearchibald added a commit to jakearchibald/BackgroundSync that referenced this issue Dec 2, 2016
jakearchibald Reject if active worker is null. e05a71d
jakearchibald referenced this issue in WICG/BackgroundSync Dec 2, 2016
Open

Reject if active worker is null. #134

Collaborator

Ok, I've updated the spec to update if there's no active worker. Also filed w3c/push-api#230 and WICG/BackgroundSync#134.