Associate VariationIDs with the two new IDCollectionKeys.

The keys are GOOGLE_WEB_PROPERTIES_FIRST_PARTY and
GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY.

The new keys will be used to limit the set of experiment IDs included in
X-Client-Data headers sent in third-party contexts when the
RestrictGoogleWebVisibility feature is enabled. Note that the feature is
disabled by default.

Also, update relevant code to consider IDs associated with the new
FIRST_PARTY groups. To do this, GetVariationsVector() and
GetVariationsString() were changed to accept collections of
IDCollectionKeys.

Bug: 1094303
Change-Id: If931d8281f5a687548427101e07f3b3075cd349b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367989Reviewed-by: Dan H <harringtond@chromium.org>
Reviewed-by: Matthias Körber <koerber@google.com>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Cr-Commit-Position: refs/heads/master@{#806803}

chrome/browser/ui/webui/snippets_internals/snippets_internals_page_handler.cc

@@ -50,8 +50,10 @@ std::set<variations::VariationID> GetSnippetsExperiments() {
 
     for (variations::IDCollectionKey key :
          {variations::GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
+          variations::GOOGLE_WEB_PROPERTIES_FIRST_PARTY,
           variations::GOOGLE_WEB_PROPERTIES_SIGNED_IN,
-          variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT}) {
+          variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
+          variations::GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY}) {
       const variations::VariationID id = variations::GetGoogleVariationID(
           key, trial->trial_name(), trial->group_name());
       if (id != variations::EMPTY_ID) {

components/autofill/core/browser/autofill_download_manager.cc

@@ -1062,7 +1062,8 @@ void AutofillDownloadManager::InitActiveExperiments() {
   delete active_experiments_;
   active_experiments_ = new std::vector<variations::VariationID>(
       variations_ids_provider->GetVariationsVector(
-          variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT));
+          {variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
+           variations::GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY}));
   base::EraseIf(*active_experiments_, [](variations::VariationID id) {
     return !IsAutofillExperimentId(id);
   });

components/variations/variations_associated_data.h

@@ -64,9 +64,10 @@ enum IDCollectionKey {
   // transmitted via the X-Client-Data header. These IDs are transmitted in
   // first- and third-party contexts.
   GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
-  // Used when kRestrictGoogleWebVisibility is enabled. The IDs in this
-  // collection are used by Google web properties and are transmitted via the
-  // X-Client-Data header. These IDs are transmitted in first-party contexts.
+  // The IDs in this collection are used by Google web properties and are
+  // transmitted via the X-Client-Data header. When kRestrictGoogleWebVisibility
+  // is enabled, these IDs are transmitted in only first-party contexts;
+  // otherwise, these IDs are transmitted in first- and third-party contexts.
   GOOGLE_WEB_PROPERTIES_FIRST_PARTY,
   // This collection is used by Google web properties for signed in users only,
   // transmitted through the X-Client-Data header.
@@ -75,10 +76,11 @@ enum IDCollectionKey {
   // server-side experimental behavior and are transmitted via the X-Client-Data
   // header. These IDs are transmitted in first- and third-party contexts.
   GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
-  // Used when kRestrictGoogleWebVisibility is enabled. The IDs in this
-  // collection are used by Google web properties to trigger server-side
-  // experimental behavior and are transmitted via the X-Client-Data header.
-  // These IDs are transmitted in first-party contexts.
+  // The IDs in this collection are used by Google web properties to trigger
+  // server-side experimental behavior and are transmitted via the X-Client-Data
+  // header. When kRestrictGoogleWebVisibility is enabled, these IDs are
+  // transmitted in only first-party contexts; otherwise, these IDs are
+  // transmitted in first- and third-party contexts.
   GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY,
   // This collection is used by the Google App and is passed at the time
   // the cross-app communication is triggered.

components/variations/variations_ids_provider.cc

@@ -7,9 +7,6 @@
 #include <stddef.h>
 
 #include <algorithm>
-#include <set>
-#include <string>
-#include <vector>
 
 #include "base/base64.h"
 #include "base/memory/singleton.h"
@@ -55,44 +52,44 @@ std::string VariationsIdsProvider::GetClientDataHeader(bool is_signed_in) {
   return variation_ids_header_copy;
 }
 
-std::string VariationsIdsProvider::GetVariationsString(IDCollectionKey key) {
-  InitVariationIDsCacheIfNeeded();
-
+std::string VariationsIdsProvider::GetVariationsString(
+    const std::set<IDCollectionKey>& keys) {
   // Construct a space-separated string with leading and trailing spaces from
-  // the variations set. Note: The ids in it will be in sorted order per the
+  // the VariationIDs set. The IDs in the string are in sorted order as per the
   // std::set contract.
   std::string ids_string = " ";
-  {
-    base::AutoLock scoped_lock(lock_);
-    for (const VariationIDEntry& entry : GetAllVariationIds()) {
-      if (entry.second == key) {
-        ids_string.append(base::NumberToString(entry.first));
-        ids_string.push_back(' ');
-      }
-    }
+
+  for (const VariationID& id : GetVariationsVector(keys)) {
+    ids_string.append(base::NumberToString(id));
+    ids_string.push_back(' ');
   }
+
   return ids_string;
 }
 
 std::string VariationsIdsProvider::GetGoogleAppVariationsString() {
-  return GetVariationsString(GOOGLE_APP);
+  return GetVariationsString({GOOGLE_APP});
 }
 
 std::string VariationsIdsProvider::GetVariationsString() {
-  return GetVariationsString(GOOGLE_WEB_PROPERTIES_ANY_CONTEXT);
+  return GetVariationsString(
+      {GOOGLE_WEB_PROPERTIES_ANY_CONTEXT, GOOGLE_WEB_PROPERTIES_FIRST_PARTY});
 }
 
 std::vector<VariationID> VariationsIdsProvider::GetVariationsVector(
-    IDCollectionKey key) {
-  return GetVariationsVectorImpl(std::set<IDCollectionKey>{key});
+    const std::set<IDCollectionKey>& keys) {
+  return GetVariationsVectorImpl(keys);
 }
 
 std::vector<VariationID>
 VariationsIdsProvider::GetVariationsVectorForWebPropertiesKeys() {
   const std::set<IDCollectionKey> web_properties_keys{
-      variations::GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
-      variations::GOOGLE_WEB_PROPERTIES_SIGNED_IN,
-      variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT};
+      GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
+      GOOGLE_WEB_PROPERTIES_FIRST_PARTY,
+      GOOGLE_WEB_PROPERTIES_SIGNED_IN,
+      GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
+      GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY,
+  };
   return GetVariationsVectorImpl(web_properties_keys);
 }
 
@@ -168,11 +165,7 @@ void VariationsIdsProvider::OnFieldTrialGroupFinalized(
     const std::string& group_name) {
   base::AutoLock scoped_lock(lock_);
   const size_t old_size = variation_ids_set_.size();
-  CacheVariationsId(trial_name, group_name, GOOGLE_WEB_PROPERTIES_ANY_CONTEXT);
-  CacheVariationsId(trial_name, group_name, GOOGLE_WEB_PROPERTIES_SIGNED_IN);
-  CacheVariationsId(trial_name, group_name,
-                    GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT);
-  CacheVariationsId(trial_name, group_name, GOOGLE_APP);
+  CacheVariationsId(trial_name, group_name);
   if (variation_ids_set_.size() != old_size)
     UpdateVariationIDsHeaderValue();
 }
@@ -215,13 +208,7 @@ void VariationsIdsProvider::InitVariationIDsCacheIfNeeded() {
   base::FieldTrialList::GetActiveFieldTrialGroups(&initial_groups);
 
   for (const auto& entry : initial_groups) {
-    CacheVariationsId(entry.trial_name, entry.group_name,
-                      GOOGLE_WEB_PROPERTIES_ANY_CONTEXT);
-    CacheVariationsId(entry.trial_name, entry.group_name,
-                      GOOGLE_WEB_PROPERTIES_SIGNED_IN);
-    CacheVariationsId(entry.trial_name, entry.group_name,
-                      GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT);
-    CacheVariationsId(entry.trial_name, entry.group_name, GOOGLE_APP);
+    CacheVariationsId(entry.trial_name, entry.group_name);
   }
   UpdateVariationIDsHeaderValue();
 
@@ -229,11 +216,13 @@ void VariationsIdsProvider::InitVariationIDsCacheIfNeeded() {
 }
 
 void VariationsIdsProvider::CacheVariationsId(const std::string& trial_name,
-                                              const std::string& group_name,
-                                              IDCollectionKey key) {
-  const VariationID id = GetGoogleVariationID(key, trial_name, group_name);
-  if (id != EMPTY_ID)
-    variation_ids_set_.insert(VariationIDEntry(id, key));
+                                              const std::string& group_name) {
+  for (int i = 0; i < ID_COLLECTION_COUNT; ++i) {
+    IDCollectionKey key = static_cast<IDCollectionKey>(i);
+    const VariationID id = GetGoogleVariationID(key, trial_name, group_name);
+    if (id != EMPTY_ID)
+      variation_ids_set_.insert(VariationIDEntry(id, key));
+  }
 }
 
 void VariationsIdsProvider::UpdateVariationIDsHeaderValue() {
@@ -270,16 +259,38 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto(
       case GOOGLE_WEB_PROPERTIES_ANY_CONTEXT:
         proto.add_variation_id(entry.first);
         break;
+      case GOOGLE_WEB_PROPERTIES_FIRST_PARTY:
+        if (base::FeatureList::IsEnabled(
+                internal::kRestrictGoogleWebVisibility)) {
+          // TODO(crbug/1094303): Send fewer VariationIDs in third-party
+          // contexts by excluding IDs associated with
+          // GOOGLE_WEB_PROPERTIES_FIRST_PARTY.
+          break;
+        }
+        // When the feature is not enabled, treat VariationIDs associated with
+        // GOOGLE_WEB_PROPERTIES_FIRST_PARTY in the same way as those
+        // associated with GOOGLE_WEB_PROPERTIES_ANY_CONTEXT.
+        proto.add_variation_id(entry.first);
+        break;
       case GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT:
         proto.add_trigger_variation_id(entry.first);
         break;
+      case GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY:
+        if (base::FeatureList::IsEnabled(
+                internal::kRestrictGoogleWebVisibility)) {
+          // TODO(crbug/1094303): Send fewer VariationIDs in third-party
+          // contexts by excluding IDs associated with
+          // GOOGLE_WEB_PROPERTIES_FIRST_PARTY.
+          break;
+        }
+        // When the feature is not enabled, treat VariationIDs associated with
+        // GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY in the same way as those
+        // associated with GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT.
+        proto.add_trigger_variation_id(entry.first);
+        break;
       case GOOGLE_APP:
         // These IDs should not be added into Google Web headers.
         break;
-      case GOOGLE_WEB_PROPERTIES_FIRST_PARTY:
-      case GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY:
-        // TODO(crbug/1094303): Add support for the above IDCollectionKeys.
-        break;
       case ID_COLLECTION_COUNT:
         // This case included to get full enum coverage for switch, so that
         // new enums introduce compiler warnings. Nothing to do for this.
@@ -385,7 +396,7 @@ std::vector<VariationID> VariationsIdsProvider::GetVariationsVectorImpl(
       result.push_back(entry.first);
   }
 
-  // Make sure each enry is unique. As a side-effect, the output will be sorted.
+  // Make sure each entry is unique. As a side effect, the output is sorted.
   std::sort(result.begin(), result.end());
   result.erase(std::unique(result.begin(), result.end()), result.end());
   return result;

components/variations/variations_ids_provider.h

@@ -64,9 +64,10 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
   // apps.
   std::string GetGoogleAppVariationsString();
 
-  // Returns the collection of variation ids matching the given |key|. Each
-  // entry in the returned vector will be unique.
-  std::vector<VariationID> GetVariationsVector(IDCollectionKey key);
+  // Returns the collection of VariationIDs associated with |keys|. Each entry
+  // in the returned vector is unique.
+  std::vector<VariationID> GetVariationsVector(
+      const std::set<IDCollectionKey>& keys);
 
   // Returns the collection of variations ids for all Google Web Properties
   // related keys.
@@ -132,7 +133,7 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
   // Returns a space-separated string containing the list of current active
   // variations (as would be reported in the |variation_id| repeated field of
   // the ClientVariations proto) for a given ID collection.
-  std::string GetVariationsString(IDCollectionKey key);
+  std::string GetVariationsString(const std::set<IDCollectionKey>& keys);
 
   // base::FieldTrialList::Observer:
   // This will add the variation ID associated with |trial_name| and
@@ -149,11 +150,10 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
   // new variation IDs.
   void InitVariationIDsCacheIfNeeded();
 
-  // Looks up the associated id for the given trial/group and adds an entry for
-  // it to |variation_ids_set_| if found.
+  // Looks up the VariationID associated with |trial_name| and |group_name|, and
+  // if found, adds an entry for it to |variation_ids_set_|.
   void CacheVariationsId(const std::string& trial_name,
-                         const std::string& group_name,
-                         IDCollectionKey key);
+                         const std::string& group_name);
 
   // Takes whatever is currently in |variation_ids_set_| and recreates
   // |variation_ids_header_| with it.  Assumes the the |lock_| is currently

components/variations/variations_seed_processor.cc

@@ -13,6 +13,7 @@
 #include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/variations/client_filterable_state.h"
 #include "components/variations/processed_study.h"
@@ -35,31 +36,60 @@ void RegisterExperimentParams(const Study& study,
     AssociateVariationParams(study.name(), experiment.name(), params);
 }
 
-// If there are variation ids associated with |experiment|, register the
-// variation ids.
-void RegisterVariationIds(const Study_Experiment& experiment,
-                          const std::string& trial_name) {
-  if (experiment.has_google_web_experiment_id()) {
-    const VariationID variation_id =
-        static_cast<VariationID>(experiment.google_web_experiment_id());
-    AssociateGoogleVariationIDForce(GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
-                                    trial_name, experiment.name(),
-                                    variation_id);
-  }
-  if (experiment.has_google_web_trigger_experiment_id()) {
-    const VariationID variation_id =
-        static_cast<VariationID>(experiment.google_web_trigger_experiment_id());
-    AssociateGoogleVariationIDForce(GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
-                                    trial_name, experiment.name(),
-                                    variation_id);
+// Returns the IDCollectionKey with which |experiment| should be associated.
+// Returns nullopt when |experiment| doesn't have a Google web or Google web
+// trigger experiment ID.
+base::Optional<IDCollectionKey> GetKeyForWebExperiment(
+    const Study_Experiment& experiment) {
+  bool has_web_experiment_id = experiment.has_google_web_experiment_id();
+  bool has_web_trigger_experiment_id =
+      experiment.has_google_web_trigger_experiment_id();
+
+  if (!has_web_experiment_id && !has_web_trigger_experiment_id)
+    return base::nullopt;
+
+  // An experiment cannot have both |google_web_experiment_id| and
+  // |google_trigger_web_experiment_id|. This is enforced by the variations
+  // server before generating a variations seed.
+  DCHECK(!(has_web_experiment_id && has_web_trigger_experiment_id));
+
+  Study_GoogleWebVisibility visibility = experiment.google_web_visibility();
+  if (visibility == Study_GoogleWebVisibility_FIRST_PARTY) {
+    return has_web_trigger_experiment_id
+               ? GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY
+               : GOOGLE_WEB_PROPERTIES_FIRST_PARTY;
   }
+  return has_web_trigger_experiment_id
+             ? GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT
+             : GOOGLE_WEB_PROPERTIES_ANY_CONTEXT;
+}
 
+// If there are VariationIDs associated with |experiment|, register the
+// VariationIDs.
+void RegisterVariationIds(const Study_Experiment& experiment,
+                          const std::string& trial_name) {
   if (experiment.has_google_app_experiment_id()) {
     const VariationID variation_id =
         static_cast<VariationID>(experiment.google_app_experiment_id());
     AssociateGoogleVariationIDForce(GOOGLE_APP, trial_name, experiment.name(),
                                     variation_id);
   }
+
+  base::Optional<IDCollectionKey> key = GetKeyForWebExperiment(experiment);
+  if (!key.has_value())
+    return;
+
+  // An experiment cannot have both |google_web_experiment_id| and
+  // |google_trigger_web_experiment_id|. See GetKeyForWebExperiment() for more
+  // details.
+  const VariationID variation_id =
+      experiment.has_google_web_trigger_experiment_id()
+          ? static_cast<VariationID>(
+                experiment.google_web_trigger_experiment_id())
+          : static_cast<VariationID>(experiment.google_web_experiment_id());
+
+  AssociateGoogleVariationIDForce(key.value(), trial_name, experiment.name(),
+                                  variation_id);
 }
 
 // Executes |callback| on every override defined by |experiment|.

components/variations/variations_seed_processor_unittest.cc

@@ -27,6 +27,7 @@
 #include "base/test/scoped_field_trial_list_resetter.h"
 #include "components/variations/client_filterable_state.h"
 #include "components/variations/processed_study.h"
+#include "components/variations/proto/study.pb.h"
 #include "components/variations/study_filtering.h"
 #include "components/variations/variations_associated_data.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -182,6 +183,23 @@ TEST_F(VariationsSeedProcessorTest, AllowForceGroupAndVariationId) {
   EXPECT_EQ(kExperimentId, id);
 }
 
+TEST_F(VariationsSeedProcessorTest, AllowForceGroupAndVariationId_FirstParty) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
+
+  Study study = CreateStudyWithFlagGroups(100, 0, 0);
+  Study_Experiment* experiment1 = study.mutable_experiment(1);
+  experiment1->set_google_web_experiment_id(kExperimentId);
+  experiment1->set_google_web_visibility(Study_GoogleWebVisibility_FIRST_PARTY);
+
+  EXPECT_TRUE(CreateTrialFromStudy(study));
+  EXPECT_EQ(kFlagGroup1Name,
+            base::FieldTrialList::FindFullName(kFlagStudyName));
+
+  VariationID id = GetGoogleVariationID(GOOGLE_WEB_PROPERTIES_FIRST_PARTY,
+                                        kFlagStudyName, kFlagGroup1Name);
+  EXPECT_EQ(kExperimentId, id);
+}
+
 // Test that the group for kForcingFlag1 is forced.
 TEST_F(VariationsSeedProcessorTest, ForceGroupWithFlag1) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);

ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.mm

@@ -702,7 +702,8 @@ NSString* SerializedPref(const PrefService::Preference* pref) {
   variations::VariationsIdsProvider* provider =
       variations::VariationsIdsProvider::GetInstance();
   std::vector<variations::VariationID> ids = provider->GetVariationsVector(
-      variations::GOOGLE_WEB_PROPERTIES_ANY_CONTEXT);
+      {variations::GOOGLE_WEB_PROPERTIES_ANY_CONTEXT,
+       variations::GOOGLE_WEB_PROPERTIES_FIRST_PARTY});
   return std::find(ids.begin(), ids.end(), variationID) != ids.end();
 }
 
@@ -710,7 +711,8 @@ NSString* SerializedPref(const PrefService::Preference* pref) {
   variations::VariationsIdsProvider* provider =
       variations::VariationsIdsProvider::GetInstance();
   std::vector<variations::VariationID> ids = provider->GetVariationsVector(
-      variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT);
+      {variations::GOOGLE_WEB_PROPERTIES_TRIGGER_ANY_CONTEXT,
+       variations::GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY});
   return std::find(ids.begin(), ids.end(), variationID) != ids.end();
 }