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

Issue 18497002: [Refactoring] Extract CachedResourceLoader::canAccess() (Closed)

Created:
7 years, 5 months ago by Hajime Morrita
Modified:
7 years, 5 months ago
Reviewers:
abarth-chromium
CC:
blink-reviews, dglazkov+blink, Nate Chapin, eae+blinkwatch, adamk+blink_chromium.org, gavinp+loader_chromium.org
Visibility:
Public.

Description

[Refactoring] Extract CachedResourceLoader::canAccess() This moves origin-based access control decision from ScriptElement to CachedResourceLoader. Now the flag from the CORS-setting attribute @crossorigin is stored as an ResourceLoaderOption instead of ScriptElement::m_usesRequestControl and the decision is made by CachedResouceLoader::canAccess() instead of ScriptElement::notifyDone(). As a result, direct dependency to ResoruceRequest from ScriptElement is also fade away. This abstraction will be used by HTML Imports as well. TEST=none BUG=240592 R=abarth@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153471

Patch Set 1 #

Total comments: 2

Patch Set 2 : For landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -15 lines) Patch
M Source/core/dom/ScriptElement.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/dom/ScriptElement.cpp View 4 chunks +2 lines, -11 lines 0 comments Download
M Source/core/loader/ResourceLoaderOptions.h View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/loader/cache/CachedResource.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/loader/cache/CachedResourceLoader.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/loader/cache/CachedResourceLoader.cpp View 1 1 chunk +24 lines, -0 lines 0 comments Download
M Source/core/loader/cache/CachedResourceRequest.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/loader/cache/CachedResourceRequest.cpp View 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Hajime Morrita
This is a variation of https://codereview.chromium.org/18211006/
7 years, 5 months ago (2013-07-02 09:05:31 UTC) #1
abarth-chromium
LGTM https://codereview.chromium.org/18497002/diff/1/Source/core/loader/cache/CachedResourceLoader.cpp File Source/core/loader/cache/CachedResourceLoader.cpp (right): https://codereview.chromium.org/18497002/diff/1/Source/core/loader/cache/CachedResourceLoader.cpp#newcode434 Source/core/loader/cache/CachedResourceLoader.cpp:434: && !resource->passesAccessControlCheck(m_document->securityOrigin(), error)) { Technically we need to ...
7 years, 5 months ago (2013-07-02 18:41:56 UTC) #2
Hajime Morrita
On 2013/07/02 18:41:56, abarth wrote: > LGTM > > https://codereview.chromium.org/18497002/diff/1/Source/core/loader/cache/CachedResourceLoader.cpp > File Source/core/loader/cache/CachedResourceLoader.cpp (right): > ...
7 years, 5 months ago (2013-07-03 00:36:26 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/morrita@chromium.org/18497002/7001
7 years, 5 months ago (2013-07-03 00:50:17 UTC) #4
Hajime Morrita
Committed patchset #2 manually as r153471 (presubmit successful).
7 years, 5 months ago (2013-07-03 05:22:45 UTC) #5
Hajime Morrita
After a quick look, I found that it isn't trivial to make redirect chain working, ...
7 years, 5 months ago (2013-07-03 08:34:35 UTC) #6
abarth-chromium
7 years, 5 months ago (2013-07-03 18:14:33 UTC) #7
Message was sent while issue was closed.
On 2013/07/03 08:34:35, morrita1 wrote:
> I'm not sure how critical this is.

It's good to have the bug on file, but I wouldn't panic about fixing it.

Powered by Google App Engine
This is Rietveld 408576698