Bug 1500448 - Avoid the parsing of the FeaturePolicy 'allow' attribute when not needed, r=ckerschb
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 20 Oct 2018 06:08:46 +0200
changeset 490461 b6174ebcb60c2e4f981215bc99fa37025c11a7a0
parent 490460 53958ec4b3c9c48ee650d2a6e470e44fa47fc8d0
child 490572 8784cdbf16d36c6e10a969bfaf47725ecec3b080
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersckerschb
bugs1500448
milestone64.0a1
Bug 1500448 - Avoid the parsing of the FeaturePolicy 'allow' attribute when not needed, r=ckerschb
dom/html/HTMLIFrameElement.cpp
dom/html/HTMLIFrameElement.h
--- a/dom/html/HTMLIFrameElement.cpp
+++ b/dom/html/HTMLIFrameElement.cpp
@@ -69,17 +69,17 @@ HTMLIFrameElement::BindToTree(nsIDocumen
 {
   nsresult rv = nsGenericHTMLFrameElement::BindToTree(aDocument, aParent,
                                                       aBindingParent);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   if (StaticPrefs::dom_security_featurePolicy_enabled()) {
-    RefreshFeaturePolicy();
+    RefreshFeaturePolicy(true /* parse the feature policy attribute */);
   }
   return NS_OK;
 }
 
 bool
 HTMLIFrameElement::ParseAttribute(int32_t aNamespaceID,
                                   nsAtom* aAttribute,
                                   const nsAString& aValue,
@@ -185,23 +185,27 @@ HTMLIFrameElement::AfterSetAttr(int32_t 
     if (aName == nsGkAtoms::sandbox) {
       if (mFrameLoader) {
         // If we have an nsFrameLoader, apply the new sandbox flags.
         // Since this is called after the setter, the sandbox flags have
         // alreay been updated.
         mFrameLoader->ApplySandboxFlags(GetSandboxFlags());
       }
     }
-    if ((aName == nsGkAtoms::allow ||
-         aName == nsGkAtoms::src ||
-         aName == nsGkAtoms::srcdoc ||
-         aName == nsGkAtoms::sandbox ||
-         aName == nsGkAtoms::allowpaymentrequest) &&
-        StaticPrefs::dom_security_featurePolicy_enabled()) {
-      RefreshFeaturePolicy();
+
+    if (StaticPrefs::dom_security_featurePolicy_enabled()) {
+      if (aName == nsGkAtoms::allow ||
+          aName == nsGkAtoms::src ||
+          aName == nsGkAtoms::srcdoc ||
+          aName == nsGkAtoms::sandbox) {
+        RefreshFeaturePolicy(true /* parse the feature policy attribute */);
+      } else if (aName == nsGkAtoms::allowfullscreen ||
+                 aName == nsGkAtoms::allowpaymentrequest) {
+        RefreshFeaturePolicy(false /* parse the feature policy attribute */);
+      }
     }
   }
   return nsGenericHTMLFrameElement::AfterSetAttr(aNameSpaceID, aName,
                                                  aValue, aOldValue,
                                                  aMaybeScriptedPrincipal,
                                                  aNotify);
 }
 
@@ -274,43 +278,43 @@ HTMLIFrameElement::GetFeaturePolicyDefau
   if (!principal) {
     principal = NodePrincipal();
   }
 
   return principal.forget();
 }
 
 void
-HTMLIFrameElement::RefreshFeaturePolicy()
+HTMLIFrameElement::RefreshFeaturePolicy(bool aParseAllowAttribute)
 {
   MOZ_ASSERT(StaticPrefs::dom_security_featurePolicy_enabled());
-  mFeaturePolicy->ResetDeclaredPolicy();
+
+  if (aParseAllowAttribute) {
+    mFeaturePolicy->ResetDeclaredPolicy();
 
-  // The origin can change if 'src' and 'srcdoc' attributes change.
-  nsCOMPtr<nsIPrincipal> origin = GetFeaturePolicyDefaultOrigin();
-  MOZ_ASSERT(origin);
-  mFeaturePolicy->SetDefaultOrigin(origin);
+    // The origin can change if 'src' and 'srcdoc' attributes change.
+    nsCOMPtr<nsIPrincipal> origin = GetFeaturePolicyDefaultOrigin();
+    MOZ_ASSERT(origin);
+    mFeaturePolicy->SetDefaultOrigin(origin);
 
-  nsAutoString allow;
-  GetAttr(nsGkAtoms::allow, allow);
+    nsAutoString allow;
+    GetAttr(nsGkAtoms::allow, allow);
 
-  if (!allow.IsEmpty()) {
-    // Set or reset the FeaturePolicy directives.
-    mFeaturePolicy->SetDeclaredPolicy(OwnerDoc(), allow, NodePrincipal(),
-                                      origin);
+    if (!allow.IsEmpty()) {
+      // Set or reset the FeaturePolicy directives.
+      mFeaturePolicy->SetDeclaredPolicy(OwnerDoc(), allow, NodePrincipal(),
+                                        origin);
+    }
+
+    mFeaturePolicy->InheritPolicy(OwnerDoc()->Policy());
   }
 
-  mFeaturePolicy->InheritPolicy(OwnerDoc()->Policy());
-
   if (AllowPaymentRequest()) {
     mFeaturePolicy->MaybeSetAllowedPolicy(NS_LITERAL_STRING("payment"));
   }
 
   if (AllowFullscreen()) {
     mFeaturePolicy->MaybeSetAllowedPolicy(NS_LITERAL_STRING("fullscreen"));
   }
-
-  // TODO: https://wicg.github.io/feature-policy/#process-feature-policy-attributes
-  // requires to check allowusermediarequest
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/html/HTMLIFrameElement.h
+++ b/dom/html/HTMLIFrameElement.h
@@ -218,17 +218,17 @@ protected:
                                           bool aNotify) override;
 
 private:
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     MappedDeclarations&);
 
   static const DOMTokenListSupportedToken sSupportedSandboxTokens[];
 
-  void RefreshFeaturePolicy();
+  void RefreshFeaturePolicy(bool aParseAllowAttribute);
 
   // If this iframe has a 'srcdoc' attribute, the document's origin will be
   // returned. Otherwise, if this iframe has a 'src' attribute, the origin will
   // be the parsing of its value as URL. If the URL is invalid, or 'src'
   // attribute doesn't exist, the origin will be the document's origin.
   already_AddRefed<nsIPrincipal>
   GetFeaturePolicyDefaultOrigin() const;