Replicate a parsed feature policy representation so it doesn't need to be parsed in the browser pro…

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index 5e0a3e295f57422ac552dc5c5b2bfc1134d2f387..318d947cf55af837004b34631deb185b4b5aab54 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -635,8 +635,28 @@ class SitePerProcessFeaturePolicyBrowserTest
     command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
                                     "FeaturePolicy");
   }
+
+  std::vector<FeaturePolicyParsedWhitelist> CreateExpectedFeaturePolicyHeader(
+      const std::string& feature_name,
+      bool matches_all_origins,
+      const std::vector<std::string>& origins) {
+    std::vector<FeaturePolicyParsedWhitelist> result(1);
+    result[0].feature_name = feature_name;
+    result[0].matches_all_origins = matches_all_origins;
+    for (const std::string& origin_string : origins)
+      result[0].origins.push_back(url::Origin(GURL(origin_string)));
+    return result;
+  }
 };
 
+bool operator==(const FeaturePolicyParsedWhitelist& first,

[alexmos, 2016/12/01 02:04:19]

Any value in putting this next to the struct definition in
frame_replication_state, or do you envision this being useful only in this test?

[raymes, 2016/12/01 03:03:09]

Right now it's only used from the test and I can't see a use outside from it
atm, so I was thinking to leave it here. We could always move it out if needed.

[alexmos, 2016/12/01 22:27:54]

Acknowledged.

+                const FeaturePolicyParsedWhitelist& second) {
+  return std::tie(first.feature_name, first.matches_all_origins,
+                  first.origins) == std::tie(second.feature_name,
+                                             second.matches_all_origins,
+                                             second.origins);
+}
+
 double GetFrameDeviceScaleFactor(const ToRenderFrameHost& adapter) {
   double device_scale_factor;
   const char kGetFrameDeviceScaleFactor[] =
@@ -8686,19 +8706,21 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessFeaturePolicyBrowserTest,
   EXPECT_TRUE(NavigateToURL(shell(), start_url));
 
   FrameTreeNode* root = web_contents()->GetFrameTree()->root();
-  EXPECT_EQ("{\"vibrate\":[\"self\"]}",
+  EXPECT_EQ(CreateExpectedFeaturePolicyHeader("vibrate", false,

[alexmos, 2016/12/01 02:04:19]

nit: the tests might be a bit more readable if instead of
CreateExpectedFeaturePolicyHeader you had two helpers: one that takes the
feature name + origins vector, and another for the matches_all_origins case that
takes just the feature name.

[raymes, 2016/12/01 03:03:09]

Done.

+                                              {start_url.GetOrigin().spec()}),
             root->current_replication_state().feature_policy_header);
 
   // When the main frame navigates to a page with a new policy, it should
   // overwrite the old one.
   EXPECT_TRUE(NavigateToURL(shell(), first_nav_url));
-  EXPECT_EQ("{\"vibrate\":[\"*\"]}",
+  EXPECT_EQ(CreateExpectedFeaturePolicyHeader("vibrate", true,
+                                              std::vector<std::string>()),
             root->current_replication_state().feature_policy_header);
 
   // When the main frame navigates to a page without a policy, the replicated
   // policy header should be cleared.
   EXPECT_TRUE(NavigateToURL(shell(), second_nav_url));
-  EXPECT_EQ("", root->current_replication_state().feature_policy_header);
+  EXPECT_TRUE(root->current_replication_state().feature_policy_header.empty());
 }
 
 IN_PROC_BROWSER_TEST_F(SitePerProcessFeaturePolicyBrowserTest,
@@ -8712,19 +8734,21 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessFeaturePolicyBrowserTest,
   EXPECT_TRUE(NavigateToURL(shell(), start_url));
 
   FrameTreeNode* root = web_contents()->GetFrameTree()->root();
-  EXPECT_EQ("{\"vibrate\":[\"self\"]}",
+  EXPECT_EQ(CreateExpectedFeaturePolicyHeader("vibrate", false,
+                                              {start_url.GetOrigin().spec()}),
             root->current_replication_state().feature_policy_header);
 
   // When the main frame navigates to a page with a new policy, it should
   // overwrite the old one.
   EXPECT_TRUE(NavigateToURL(shell(), first_nav_url));
-  EXPECT_EQ("{\"vibrate\":[\"*\"]}",
+  EXPECT_EQ(CreateExpectedFeaturePolicyHeader("vibrate", true,
+                                              std::vector<std::string>()),
             root->current_replication_state().feature_policy_header);
 
   // When the main frame navigates to a page without a policy, the replicated
   // policy header should be cleared.
   EXPECT_TRUE(NavigateToURL(shell(), second_nav_url));
-  EXPECT_EQ("", root->current_replication_state().feature_policy_header);
+  EXPECT_TRUE(root->current_replication_state().feature_policy_header.empty());
 }
 
 // Test that the replicated feature policy header is correct in subframes as
@@ -8740,28 +8764,34 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessFeaturePolicyBrowserTest,
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
 
   FrameTreeNode* root = web_contents()->GetFrameTree()->root();
-  EXPECT_EQ("{\"vibrate\":[\"self\", \"http://example.com/\"]}",
+  EXPECT_EQ(CreateExpectedFeaturePolicyHeader(
+                "vibrate", false,
+                {main_url.GetOrigin().spec(), "http://example.com/"}),
             root->current_replication_state().feature_policy_header);
   EXPECT_EQ(1UL, root->child_count());
   EXPECT_EQ(
-      "{\"vibrate\":[\"self\"]}",
+      CreateExpectedFeaturePolicyHeader("vibrate", false,
+                                        {main_url.GetOrigin().spec()}),
       root->child_at(0)->current_replication_state().feature_policy_header);
 
   // Navigate the iframe cross-site.
   NavigateFrameToURL(root->child_at(0), first_nav_url);
   EXPECT_EQ(
-      "{\"vibrate\":[\"*\"]}",
+      CreateExpectedFeaturePolicyHeader("vibrate", true,
+                                        std::vector<std::string>()),
       root->child_at(0)->current_replication_state().feature_policy_header);
 
   // Navigate the iframe to another location, this one with no policy header
   NavigateFrameToURL(root->child_at(0), second_nav_url);
-  EXPECT_EQ(
-      "", root->child_at(0)->current_replication_state().feature_policy_header);
+  EXPECT_TRUE(root->child_at(0)
+                  ->current_replication_state()
+                  .feature_policy_header.empty());
 
   // Navigate the iframe back to a page with a policy
   NavigateFrameToURL(root->child_at(0), first_nav_url);
   EXPECT_EQ(
-      "{\"vibrate\":[\"*\"]}",
+      CreateExpectedFeaturePolicyHeader("vibrate", true,
+                                        std::vector<std::string>()),
       root->child_at(0)->current_replication_state().feature_policy_header);
 }