Move most Feature Policy code into content/

content/child/blink_platform_impl.cc

Index: content/child/blink_platform_impl.cc
diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc
index 7c6dfda8dbe3b236e86e401a012c9ad968530544..ef1703731c00c409395fa99bc1102a33da21c364 100644
--- a/content/child/blink_platform_impl.cc
+++ b/content/child/blink_platform_impl.cc
@@ -39,6 +39,7 @@
 #include "content/app/strings/grit/content_strings.h"
 #include "content/child/child_thread_impl.h"
 #include "content/child/content_child_helpers.h"
+#include "content/child/feature_policy/feature_policy_platform.h"
 #include "content/child/notifications/notification_dispatcher.h"
 #include "content/child/notifications/notification_manager.h"
 #include "content/child/push_messaging/push_dispatcher.h"
@@ -856,4 +857,24 @@ int BlinkPlatformImpl::domKeyEnumFromString(const WebString& key_string) {
       ui::KeycodeConverter::KeyStringToDomKey(key_string.utf8()));
 }
 
+blink::WebFeaturePolicy* BlinkPlatformImpl::createFeaturePolicy(
+    const blink::WebFeaturePolicy* parentPolicy,
+    const blink::WebParsedFeaturePolicyHeader& policyHeader,
+    const blink::WebSecurityOrigin& origin) {
+  std::unique_ptr<FeaturePolicy> policy = FeaturePolicy::CreateFromParentPolicy(
+      static_cast<const FeaturePolicy*>(parentPolicy), url::Origin(origin));
+  policy->SetHeaderPolicy(FeaturePolicyHeaderFromWeb(policyHeader));

[raymes, 2017/01/19 02:40:57]

It looks like we never need to access anything in the
WebParsedFeaturePolicyHeader in blink. So I was thinking we could potentially
make it similar to WebFeaturePolicy in the sense that the parsing code lives in
content/ and returns an opaque pointer to the header which can be passed around
in blink, but always just ends up going back into content. This might save us
needing these conversion functions to go to/from blink/content classes. It would
also save needing to expose those structs publicly. What do you think about that
approach? Are there some downsides I'm missing?

[iclelland, 2017/01/19 05:40:40]

I think that might make sense as the final step in this move -- I'm not 100%
certain, though, so I haven't done it yet.

Technically moving it isn't a big deal; It means switching to a different JSON
parser, using the one in base, likely, and switching to using all of those
types, which should be a pretty mechanical change.

The bigger difference to me is that it moves header parsing out of blink, which
(I believe) is different than what we do for all other headers.

+  return policy.release();
+}
+
+bool BlinkPlatformImpl::isFeatureEnabledByPolicy(

[raymes, 2017/01/19 02:40:56]

I don't have a problem with this being a standalone function at all, but I was
curious why it was easier to do it this way (than to have the function on the
WebFeaturePolicy object)?

[iclelland, 2017/01/19 05:40:40]

That caused me some grief -- it seems like the obvious way to do it, as a
virtual method on WebFeaturePolicy, taking a WebSecurityOrigin. Implementing
that method in content/common requires linking against all of the code for that
class, and  related classes, from Blink -- that's the reason I was getting
compile failures in the first patch.

I figured the best way out of that mess is to define all of the functions that
deal with WebSecurityOrigin in content/{child,renderer} instead, and have
content/common work only with url::Origin.

(I'll talk to mkwst@ about the plans for unifying the two classes so that we can
eliminate a lot of the conversion functions, but until that happens, I think
this is the cleanest way.)

+    const blink::WebFeaturePolicy* policy,
+    blink::WebFeaturePolicyFeature feature,
+    const blink::WebSecurityOrigin& origin) {
+  //  if (!policy)
+  //    return FeaturePolicy::IsFeatureAllowedByDefault("");

[raymes, 2017/01/19 02:40:56]

Hmm, what is this bit for?

[iclelland, 2017/01/19 05:40:40]

Thanks, that's not moved yet -- it was a placeholder to define the behaviour for
features in case there was no feature policy on the document at all.
That behaviour is actually implemented in
ConditionalFeatures::isFeatureEnabledInFrame, for the case where the runtime
flag is disabled, but I was considering moving it into content/.

+  return static_cast<const FeaturePolicy*>(policy)->IsFeatureEnabledForOrigin(
+      feature, url::Origin(origin));
+}
+
 }  // namespace content