Move 'frame-src' CSP checks into FrameFetchContext.

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Index: third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
index 57f5d624403456f6e2518b595e38f9a6ed975c5a..f72bd679a2d4b412db0f3b233bec133682640911 100644
--- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
@@ -526,6 +526,23 @@ ResourceRequestBlockedReason FrameFetchContext::canRequestInternal(Resource::Typ
     // I believe it's the Resource::Raw case.
     const ContentSecurityPolicy* csp = m_document ? m_document->contentSecurityPolicy() : nullptr;
 
+    // If we're loading a frame, grab it's parent's policy for 'frame-src' checks:
+    if (!csp && type == Resource::MainResource) {

[Yoav Weiss, 2016/06/02 09:15:14]

I understand that the assumption here is that csp is NULL (because m_document
is) when we're navigating to a frame.
Can you make that assumption explicit (comment, etc)?

+        if (Frame* parentFrame = frame()->tree().parent()) {
+            csp = parentFrame->securityContext()->contentSecurityPolicy();
+            if (!csp->allowChildFrameFromSource(url, redirectStatus, cspReporting)) {
+                // TODO(mkwst): If we cancel the request after a redirect, we never instantiate
+                // a document, and therefore don't inherit the loader's sandbox flags, or trigger
+                // a load event. This is strange.
+                if (redirectStatus == ResourceRequest::RedirectStatus::FollowedRedirect) {
+                    frame()->document()->enforceSandboxFlags(SandboxOrigin);
+                    frame()->owner()->dispatchLoad();
+                }
+                return ResourceRequestBlockedReasonCSP;
+            }

[Yoav Weiss, 2016/06/02 09:15:14]

should we return here? Or nullify csp? Otherwise, we've change csp to the
parent's, which may change code paths further down the function? (e.g. we'd call
csp->allowRequest when we previously didn't) I'm not sure that outcome would be
different, but still..

Alternatively, isolating the frame logic in a helper function would help making
sure it doesn't have such side effects on the rest of the function.

+        }
+    }
+
     if (csp) {
         if (!shouldBypassMainWorldCSP && !csp->allowRequest(resourceRequest.requestContext(), url, redirectStatus, cspReporting))
             return ResourceRequestBlockedReasonCSP;