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

Issue 137983010: (Re)organize handling of CORS access control during resource loading. (Closed)

Created:
6 years, 11 months ago by sof
Modified:
6 years, 11 months ago
CC:
blink-reviews, nessy, gavinp+loader_chromium.org, gasubic, eae+blinkwatch, ed+blinkwatch_opera.com, eric.carlson_apple.com, fs, dglazkov+blink, apavlov+blink_chromium.org, adamk+blink_chromium.org, darktears, vcarbune.chromium, philipj_slow, Inactive
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

(Re)organize handling of CORS access control during resource loading. Common up and simplify the various pieces of code that handles CORS access control, avoiding per resource type handling. That is, a ResourceLoader's host will now be called upon by the loader when it receives the initial response. The host deciding if a response should be allowed loaded or not: bool ResourceLoaderHost::canAccessResource(Resource*, const KURL&); If not, the loader cancels the loading of the resource and signals an error. The loader host is responsible for emitting a suitable console error message. Switch over the 'loader' resource clients to rely on that, avoiding custom CORS checks (and error reporting) for scripts, imports, images, text tracks. As the resource loader now needs to know if a load operation is CORS enabled or not, this is now/again recorded by ResourceLoaderOptions. R=abarth@chromium.org,japhet@chromium.org BUG=178787 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165877

Patch Set 1 #

Patch Set 2 : Remove inappropriate DEFINE_STATIC_LOCAL() use #

Patch Set 3 : HTMLImportLoader no longer needs a ResourceFetcher #

Unified diffs Side-by-side diffs Delta from patch set Stats (+197 lines, -146 lines) Patch
M LayoutTests/http/tests/htmlimports/cross-origin-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/htmlimports/redirect-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/http/tests/security/img-with-failed-cors-check-fails-to-load-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/resources/cors-script.php View 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/security/script-crossorigin-fails-cross-origin-2-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
A LayoutTests/http/tests/security/script-crossorigin-loads-correctly-dual.html View 1 chunk +24 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/security/script-crossorigin-loads-correctly-dual-expected.txt View 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/security/text-track-crossorigin-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/http/tests/security/video-poster-cross-origin-crash-expected.txt View 1 chunk +1 line, -2 lines 0 comments Download
M LayoutTests/http/tests/security/video-poster-cross-origin-crash2-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSImageSetValue.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSImageSetValue.cpp View 3 chunks +4 lines, -4 lines 0 comments Download
M Source/core/css/CSSImageValue.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/css/CSSImageValue.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/css/resolver/StyleResourceLoader.cpp View 3 chunks +6 lines, -5 lines 0 comments Download
M Source/core/dom/ScriptLoader.h View 3 chunks +0 lines, -8 lines 0 comments Download
M Source/core/dom/ScriptLoader.cpp View 5 chunks +2 lines, -19 lines 0 comments Download
M Source/core/fetch/FetchRequest.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/fetch/Resource.h View 1 2 chunks +3 lines, -0 lines 0 comments Download
M Source/core/fetch/Resource.cpp View 2 chunks +59 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.h View 4 chunks +3 lines, -8 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.cpp View 5 chunks +17 lines, -20 lines 0 comments Download
M Source/core/fetch/ResourceLoader.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceLoader.cpp View 2 chunks +18 lines, -2 lines 0 comments Download
M Source/core/fetch/ResourceLoaderHost.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/fetch/ResourceLoaderOptions.h View 4 chunks +10 lines, -0 lines 0 comments Download
M Source/core/html/HTMLImportChild.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLImportLoader.h View 1 2 3 chunks +5 lines, -3 lines 0 comments Download
M Source/core/html/HTMLImportLoader.cpp View 1 2 3 chunks +4 lines, -12 lines 0 comments Download
M Source/core/html/parser/HTMLScriptRunner.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/loader/ImageLoader.cpp View 3 chunks +4 lines, -24 lines 0 comments Download
M Source/core/loader/TextTrackLoader.h View 1 chunk +1 line, -2 lines 0 comments Download
M Source/core/loader/TextTrackLoader.cpp View 1 3 chunks +7 lines, -18 lines 0 comments Download
M Source/core/xml/parser/XMLDocumentParser.cpp View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
sof
At your own leisure, please take a look. Some changes to try to make CORS ...
6 years, 11 months ago (2014-01-26 22:50:27 UTC) #1
abarth-chromium
LGTM This approach looks much better. Thanks!
6 years, 11 months ago (2014-01-27 18:00:43 UTC) #2
Nate Chapin
On 2014/01/27 18:00:43, abarth wrote: > LGTM > > This approach looks much better. Thanks! ...
6 years, 11 months ago (2014-01-27 18:08:14 UTC) #3
sof
On 2014/01/27 18:08:14, Nate Chapin wrote: > On 2014/01/27 18:00:43, abarth wrote: > > LGTM ...
6 years, 11 months ago (2014-01-27 18:43:47 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/137983010/50002
6 years, 11 months ago (2014-01-27 18:44:09 UTC) #5
commit-bot: I haz the power
6 years, 11 months ago (2014-01-27 19:58:48 UTC) #6
Message was sent while issue was closed.
Change committed as 165877

Powered by Google App Engine
This is Rietveld 408576698