Adding Embedding-CSP HTTP header

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

Index: third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
index 745036a4cf56f3586a19b52e29144c95dc97e656..effca0c363b4d1377ed97a7cb1cd7987ada410cc 100644
--- a/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
@@ -37,6 +37,7 @@
 #include "core/frame/FrameOwner.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/Settings.h"
+#include "core/html/HTMLIFrameElement.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/EmptyClients.h"
 #include "core/page/Page.h"
@@ -163,9 +164,9 @@ class FrameFetchContextDisplayedCertificateErrorsTest
   KURL mainResourceUrl;
 };
 
-class FrameFetchContextUpgradeTest : public FrameFetchContextTest {
+class FrameFetchContextModifyRequestTest : public FrameFetchContextTest {

[Mike West, 2016/10/06 13:30:21]

I would have expected you to need to set the runtime enabled flag for this
feature for these tests to pass... Do they pass? If so, what am I missing? :)

[amalika, 2016/10/06 18:54:57]

From my understanding this might be because site_per_proces_browsertests
explicitly overrides SetUpCommandLine in 
https://cs.chromium.org/chromium/src/content/browser/site_per_process_browser...
? 
But not too sure. I tried looking for confirmation but couldnt find it in 15
minutes or so I stopped... But I double checked with printf statement and the
flag is set to true in FrameFetchContext test. (Also in that file
setCorsRFC1918Enabled flag is explicitly set to false first so I am guessing all
RuntimeEnabledFeatures are set by default for tests?
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/Fr...)

  public:
-  FrameFetchContextUpgradeTest()
+  FrameFetchContextModifyRequestTest()
       : exampleOrigin(SecurityOrigin::create(
             KURL(ParsedURLString, "https://example.test/"))),
         secureOrigin(SecurityOrigin::create(
@@ -189,7 +190,7 @@ class FrameFetchContextUpgradeTest : public FrameFetchContextTest {
     fetchRequest.mutableResourceRequest().setRequestContext(requestContext);
     fetchRequest.mutableResourceRequest().setFrameType(frameType);
 
-    fetchContext->upgradeInsecureRequest(fetchRequest.mutableResourceRequest());
+    fetchContext->modifyRequestForCSP(fetchRequest.mutableResourceRequest());
 
     EXPECT_EQ(expectedURL.getString(),
               fetchRequest.resourceRequest().url().getString());
@@ -202,9 +203,9 @@ class FrameFetchContextUpgradeTest : public FrameFetchContextTest {
     EXPECT_EQ(expectedURL.path(), fetchRequest.resourceRequest().url().path());
   }
 
-  void expectHTTPSHeader(const char* input,
-                         WebURLRequest::FrameType frameType,
-                         bool shouldPrefer) {
+  void expectUpgradeInsecureRequestHeader(const char* input,
+                                          WebURLRequest::FrameType frameType,
+                                          bool shouldPrefer) {
     KURL inputURL(ParsedURLString, input);
 
     FetchRequest fetchRequest =
@@ -213,27 +214,59 @@ class FrameFetchContextUpgradeTest : public FrameFetchContextTest {
         WebURLRequest::RequestContextScript);
     fetchRequest.mutableResourceRequest().setFrameType(frameType);
 
-    fetchContext->upgradeInsecureRequest(fetchRequest.mutableResourceRequest());
+    fetchContext->modifyRequestForCSP(fetchRequest.mutableResourceRequest());
 
     EXPECT_EQ(shouldPrefer ? String("1") : String(),
               fetchRequest.resourceRequest().httpHeaderField(
                   HTTPNames::Upgrade_Insecure_Requests));
 
-    // Calling upgradeInsecureRequest more than once shouldn't affect the
+    // Calling modifyRequestForCSP more than once shouldn't affect the
     // header.
     if (shouldPrefer) {
-      fetchContext->upgradeInsecureRequest(
-          fetchRequest.mutableResourceRequest());
+      fetchContext->modifyRequestForCSP(fetchRequest.mutableResourceRequest());
       EXPECT_EQ("1", fetchRequest.resourceRequest().httpHeaderField(
                          HTTPNames::Upgrade_Insecure_Requests));
     }
   }
 
+  void expectSetEmbeddingCSPRequestHeader(
+      const char* input,
+      WebURLRequest::FrameType frameType,
+      const AtomicString& expectedEmbeddingCSP) {
+    KURL inputURL(ParsedURLString, input);
+
+    FetchRequest fetchRequest =
+        FetchRequest(ResourceRequest(inputURL), FetchInitiatorInfo());
+    fetchRequest.mutableResourceRequest().setRequestContext(
+        WebURLRequest::RequestContextScript);
+    fetchRequest.mutableResourceRequest().setFrameType(frameType);
+
+    fetchContext->modifyRequestForCSP(fetchRequest.mutableResourceRequest());
+
+    EXPECT_EQ(expectedEmbeddingCSP,
+              fetchRequest.resourceRequest().httpHeaderField(
+                  HTTPNames::Embedding_CSP));
+  }
+
+  const AtomicString& setFrameOwnerBasedOnFrameType(
+      WebURLRequest::FrameType frameType,
+      HTMLIFrameElement* iframe,
+      const AtomicString& potentialValue) {
+    if (frameType == WebURLRequest::FrameTypeNested) {

[Mike West, 2016/10/06 13:30:21]

Nit: We generally try to do the quick exit first. So, perhaps reverse this into:

```
if (frameType != WebURLRequest::FrameTypeNested) {
  document->frame()->setOwner(nullptr);
  return nullAtom;
}

iframe->setAttribute(...);
...
return ...;
```

+      iframe->setAttribute(HTMLNames::cspAttr, potentialValue);
+      document->frame()->setOwner(iframe);
+      return potentialValue;
+    }
+
+    document->frame()->setOwner(0);

[Mike West, 2016/10/06 13:30:21]

Nit: s/0/nullptr/

+    return nullAtom;
+  }
+
   RefPtr<SecurityOrigin> exampleOrigin;
   RefPtr<SecurityOrigin> secureOrigin;
 };
 
-TEST_F(FrameFetchContextUpgradeTest, UpgradeInsecureResourceRequests) {
+TEST_F(FrameFetchContextModifyRequestTest, UpgradeInsecureResourceRequests) {
   struct TestCase {
     const char* original;
     const char* upgraded;
@@ -291,7 +324,8 @@ TEST_F(FrameFetchContextUpgradeTest, UpgradeInsecureResourceRequests) {
   }
 }
 
-TEST_F(FrameFetchContextUpgradeTest, DoNotUpgradeInsecureResourceRequests) {
+TEST_F(FrameFetchContextModifyRequestTest,
+       DoNotUpgradeInsecureResourceRequests) {
   FrameFetchContext::provideDocumentToContext(*fetchContext, document.get());
   document->setSecurityOrigin(secureOrigin);
   document->setInsecureRequestPolicy(kLeaveInsecureRequestsAlone);
@@ -317,7 +351,7 @@ TEST_F(FrameFetchContextUpgradeTest, DoNotUpgradeInsecureResourceRequests) {
                 "ftp://example.test:1212/image.png");
 }
 
-TEST_F(FrameFetchContextUpgradeTest, SendHTTPSHeader) {
+TEST_F(FrameFetchContextModifyRequestTest, SendUpgradeInsecureRequestHeader) {
   struct TestCase {
     const char* toRequest;
     WebURLRequest::FrameType frameType;
@@ -340,20 +374,68 @@ TEST_F(FrameFetchContextUpgradeTest, SendHTTPSHeader) {
   // the tests both before and after providing a document to the context.
   for (const auto& test : tests) {
     document->setInsecureRequestPolicy(kLeaveInsecureRequestsAlone);
-    expectHTTPSHeader(test.toRequest, test.frameType, test.shouldPrefer);
+    expectUpgradeInsecureRequestHeader(test.toRequest, test.frameType,
+                                       test.shouldPrefer);
 
     document->setInsecureRequestPolicy(kUpgradeInsecureRequests);
-    expectHTTPSHeader(test.toRequest, test.frameType, test.shouldPrefer);
+    expectUpgradeInsecureRequestHeader(test.toRequest, test.frameType,
+                                       test.shouldPrefer);
   }
 
   FrameFetchContext::provideDocumentToContext(*fetchContext, document.get());
 
   for (const auto& test : tests) {
     document->setInsecureRequestPolicy(kLeaveInsecureRequestsAlone);
-    expectHTTPSHeader(test.toRequest, test.frameType, test.shouldPrefer);
+    expectUpgradeInsecureRequestHeader(test.toRequest, test.frameType,
+                                       test.shouldPrefer);
 
     document->setInsecureRequestPolicy(kUpgradeInsecureRequests);
-    expectHTTPSHeader(test.toRequest, test.frameType, test.shouldPrefer);
+    expectUpgradeInsecureRequestHeader(test.toRequest, test.frameType,
+                                       test.shouldPrefer);
+  }
+}
+
+TEST_F(FrameFetchContextModifyRequestTest, SendExpectedEmbeddingCSPHeader) {
+  struct TestCase {
+    const char* toRequest;
+    WebURLRequest::FrameType frameType;
+  } tests[] = {
+      {"https://example.test/page.html", WebURLRequest::FrameTypeAuxiliary},
+      {"https://example.test/page.html", WebURLRequest::FrameTypeNested},
+      {"https://example.test/page.html", WebURLRequest::FrameTypeNone},
+      {"https://example.test/page.html", WebURLRequest::FrameTypeTopLevel}};
+
+  HTMLIFrameElement* iframe = HTMLIFrameElement::create(*document);
+  const AtomicString& requiredCSP = AtomicString("default-src 'none'");
+  const AtomicString& anotherRequiredCSP = AtomicString("default-src 'self'");
+
+  // This should work correctly both when the FrameFetchContext has a Document,
+  // and when it doesn't (e.g. during main frame navigations), so run through
+  // the tests both before and after providing a document to the context.

[Mike West, 2016/10/06 13:30:21]

I don't think this comment is accurate for this header.
`Upgrade-Insecure-Requests: 1` is sent for all navigational requests, top-level
or nested. This new header is sent only for navigational requests in a nested
context. I agree that the behavior should be the same regardless of whether or
not a `Document` is present, but I'm not sure we can ever do a nested load
without one. :)

+  for (const auto& test : tests) {
+    AtomicString expectedRequiredCSP =
+        setFrameOwnerBasedOnFrameType(test.frameType, iframe, requiredCSP);
+    expectSetEmbeddingCSPRequestHeader(test.toRequest, test.frameType,

[Mike West, 2016/10/06 13:30:21]

Using the return value to set the expectation is somewhat more magical than I
think is comprehensible. :) Would you consider something like:

```
expectSetEmbeddingCSPRequestHeader(test.toRequest, test.frameType,
    test.frameType == WebURLRequest::FrameTypeNested ? requiredCSP : nullAtom);

```

?

+                                       expectedRequiredCSP);
+
+    expectedRequiredCSP = setFrameOwnerBasedOnFrameType(test.frameType, iframe,
+                                                        anotherRequiredCSP);
+    expectSetEmbeddingCSPRequestHeader(test.toRequest, test.frameType,
+                                       expectedRequiredCSP);
+  }
+
+  FrameFetchContext::provideDocumentToContext(*fetchContext, document.get());
+
+  for (const auto& test : tests) {
+    AtomicString expectedRequiredCSP =
+        setFrameOwnerBasedOnFrameType(test.frameType, iframe, requiredCSP);
+    expectSetEmbeddingCSPRequestHeader(test.toRequest, test.frameType,
+                                       expectedRequiredCSP);
+
+    expectedRequiredCSP = setFrameOwnerBasedOnFrameType(test.frameType, iframe,
+                                                        anotherRequiredCSP);
+    expectSetEmbeddingCSPRequestHeader(test.toRequest, test.frameType,
+                                       expectedRequiredCSP);
   }
 }