Add UseCounter for old allow attribute syntax

[email protected] (cherry picked from commit a74fa69) Bug: 726739, 761009 Change-Id: Ifd0a8e7a020e20b86fe218b607ac69a041b73ce3 Reviewed-on: https://chromium-review.googlesource.com/643759 Commit-Queue: Luna Lu <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#499172} Reviewed-on: https://chromium-review.googlesource.com/655226 Reviewed-by: Ian Clelland <[email protected]> Cr-Commit-Position: refs/branch-heads/3202@{#65} Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
nwjs · Sep 7, 2017 · 3fef18a · 3fef18a
1 parent 687bc1b
commit 3fef18a
Show file tree

Hide file tree

Showing 13 changed files with 69 additions and 29 deletions.
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp b/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp
@@ -77,7 +77,7 @@ void HTMLFrameElement::ParseAttribute(
 }
 
 Vector<WebParsedFeaturePolicyDeclaration>
-HTMLFrameElement::ConstructContainerPolicy(Vector<String>*) const {
+HTMLFrameElement::ConstructContainerPolicy(Vector<String>*, bool*) const {
   // Frame elements are not allowed to enable the fullscreen feature. Add an
   // empty whitelist for the fullscreen feature so that the framed content is
   // unable to use the API, regardless of origin.

diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElement.h b/third_party/WebKit/Source/core/html/HTMLFrameElement.h
@@ -40,7 +40,8 @@ class CORE_EXPORT HTMLFrameElement final : public HTMLFrameElementBase {
   bool NoResize() const;
 
   Vector<WebParsedFeaturePolicyDeclaration> ConstructContainerPolicy(
-      Vector<String>*) const override;
+      Vector<String>* /* messages */,
+      bool* /* old_syntax */) const override;
 
  private:
   explicit HTMLFrameElement(Document&);

diff --git a/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp b/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp
@@ -144,7 +144,7 @@ void HTMLFrameOwnerElement::SetSandboxFlags(SandboxFlags flags) {
   sandbox_flags_ = flags;
   // Recalculate the container policy in case the allow-same-origin flag has
   // changed.
-  container_policy_ = ConstructContainerPolicy(nullptr);
+  container_policy_ = ConstructContainerPolicy(nullptr, nullptr);
 
   // Don't notify about updates if ContentFrame() is null, for example when
   // the subframe hasn't been created yet.
@@ -165,8 +165,9 @@ void HTMLFrameOwnerElement::DisposePluginSoon(PluginView* plugin) {
     plugin->Dispose();
 }
 
-void HTMLFrameOwnerElement::UpdateContainerPolicy(Vector<String>* messages) {
-  container_policy_ = ConstructContainerPolicy(messages);
+void HTMLFrameOwnerElement::UpdateContainerPolicy(Vector<String>* messages,
+                                                  bool* old_syntax) {
+  container_policy_ = ConstructContainerPolicy(messages, old_syntax);
   // Don't notify about updates if ContentFrame() is null, for example when
   // the subframe hasn't been created yet.
   if (ContentFrame()) {

diff --git a/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h b/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h
@@ -131,12 +131,19 @@ class CORE_EXPORT HTMLFrameOwnerElement : public HTMLElement,
   // Return a feature policy container policy for this frame, based on the
   // frame attributes and the effective origin specified in the frame
   // attributes.
+  // If |old_syntax| (bool*) is not null, it will be set true if the deprecated
+  // space-deparated feature list syntax is detected.
+  // TODO(loonybear): remove the boolean once the space separated feature list
+  // syntax is deprecated.
+  // https://crbug.com/761009.
   virtual Vector<WebParsedFeaturePolicyDeclaration> ConstructContainerPolicy(
-      Vector<String>*) const = 0;
+      Vector<String>* /*  messages */,
+      bool* /* old_syntax */) const = 0;
 
   // Update the container policy and notify the frame loader client of any
   // changes.
-  void UpdateContainerPolicy(Vector<String>* messages = nullptr);
+  void UpdateContainerPolicy(Vector<String>* messages = nullptr,
+                             bool* old_syntax = nullptr);
 
  private:
   // Intentionally private to prevent redundant checks when the type is

diff --git a/third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp b/third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp
@@ -178,15 +178,23 @@ void HTMLIFrameElement::ParseAttribute(
     if (allow_ != value) {
       allow_ = value;
       Vector<String> messages;
-      UpdateContainerPolicy(&messages);
-      UseCounter::Count(GetDocument(),
-                        WebFeature::kFeaturePolicyAllowAttribute);
+      bool old_syntax = false;
+      UpdateContainerPolicy(&messages, &old_syntax);
       if (!messages.IsEmpty()) {
         for (const String& message : messages) {
           GetDocument().AddConsoleMessage(ConsoleMessage::Create(
               kOtherMessageSource, kWarningMessageLevel, message));
         }
       }
+
+      if (old_syntax) {
+        UseCounter::Count(
+            GetDocument(),
+            WebFeature::kFeaturePolicyAllowAttributeDeprecatedSyntax);
+      } else {
+        UseCounter::Count(GetDocument(),
+                          WebFeature::kFeaturePolicyAllowAttribute);
+      }
     }
   } else {
     if (name == srcAttr)
@@ -196,11 +204,13 @@ void HTMLIFrameElement::ParseAttribute(
 }
 
 Vector<WebParsedFeaturePolicyDeclaration>
-HTMLIFrameElement::ConstructContainerPolicy(Vector<String>* messages) const {
+HTMLIFrameElement::ConstructContainerPolicy(Vector<String>* messages,
+                                            bool* old_syntax) const {
   RefPtr<SecurityOrigin> src_origin = GetOriginForFeaturePolicy();
   RefPtr<SecurityOrigin> self_origin = GetDocument().GetSecurityOrigin();
   Vector<WebParsedFeaturePolicyDeclaration> container_policy =
-      ParseFeaturePolicyAttribute(allow_, self_origin, src_origin, messages);
+      ParseFeaturePolicyAttribute(allow_, self_origin, src_origin, messages,
+                                  old_syntax);
 
   // If allowfullscreen attribute is present and no fullscreen policy is set,
   // enable the feature for all origins.

diff --git a/third_party/WebKit/Source/core/html/HTMLIFrameElement.h b/third_party/WebKit/Source/core/html/HTMLIFrameElement.h
@@ -46,7 +46,8 @@ class CORE_EXPORT HTMLIFrameElement final
   DOMTokenList* sandbox() const;
 
   Vector<WebParsedFeaturePolicyDeclaration> ConstructContainerPolicy(
-      Vector<String>* messages = nullptr) const override;
+      Vector<String>* /* messages */,
+      bool* /* old_syntax */) const override;
 
  private:
   explicit HTMLIFrameElement(Document&);

diff --git a/third_party/WebKit/Source/core/html/HTMLIFrameElementTest.cpp b/third_party/WebKit/Source/core/html/HTMLIFrameElementTest.cpp
@@ -212,7 +212,7 @@ TEST_F(HTMLIFrameElementTest, AllowAttributeContainerPolicy) {
   EXPECT_EQ(1UL, container_policy2[1].origins.size());
   EXPECT_EQ("http://example.net", container_policy2[1].origins[0].ToString());
 
-  // TODO(lunalu): Remove this test when deprecating the old syntax.
+  // TODO(loonybear): Remove this test when deprecating the old syntax.
   // Test for supporting old allow syntax.
   frame_element->setAttribute(HTMLNames::allowAttr, "payment fullscreen");
 
@@ -296,7 +296,7 @@ TEST_F(HTMLIFrameElementTest, ConstructEmptyContainerPolicy) {
   HTMLIFrameElement* frame_element = HTMLIFrameElement::Create(*document);
 
   WebParsedFeaturePolicy container_policy =
-      frame_element->ConstructContainerPolicy(nullptr);
+      frame_element->ConstructContainerPolicy(nullptr, nullptr);
   EXPECT_EQ(0UL, container_policy.size());
 }
 
@@ -311,7 +311,7 @@ TEST_F(HTMLIFrameElementTest, ConstructContainerPolicy) {
   HTMLIFrameElement* frame_element = HTMLIFrameElement::Create(*document);
   frame_element->setAttribute(HTMLNames::allowAttr, "payment; usb");
   WebParsedFeaturePolicy container_policy =
-      frame_element->ConstructContainerPolicy(nullptr);
+      frame_element->ConstructContainerPolicy(nullptr, nullptr);
   EXPECT_EQ(2UL, container_policy.size());
   EXPECT_EQ(WebFeaturePolicyFeature::kPayment, container_policy[0].feature);
   EXPECT_FALSE(container_policy[0].matches_all_origins);
@@ -339,7 +339,7 @@ TEST_F(HTMLIFrameElementTest, ConstructContainerPolicyWithAllowFullscreen) {
   frame_element->SetBooleanAttribute(HTMLNames::allowfullscreenAttr, true);
 
   WebParsedFeaturePolicy container_policy =
-      frame_element->ConstructContainerPolicy(nullptr);
+      frame_element->ConstructContainerPolicy(nullptr, nullptr);
   EXPECT_EQ(1UL, container_policy.size());
   EXPECT_EQ(WebFeaturePolicyFeature::kFullscreen, container_policy[0].feature);
   EXPECT_TRUE(container_policy[0].matches_all_origins);
@@ -358,7 +358,7 @@ TEST_F(HTMLIFrameElementTest, ConstructContainerPolicyWithAllowPaymentRequest) {
   frame_element->SetBooleanAttribute(HTMLNames::allowpaymentrequestAttr, true);
 
   WebParsedFeaturePolicy container_policy =
-      frame_element->ConstructContainerPolicy(nullptr);
+      frame_element->ConstructContainerPolicy(nullptr, nullptr);
   EXPECT_EQ(2UL, container_policy.size());
   EXPECT_EQ(WebFeaturePolicyFeature::kUsb, container_policy[0].feature);
   EXPECT_FALSE(container_policy[0].matches_all_origins);
@@ -388,7 +388,7 @@ TEST_F(HTMLIFrameElementTest, ConstructContainerPolicyWithAllowAttributes) {
   frame_element->SetBooleanAttribute(HTMLNames::allowpaymentrequestAttr, true);
 
   WebParsedFeaturePolicy container_policy =
-      frame_element->ConstructContainerPolicy(nullptr);
+      frame_element->ConstructContainerPolicy(nullptr, nullptr);
   EXPECT_EQ(3UL, container_policy.size());
   EXPECT_EQ(WebFeaturePolicyFeature::kPayment, container_policy[0].feature);
   EXPECT_FALSE(container_policy[0].matches_all_origins);

diff --git a/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp b/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp
@@ -269,7 +269,7 @@ bool HTMLPlugInElement::ShouldAccelerate() const {
 }
 
 Vector<WebParsedFeaturePolicyDeclaration>
-HTMLPlugInElement::ConstructContainerPolicy(Vector<String>*) const {
+HTMLPlugInElement::ConstructContainerPolicy(Vector<String>*, bool*) const {
   // Plugin elements (<object> and <embed>) are not allowed to enable the
   // fullscreen feature. Add an empty whitelist for the fullscreen feature so
   // that the nested browsing context is unable to use the API, regardless of

diff --git a/third_party/WebKit/Source/core/html/HTMLPlugInElement.h b/third_party/WebKit/Source/core/html/HTMLPlugInElement.h
@@ -84,7 +84,8 @@ class CORE_EXPORT HTMLPlugInElement
   void CreatePluginWithoutLayoutObject();
 
   virtual Vector<WebParsedFeaturePolicyDeclaration> ConstructContainerPolicy(
-      Vector<String>*) const;
+      Vector<String>* /* messages */,
+      bool* /* old_syntax */) const;
 
  protected:
   HTMLPlugInElement(const QualifiedName& tag_name,

diff --git a/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp b/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.cpp
@@ -15,7 +15,7 @@
 namespace blink {
 
 namespace {
-// TODO(lunalu): Deprecate the methods in this namesapce when deprecating old
+// TODO(loonybear): Deprecate the methods in this namesapce when deprecating old
 // allow syntax.
 bool IsValidOldAllowSyntax(const String& policy,
                            RefPtr<SecurityOrigin> src_origin) {
@@ -92,22 +92,27 @@ Vector<WebParsedFeaturePolicyDeclaration> ParseFeaturePolicyAttribute(
     const String& policy,
     RefPtr<SecurityOrigin> self_origin,
     RefPtr<SecurityOrigin> src_origin,
-    Vector<String>* messages) {
+    Vector<String>* messages,
+    bool* old_syntax) {
   return ParseFeaturePolicy(policy, self_origin, src_origin, messages,
-                            GetDefaultFeatureNameMap());
+                            GetDefaultFeatureNameMap(), old_syntax);
 }
 
 Vector<WebParsedFeaturePolicyDeclaration> ParseFeaturePolicy(
     const String& policy,
     RefPtr<SecurityOrigin> self_origin,
     RefPtr<SecurityOrigin> src_origin,
     Vector<String>* messages,
-    const FeatureNameMap& feature_names) {
+    const FeatureNameMap& feature_names,
+    bool* old_syntax) {
   // Temporarily supporting old allow syntax:
   //     allow = "feature1 feature2 feature3 ... "
-  // TODO(lunalu): depracate this old syntax in the future.
-  if (IsValidOldAllowSyntax(policy, src_origin))
+  // TODO(loonybear): depracate this old syntax in the future.
+  if (IsValidOldAllowSyntax(policy, src_origin)) {
+    if (old_syntax)
+      *old_syntax = true;
     return ParseOldAllowSyntax(policy, src_origin, messages, feature_names);
+  }
 
   Vector<WebParsedFeaturePolicyDeclaration> whitelists;
   BitVector features_specified(

diff --git a/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.h b/third_party/WebKit/Source/platform/feature_policy/FeaturePolicy.h
@@ -40,22 +40,34 @@ ParseFeaturePolicyHeader(const String& policy,
 // input will cause as warning message to be appended to it.
 // Example of a feature policy string:
 //     "vibrate a.com 'src'; fullscreen 'none'; payment 'self', payment *".
+// If |old_syntax| is not null, it will be set true if the deprecated
+// space-deparated feature list syntax is detected.
+// TODO(loonybear): remove the boolean once the space separated feature list
+// syntax is deprecated.
+// https://crbug.com/761009.
 PLATFORM_EXPORT Vector<WebParsedFeaturePolicyDeclaration>
 ParseFeaturePolicyAttribute(const String& policy,
                             RefPtr<SecurityOrigin> self_origin,
                             RefPtr<SecurityOrigin> src_origin,
-                            Vector<String>* messages);
+                            Vector<String>* messages,
+                            bool* old_syntax);
 
 // Converts a feature policy string into a vector of whitelists (see comments
 // above), with an explicit FeatureNameMap. This algorithm is called by both
 // header policy parsing and container policy parsing. |self_origin| and
 // |src_origin| are both nullable.
+// If |old_syntax| is not null, it will be set true if the deprecated
+// space-deparated feature list syntax is detected.
+// TODO(loonybear): remove the boolean once the space separated feature list
+// syntax is deprecated.
+// https://crbug.com/761009.
 PLATFORM_EXPORT Vector<WebParsedFeaturePolicyDeclaration> ParseFeaturePolicy(
     const String& policy,
     RefPtr<SecurityOrigin> self_origin,
     RefPtr<SecurityOrigin> src_origin,
     Vector<String>* messages,
-    const FeatureNameMap& feature_names);
+    const FeatureNameMap& feature_names,
+    bool* old_syntax = nullptr);
 
 // Verifies whether feature policy is enabled and |feature| is supported in
 // feature policy.

diff --git a/third_party/WebKit/public/platform/web_feature.mojom b/third_party/WebKit/public/platform/web_feature.mojom
@@ -1677,6 +1677,7 @@ enum WebFeature {
   kWebkitBoxLineClampOneChildIsLayoutBlockFlowInline = 2142,
   kWebkitBoxLineClampManyChildren = 2143,
   kWebkitBoxLineClampDoesSomething = 2144,
+  kFeaturePolicyAllowAttributeDeprecatedSyntax = 2145,
 
   // Add new features immediately above this line. Don't change assigned
   // numbers of any item, and don't reuse removed slots.

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -16268,6 +16268,7 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="2142" label="WebkitBoxLineClampOneChildIsLayoutBlockFlowInline"/>
   <int value="2143" label="WebkitBoxLineClampManyChildren"/>
   <int value="2144" label="WebkitBoxLineClampDoesSomething"/>
+  <int value="2145" label="FeaturePolicyAllowAttributeDeprecatedSyntax"/>
 </enum>
 
 <enum name="FeedbackSource">