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 500752 b6174ebcb60c2e4f981215bc99fa37025c11a7a0
parent 500751 53958ec4b3c9c48ee650d2a6e470e44fa47fc8d0
child 500753 8784cdbf16d36c6e10a969bfaf47725ecec3b080
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1500448
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;