Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 471199 Add support for link rel=preload
Starred by 31 users Project Member Reported by y...@yoav.ws, Mar 27 2015 Back to list
Status: Fixed
Owner:
Closed: Jan 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Launch-OWP
Launch-Accessibility: ----
Launch-Legal: ----
Launch-M-Approved: ----
Launch-M-Target: ----
Launch-Privacy: ----
Launch-Security: ----
Launch-Status: ----
Launch-Test: ----
Launch-UI: ----


Sign in to add a comment
(See http://www.chromium.org/blink#launch-process for an overview)

Change description:
Add support for <link rel="preload"> to tell the browser that it should download a certain resource in a certain priority, since this resource will be needed later during current navigation.

Changes to API surface:
* <link rel=preload> would trigger a resource download
* The `as` attribute would provide it with context (and corresponding download priority)
* Equivalent `Link` header support.

Links:
Specification: https://w3c.github.io/preload/
Public standards discussion: https://github.com/w3c/preload/

Support in other browsers:
Internet Explorer: Not supported
Firefox: Not supported
Safari: Not supported

*Make sure to fill in any labels with a -?, including all OSes this change
affects. Feel free to leave other labels at the defaults.

 
Blocking: chromium:474445
Project Member Comment 2 by bugdroid1@chromium.org, Apr 12 2015
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=193605

------------------------------------------------------------------
r193605 | yoav@yoav.ws | 2015-04-12T22:27:32.027909Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/dom/HTMLLinkElement/link-preload-validity.html?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/ResourceFetcher.h?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/LinkRelAttribute.cpp?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/LinkLoader.cpp?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/LinkRelAttribute.h?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/RuntimeEnabledFeatures.in?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.in?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/LinkLoader.h?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.h?r1=193605&r2=193604&pathrev=193605
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/dom/HTMLLinkElement/link-preload-validity-expected.txt?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLLinkElement.cpp?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/ResourceFetcher.cpp?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLAttributeNames.in?r1=193605&r2=193604&pathrev=193605
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLLinkElement.h?r1=193605&r2=193604&pathrev=193605

Add initial link preload support

An initial version of <link rel=preload>: https://w3c.github.io/preload/
Doesn't include:
* Priorities implementation
* Link header support
* Mutation tests  

BUG= 471199 

Review URL: https://codereview.chromium.org/1060863003
-----------------------------------------------------------------
Comment 3 by y...@yoav.ws, May 19 2015
Blockedon: chromium:489646
Comment 4 by y...@yoav.ws, May 19 2015
Blockedon: chromium:489647
Project Member Comment 5 by bugdroid1@chromium.org, May 19 2015
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=195532

------------------------------------------------------------------
r195532 | yoav@yoav.ws | 2015-05-19T14:10:18.782270Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/Resource.h?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/ResourceFetcher.h?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/LinkLoader.cpp?r1=195532&r2=195531&pathrev=195532
   A http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/LinkLoaderTest.cpp?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/core.gypi?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/loader/FrameFetchContext.cpp?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/FetchRequest.h?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/Resource.cpp?r1=195532&r2=195531&pathrev=195532
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/fetch/ResourceFetcher.cpp?r1=195532&r2=195531&pathrev=195532

Enable `as` based priorities for <link rel=preload>

The `as` attribute on <link rel=preload> was implemented as a placeholder, but didn't really do anything.

This CL adds support for true resource prioritization based on the `as` attribute, as well as LinkLoader unit tests for preload, and fixing a bug related to preload triggering a download even before it was added to the document.
 
BUG= 471199 

Review URL: https://codereview.chromium.org/1141883003
-----------------------------------------------------------------
Do you believe this feature may have any possible security implications? If so, we should make sure Chrome's security team has approved before this goes out.
Cc: owe...@chromium.org
The spec is now a working draft: http://www.w3.org/TR/2015/WD-preload-20150721/
Comment 9 by y...@yoav.ws, Aug 31 2015
Cc: japhet@chromium.org
So, with the spec settling down on `as`, here's the implementation plan I have in mind:
* Since such feature with little user-visibility, can pass broken undetected, I intend to get a full blown test suite in place. The test suite should make sure that 
  - preloaded resources are reused and not downloaded twice for *all* content types (ಠ_ಠ => background images)
  - Preload requests sent via Link headers don't get terminated when navigation starts
  - They don't delay onload, get terminated when user navigates away (might be tricky with previous requirement) or when removed from DOM.
  - If anyone is interested in helping out, that would be the best place to start.
* In order to make all the above work, I think we're going to need a new kind of loader (PreloadLoader?) which will resist navigation starts (link PingLoader), will not block onload, etc
* Also, we'd need to change background images loading to make sure they check for previous preloads before loading a new resource, which isn't the case today.

Thoughts?
  - Preload
Comment 10 by y...@yoav.ws, Aug 31 2015
Labels: -M-45 M-47
Cc: igrigo...@chromium.org
Labels: Cr-Internals-Network
+cr-internals-network for feedback.

Re, #9.. 

- test suite: yes, please!
- "will resist navigation starts": can you elaborate a bit more on what you have in mind here?
- Background images: this is CSS specific, and is the issue here that those use an alternate fetch route today?

Comment 13 by y...@yoav.ws, Sep 2 2015
navigation:
Currently, when sending out Link header based preload, the requests are sent out *before* the navigation starts, and then get terminated once it does. That's obviously not the way it should be, so that needs fixing, probably by making these preloads behave more like PingLoader, but only for the navigation of the page they're in (so they won't persist across future navigations).

Background images:
The issue is that they use an alternate fetch route which ignores the possibility of a previous preload, since the preloadScanner doesn't pick them up.
The network stack inside the browser should already handle de-duping connections. If there's a preload in process the background image should share it I think.
Comment 15 by y...@yoav.ws, Nov 4 2015
Blockedon: chromium:549643
Comment 16 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552283
Comment 17 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552288
Comment 18 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552289
Comment 19 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552290
Comment 20 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552291
Comment 21 by y...@yoav.ws, Nov 6 2015
Blockedon: chromium:552294
Comment 22 by y...@yoav.ws, Nov 9 2015
Labels: -Cr-Internals-Network Cr-Blink-Loader
Comment 23 by y...@yoav.ws, Nov 10 2015
Blockedon: chromium:553945
Comment 24 by y...@yoav.ws, Dec 1 2015
Labels: -M-47 M-49
Cc: kenjibaheux@chromium.org
Yoav, I see a handful unresolved issues in the "blocked on" field. Is there an MVP still on track for M49 (crossing fingers)? Any other blockers (spec, conformance tests, ...)? 

Thanks!
Comment 26 by y...@yoav.ws, Jan 8 2016
There are still 3 open issues:
* onload events don't fire for preload links
* resource download doesn't stop when a preload link is removed
* HTMLPreloadScanner doesn't support <link rel=preload>

I'm currently working on the preload scanner support: https://codereview.chromium.org/1563263002/
(simply because it's the easiest to resolve)

While I think an MVP can get by without being able to dynamically terminating preloaded resources, the onload support is fairly important for many of the use cases. Since it's not something that developers can feature detect and work around, I consider it a shipping blocker. Let me know if you think otherwise.

Since it's a pretty big issue, I'm not sure we can get it in time for shipping in 49...

Kenji/Yoav - do you see security review as important for this change?
Comment 28 by y...@yoav.ws, Jan 8 2016
I don't think there's a particular need, as I don't think the API exposes any new security risks. With that said, I don't think it would hurt either :)

Do you folks think that I should send an intent to ship to get things rolling (even if we will wait for onload to be implemented in order to ship)?
Re, intent to ship: yes, modulo two things..

1) we have a plausible path towards addressing the remaining issues (from what I gather, onload mostly)
2) I'd suggest we resolve #7, #34, #37 (GitHub) issues as well.

With that in place I think we're in great shape for an i2s.
Comment 30 by y...@yoav.ws, Jan 12 2016
I now have a path to resolve onload. Hopefully will have a CL for review tomorrow. The dynamic removal issue (https://code.google.com/p/chromium/issues/detail?id=489646) is less critical IMO.

#7 - easy to fix once https://github.com/whatwg/dom/pull/144 lands
#34 and #37 - do you think we should block shipping on them? 
Comment 31 by y...@yoav.ws, Jan 13 2016
Blockedon: chromium:577095
- #7: yep, thanks for working on that!
- #34: I believe this should do it: https://github.com/w3c/preload/pull/40.
- #37: After another once-over, I think Preload spec already has the right bits in place. We need Fetch to update it's type definition: https://github.com/w3c/preload/issues/37#issuecomment-171403547
Comment 33 by y...@yoav.ws, Jan 18 2016
Labels: -M-49 M-50
I doubt we'd make it for M49, so moving the label to M50.
Project Member Comment 34 by bugdroid1@chromium.org, Jan 19 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b08ddc2f37a6463ee7edc3555cb5b1b4e0d71f9

commit 1b08ddc2f37a6463ee7edc3555cb5b1b4e0d71f9
Author: yoav <yoav@yoav.ws>
Date: Tue Jan 19 11:34:45 2016

Added tests for more preload as values

The CL makes sure that all `as` types are properly handled and preloaded.
It only adds tests, without any functional changes.

BUG= 471199 

Review URL: https://codereview.chromium.org/1605503002

Cr-Commit-Position: refs/heads/master@{#370096}

[modify] http://crrev.com/1b08ddc2f37a6463ee7edc3555cb5b1b4e0d71f9/third_party/WebKit/LayoutTests/http/tests/preload/download_resources.html
[modify] http://crrev.com/1b08ddc2f37a6463ee7edc3555cb5b1b4e0d71f9/third_party/WebKit/LayoutTests/http/tests/preload/single_download_preload.html
[add] http://crrev.com/1b08ddc2f37a6463ee7edc3555cb5b1b4e0d71f9/third_party/WebKit/LayoutTests/http/tests/resources/test.oga

Comment 35 by y...@yoav.ws, Jan 21 2016
Blockedon: chromium:568025
Comment 36 by y...@yoav.ws, Jan 21 2016
Blockedon: chromium:579914
Project Member Comment 37 by bugdroid1@chromium.org, Jan 21 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0661feafc9a84f03b04dd3719b8aaa255dfaec63

commit 0661feafc9a84f03b04dd3719b8aaa255dfaec63
Author: yoav <yoav@yoav.ws>
Date: Thu Jan 21 11:04:48 2016

Add use counter for preload link headers

When adding use counters for link headers, preload was left out. This CL
fixes that historical mistake.

BUG= 471199 

Review URL: https://codereview.chromium.org/1602203005

Cr-Commit-Position: refs/heads/master@{#370666}

[modify] http://crrev.com/0661feafc9a84f03b04dd3719b8aaa255dfaec63/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] http://crrev.com/0661feafc9a84f03b04dd3719b8aaa255dfaec63/third_party/WebKit/Source/core/loader/LinkLoader.cpp

Comment 38 by y...@yoav.ws, Jan 23 2016
Blockedon: chromium:580758
Comment 40 by y...@yoav.ws, Jan 28 2016
Status: Fixed
Sign in to add a comment