Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2602233002:
Prefetch Link header support. (Closed)
|
Created:
3 years, 11 months ago by Yoav Weiss Modified:
3 years, 11 months ago Reviewers:
Charlie Harrison CC:
chromium-reviews, tyoshino+watch_chromium.org, gavinp+prerender_chromium.org, loading-reviews_chromium.org, gavinp+loader_chromium.org, blink-reviews, Nate Chapin Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPrefetch Link header support.
This CL adds support for Link header based prefetch.
Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0EwAJ
BUG=658274
Committed: https://crrev.com/f5845f6d9b9b916af212b904a88117ae39a5f514
Cr-Commit-Position: refs/heads/master@{#441165}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Test based of perfObserver #
Messages
Total messages: 23 (15 generated)
The CQ bit was checked by yoav@yoav.ws to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Prefetch Link header support. BUG= ========== to ========== Prefetch Link header support. This CL adds support for Link header based prefetch. BUG=658274 ==========
yoav@yoav.ws changed reviewers: + csharrison@chromium.org
Hey Charlie, A small patch to review as a warm up after the holidays :) I'll also send an I2S for the feature
Description was changed from ========== Prefetch Link header support. This CL adds support for Link header based prefetch. BUG=658274 ========== to ========== Prefetch Link header support. This CL adds support for Link header based prefetch. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0... BUG=658274 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm % nit and approval for I2S https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php (right): https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php:13: <script src="/resources/slow-script.pl?delay=200"></script> Why do we need to load this script at all?
On 2017/01/01 18:36:05, Charlie Harrison (back Jan 3) wrote: > lgtm % nit and approval for I2S > > https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... > File third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php > (right): > > https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... > third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php:13: > <script src="/resources/slow-script.pl?delay=200"></script> > Why do we need to load this script at all? It's basically a timeout to delay the onload event. Many preload tests are using the same method. At the same time, I'll try to switch to a PerformanceObserver based way to check this kind of downloads actually happened, as this method runs a risk of flakiness on extremely slow bots.
The CQ bit was checked by yoav@yoav.ws to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/01/01 20:00:46, Yoav Weiss wrote: > On 2017/01/01 18:36:05, Charlie Harrison (back Jan 3) wrote: > > lgtm % nit and approval for I2S > > > > > https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... > > File > third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php > > (right): > > > > > https://codereview.chromium.org/2602233002/diff/1/third_party/WebKit/LayoutTe... > > > third_party/WebKit/LayoutTests/http/tests/prefetch/link_header_prefetch.php:13: > > <script src="/resources/slow-script.pl?delay=200"></script> > > Why do we need to load this script at all? > > It's basically a timeout to delay the onload event. Many preload tests are using > the same method. > > At the same time, I'll try to switch to a PerformanceObserver based way to check > this kind of downloads actually happened, as this method runs a risk of > flakiness on extremely slow bots. modified tests to get rid of timeout (I should probably do the same for preload's tests).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
still lgtm. I was thinking that the prefetch delays the onload event but of course it doesn't :P
The CQ bit was checked by yoav@yoav.ws
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483463246533090,
"parent_rev": "e0ddd88af0b5089c15bccbb25c336a48885ec73a", "commit_rev":
"a7b7ea161aaa321cef6d1f8443fb756305d191d9"}
Message was sent while issue was closed.
Description was changed from ========== Prefetch Link header support. This CL adds support for Link header based prefetch. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0... BUG=658274 ========== to ========== Prefetch Link header support. This CL adds support for Link header based prefetch. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0... BUG=658274 Review-Url: https://codereview.chromium.org/2602233002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Prefetch Link header support. This CL adds support for Link header based prefetch. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0... BUG=658274 Review-Url: https://codereview.chromium.org/2602233002 ========== to ========== Prefetch Link header support. This CL adds support for Link header based prefetch. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/8Zo2HiNEs94/h8mDVkx0... BUG=658274 Committed: https://crrev.com/f5845f6d9b9b916af212b904a88117ae39a5f514 Cr-Commit-Position: refs/heads/master@{#441165} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f5845f6d9b9b916af212b904a88117ae39a5f514 Cr-Commit-Position: refs/heads/master@{#441165} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
