Enable the RestrictGoogleWebVisibility feature.

Bug: 1094303
Change-Id: I7471695cb8b1248fe0d3005635ed7310a9ac8296
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462247
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816730}

components/variations/BUILD.gn

@@ -192,6 +192,7 @@ source_set("unit_tests") {
 
   deps = [
     ":variations",
+    ":variations_features",
     ":variations_mojom",
     "net",
     "proto",

components/variations/variations_features.cc

@@ -9,7 +9,7 @@ namespace variations {
 namespace internal {
 
 const base::Feature kRestrictGoogleWebVisibility{
-    "RestrictGoogleWebVisibility", base::FEATURE_DISABLED_BY_DEFAULT};
+    "RestrictGoogleWebVisibility", base::FEATURE_ENABLED_BY_DEFAULT};
 
 }  // namespace internal
 }  // namespace variations

components/variations/variations_ids_provider.h

@@ -144,7 +144,7 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
                            ForceDisableVariationIds_ValidCommandLine);
   FRIEND_TEST_ALL_PREFIXES(VariationsIdsProviderTest,
                            ForceDisableVariationIds_Invalid);
-  FRIEND_TEST_ALL_PREFIXES(VariationsIdsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(VariationsIdsProviderTestWithRestrictedVisibility,
                            OnFieldTrialGroupFinalized);
   FRIEND_TEST_ALL_PREFIXES(VariationsIdsProviderTest,
                            GetGoogleAppVariationsString);

components/variations/variations_ids_provider_unittest.cc

@@ -7,14 +7,18 @@
 #include <string>
 
 #include "base/base64.h"
+#include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
 #include "base/run_loop.h"
+#include "base/stl_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "components/variations/entropy_provider.h"
 #include "components/variations/proto/client_variations.pb.h"
 #include "components/variations/proto/study.pb.h"
 #include "components/variations/variations.mojom.h"
 #include "components/variations/variations_associated_data.h"
+#include "components/variations/variations_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace variations {
@@ -70,6 +74,25 @@ class VariationsIdsProviderTest : public ::testing::Test {
   base::test::SingleThreadTaskEnvironment task_environment_;
 };
 
+// Used for testing the kRestrictGoogleWebVisibility feature.
+class VariationsIdsProviderTestWithRestrictedVisibility
+    : public VariationsIdsProviderTest,
+      public ::testing::WithParamInterface<bool> {
+ public:
+  VariationsIdsProviderTestWithRestrictedVisibility() {
+    if (GetParam()) {
+      scoped_feature_list_.InitAndEnableFeature(
+          internal::kRestrictGoogleWebVisibility);
+    } else {
+      scoped_feature_list_.InitAndDisableFeature(
+          internal::kRestrictGoogleWebVisibility);
+    }
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
 TEST_F(VariationsIdsProviderTest, ForceVariationIds_Valid) {
   VariationsIdsProvider provider;
 
@@ -173,7 +196,12 @@ TEST_F(VariationsIdsProviderTest, ForceDisableVariationIds_Invalid) {
   EXPECT_TRUE(provider.GetClientDataHeaders(/*is_signed_in=*/false).is_null());
 }
 
-TEST_F(VariationsIdsProviderTest, OnFieldTrialGroupFinalized) {
+INSTANTIATE_TEST_SUITE_P(All,
+                         VariationsIdsProviderTestWithRestrictedVisibility,
+                         ::testing::Bool());
+
+TEST_P(VariationsIdsProviderTestWithRestrictedVisibility,
+       OnFieldTrialGroupFinalized) {
   VariationsIdsProvider provider;
   provider.InitVariationIDsCacheIfNeeded();
 
@@ -191,7 +219,7 @@ TEST_F(VariationsIdsProviderTest, OnFieldTrialGroupFinalized) {
   ASSERT_EQ(default_name, trial_3->group_name());
 
   scoped_refptr<base::FieldTrial> trial_4(CreateTrialAndAssociateId(
-      "t4", default_name, GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT, 44));
+      "t4", default_name, GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY, 44));
   ASSERT_EQ(default_name, trial_4->group_name());
 
   scoped_refptr<base::FieldTrial> trial_5(CreateTrialAndAssociateId(
@@ -206,37 +234,99 @@ TEST_F(VariationsIdsProviderTest, OnFieldTrialGroupFinalized) {
   {
     variations::mojom::VariationsHeadersPtr headers =
         provider.GetClientDataHeaders(/*is_signed_in=*/false);
-    const std::string variations =
+    const std::string variations_header_first_party = headers->headers_map.at(
+        variations::mojom::GoogleWebVisibility::FIRST_PARTY);
+    const std::string variations_header_any_context =
         headers->headers_map.at(variations::mojom::GoogleWebVisibility::ANY);
 
-    std::set<VariationID> variation_ids;
-    std::set<VariationID> trigger_ids;
-    ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids));
-    EXPECT_EQ(2U, variation_ids.size());
-    EXPECT_TRUE(variation_ids.find(11) != variation_ids.end());
-    EXPECT_TRUE(variation_ids.find(22) != variation_ids.end());
-    EXPECT_EQ(2U, trigger_ids.size());
-    EXPECT_TRUE(trigger_ids.find(33) != trigger_ids.end());
-    EXPECT_TRUE(trigger_ids.find(44) != trigger_ids.end());
+    std::set<VariationID> ids_first_party;
+    std::set<VariationID> trigger_ids_first_party;
+    ASSERT_TRUE(ExtractVariationIds(variations_header_first_party,
+                                    &ids_first_party,
+                                    &trigger_ids_first_party));
+    std::set<VariationID> ids_any_context;
+    std::set<VariationID> trigger_ids_any_context;
+    ASSERT_TRUE(ExtractVariationIds(variations_header_any_context,
+                                    &ids_any_context,
+                                    &trigger_ids_any_context));
+
+    EXPECT_EQ(2U, ids_first_party.size());
+    EXPECT_TRUE(base::Contains(ids_first_party, 11));
+    EXPECT_TRUE(base::Contains(ids_first_party, 22));
+    EXPECT_EQ(2U, trigger_ids_first_party.size());
+    EXPECT_TRUE(base::Contains(trigger_ids_first_party, 33));
+    EXPECT_TRUE(base::Contains(trigger_ids_first_party, 44));
+
+    if (base::FeatureList::IsEnabled(internal::kRestrictGoogleWebVisibility)) {
+      // When the feature is enabled, IDs associated with FIRST_PARTY
+      // IDCollectionKeys should be excluded from the variations header that may
+      // be sent in third-party contexts.
+      EXPECT_EQ(1U, ids_any_context.size());
+      EXPECT_TRUE(base::Contains(ids_any_context, 11));
+      EXPECT_EQ(1U, trigger_ids_any_context.size());
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 33));
+    } else {
+      // When the feature is disabled, IDs associated with FIRST_PARTY
+      // IDCollectionKeys should be included in the variations header that may
+      // be sent in third-party contexts.
+      EXPECT_EQ(2U, ids_any_context.size());
+      EXPECT_TRUE(base::Contains(ids_any_context, 11));
+      EXPECT_TRUE(base::Contains(ids_any_context, 22));
+      EXPECT_EQ(2U, trigger_ids_any_context.size());
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 33));
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 44));
+    }
   }
 
   // Now, get signed-in ids.
   {
     variations::mojom::VariationsHeadersPtr headers =
         provider.GetClientDataHeaders(/*is_signed_in=*/true);
-    const std::string variations =
+    const std::string variations_header_first_party = headers->headers_map.at(
+        variations::mojom::GoogleWebVisibility::FIRST_PARTY);
+    const std::string variations_header_any_context =
         headers->headers_map.at(variations::mojom::GoogleWebVisibility::ANY);
 
-    std::set<VariationID> variation_ids;
-    std::set<VariationID> trigger_ids;
-    ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids));
-    EXPECT_EQ(3U, variation_ids.size());
-    EXPECT_TRUE(variation_ids.find(11) != variation_ids.end());
-    EXPECT_TRUE(variation_ids.find(22) != variation_ids.end());
-    EXPECT_TRUE(variation_ids.find(55) != variation_ids.end());
-    EXPECT_EQ(2U, trigger_ids.size());
-    EXPECT_TRUE(trigger_ids.find(33) != trigger_ids.end());
-    EXPECT_TRUE(trigger_ids.find(44) != trigger_ids.end());
+    std::set<VariationID> ids_first_party;
+    std::set<VariationID> trigger_ids_first_party;
+    ASSERT_TRUE(ExtractVariationIds(variations_header_first_party,
+                                    &ids_first_party,
+                                    &trigger_ids_first_party));
+    std::set<VariationID> ids_any_context;
+    std::set<VariationID> trigger_ids_any_context;
+    ASSERT_TRUE(ExtractVariationIds(variations_header_any_context,
+                                    &ids_any_context,
+                                    &trigger_ids_any_context));
+
+    EXPECT_EQ(3U, ids_first_party.size());
+    EXPECT_TRUE(base::Contains(ids_first_party, 11));
+    EXPECT_TRUE(base::Contains(ids_first_party, 22));
+    EXPECT_TRUE(base::Contains(ids_any_context, 55));
+    EXPECT_EQ(2U, trigger_ids_first_party.size());
+    EXPECT_TRUE(base::Contains(trigger_ids_first_party, 33));
+    EXPECT_TRUE(base::Contains(trigger_ids_first_party, 44));
+
+    if (base::FeatureList::IsEnabled(internal::kRestrictGoogleWebVisibility)) {
+      // When the feature is enabled, IDs associated with FIRST_PARTY
+      // IDCollectionKeys should be excluded from the variations header that may
+      // be sent in third-party contexts.
+      EXPECT_EQ(2U, ids_any_context.size());
+      EXPECT_TRUE(base::Contains(ids_any_context, 11));
+      EXPECT_TRUE(base::Contains(ids_any_context, 55));
+      EXPECT_EQ(1U, trigger_ids_any_context.size());
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 33));
+    } else {
+      // When the feature is disabled, IDs associated with FIRST_PARTY
+      // IDCollectionKeys should be included in the variations header that may
+      // be sent in third-party contexts.
+      EXPECT_EQ(3U, ids_any_context.size());
+      EXPECT_TRUE(base::Contains(ids_any_context, 11));
+      EXPECT_TRUE(base::Contains(ids_any_context, 22));
+      EXPECT_TRUE(base::Contains(ids_any_context, 55));
+      EXPECT_EQ(2U, trigger_ids_any_context.size());
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 33));
+      EXPECT_TRUE(base::Contains(trigger_ids_any_context, 44));
+    }
   }
 }