Allow cross-origin cssRules access to CORS-fetched stylesheet.

Allow cross-origin cssRules access to CORS-fetched stylesheet.

When accessing the cssRules property on CSSStyleSheet an origin check is
performed for non-inline stylesheets. Should that stylesheet have been
loaded from another origin following CORS, the access ought to be
permitted, but wasn't.

Address by having the link element (LinkStyle) record if the stylesheet
it creates was successfully fetched using CORS. If it was, the stylesheet
access check will succeed.

R=mkwst
BUG=467672
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192066

[sof, 2015-03-17 17:03:08]

sigbjornf@opera.com changed reviewers:
+ japhet@chromium.org, mkwst@chromium.org

[sof, 2015-03-17 17:03:09]

Please take a look.

Enables cross-origin script access to these stylesheets, so we need to be very
careful here. The access is pinned to the originally accessing origin of the
CORS stylesheet, so I believe this restricts matters sufficiently.

[Mike West, 2015-03-17 20:05:46]

LGTM % comments. Thanks!

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
File Source/core/html/HTMLLinkElement.cpp (right):

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
Source/core/html/HTMLLinkElement.cpp:684: if
(!m_owner->document().securityOrigin()->canRequest(baseURL))
I'm not sure it's useful to check canRequest here, as opposed to setting the
origin regardless. If we got here, we passed CORS, right? Since
`CSSStyleSheet::canAccessRules` checks canRequest anyway, it seems like we're
just doing extra work here. Adding a ref to the SecurityOrigin unconditionally
seems cheaper than doing the canRequest() check.

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
File Source/core/html/HTMLLinkElement.h (right):

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
Source/core/html/HTMLLinkElement.h:110: ASSERT(!m_fetchFollowingCORS);
Why ASSERT here?

[sof, 2015-03-17 21:52:15]

Thanks for finding the time for the careful review.

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
File Source/core/html/HTMLLinkElement.cpp (right):

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
Source/core/html/HTMLLinkElement.cpp:684: if
(!m_owner->document().securityOrigin()->canRequest(baseURL))
On 2015/03/17 20:05:46, Mike West wrote:
> I'm not sure it's useful to check canRequest here, as opposed to setting the
> origin regardless. If we got here, we passed CORS, right? Since
> `CSSStyleSheet::canAccessRules` checks canRequest anyway, it seems like we're
> just doing extra work here. Adding a ref to the SecurityOrigin unconditionally
> seems cheaper than doing the canRequest() check.

Yes, it serves no purpose to do this check here (now), but the CORS-laced fetch
may end up being to a same-origin resource, so no guarantee that we have passed
any CORS access check on the response at this point. But the check is redundant
given how it is used on the other side. Dropped.

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
File Source/core/html/HTMLLinkElement.h (right):

https://codereview.chromium.org/1011103002/diff/60001/Source/core/html/HTMLLi...
Source/core/html/HTMLLinkElement.h:110: ASSERT(!m_fetchFollowingCORS);
On 2015/03/17 20:05:46, Mike West wrote:
> Why ASSERT here?

Just sanity checking that it represents a state transition.

[sof, 2015-03-18 07:27:58]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-03-18 07:27:58]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1011103002/#ps100001
(title: "compile fix..")

[commit-bot: I haz the power, 2015-03-18 07:28:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1011103002/100001

[commit-bot: I haz the power, 2015-03-18 07:31:16]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192066