Make content::FeaturePolicy implement WebFeaturePolicy, and use it in blink code

third_party/WebKit/Source/bindings/core/v8/ConditionalFeatures.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ConditionalFeatures.h"
 
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/V8Document.h"
 #include "bindings/core/v8/V8HTMLLinkElement.h"
 #include "bindings/core/v8/V8Navigator.h"
 #include "bindings/core/v8/V8Window.h"
 #include "core/dom/ExecutionContext.h"
-#include "core/frame/LocalFrame.h"
+#include "core/frame/Frame.h"
 #include "core/origin_trials/OriginTrials.h"
+#include "public/platform/Platform.h"
 
 namespace blink {
 
 void installConditionalFeaturesCore(const WrapperTypeInfo* wrapperTypeInfo,
                                     const ScriptState* scriptState,
                                     v8::Local<v8::Object> prototypeObject,
                                     v8::Local<v8::Function> interfaceObject) {
   // TODO(iclelland): Generate all of this logic at compile-time, based on the
   // configuration of origin trial enabled attributes and interfaces in IDL
   // files. (crbug.com/615060)
@@ skipping 109 matching lines @@
 void installPendingConditionalFeature(const String& feature,
                                       const ScriptState* scriptState) {
   DCHECK(scriptState);
   DCHECK(scriptState->context() == scriptState->isolate()->GetCurrentContext());
   DCHECK(scriptState->perContextData());
   DCHECK(scriptState->world().isMainWorld());
 
   (*s_installPendingConditionalFeatureFunction)(feature, scriptState);
 }
 
-bool isFeatureEnabledInFrame(const FeaturePolicy::Feature& feature,
-                             const LocalFrame* frame) {
-  // If there is no frame, or if feature policy is disabled, use defaults.
-  bool enabledByDefault =
-      (feature.defaultPolicy == FeaturePolicy::FeatureDefault::EnableForAll ||
-       (feature.defaultPolicy == FeaturePolicy::FeatureDefault::EnableForSelf &&
-        !frame->isCrossOriginSubframe()));
-  if (!RuntimeEnabledFeatures::featurePolicyEnabled() || !frame)
-    return enabledByDefault;
-  FeaturePolicy* featurePolicy = frame->securityContext()->getFeaturePolicy();
+bool isFeatureEnabledInFrame(WebFeaturePolicyFeature feature,
+                             const Frame* frame) {
+  SecurityOrigin* origin = nullptr;
+  if (frame)
+    origin = frame->securityContext()->getSecurityOrigin();
+
+  // TODO: Remove this check when FP ships. This sets the static policy for
+  // fullscreen and vibrate to be allowed at the top-level, but not in cross-
+  // origin content. With FP enabled, this logic is centralized in the
+  // FeaturePolicy class.
+  if (!RuntimeEnabledFeatures::featurePolicyEnabled() || !frame) {

[raymes, 2017/02/13 04:45:22]

Currently the only callsites I see for this check that FP is enabled prior to
calling this function. Maybe some of these changes could be put into an
orthogonal CL?

[iclelland, 2017/02/23 20:04:12]

Agreed; most of this should be removed; when feature policy is disabled, this
shouldn't be called at all. That will simplify a lot of the code, including the
awkward edge-case handling below.

+    bool isSameOriginSubframe =
+        frame &&
+        (frame->isMainFrame() ||
+         origin->canAccess(
+             frame->tree().top()->securityContext()->getSecurityOrigin()));
+    return (isSameOriginSubframe ||

[raymes, 2017/02/13 04:45:22]

This might be a bit simpler to read as:
if (isSameOriginSubframe) {
  return feature != WebFeaturePolicyFeature::x && 
         feature != WebFeaturePolicyFeature::y;
}
  

[iclelland, 2017/02/23 20:04:12]

Done.

+            !(feature == WebFeaturePolicyFeature::Fullscreen ||

[raymes, 2017/02/13 04:45:22]

Does this mean fullscreen won't work at all in a cross-origin iframe? Shouldn't
it work if the attribute is provided? Same with payments?

[iclelland, 2017/02/23 20:04:12]

That edge case actually isn't ever hit -- it was a rough "is this page cross
origin" check for the features which are disabled by default in cross-origin
frames (like vibrate), to handle the case when FeaturePolicy was not enabled.
Fullscreen won't run this code, though, in that case; if FP is disabled, it just
walks up the frame tree looking for 'allowfullscreen' attributes on frame
owners. Same with payments. This is really just for vibrate. (And then vibrate
went ahead and used the runtime feature check, so this whole block is unused
now.)

+              feature == WebFeaturePolicyFeature::Geolocation ||

[raymes, 2017/02/13 04:45:22]

Should geolocation be here?

[iclelland, 2017/02/23 20:04:12]

It was just all of the features that had a default defined in the spec of
["self"]. 

+              feature == WebFeaturePolicyFeature::Payment ||
+              feature == WebFeaturePolicyFeature::Vibrate));
+  }
+  WebFeaturePolicy* featurePolicy =
+      frame->securityContext()->getFeaturePolicy();
   // The policy should always be initialized before checking it to ensure we
   // properly inherit the parent policy.
   DCHECK(featurePolicy);
 
   // Otherwise, check policy.
-  return featurePolicy->isFeatureEnabled(feature);
+  return Platform::current()->isFeatureEnabledByPolicy(featurePolicy, feature);
 }
 
 }  // namespace blink