Move container policy logic to frame owner classes.

third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp

 /*
  * Copyright (C) 2006, 2007, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 190 matching lines @@
 }
 
 DOMWindow* HTMLFrameOwnerElement::contentWindow() const {
   return content_frame_ ? content_frame_->DomWindow() : 0;
 }
 
 void HTMLFrameOwnerElement::SetSandboxFlags(SandboxFlags flags) {
   sandbox_flags_ = flags;
   // Recalculate the container policy in case the allow-same-origin flag has
   // changed.
   container_policy_ = GetContainerPolicyFromAllowedFeatures(

[raymes, 2017/06/07 23:54:38]

Hmm - should this use the same ConstructContainerPolicy code? I don't know this
code well :( If so, does it mean we can remove
GetContainerPolicyFromAllowedFeatures ?

[iclelland, 2017/06/12 17:32:17]

Good catch -- thanks. Updated.

[raymes, 2017/06/12 22:54:26]

Can we remove GetContainerPolicyFromAllowedFeatures from FeaturePolicy.cpp now?
I thin kit's dead code?

       AllowedFeatures(), AllowFullscreen(), AllowPaymentRequest(),
       GetOriginForFeaturePolicy());
 
   // Don't notify about updates if ContentFrame() is null, for example when
   // the subframe hasn't been created yet.
   if (ContentFrame()) {
     GetDocument().GetFrame()->Loader().Client()->DidChangeFramePolicy(
         ContentFrame(), sandbox_flags_, container_policy_);
   }
 }
 
 bool HTMLFrameOwnerElement::IsKeyboardFocusable() const {
   return content_frame_ && HTMLElement::IsKeyboardFocusable();
 }
 
 void HTMLFrameOwnerElement::DisposeFrameOrPluginSoon(
     FrameOrPlugin* frame_or_plugin) {
   if (g_update_suspend_count) {
     FrameOrPluginsPendingDispose().insert(frame_or_plugin);
   } else {
     frame_or_plugin->Dispose();
   }
 }
 
+Vector<WebParsedFeaturePolicyDeclaration>
+HTMLFrameOwnerElement::ConstructContainerPolicy() const {
+  RefPtr<SecurityOrigin> origin = GetOriginForFeaturePolicy();
+  Vector<WebParsedFeaturePolicyDeclaration> whitelists;
+  for (const WebFeaturePolicyFeature feature : AllowedFeatures()) {
+    WebParsedFeaturePolicyDeclaration whitelist;
+    whitelist.feature = feature;
+    whitelist.origins = Vector<WebSecurityOrigin>(1UL, {origin});
+    whitelists.push_back(whitelist);
+  }
+  return whitelists;
+}

[raymes, 2017/06/07 23:54:38]

Is this actually needed or can this be a pure virtual function with subclasses
implementing what they need? If possible that might be preferable, simply just
to reduce the amount of indirection?

[iclelland, 2017/06/12 17:32:17]

i had expected that it would be common code to handle the 'allow' attribute, but
that's only been defined on <iframe>, so maybe it makes sense to implement it
there instead.

I've moved this logic to HTMLIFrameElement, and removed the call to this method
(and the code that compensated for <fullscreen> from HTMLFrameElement. There's a
simple no-specific-container-policy implementation for plugin elements as well.

+
 void HTMLFrameOwnerElement::UpdateContainerPolicy() {
-  container_policy_ = GetContainerPolicyFromAllowedFeatures(
-      AllowedFeatures(), AllowFullscreen(), AllowPaymentRequest(),
-      GetOriginForFeaturePolicy());
+  container_policy_ = ConstructContainerPolicy();
   // Don't notify about updates if ContentFrame() is null, for example when
   // the subframe hasn't been created yet.
   if (ContentFrame()) {
     GetDocument().GetFrame()->Loader().Client()->DidChangeFramePolicy(
         ContentFrame(), sandbox_flags_, container_policy_);
   }
 }
 
 void HTMLFrameOwnerElement::FrameOwnerPropertiesChanged() {
   // Don't notify about updates if ContentFrame() is null, for example when
@@ skipping 111 matching lines @@
 }
 
 DEFINE_TRACE(HTMLFrameOwnerElement) {
   visitor->Trace(content_frame_);
   visitor->Trace(widget_);
   HTMLElement::Trace(visitor);
   FrameOwner::Trace(visitor);
 }
 
 }  // namespace blink