The default bug view has changed. See this FAQ.

Add support for <link rel=preload>

ASSIGNED
Assigned to

Status

()

Core
DOM: Core & HTML
P2
enhancement
ASSIGNED
a year ago
13 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

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

a year 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
Component: Untriaged → Networking
Product: Firefox → Core

Updated

a year ago
Severity: normal → enhancement
OS: Unspecified → All
Hardware: Unspecified → All
Component: Networking → DOM: Core & HTML
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

a year ago
Priority: -- → P2

Updated

a year ago
Blocks: 1229634

Comment 2

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

Comment 3

10 months 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?
(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)
Flags: needinfo?(dd.mozilla)
Flags: needinfo?(dd.mozilla)
(Assignee)

Comment 5

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

Updated

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

Comment 6

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

Comment 7

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

Updated

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

Comment 8

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

Comment 9

7 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

7 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

7 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

7 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

7 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

7 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)
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 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

7 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)
(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

15 days ago
Any recent progress on preload support?
You need to log in before you can comment on or make changes to this bug.