Remove variation parameters from user_classifier.

We are not settings these parameters, and have no plans to set them.

Bug: 1044139
Change-Id: I7c024b5bf01fafc8771ea2cd3f511ead085360ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012885Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736193}

components/feed/core/user_classifier.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <cfloat>
+#include <cmath>
 #include <string>
 
 #include "base/metrics/histogram_macros.h"
@@ -17,7 +18,6 @@
 #include "components/feed/feed_feature_list.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
-#include "components/variations/variations_associated_data.h"
 
 namespace feed {
 
@@ -25,30 +25,27 @@ namespace {
 
 // The discount rate for computing the discounted-average rates. Must be
 // strictly larger than 0 and strictly smaller than 1!
-const double kDiscountRatePerDay = 0.25;
-const char kDiscountRatePerDayParam[] = "user_classifier_discount_rate_per_day";
+constexpr double kDiscountRatePerDay = 0.25;
+static_assert(kDiscountRatePerDay > 0 && kDiscountRatePerDay < 1,
+              "invalid value");
+// Compute discount_rate_per_hour such that
+//   kDiscountRatePerDay = 1 - e^{-kDiscountRatePerHour * 24}.
+const double kDiscountRatePerHour =
+    std::log(1.0 / (1.0 - kDiscountRatePerDay)) / 24.0;
 
 // Never consider any larger interval than this (so that extreme situations such
 // as losing your phone or going for a long offline vacation do not skew the
 // average too much).
-// When overriding via variation parameters, it is better to use smaller values
-// than |kMaxHours| as this it the maximum value reported in the histograms.
 const double kMaxHours = 7 * 24;
-const char kMaxHoursParam[] = "user_classifier_max_hours";
 
 // Ignore events within |kMinHours| hours since the last event (|kMinHours| is
 // the length of the browsing session where subsequent events of the same type
 // do not count again).
 const double kMinHours = 0.5;
-const char kMinHoursParam[] = "user_classifier_min_hours";
 
 // Classification constants.
 const double kActiveConsumerClicksAtLeastOncePerHours = 96;
-const char kActiveConsumerClicksAtLeastOncePerHoursParam[] =
-    "user_classifier_active_consumer_clicks_at_least_once_per_hours";
 const double kRareUserViewsAtMostOncePerHours = 96;
-const char kRareUserViewsAtMostOncePerHoursParam[] =
-    "user_classifier_rare_user_views_at_most_once_per_hours";
 
 // Histograms for logging the estimated average hours to next event. During
 // launch these must match legacy histogram names.
@@ -61,67 +58,35 @@ const char kHistogramAverageHoursToUseSuggestions[] =
 const UserClassifier::Event kEvents[] = {
     UserClassifier::Event::kSuggestionsViewed,
     UserClassifier::Event::kSuggestionsUsed};
-
-// Arrays of pref names, indexed by Event's int value.
-const char* kRateKeys[] = {prefs::kUserClassifierAverageSuggestionsViwedPerHour,
-                           prefs::kUserClassifierAverageSuggestionsUsedPerHour};
-const char* kLastTimeKeys[] = {prefs::kUserClassifierLastTimeToViewSuggestions,
-                               prefs::kUserClassifierLastTimeToUseSuggestions};
-
-// Default lengths of the intervals for new users for the events.
-const double kInitialHoursBetweenEvents[] = {24, 120};
-const char* kInitialHoursBetweenEventsParams[] = {
-    "user_classifier_default_interval_suggestions_viewed",
-    "user_classifier_default_interval_suggestions_used"};
-
-// This verifies that each of the arrays has exactly the same number of values
-// as the number of enum values in UserClassifier::Event. These arrays are all
-// indexed by the integer value of UserClassifier::Event values.
 static_assert(base::size(kEvents) ==
-                      static_cast<int>(UserClassifier::Event::kMaxValue) + 1 &&
-                  base::size(kRateKeys) ==
-                      static_cast<int>(UserClassifier::Event::kMaxValue) + 1 &&
-                  base::size(kLastTimeKeys) ==
-                      static_cast<int>(UserClassifier::Event::kMaxValue) + 1 &&
-                  base::size(kInitialHoursBetweenEvents) ==
-                      static_cast<int>(UserClassifier::Event::kMaxValue) + 1 &&
-                  base::size(kInitialHoursBetweenEventsParams) ==
-                      static_cast<int>(UserClassifier::Event::kMaxValue) + 1,
-              "Fill in info for all event types.");
-
-// Computes the discount rate.
-double GetDiscountRatePerHour() {
-  double discount_rate_per_day = variations::GetVariationParamByFeatureAsDouble(
-      kInterestFeedContentSuggestions, kDiscountRatePerDayParam,
-      kDiscountRatePerDay);
-  // Check for illegal values.
-  if (discount_rate_per_day <= 0 || discount_rate_per_day >= 1) {
-    DLOG(WARNING) << "Illegal value " << discount_rate_per_day
-                  << " for the parameter " << kDiscountRatePerDayParam
-                  << " (must be strictly between 0 and 1; the default "
-                  << kDiscountRatePerDay << " is used, instead).";
-    discount_rate_per_day = kDiscountRatePerDay;
-  }
-  // Compute discount_rate_per_hour such that
-  //   discount_rate_per_day = 1 - e^{-discount_rate_per_hour * 24}.
-  return std::log(1.0 / (1.0 - discount_rate_per_day)) / 24.0;
-}
+                  static_cast<int>(UserClassifier::Event::kMaxValue) + 1,
+              "kEvents should have all enum values.");
 
-double GetInitialHoursBetweenEvents(UserClassifier::Event event) {
-  return variations::GetVariationParamByFeatureAsDouble(
-      kInterestFeedContentSuggestions,
-      kInitialHoursBetweenEventsParams[static_cast<int>(event)],
-      kInitialHoursBetweenEvents[static_cast<int>(event)]);
+const char* GetRateKey(UserClassifier::Event event) {
+  switch (event) {
+    case UserClassifier::Event::kSuggestionsViewed:
+      return prefs::kUserClassifierAverageSuggestionsViwedPerHour;
+    case UserClassifier::Event::kSuggestionsUsed:
+      return prefs::kUserClassifierAverageSuggestionsUsedPerHour;
+  }
 }
 
-double GetMinHours() {
-  return variations::GetVariationParamByFeatureAsDouble(
-      kInterestFeedContentSuggestions, kMinHoursParam, kMinHours);
+const char* GetLastTimeKey(UserClassifier::Event event) {
+  switch (event) {
+    case UserClassifier::Event::kSuggestionsViewed:
+      return prefs::kUserClassifierLastTimeToViewSuggestions;
+    case UserClassifier::Event::kSuggestionsUsed:
+      return prefs::kUserClassifierLastTimeToUseSuggestions;
+  }
 }
 
-double GetMaxHours() {
-  return variations::GetVariationParamByFeatureAsDouble(
-      kInterestFeedContentSuggestions, kMaxHoursParam, kMaxHours);
+double GetInitialHoursBetweenEvents(UserClassifier::Event event) {
+  switch (event) {
+    case UserClassifier::Event::kSuggestionsViewed:
+      return 24;
+    case UserClassifier::Event::kSuggestionsUsed:
+      return 120;
+  }
 }
 
 // Returns the new value of the rate using its |old_value|, assuming
@@ -178,21 +143,7 @@ double GetRateForEstimateHoursBetweenEvents(double estimate_hours,
 }  // namespace
 
 UserClassifier::UserClassifier(PrefService* pref_service, base::Clock* clock)
-    : pref_service_(pref_service),
-      clock_(clock),
-      discount_rate_per_hour_(GetDiscountRatePerHour()),
-      min_hours_(GetMinHours()),
-      max_hours_(GetMaxHours()),
-      active_consumer_clicks_at_least_once_per_hours_(
-          variations::GetVariationParamByFeatureAsDouble(
-              kInterestFeedContentSuggestions,
-              kActiveConsumerClicksAtLeastOncePerHoursParam,
-              kActiveConsumerClicksAtLeastOncePerHours)),
-      rare_viewer_opens_surface_at_most_once_per_hours_(
-          variations::GetVariationParamByFeatureAsDouble(
-              kInterestFeedContentSuggestions,
-              kRareUserViewsAtMostOncePerHoursParam,
-              kRareUserViewsAtMostOncePerHours)) {
+    : pref_service_(pref_service), clock_(clock) {
   // The pref_service_ can be null in tests.
   if (!pref_service_) {
     return;
@@ -214,25 +165,19 @@ UserClassifier::~UserClassifier() = default;
 
 // static
 void UserClassifier::RegisterProfilePrefs(PrefRegistrySimple* registry) {
-  double discount_rate = GetDiscountRatePerHour();
-  double min_hours = GetMinHours();
-  double max_hours = GetMaxHours();
-
   for (Event event : kEvents) {
     double default_rate = GetRateForEstimateHoursBetweenEvents(
-        GetInitialHoursBetweenEvents(event), discount_rate, min_hours,
-        max_hours);
-    registry->RegisterDoublePref(kRateKeys[static_cast<int>(event)],
-                                 default_rate);
-    registry->RegisterTimePref(kLastTimeKeys[static_cast<int>(event)],
-                               base::Time());
+        GetInitialHoursBetweenEvents(event), kDiscountRatePerHour, kMinHours,
+        kMaxHours);
+    registry->RegisterDoublePref(GetRateKey(event), default_rate);
+    registry->RegisterTimePref(GetLastTimeKey(event), base::Time());
   }
 }
 
 void UserClassifier::OnEvent(Event event) {
   double metric_value = UpdateRateOnEvent(event);
-  double avg = GetEstimateHoursBetweenEvents(
-      metric_value, discount_rate_per_hour_, min_hours_, max_hours_);
+  double avg = GetEstimateHoursBetweenEvents(metric_value, kDiscountRatePerHour,
+                                             kMinHours, kMaxHours);
   // We use kMaxHours as the max value below as the maximum value for the
   // histograms must be constant.
   switch (event) {
@@ -249,8 +194,8 @@ void UserClassifier::OnEvent(Event event) {
 
 double UserClassifier::GetEstimatedAvgTime(Event event) const {
   double rate = GetUpToDateRate(event);
-  return GetEstimateHoursBetweenEvents(rate, discount_rate_per_hour_,
-                                       min_hours_, max_hours_);
+  return GetEstimateHoursBetweenEvents(rate, kDiscountRatePerHour, kMinHours,
+                                       kMaxHours);
 }
 
 UserClass UserClassifier::GetUserClass() const {
@@ -260,12 +205,12 @@ UserClass UserClassifier::GetUserClass() const {
   }
 
   if (GetEstimatedAvgTime(Event::kSuggestionsViewed) >=
-      rare_viewer_opens_surface_at_most_once_per_hours_) {
+      kRareUserViewsAtMostOncePerHours) {
     return UserClass::kRareSuggestionsViewer;
   }
 
   if (GetEstimatedAvgTime(Event::kSuggestionsUsed) <=
-      active_consumer_clicks_at_least_once_per_hours_) {
+      kActiveConsumerClicksAtLeastOncePerHours) {
     return UserClass::kActiveSuggestionsConsumer;
   }
 
@@ -304,9 +249,9 @@ double UserClassifier::UpdateRateOnEvent(Event event) {
   }
 
   double hours_since_last_time =
-      std::min(max_hours_, GetHoursSinceLastTime(event));
+      std::min(kMaxHours, GetHoursSinceLastTime(event));
   // Ignore events within the same "browsing session".
-  if (hours_since_last_time < min_hours_) {
+  if (hours_since_last_time < kMinHours) {
     return GetUpToDateRate(event);
   }
 
@@ -315,7 +260,7 @@ double UserClassifier::UpdateRateOnEvent(Event event) {
   double rate = GetRate(event);
   // Add 1 to the discounted rate as the event has happened right now.
   double new_rate =
-      1 + DiscountRate(rate, hours_since_last_time, discount_rate_per_hour_);
+      1 + DiscountRate(rate, hours_since_last_time, kDiscountRatePerHour);
   SetRate(event, new_rate);
   return new_rate;
 }
@@ -326,11 +271,11 @@ double UserClassifier::GetUpToDateRate(Event event) const {
     return 0;
   }
 
-  double hours_since_last_time =
-      std::min(max_hours_, GetHoursSinceLastTime(event));
+  const double hours_since_last_time =
+      std::min(kMaxHours, GetHoursSinceLastTime(event));
 
-  double rate = GetRate(event);
-  return DiscountRate(rate, hours_since_last_time, discount_rate_per_hour_);
+  const double rate = GetRate(event);
+  return DiscountRate(rate, hours_since_last_time, kDiscountRatePerHour);
 }
 
 double UserClassifier::GetHoursSinceLastTime(Event event) const {
@@ -339,29 +284,28 @@ double UserClassifier::GetHoursSinceLastTime(Event event) const {
   }
 
   base::TimeDelta since_last_time =
-      clock_->Now() -
-      pref_service_->GetTime(kLastTimeKeys[static_cast<int>(event)]);
+      clock_->Now() - pref_service_->GetTime(GetLastTimeKey(event));
   return since_last_time.InSecondsF() / 3600;
 }
 
 bool UserClassifier::HasLastTime(Event event) const {
-  return pref_service_->HasPrefPath(kLastTimeKeys[static_cast<int>(event)]);
+  return pref_service_->HasPrefPath(GetLastTimeKey(event));
 }
 
 void UserClassifier::SetLastTimeToNow(Event event) {
-  pref_service_->SetTime(kLastTimeKeys[static_cast<int>(event)], clock_->Now());
+  pref_service_->SetTime(GetLastTimeKey(event), clock_->Now());
 }
 
 double UserClassifier::GetRate(Event event) const {
-  return pref_service_->GetDouble(kRateKeys[static_cast<int>(event)]);
+  return pref_service_->GetDouble(GetRateKey(event));
 }
 
 void UserClassifier::SetRate(Event event, double rate) {
-  pref_service_->SetDouble(kRateKeys[static_cast<int>(event)], rate);
+  pref_service_->SetDouble(GetRateKey(event), rate);
 }
 
 void UserClassifier::ClearRate(Event event) {
-  pref_service_->ClearPref(kRateKeys[static_cast<int>(event)]);
+  pref_service_->ClearPref(GetRateKey(event));
 }
 
 }  // namespace feed

components/feed/core/user_classifier.h

@@ -88,15 +88,6 @@ class UserClassifier {
   PrefService* pref_service_;
   base::Clock* clock_;
 
-  // Params of the rate.
-  const double discount_rate_per_hour_;
-  const double min_hours_;
-  const double max_hours_;
-
-  // Params of the classification.
-  const double active_consumer_clicks_at_least_once_per_hours_;
-  const double rare_viewer_opens_surface_at_most_once_per_hours_;
-
   DISALLOW_COPY_AND_ASSIGN(UserClassifier);
 };
 

components/feed/core/user_classifier_unittest.cc

@@ -9,10 +9,8 @@
 #include <utility>
 
 #include "base/test/metrics/histogram_tester.h"
-#include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_clock.h"
 #include "base/time/time.h"
-#include "components/feed/feed_feature_list.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -80,30 +78,6 @@ TEST_F(FeedUserClassifierTest,
               Eq(UserClass::kActiveSuggestionsConsumer));
 }
 
-TEST_F(FeedUserClassifierTest,
-       ShouldBecomeActiveSuggestionsConsumerByClickingOftenWithDecreasedParam) {
-  // Increase the param to one half.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      kInterestFeedContentSuggestions,
-      {{"user_classifier_active_consumer_clicks_at_least_once_per_hours",
-        "36"}});
-  UserClassifier* user_classifier = CreateUserClassifier();
-
-  // After two clicks still only an active user.
-  user_classifier->OnEvent(UserClassifier::Event::kSuggestionsUsed);
-  test_clock()->Advance(base::TimeDelta::FromHours(1));
-  user_classifier->OnEvent(UserClassifier::Event::kSuggestionsUsed);
-  EXPECT_THAT(user_classifier->GetUserClass(),
-              Eq(UserClass::kActiveSuggestionsViewer));
-
-  // One more click to become an active consumer.
-  test_clock()->Advance(base::TimeDelta::FromHours(1));
-  user_classifier->OnEvent(UserClassifier::Event::kSuggestionsUsed);
-  EXPECT_THAT(user_classifier->GetUserClass(),
-              Eq(UserClass::kActiveSuggestionsConsumer));
-}
-
 TEST_F(FeedUserClassifierTest,
        ShouldBecomeRareSuggestionsViewerUserByNoActivity) {
   UserClassifier* user_classifier = CreateUserClassifier();
@@ -119,26 +93,6 @@ TEST_F(FeedUserClassifierTest,
               Eq(UserClass::kRareSuggestionsViewer));
 }
 
-TEST_F(FeedUserClassifierTest,
-       ShouldBecomeRareSuggestionsViewerByNoActivityWithDecreasedParam) {
-  // Decrease the param to one half.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      kInterestFeedContentSuggestions,
-      {{"user_classifier_rare_user_views_at_most_once_per_hours", "48"}});
-  UserClassifier* user_classifier = CreateUserClassifier();
-
-  // After one days of waiting still an active user.
-  test_clock()->Advance(base::TimeDelta::FromDays(1));
-  EXPECT_THAT(user_classifier->GetUserClass(),
-              Eq(UserClass::kActiveSuggestionsViewer));
-
-  // One more day to become a rare user.
-  test_clock()->Advance(base::TimeDelta::FromDays(1));
-  EXPECT_THAT(user_classifier->GetUserClass(),
-              Eq(UserClass::kRareSuggestionsViewer));
-}
-
 class FeedUserClassifierEventTest
     : public FeedUserClassifierTest,
       public ::testing::WithParamInterface<
@@ -220,31 +174,6 @@ TEST_P(FeedUserClassifierEventTest, ShouldIgnoreSubsequentEventsForHalfAnHour) {
   EXPECT_THAT(user_classifier->GetEstimatedAvgTime(event), Lt(old_rate));
 }
 
-TEST_P(FeedUserClassifierEventTest,
-       ShouldIgnoreSubsequentEventsWithIncreasedLimit) {
-  UserClassifier::Event event = GetParam().first;
-  // Increase the min_hours to 1.0, i.e. 60 minutes.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      kInterestFeedContentSuggestions, {{"user_classifier_min_hours", "1.0"}});
-  UserClassifier* user_classifier = CreateUserClassifier();
-
-  // The initial event.
-  user_classifier->OnEvent(event);
-  // Subsequent events get ignored for the next 60 minutes.
-  for (int i = 0; i < 11; i++) {
-    test_clock()->Advance(base::TimeDelta::FromMinutes(5));
-    double old_rate = user_classifier->GetEstimatedAvgTime(event);
-    user_classifier->OnEvent(event);
-    EXPECT_THAT(user_classifier->GetEstimatedAvgTime(event), Eq(old_rate));
-  }
-  // An event 60 minutes after the initial event is finally not ignored.
-  test_clock()->Advance(base::TimeDelta::FromMinutes(5));
-  double old_rate = user_classifier->GetEstimatedAvgTime(event);
-  user_classifier->OnEvent(event);
-  EXPECT_THAT(user_classifier->GetEstimatedAvgTime(event), Lt(old_rate));
-}
-
 TEST_P(FeedUserClassifierEventTest, ShouldCapDelayBetweenEvents) {
   UserClassifier::Event event = GetParam().first;
   UserClassifier* user_classifier = CreateUserClassifier();
@@ -267,33 +196,6 @@ TEST_P(FeedUserClassifierEventTest, ShouldCapDelayBetweenEvents) {
               Eq(rate_after_a_year));
 }
 
-TEST_P(FeedUserClassifierEventTest,
-       ShouldCapDelayBetweenEventsWithDecreasedLimit) {
-  UserClassifier::Event event = GetParam().first;
-  // Decrease the max_hours to 72, i.e. 3 days.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      kInterestFeedContentSuggestions, {{"user_classifier_max_hours", "72"}});
-  UserClassifier* user_classifier = CreateUserClassifier();
-
-  // The initial event.
-  user_classifier->OnEvent(event);
-  // Wait for an insane amount of time
-  test_clock()->Advance(base::TimeDelta::FromDays(365));
-  user_classifier->OnEvent(event);
-  double rate_after_a_year = user_classifier->GetEstimatedAvgTime(event);
-
-  // Now repeat the same with s/one year/two days.
-  user_classifier->ClearClassificationForDebugging();
-  user_classifier->OnEvent(event);
-  test_clock()->Advance(base::TimeDelta::FromDays(3));
-  user_classifier->OnEvent(event);
-
-  // The results should be the same.
-  EXPECT_THAT(user_classifier->GetEstimatedAvgTime(event),
-              Eq(rate_after_a_year));
-}
-
 INSTANTIATE_TEST_SUITE_P(
     All,  // An empty prefix for the parametrized tests names (no need to
           // distinguish the only instance we make here).