Avoid implicit conversion of Layer::salt() to bool.

Due to an implicit conversion of `salt` to bool, virtually all layers, regardless of `id` or `salt`, effectively used the salt value of `1`.

User-to-layer assignments before and after this CL will be shuffled, therefore it might be necessary to cherry-pick this CL to the earliest
version when we want to land this feature.

(cherry picked from commit 788e2c7)

Bug: 1405592, b/260609574
Change-Id: Ia24e36644e49a2e24bd0203e34bdee7a10ff2c5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4139578
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1090042}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4147987
Auto-Submit: Steven Holte <holte@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#179}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

components/variations/uniformity_unittest.cc

@@ -25,12 +25,15 @@ namespace {
 
 // For these tests, we use a small LES range to make the expectations simpler.
 const uint32_t kMaxEntropy = 20;
+const uint32_t kLayerId = 1;
+const uint32_t kLayerSalt = kLayerId;
 const char kStudyName[] = "Uniformity";
 
 struct LayerStudySeedOptions {
   bool layer_constrain_study = true;
   bool force_low_entropy_layer = true;
   bool force_low_entropy = false;
+  uint32_t salt = kLayerSalt;
   uint32_t slot_multiplier = 1;
 };
 
@@ -41,8 +44,8 @@ struct LayerStudySeedOptions {
 VariationsSeed LayerStudySeed(LayerStudySeedOptions options) {
   VariationsSeed seed;
   Layer* layer = seed.add_layers();
-  layer->set_id(42);
-  layer->set_salt(42);
+  layer->set_id(kLayerId);
+  layer->set_salt(options.salt);
   layer->set_num_slots(10 * options.slot_multiplier);
   if (options.force_low_entropy_layer)
     layer->set_entropy_mode(Layer::LOW);
@@ -64,7 +67,7 @@ VariationsSeed LayerStudySeed(LayerStudySeedOptions options) {
 
   if (options.layer_constrain_study) {
     LayerMemberReference* layer_membership = study->mutable_layer();
-    layer_membership->set_layer_id(42);
+    layer_membership->set_layer_id(kLayerId);
     layer_membership->set_layer_member_id(82);
   }
   // Use 3 arms, which does not divide
@@ -274,6 +277,23 @@ TEST(VariationsUniformityTest, DefaultEntropyLayerDefaultEntropyStudy) {
   EXPECT_THAT(assignments, ::testing::ElementsAreArray(expected));
 }
 
+// Not specifying the `salt` field for the layer should fall back to using the
+// `id` field as salt. Different salt values should result in different layer
+// exclusions.
+TEST(VariationsUniformityTest, LayerSalt) {
+  auto assignments = GetUniformityAssignments(
+      LayerStudySeed({.force_low_entropy_layer = false}));
+
+  auto assignments_salt_not_specified = GetUniformityAssignments(
+      LayerStudySeed({.force_low_entropy_layer = false, .salt = 0}));
+
+  auto assignments_alternative_salt = GetUniformityAssignments(LayerStudySeed(
+      {.force_low_entropy_layer = false, .salt = kLayerSalt + 1}));
+
+  EXPECT_EQ(assignments, assignments_salt_not_specified);
+  EXPECT_NE(assignments, assignments_alternative_salt);
+}
+
 // When enable_benchmarking is passed, layered studies should never activate.
 TEST(VariationsUniformityTest, BenchmarkingDisablesLayeredStudies) {
   std::vector<std::string> expected(
@@ -346,4 +366,4 @@ TEST(VariationsUniformityTest, SessionEntropyStudyChiSquare) {
   }
 }
 
-}  // namespace variations
+}  // namespace variations

components/variations/variations_layers.cc

@@ -213,7 +213,7 @@ void VariationsLayers::ConstructLayer(const EntropyProviders& entropy_providers,
   const auto& entropy_provider = (layer_proto.entropy_mode() != Layer::LOW)
                                      ? entropy_providers.default_entropy()
                                      : entropy_providers.low_entropy();
-  uint32_t salt = layer_proto.salt() || layer_proto.id();
+  uint32_t salt = layer_proto.salt() ? layer_proto.salt() : layer_proto.id();
   ValueInRange pseudorandom = {
       .value = entropy_provider.GetPseudorandomValue(salt, range),
       .range = static_cast<uint32_t>(range),