Add support for <link rel=preload>

ASSIGNED
Assigned to

Status

()

Core
DOM: Core & HTML
P2
enhancement
ASSIGNED
2 years ago
2 days ago

People

(Reporter: Ilya Grigorik, Assigned: dragana)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Trunk
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:p1][necko-active], URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36



Actual results:

Preload is not supported.


Expected results:

Preload provides a declarative fetch primitive that initiates an early fetch and separates fetching from resource execution.

- spec: https://w3c.github.io/preload/
- use cases: https://w3c.github.io/preload/#use-cases

Updated

2 years ago
Component: Untriaged → Networking
Product: Firefox → Core

Updated

2 years ago
Severity: normal → enhancement
OS: Unspecified → All
Hardware: Unspecified → All
Component: Networking → DOM: Core & HTML

Comment 1

2 years ago
This would be extremely helpful for authors with web font loading, which are currently loaded only as needed.

Blog post by one of the Adobe Typekit devs:

http://www.bramstein.com/writing/preload-hints-for-web-fonts.html

Updated

2 years ago
Priority: -- → P2

Updated

2 years ago
Blocks: 1229634

Comment 2

a year ago
Chrome 50 already has this, already leveraged by many sites to load lot faster.

Comment 3

a year ago
+1. This is the feature that will kill the Flash of Unstyled Text once and for all. This is a significant user-facing issue, at least experientially, and also a Chrome parity issue. Is anyone working on this?

Comment 4

11 months ago
(It is a bit silly to have separate specs and in general having too many random underdefined pre* stuff, but oh well, I guess we should just implement this. Preload is somewhat well defined actually)

I wonder how much in practice prefetch and preload should differ in our implementation.

There is of course the http/2 part, but if talking only about the DOM part of this all.

Dragana, would you be interested in to take a look at this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dd.mozilla)

Updated

11 months ago
Flags: needinfo?(dd.mozilla)

Updated

11 months ago
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 5

11 months ago
I will take it.
Flags: needinfo?(dd.mozilla)
(Assignee)

Updated

11 months ago
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
(Assignee)

Comment 6

9 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24d2ba7c9579
(Assignee)

Comment 7

9 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0dd32c8ad367

Updated

9 months ago
Keywords: dev-doc-needed
(Assignee)

Comment 8

9 months ago
Created attachment 8784806 [details] [diff] [review]
bug_1222633.patch
(Assignee)

Comment 9

9 months ago
Created attachment 8784807 [details] [diff] [review]
bug_1222633.patch

Here is the first version. I have implemented it similar to prefetch.

In current implementation, when a new load starts we kill all prefetches. I have change that, anyway I think we need a better way to decide what to kill and what not, e.g. if load is almost over why killing it or if load only has small amount of data why killing it we are going to get that data anyway because closing a connection takes a 1/2rtt.
Attachment #8784806 - Attachment is obsolete: true
Attachment #8784807 - Flags: feedback?(bugs)
(Reporter)

Comment 10

9 months ago
A couple of questions after a quick scan through the code...

The load is initiated via nsPrefetchService. Does this imply some form of prioritization on the request? Note that prefetch and preload have *very* different load semantics: one is a "I might need this on next navigation, fetch it with low priority", and preload is "initiate this fetch right now and treat it as resource type defined by `as` attribute.. same priority, same CSP policy, everything". 

I don't see `as` being used anywhere for security or prioritization. Is this WIP?

It's OK to cancel any and all outstanding preloads when the fetch group is terminated. That's not the case for prefetch, which should be allowed to continue.
(Assignee)

Comment 11

9 months ago
(In reply to Ilya Grigorik from comment #10)
> A couple of questions after a quick scan through the code...
> 
> The load is initiated via nsPrefetchService. Does this imply some form of
> prioritization on the request? Note that prefetch and preload have *very*
> different load semantics: one is a "I might need this on next navigation,
> fetch it with low priority", and preload is "initiate this fetch right now
> and treat it as resource type defined by `as` attribute.. same priority,
> same CSP policy, everything". 
> 

Preload will be started immediately and prefetch will be started only after the main page finishes loading. 
'as' is not currently used (this is work in progress and I am not a dom person so I will need help here from Olli. I am not sure what is already implemented and what not)

> I don't see `as` being used anywhere for security or prioritization. Is this
> WIP?

This is work in progress :)
Prefetch does not have priority build in, but that will be a different bug.

> 
> It's OK to cancel any and all outstanding preloads when the fetch group is
> terminated. That's not the case for prefetch, which should be allowed to
> continue.

Currently I am not canceling preload when the fetch group finishes, but that will be ok to implement.
(Reporter)

Comment 12

9 months ago
(In reply to Dragana Damjanovic [:dragana] from comment #11)
> Preload will be started immediately and prefetch will be started only after
> the main page finishes loading. 

Cool. For my own benefit, where and how is this controlled in the code?

> > I don't see `as` being used anywhere for security or prioritization. Is this
> > WIP?

Gotcha. Thanks! Excited to see this coming together :)
(Assignee)

Comment 13

9 months ago
(In reply to Ilya Grigorik from comment #12)
> (In reply to Dragana Damjanovic [:dragana] from comment #11)
> > Preload will be started immediately and prefetch will be started only after
> > the main page finishes loading. 
> 
> Cool. For my own benefit, where and how is this controlled in the code?

Copying this for you, I  figured out that I have a bug :) Thanks :) 

In nsPrefetchService::PrefetchPreload

If we have the same prefetch already queued, move it from mQueue to mCurrentNodes (currently active prefetches)
if (!(aType & nsStyleLinkElement::ePRELOAD)) {
  return added ? NS_OK : NS_ERROR_ABORT;
} else {
  // It is a preload, we will activate fetch.
  LOG(("This is a preload."));
  mQueue.erase(nodeIt);
  rv = node->OpenChannel();
  if (NS_SUCCEEDED(rv)) {
    mCurrentNodes.AppendElement(node);
  } else {
    DispatchEvent(node, false);
  }
  return NS_OK;
}

and if it is a completely a new link, put it directly to the mCurrentNodes and start it:
if (!(aType & nsStyleLinkElement::ePRELOAD)) {
  rv = EnqueueURI(aURI, aReferrerURI, aSource,
                 getter_AddRefs(enqueuedNode));
  NS_ENSURE_SUCCESS(rv, rv);
} else {
  LOG(("This is a preload, so start the prefetch.\n"));
  enqueuedNode = new nsPrefetchNode(this, aURI, aReferrerURI,
                                    aSource);
  rv = enqueuedNode->OpenChannel();
  if (NS_SUCCEEDED(rv)) {
    mCurrentNodes.AppendElement(enqueuedNode);
  } else {
    DispatchEvent(enqueuedNode, false);
  }
}
(Assignee)

Comment 14

9 months ago
Created attachment 8784894 [details] [diff] [review]
bug_1222633.patch
Attachment #8784807 - Attachment is obsolete: true
Attachment #8784807 - Flags: feedback?(bugs)
Attachment #8784894 - Flags: feedback?(bugs)

Comment 15

9 months ago
Aha, the spec doesn't actually define at all what to do with the preloaded resources.
I guess we'll just use some cache.

Comment 16

9 months ago
Comment on attachment 8784894 [details] [diff] [review]
bug_1222633.patch

> nsPrefetchService::StopPrefetching()
> {
>     mStopCount++;
> 
>     LOG(("StopPrefetching [stopcount=%d]\n", mStopCount));
>-
>-    // only kill the prefetch queue if we are actively prefetching right now
>-    if (mCurrentNodes.IsEmpty()) {
>-        return;
>-    }
>-
>-    for (uint32_t i = 0; i < mCurrentNodes.Length(); ++i) {
>-        mCurrentNodes[i]->CancelChannel(NS_BINDING_ABORTED);
>-    }
>-    mCurrentNodes.Clear();
>-    EmptyQueue();
>+    // We will not kill already started prefetch.
Why not?



So to support 'as' I think one would need to change nsPrefetchNode::OpenChannel() so that right contentpolicy type could be passed.

But yes, makes sense to reuse the existing pre* stuff.

(sorry about delay. I tend to process feedback queue after review queue)
Attachment #8784894 - Flags: feedback?(bugs) → feedback+
(Assignee)

Comment 17

9 months ago
(In reply to Olli Pettay [:smaug] from comment #16)
> Comment on attachment 8784894 [details] [diff] [review]
> bug_1222633.patch
> 
> > nsPrefetchService::StopPrefetching()
> > {
> >     mStopCount++;
> > 
> >     LOG(("StopPrefetching [stopcount=%d]\n", mStopCount));
> >-
> >-    // only kill the prefetch queue if we are actively prefetching right now
> >-    if (mCurrentNodes.IsEmpty()) {
> >-        return;
> >-    }
> >-
> >-    for (uint32_t i = 0; i < mCurrentNodes.Length(); ++i) {
> >-        mCurrentNodes[i]->CancelChannel(NS_BINDING_ABORTED);
> >-    }
> >-    mCurrentNodes.Clear();
> >-    EmptyQueue();
> >+    // We will not kill already started prefetch.
> Why not?

We should not stop preloads and for prefetchs if we already started transfer, depends on how much was already transferred, how big is the file, and there is still sometime until we actually cancel connection (until close connection is received by the server), it is sometime no use in canceling it. We should add some logic to decide on that, e.g. do we already have response headers and how much data is already transmitted. I would add that in a separate bug.

> 
> 
> 
> So to support 'as' I think one would need to change
> nsPrefetchNode::OpenChannel() so that right contentpolicy type could be
> passed.
> 

We probably have a function that parses request destination similar to 'as'. Can you point me to it? Thanks.


> But yes, makes sense to reuse the existing pre* stuff.
> 
> (sorry about delay. I tend to process feedback queue after review queue)
Flags: needinfo?(bugs)

Comment 18

9 months ago
(In reply to Dragana Damjanovic [:dragana] from comment #17)
> We probably have a function that parses request destination similar to 'as'.
> Can you point me to it? Thanks.

Apparently we don't have quite the right list of things. Our service worker and related stuff seem to use older version of the spec.
bkelly might know if we have patches to update our impl to support the "destination"s which Fetch spec is talking about.
Flags: needinfo?(bugs) → needinfo?(bkelly)
(In reply to Olli Pettay [:smaug] from comment #18)
> Apparently we don't have quite the right list of things. Our service worker
> and related stuff seem to use older version of the spec.
> bkelly might know if we have patches to update our impl to support the
> "destination"s which Fetch spec is talking about.

AFAIK no one has started work on updating the code to this part of the spec.
Flags: needinfo?(bkelly)

Comment 20

2 months ago
Any recent progress on preload support?
Marking [qf] for triage, not sure if this appropriate as this isn't just a fix.

To add reason, this was brought up by partners that heavily use preload, especially for prioritizing their core scripts that handle loading and rendering of page components. Examples are Facebook (uses Big Pipe for generating page from pagelets) and Zalando (tailor is their "streaming layout service").
Whiteboard: [qf]

Updated

2 months ago
Flags: needinfo?(jduell.mcbugs)
(Assignee)

Comment 22

2 months ago
I will try to find time to work on this, targeting ff55
If Facebook will use this than it is a qf:p1. Kannan will ask Facebook for details. Otherwise we may wish to move it to a p3.
Flags: needinfo?(kvijayan)
Whiteboard: [qf] → [qf:p1]
Facebook is already using rel=preload for CSS and JS.
Flags: needinfo?(kvijayan)

Comment 25

a month ago
We use preload at search results page of Yandex for loading core sctipts from page header that flushes to user at the very beginning of request handling.

You can see it in this url: https://yandex.com/search/?text=cats

And we will be happy if this feature appears in Firefox :)
Flags: needinfo?(jduell.mcbugs)
Whiteboard: [qf:p1] → [qf:p1][necko-active]

Comment 26

a month ago
This is probably already known, but preload can also be used in a http header. ie not just in the html response

Comment 27

a month ago
(In reply to Simon Cropp from comment #26)
> This is probably already known, but preload can also be used in a http
> header. ie not just in the html response

Oh, I thought this bug was (only) about the http response header. http://caniuse.com/#search=preload
But the HTTP Response Header "Link: <https://example.com/other/styles.css>; rel=preload; as=style" isn't supported right now, is it? https://w3c.github.io/preload/
The current rel=prefetch (http://caniuse.com/#feat=link-rel-prefetch) is more like a "should" and get's loaded at the end of all requests, so it's inadequate to preload a css plus fonts/images with a reponse header of index.html. The css url could get read fast from the html, but not the images/fonts of the css (we have to load first). Preloading would be good here.
(Assignee)

Comment 28

a month ago
(In reply to Darkspirit from comment #27)
> (In reply to Simon Cropp from comment #26)
> > This is probably already known, but preload can also be used in a http
> > header. ie not just in the html response
> 
> Oh, I thought this bug was (only) about the http response header.
> http://caniuse.com/#search=preload
> But the HTTP Response Header "Link: <https://example.com/other/styles.css>;
> rel=preload; as=style" isn't supported right now, is it?
> https://w3c.github.io/preload/
> The current rel=prefetch (http://caniuse.com/#feat=link-rel-prefetch) is
> more like a "should" and get's loaded at the end of all requests, so it's
> inadequate to preload a css plus fonts/images with a reponse header of
> index.html. The css url could get read fast from the html, but not the
> images/fonts of the css (we have to load first). Preloading would be good
> here.

Preload is not implemented at all. Prefetch is implemented but that is a different thing.

Comment 29

16 days ago
That will be really great to have preload working on Firefox... Hope to see that soon!
(Assignee)

Comment 30

9 days ago
Created attachment 8869499 [details] [diff] [review]
bug_1222633.patch

This is a new version.

I have some questions:
I have not implemented "media" part. Do we have implementation of its parser somewhere?

I have implemented checking for "type" attribute. I am not sure I have all the types listed. For some types, e.g. images, I found helper functions for some other I have not found any. If there are some helper functions I have not found, please let me know.

"as" attribute is a bit confusing. The draft says that if a "as" is empty string, the preload should be treated as XHR. If "as" attribute is not a correct destination, preload should dispatch an error. On the other hand there are tests that expect that if you set a incorrect destination "as" attribute should be set to empty string, which is putting this 2 cases into one. I decided to implement one version for now and we can change this later.

nsPrefetchService will stop preloads as soon as there as no active loads any more. This will cause some of the web-platform tests to fail, because the page dispatching reload finish loading. Some test will run fine if run as a single test, but fail if I run a whole directory(https://treeherder.mozilla.org/#/jobs?repo=try&revision=937d9572f092501d592e3134cf57c3a72e23aa90). 


For now (for firefox55) we will not implement nocache version. If a page wants to preload resources it must send a response that is cachable. The nocache version is a lot of work that we do not have cycles for at this moment.

Some web-platform tests fail because the tests uses PerformanceResourceTiming to check whether load(not preload) uses the preloaded resources. Since we store data in the cache and loads from the cache have performance entry. The tests expect that loads using preloaded resources do not have such a entry. Even if we implement "nocache" version of preload this test will failed in the same way (if I remember correctly our discussion on how to implement "nocahe" version).

For missing "as" attribute I set contentPolicy to TYPE_OTHER. Is this correct?  testing/web-platform/tests/preload/preload-csp.sub.html
will fail for that case. I am not sure if the test is not correct or as="" should have a different contectType.
Attachment #8869499 - Flags: feedback?(bugs)
(Assignee)

Comment 31

9 days ago
Created attachment 8869500 [details] [diff] [review]
bug_1222633_webtest.patch

This are some changes to the web-platfom tests. Mainly we need resources to be cachable.
(Assignee)

Updated

9 days ago
Attachment #8784894 - Attachment is obsolete: true
(Assignee)

Comment 32

6 days ago
(In reply to Dragana Damjanovic [:dragana] from comment #30)
> Created attachment 8869499 [details] [diff] [review]
> bug_1222633.patch
>
> nsPrefetchService will stop preloads as soon as there as no active loads any
> more. This will cause some of the web-platform tests to fail, because the
> page dispatching reload finish loading. Some test will run fine if run as a
> single test, but fail if I run a whole
> directory(https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=937d9572f092501d592e3134cf57c3a72e23aa90). 
> 
> 

I figured out why this is happening. I have added cache headers so the request that was suppose to prolong the load of a page are served from cache and than loads finishes and cancel preloads. So this is not a problem, the tests are wrong.

Comment 33

2 days ago
(In reply to Dragana Damjanovic [:dragana] from comment #30)
> Created attachment 8869499 [details] [diff] [review]
> bug_1222633.patch
> 
> This is a new version.
> 
> I have some questions:
> I have not implemented "media" part. Do we have implementation of its parser
> somewhere?
> 
> I have implemented checking for "type" attribute. I am not sure I have all
> the types listed. For some types, e.g. images, I found helper functions for
> some other I have not found any. If there are some helper functions I have
> not found, please let me know.
> 
> "as" attribute is a bit confusing. The draft says that if a "as" is empty
> string, the preload should be treated as XHR. If "as" attribute is not a
> correct destination, preload should dispatch an error. On the other hand
> there are tests that expect that if you set a incorrect destination "as"
> attribute should be set to empty string, which is putting this 2 cases into
> one. I decided to implement one version for now and we can change this later.

Note that the `as` attribute handling is about to change with regard to empty values and firing of onerror events. See https://groups.google.com/a/chromium.org/d/msg/blink-dev/mKkVXwMhOLc/1X2m8u7HBQAJ

> 
> nsPrefetchService will stop preloads as soon as there as no active loads any
> more. This will cause some of the web-platform tests to fail, because the
> page dispatching reload finish loading. Some test will run fine if run as a
> single test, but fail if I run a whole
> directory(https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=937d9572f092501d592e3134cf57c3a72e23aa90). 
> 
> 
> For now (for firefox55) we will not implement nocache version. If a page
> wants to preload resources it must send a response that is cachable. The
> nocache version is a lot of work that we do not have cycles for at this
> moment.
> 
> Some web-platform tests fail because the tests uses
> PerformanceResourceTiming to check whether load(not preload) uses the
> preloaded resources. Since we store data in the cache and loads from the
> cache have performance entry. The tests expect that loads using preloaded
> resources do not have such a entry. Even if we implement "nocache" version
> of preload this test will failed in the same way (if I remember correctly
> our discussion on how to implement "nocahe" version).
> 
> For missing "as" attribute I set contentPolicy to TYPE_OTHER. Is this
> correct?  testing/web-platform/tests/preload/preload-csp.sub.html
> will fail for that case. I am not sure if the test is not correct or as=""
> should have a different contectType.
(Assignee)

Comment 34

2 days ago
(In reply to Yoav Weiss from comment #33)
> (In reply to Dragana Damjanovic [:dragana] from comment #30)
> > Created attachment 8869499 [details] [diff] [review]
> > bug_1222633.patch
> > 
> > This is a new version.
> > 
> > I have some questions:
> > I have not implemented "media" part. Do we have implementation of its parser
> > somewhere?
> > 
> > I have implemented checking for "type" attribute. I am not sure I have all
> > the types listed. For some types, e.g. images, I found helper functions for
> > some other I have not found any. If there are some helper functions I have
> > not found, please let me know.
> > 
> > "as" attribute is a bit confusing. The draft says that if a "as" is empty
> > string, the preload should be treated as XHR. If "as" attribute is not a
> > correct destination, preload should dispatch an error. On the other hand
> > there are tests that expect that if you set a incorrect destination "as"
> > attribute should be set to empty string, which is putting this 2 cases into
> > one. I decided to implement one version for now and we can change this later.
> 
> Note that the `as` attribute handling is about to change with regard to
> empty values and firing of onerror events. See
> https://groups.google.com/a/chromium.org/d/msg/blink-dev/mKkVXwMhOLc/
> 1X2m8u7HBQAJ
> 


Thanks.

I have follow github issues regarding this.
You need to log in before you can comment on or make changes to this bug.