Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(173)

Issue 1131513004: Revert of Added <link rel=preconnect> crossorigin attribute (Closed)

Created:
5 years, 7 months ago by Yoav Weiss
Modified:
5 years, 7 months ago
Reviewers:
Dirk Pranke, 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

Revert of Added <link rel=preconnect> crossorigin attribute (patchset #2 id:20001 of https://codereview.chromium.org/1138743002/) Reason for revert: The patch seems to be causing preconnect related crashes: https://build.chromium.org/p/chromium.webkit/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/13855 Reverting until I get a clearer picture as to why that is the case. Original issue's description: > Added <link rel=preconnect> crossorigin attribute > > In order to support preconnect for both anonymous and non-anonymous connection pools we need to add support for the crossorigin attribute in <link rel=preconnect>. > This CL implements the core functionality of this change on the Blink side. > > BUG=468005 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195184 TBR=mkwst@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=468005 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195200

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -168 lines) Patch
D LayoutTests/fast/dom/HTMLLinkElement/link-preconnect-crossorigin.html View 1 chunk +0 lines, -14 lines 0 comments Download
D LayoutTests/fast/dom/HTMLLinkElement/link-preconnect-crossorigin-expected.txt View 1 chunk +0 lines, -9 lines 0 comments Download
D LayoutTests/fast/dom/HTMLLinkElement/link-preconnect-valid.html View 1 chunk +0 lines, -11 lines 0 comments Download
D LayoutTests/fast/dom/HTMLLinkElement/link-preconnect-valid-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/core.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
D Source/core/html/CrossOriginAttribute.h View 1 chunk +0 lines, -19 lines 0 comments Download
D Source/core/html/CrossOriginAttribute.cpp View 1 chunk +0 lines, -20 lines 0 comments Download
M Source/core/loader/LinkHeader.h View 3 chunks +0 lines, -4 lines 0 comments Download
M Source/core/loader/LinkHeader.cpp View 7 chunks +7 lines, -18 lines 0 comments Download
M Source/core/loader/LinkHeaderTest.cpp View 3 chunks +5 lines, -31 lines 0 comments Download
M Source/core/loader/LinkLoader.cpp View 4 chunks +5 lines, -12 lines 0 comments Download
M Source/platform/network/NetworkHints.h View 1 chunk +1 line, -2 lines 0 comments Download
M Source/platform/network/NetworkHints.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
D public/platform/WebCrossOriginAttribute.h View 1 chunk +0 lines, -19 lines 0 comments Download
M public/platform/WebPrescientNetworking.h View 2 chunks +0 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
Yoav Weiss
Created Revert of Added <link rel=preconnect> crossorigin attribute
5 years, 7 months ago (2015-05-11 20:56:42 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1131513004/1
5 years, 7 months ago (2015-05-11 20:57:07 UTC) #2
Dirk Pranke
lgtm
5 years, 7 months ago (2015-05-11 20:57:36 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=195200
5 years, 7 months ago (2015-05-11 20:57:46 UTC) #5
Mike West
LGTM. Thanks for reverting. Happy to look at a second attempt when you have a ...
5 years, 7 months ago (2015-05-12 03:10:59 UTC) #6
Yoav Weiss
5 years, 7 months ago (2015-05-12 07:05:03 UTC) #7
Message was sent while issue was closed.
On 2015/05/12 03:10:59, Mike West wrote:
> LGTM. Thanks for reverting. Happy to look at a second attempt when you have a
> fix.

I see the crash locally, and see what change caused it. It's the fact that we
temporarily broke the preconnect functionality that this test is testing (which
causes it to time out). I'll resubmit without calling the new interface for
preconnect (so without breaking the preconnect functionality)

Powered by Google App Engine
This is Rietveld 408576698