Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(379)

Issue 1060863003: Add initial link preload support (Closed)

Created:
2 years, 10 months ago by Yoav Weiss
Modified:
2 years, 10 months ago
Reviewers:
sof, Mike West
CC:
blink-reviews, blink-reviews-html_chromium.org, dglazkov+blink, gavinp+prerender_chromium.org, gavinp+loader_chromium.org, Nate Chapin, tyoshino+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

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 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193605

Patch Set 1 #

Patch Set 2 : build issue fix #

Total comments: 9

Patch Set 3 : Review comments #

Total comments: 2

Patch Set 4 : Review comments #

Patch Set 5 : Fix uninit member :/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -6 lines) Patch
A LayoutTests/fast/dom/HTMLLinkElement/link-preload-validity.html View 1 2 3 1 chunk +15 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLLinkElement/link-preload-validity-expected.txt View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.cpp View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M Source/core/frame/Settings.in View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/HTMLAttributeNames.in View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/HTMLLinkElement.h View 3 chunks +3 lines, -1 line 0 comments Download
M Source/core/html/HTMLLinkElement.cpp View 4 chunks +7 lines, -3 lines 0 comments Download
M Source/core/html/LinkRelAttribute.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/html/LinkRelAttribute.cpp View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M Source/core/loader/LinkLoader.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/loader/LinkLoader.cpp View 1 2 3 2 chunks +35 lines, -1 line 0 comments Download
M Source/core/testing/InternalSettings.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/testing/InternalSettings.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 36 (10 generated)
Yoav Weiss
Hey Mike, Mind taking a look? :)
2 years, 10 months ago (2015-04-05 20:15:08 UTC) #2
Mike West
Can you please: * add some tests for `as` behavior? * link to a spec ...
2 years, 10 months ago (2015-04-07 06:26:28 UTC) #3
Yoav Weiss
Thanks! Added spec link. I didn't add tests for `as` since it doesn't really do ...
2 years, 10 months ago (2015-04-07 07:26:14 UTC) #4
Mike West
https://codereview.chromium.org/1060863003/diff/20001/Source/core/fetch/ResourceFetcher.cpp File Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/1060863003/diff/20001/Source/core/fetch/ResourceFetcher.cpp#newcode338 Source/core/fetch/ResourceFetcher.cpp:338: ASSERT(request.resourceRequest().frameType() == WebURLRequest::FrameTypeNone); On 2015/04/07 at 07:26:14, Yoav Weiss ...
2 years, 10 months ago (2015-04-07 12:23:01 UTC) #5
Yoav Weiss
On 2015/04/07 12:23:01, Mike West (OOO until 8th) wrote: > https://codereview.chromium.org/1060863003/diff/20001/Source/core/fetch/ResourceFetcher.cpp > File Source/core/fetch/ResourceFetcher.cpp (right): ...
2 years, 10 months ago (2015-04-07 12:40:20 UTC) #6
Mike West
LGTM % 'as' and a nit. https://codereview.chromium.org/1060863003/diff/40001/Source/core/loader/LinkLoader.cpp File Source/core/loader/LinkLoader.cpp (right): https://codereview.chromium.org/1060863003/diff/40001/Source/core/loader/LinkLoader.cpp#newcode143 Source/core/loader/LinkLoader.cpp:143: // FIXME: Add ...
2 years, 10 months ago (2015-04-07 19:56:18 UTC) #7
Yoav Weiss
On 2015/04/07 19:56:18, Mike West wrote: > LGTM % 'as' and a nit. > > ...
2 years, 10 months ago (2015-04-09 07:00:31 UTC) #8
Mike West
LGTM, thanks for taking another pass.
2 years, 10 months ago (2015-04-09 07:17:49 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1060863003/60001
2 years, 10 months ago (2015-04-09 09:23:28 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/58507)
2 years, 10 months ago (2015-04-09 22:04:34 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1060863003/60001
2 years, 10 months ago (2015-04-10 05:13:24 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/58674)
2 years, 10 months ago (2015-04-10 18:17:33 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1060863003/60001
2 years, 10 months ago (2015-04-11 18:49:13 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/57064)
2 years, 10 months ago (2015-04-12 04:46:23 UTC) #21
Mike West
That windows bot is unhappy. Might be worth filing an infra bug. :(
2 years, 10 months ago (2015-04-12 05:14:10 UTC) #22
sof
On 2015/04/12 04:46:23, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
2 years, 10 months ago (2015-04-12 05:18:37 UTC) #23
Mike West
Oh. *ahem* I suppose I could have clicked through to read the stdout before blaming ...
2 years, 10 months ago (2015-04-12 05:21:47 UTC) #24
Yoav Weiss
On 2015/04/12 05:18:37, sof wrote: > On 2015/04/12 04:46:23, I haz the power (commit-bot) wrote: ...
2 years, 10 months ago (2015-04-12 05:21:49 UTC) #25
Yoav Weiss
On 2015/04/12 05:21:49, Yoav Weiss wrote: > On 2015/04/12 05:18:37, sof wrote: > > On ...
2 years, 10 months ago (2015-04-12 06:13:00 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1060863003/80001
2 years, 10 months ago (2015-04-12 19:04:04 UTC) #29
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://src.chromium.org/viewvc/blink?view=rev&revision=193605
2 years, 10 months ago (2015-04-12 22:27:39 UTC) #30
sof
yoav wrote: > .. > Uninitialized member var :/ Hmm, -Wuninitialized doesn't catch this statically ...
2 years, 10 months ago (2015-04-13 18:08:00 UTC) #32
Lei Zhang
On 2015/04/13 18:08:00, sof wrote: > yoav wrote: > > > .. > > Uninitialized ...
2 years, 10 months ago (2015-04-13 19:43:42 UTC) #33
Nico
2 years, 10 months ago (2015-04-13 19:59:06 UTC) #34
Nico
grr rietveld. I just wrote a reply, hit "send" and rietveld sent an empty mail ...
2 years, 10 months ago (2015-04-13 20:00:54 UTC) #35
sof
2 years, 10 months ago (2015-04-14 14:26:48 UTC) #36
Message was sent while issue was closed.
On 2015/04/13 20:00:54, Nico (away until Wed Apr 15) wrote:
> grr rietveld. I just wrote a reply, hit "send" and rietveld sent an empty mail
> :-(
> 
> This is surprisingly hard to do. I gave it a fairly serious try a bit over a
> month ago (http://marc.info/?l=cfe-dev&m=142578376706818&w=2 – somewhat
> mojibaked, but comprehensible – lists the heuristics I tried. Let me know if
you
> can think of better ones.)

Oh cool, I had no idea that you had a go at improving matters recently; thanks
for the pointer!

For Blink, I would have (idly) thought that something implementing the first
four restrictions would get you close. Will have to experiment.. :)

Powered by Google App Engine
This is Rietveld 408576698