Make GetVariationsVector (by key set) a private impl method

This CL renames/moved the GetVariationsVector method taking a set of
collection key ids to be a private GetVariationsVectorImpl method and
adjusts the call sites, tests and definition order to match.

Bug: 1073600
Change-Id: Ia921ae158e82159a695037090a53e74d1ddf8fec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243991
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779311}

components/variations/variations_http_header_provider.cc

@@ -86,32 +86,7 @@ std::string VariationsHttpHeaderProvider::GetVariationsString() {
 
 std::vector<VariationID> VariationsHttpHeaderProvider::GetVariationsVector(
     IDCollectionKey key) {
-  return GetVariationsVector(std::set<IDCollectionKey>{key});
-}
-
-std::vector<VariationID> VariationsHttpHeaderProvider::GetVariationsVector(
-    const std::set<IDCollectionKey>& keys) {
-  InitVariationIDsCacheIfNeeded();
-
-  // Get all the active variation ids while holding the lock.
-  std::set<VariationIDEntry> all_variation_ids;
-  {
-    base::AutoLock scoped_lock(lock_);
-    all_variation_ids = GetAllVariationIds();
-  }
-
-  // Copy the requested variations to the output vector.
-  std::vector<VariationID> result;
-  result.reserve(all_variation_ids.size());
-  for (const VariationIDEntry& entry : all_variation_ids) {
-    if (keys.find(entry.second) != keys.end())
-      result.push_back(entry.first);
-  }
-
-  // Make sure each enry is unique. As a side-effect, the output will be sorted.
-  std::sort(result.begin(), result.end());
-  result.erase(std::unique(result.begin(), result.end()), result.end());
-  return result;
+  return GetVariationsVectorImpl(std::set<IDCollectionKey>{key});
 }
 
 std::vector<VariationID>
@@ -120,7 +95,7 @@ VariationsHttpHeaderProvider::GetVariationsVectorForWebPropertiesKeys() {
       variations::GOOGLE_WEB_PROPERTIES,
       variations::GOOGLE_WEB_PROPERTIES_SIGNED_IN,
       variations::GOOGLE_WEB_PROPERTIES_TRIGGER};
-  return GetVariationsVector(web_properties_keys);
+  return GetVariationsVectorImpl(web_properties_keys);
 }
 
 VariationsHttpHeaderProvider::ForceIdsResult
@@ -375,4 +350,29 @@ VariationsHttpHeaderProvider::GetAllVariationIds() {
   return all_variation_ids_set;
 }
 
+std::vector<VariationID> VariationsHttpHeaderProvider::GetVariationsVectorImpl(
+    const std::set<IDCollectionKey>& keys) {
+  InitVariationIDsCacheIfNeeded();
+
+  // Get all the active variation ids while holding the lock.
+  std::set<VariationIDEntry> all_variation_ids;
+  {
+    base::AutoLock scoped_lock(lock_);
+    all_variation_ids = GetAllVariationIds();
+  }
+
+  // Copy the requested variations to the output vector.
+  std::vector<VariationID> result;
+  result.reserve(all_variation_ids.size());
+  for (const VariationIDEntry& entry : all_variation_ids) {
+    if (keys.find(entry.second) != keys.end())
+      result.push_back(entry.first);
+  }
+
+  // Make sure each enry is unique. As a side-effect, the output will be sorted.
+  std::sort(result.begin(), result.end());
+  result.erase(std::unique(result.begin(), result.end()), result.end());
+  return result;
+}
+
 }  // namespace variations

components/variations/variations_http_header_provider.h

@@ -68,12 +68,6 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
   // entry in the returned vector will be unique.
   std::vector<VariationID> GetVariationsVector(IDCollectionKey key);
 
-  // Returns the collection of variation ids matching any of the given
-  // |keys|. Each entry in the returned vector will be unique.
-  // TODO(rogerm): rename/move to private impl function.
-  std::vector<VariationID> GetVariationsVector(
-      const std::set<IDCollectionKey>& key);
-
   // Returns the collection of variations ids for all Google Web Properties
   // related keys.
   std::vector<VariationID> GetVariationsVectorForWebPropertiesKeys();
@@ -130,11 +124,11 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
                            GetVariationsString);
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
-                           GetVariationsVectorByKey);
-  FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
-                           GetVariationsVectorByKeySet);
+                           GetVariationsVector);
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
                            GetVariationsVectorForWebPropertiesKeys);
+  FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
+                           GetVariationsVectorImpl);
 
   VariationsHttpHeaderProvider();
   ~VariationsHttpHeaderProvider() override;
@@ -189,6 +183,11 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
   // default values, synthetic variations and actual field trial variations.
   std::set<VariationIDEntry> GetAllVariationIds();
 
+  // Returns the collection of variation ids matching any of the given
+  // |keys|. Each entry in the returned vector will be unique.
+  std::vector<VariationID> GetVariationsVectorImpl(
+      const std::set<IDCollectionKey>& key);
+
   // Guards access to variables below.
   base::Lock lock_;
 

components/variations/variations_http_header_provider_unittest.cc

@@ -241,7 +241,7 @@ TEST_F(VariationsHttpHeaderProviderTest, GetVariationsString) {
   EXPECT_EQ(" 100 123 124 200 ", provider.GetVariationsString());
 }
 
-TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVectorByKey) {
+TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVector) {
   base::test::SingleThreadTaskEnvironment task_environment;
   CreateTrialAndAssociateId("t1", "g1", GOOGLE_WEB_PROPERTIES, 121);
   CreateTrialAndAssociateId("t2", "g2", GOOGLE_WEB_PROPERTIES, 122);
@@ -263,7 +263,19 @@ TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVectorByKey) {
             provider.GetVariationsVector(GOOGLE_APP));
 }
 
-TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVectorByKeySet) {
+TEST_F(VariationsHttpHeaderProviderTest,
+       GetVariationsVectorForWebPropertiesKeys) {
+  CreateTrialAndAssociateId("t1", "g1", GOOGLE_WEB_PROPERTIES, 121);
+  CreateTrialAndAssociateId("t2", "g2", GOOGLE_WEB_PROPERTIES_TRIGGER, 122);
+  CreateTrialAndAssociateId("t3", "g3", GOOGLE_WEB_PROPERTIES_SIGNED_IN, 123);
+  CreateTrialAndAssociateId("t4", "g4", GOOGLE_APP, 124);  // Will be excluded.
+  VariationsHttpHeaderProvider provider;
+  provider.ForceVariationIds({"100", "t101"}, "");
+  EXPECT_EQ((std::vector<VariationID>{100, 101, 121, 122, 123}),
+            provider.GetVariationsVectorForWebPropertiesKeys());
+}
+
+TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVectorImpl) {
   base::test::SingleThreadTaskEnvironment task_environment;
   CreateTrialAndAssociateId("t1", "g1", GOOGLE_WEB_PROPERTIES, 121);
   CreateTrialAndAssociateId("t2", "g2", GOOGLE_WEB_PROPERTIES, 122);
@@ -278,26 +290,14 @@ TEST_F(VariationsHttpHeaderProviderTest, GetVariationsVectorByKeySet) {
   provider.ForceVariationIds({"100", "200", "t101"}, "");
 
   EXPECT_EQ((std::vector<VariationID>{100, 101, 121, 122, 123, 124, 200}),
-            provider.GetVariationsVector(
+            provider.GetVariationsVectorImpl(
                 {GOOGLE_WEB_PROPERTIES, GOOGLE_WEB_PROPERTIES_TRIGGER}));
   EXPECT_EQ((std::vector<VariationID>{101, 123, 124, 125}),
-            provider.GetVariationsVector({GOOGLE_WEB_PROPERTIES_SIGNED_IN,
+            provider.GetVariationsVectorImpl({GOOGLE_WEB_PROPERTIES_SIGNED_IN,
                                               GOOGLE_WEB_PROPERTIES_TRIGGER}));
   EXPECT_EQ((std::vector<VariationID>{124, 125, 126}),
-            provider.GetVariationsVector(
+            provider.GetVariationsVectorImpl(
                 {GOOGLE_APP, GOOGLE_WEB_PROPERTIES_SIGNED_IN}));
 }
 
-TEST_F(VariationsHttpHeaderProviderTest,
-       GetVariationsVectorForWebPropertiesKeys) {
-  CreateTrialAndAssociateId("t1", "g1", GOOGLE_WEB_PROPERTIES, 121);
-  CreateTrialAndAssociateId("t2", "g2", GOOGLE_WEB_PROPERTIES_TRIGGER, 122);
-  CreateTrialAndAssociateId("t3", "g3", GOOGLE_WEB_PROPERTIES_SIGNED_IN, 123);
-  CreateTrialAndAssociateId("t4", "g4", GOOGLE_APP, 124);  // Will be excluded.
-  VariationsHttpHeaderProvider provider;
-  provider.ForceVariationIds({"100", "t101"}, "");
-  EXPECT_EQ((std::vector<VariationID>{100, 101, 121, 122, 123}),
-            provider.GetVariationsVectorForWebPropertiesKeys());
-}
-
 }  // namespace variations