Introduce WebInsecureRequestPolicy.

third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

Index: third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
index 3f378187b81d6e7ce58f91da81c9143e6dc3ba2b..f705ae0df639d93da35eaaf687017811685eb621 100644
--- a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
@@ -146,10 +146,9 @@ ContentSecurityPolicy::ContentSecurityPolicy()
     , m_scriptHashAlgorithmsUsed(ContentSecurityPolicyHashAlgorithmNone)
     , m_styleHashAlgorithmsUsed(ContentSecurityPolicyHashAlgorithmNone)
     , m_sandboxMask(0)
-    , m_enforceStrictMixedContentChecking(false)
     , m_referrerPolicy(ReferrerPolicyDefault)
     , m_treatAsPublicAddress(false)
-    , m_insecureRequestsPolicy(SecurityContext::InsecureRequestsDoNotUpgrade)
+    , m_insecureRequestPolicy(kLeaveInsecureRequestsAlone)
 {
 }
 
@@ -184,16 +183,16 @@ void ContentSecurityPolicy::applyPolicySideEffectsToExecutionContext()
             UseCounter::count(document, UseCounter::SandboxViaCSP);
             document->enforceSandboxFlags(m_sandboxMask);
         }
-        if (m_enforceStrictMixedContentChecking)
-            document->enforceStrictMixedContentChecking();
         if (m_treatAsPublicAddress)
             document->setAddressSpace(WebAddressSpacePublic);
-        if (m_insecureRequestsPolicy == SecurityContext::InsecureRequestsUpgrade) {
+        if (m_insecureRequestPolicy & kUpgradeInsecureRequests) {

[Yoav Weiss, 2016/06/07 10:30:23]

Can we move the bitwise and/or logic to WebInsecureRequestPolicy.h?

Here they feel kinda magical, so I'd much rather see that logic encapsulated in
a single place (ideally as a class. Alternatively, as static functions that
operate on an input WebInsecureRequestPolicy)

[Mike West, 2016/06/07 10:39:49]

Hrm. What's magical about it? It's a bitfield, which seems like a pretty common
sort of thing. It's not clear to me that something like
`shouldUpgradeInsecureRequests(m_insecureRequestPolicy)` is going to be easier
to understand than checking the relevant bit.

I'd like not to wrap it in a class, as this data will eventually need to be
replicated through `//content/common/frame_replication_state.h` (replacing
`FrameReplicationState::should_enforce_strict_mixed_content_checking`). Simple
types are simple to replicate.

Also, note that the eventual goal is to turn `WebInsecureRequestPolicy` into an
enum. I can't do that at the moment because of exciting bugs that make `upgrade
| block` behave differently than `upgrade`. This patch is first in the line of
patches that will get rid of that behavioral difference by moving much of the
processing up to the browser process.

             UseCounter::count(document, UseCounter::UpgradeInsecureRequestsEnabled);
-            document->setInsecureRequestsPolicy(m_insecureRequestsPolicy);
+            document->setInsecureRequestsPolicy(SecurityContext::InsecureRequestsUpgrade);
             if (!securityOrigin->host().isNull())
                 document->addInsecureNavigationUpgrade(securityOrigin->host().impl()->hash());
         }
+        if (m_insecureRequestPolicy & kBlockAllMixedContent)
+            document->enforceStrictMixedContentChecking();

[Yoav Weiss, 2016/06/07 10:30:23]

Is there a particular reason this bit moved from above the "upgrade" check to be
below it?

[Mike West, 2016/06/07 10:39:49]

I just wanted to keep all the request policy bits together. The next patch folds
both of these into a single call, so it simplified the diff. *shrug* Happy to
move it back if you think it's clearer.

 
         for (const auto& consoleMessage : m_consoleMessages)
             m_executionContext->addConsoleMessage(consoleMessage);
@@ -743,11 +742,6 @@ void ContentSecurityPolicy::enforceSandboxFlags(SandboxFlags mask)
     m_sandboxMask |= mask;
 }
 
-void ContentSecurityPolicy::enforceStrictMixedContentChecking()
-{
-    m_enforceStrictMixedContentChecking = true;
-}
-
 void ContentSecurityPolicy::treatAsPublicAddress()
 {
     if (!RuntimeEnabledFeatures::corsRFC1918Enabled())
@@ -755,10 +749,14 @@ void ContentSecurityPolicy::treatAsPublicAddress()
     m_treatAsPublicAddress = true;
 }
 
-void ContentSecurityPolicy::setInsecureRequestsPolicy(SecurityContext::InsecureRequestsPolicy policy)
+void ContentSecurityPolicy::enforceStrictMixedContentChecking()
+{
+    m_insecureRequestPolicy |= kBlockAllMixedContent;
+}
+
+void ContentSecurityPolicy::upgradeInsecureRequests()
 {
-    if (policy > m_insecureRequestsPolicy)
-        m_insecureRequestsPolicy = policy;
+    m_insecureRequestPolicy |= kUpgradeInsecureRequests;
 }
 
 static String stripURLForUseInReport(Document* document, const KURL& url, RedirectStatus redirectStatus)