Adding a previews IO-thread blacklist

components/previews/previews_experiments.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/previews/previews_experiments.h"
 
 #include <map>
 #include <string>
 
 #include "base/metrics/field_trial.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "components/variations/variations_associated_data.h"
 
+namespace previews {
+
 namespace {
 
 // The group of client-side previews experiments.
 const char kClientSidePreviewsFieldTrial[] = "ClientSidePreviews";
 
 const char kEnabled[] = "Enabled";
 
 // Allow offline pages to show for prohibitively slow networks.
 const char kOfflinePagesSlowNetwork[] = "show_offline_pages";
 
+// The number of most recent previews navigations the black list looks at to
+// determine if a domain is blacklisted.
+const char kStoredHistoryLength[] = "stored_history_length";
+
+// The number of recent navigations that were opted out of that would trigger
+// the domain to be blacklisted.
+const char kOptOutThreshold[] = "opt_out_threshold";
+
+// The amount of time a domain remains blacklisted due to opt outs.
+const char kBlackListDuration[] = "black_list_duration";
+
 // The string that corresponds to enabled for the variation param experiments.
 const char kExperimentEnabled[] = "true";
 
+// Returns the parameter value of |param| as a string. If there is no value for
+// |param|, reutnrs an empty string.

[tbansal1, 2016/09/14 17:17:29]

typo in returns

[RyanSturm, 2016/09/14 18:36:43]

Done.

+std::string ParamValue(std::string param) {

[tbansal1, 2016/09/14 17:17:30]

pass |param| by const ref

[RyanSturm, 2016/09/14 18:36:43]

Done.

+  if (!IsIncludedInClientSidePreviewsExperimentsFieldTrial())
+    return std::string();
+  std::map<std::string, std::string> experiment_params;
+  if (!variations::GetVariationParams(kClientSidePreviewsFieldTrial,
+                                      &experiment_params)) {
+    return std::string();
+  }
+  std::map<std::string, std::string>::const_iterator it =
+      experiment_params.find(param);
+  if (it == experiment_params.end())
+    return std::string();
+  return it->second;
+}
+
 }  // namespace
 
-namespace previews {
+namespace params {
+
+int StoredHistoryLengthForBlackList() {
+  std::string param_value = ParamValue(kStoredHistoryLength);
+  int history_length;
+  if (!base::StringToInt(param_value, &history_length)) {
+    return 0;
+  }
+  return history_length;
+}
+
+int BlackListOptOutThreshold() {
+  std::string param_value = ParamValue(kOptOutThreshold);
+  int opt_out_threshold;
+  if (!base::StringToInt(param_value, &opt_out_threshold)) {
+    return 0;
+  }
+  return opt_out_threshold;
+}
+
+base::TimeDelta BlackListDuration() {
+  std::string param_value = ParamValue(kBlackListDuration);
+  int duration;
+  if (!base::StringToInt(param_value, &duration)) {
+    return base::TimeDelta::FromSeconds(0);
+  }
+  return base::TimeDelta::FromSeconds(duration);
+}
+
+bool BlackListParamsAreValid() {
+  return BlackListDuration() > base::TimeDelta::FromSeconds(0) &&
+         BlackListOptOutThreshold > 0 && StoredHistoryLengthForBlackList() > 0;
+}

[tbansal1, 2016/09/14 20:17:50]

nit: space between params.
also, add the comment // namespace params.

[RyanSturm, 2016/09/14 21:01:03]

Done.

+}
 
 bool IsIncludedInClientSidePreviewsExperimentsFieldTrial() {
   // By convention, an experiment in the client-side previews study enables use
   // of at least one client-side previews optimization if its name begins with
   // "Enabled."
   return base::StartsWith(
       base::FieldTrialList::FindFullName(kClientSidePreviewsFieldTrial),
       kEnabled, base::CompareCase::SENSITIVE);
 }
 
 bool IsOfflinePreviewsEnabled() {
-  if (!IsIncludedInClientSidePreviewsExperimentsFieldTrial())
-    return false;
-  std::map<std::string, std::string> experiment_params;
-  if (!variations::GetVariationParams(kClientSidePreviewsFieldTrial,
-                                      &experiment_params)) {
-    return false;
-  }
-  std::map<std::string, std::string>::const_iterator it =
-      experiment_params.find(kOfflinePagesSlowNetwork);
-  return it != experiment_params.end() && it->second == kExperimentEnabled;
+  std::string param_value = ParamValue(kOfflinePagesSlowNetwork);
+  return param_value == kExperimentEnabled;
 }
 
 bool EnableOfflinePreviewsForTesting() {
   std::map<std::string, std::string> params;
   params[kOfflinePagesSlowNetwork] = kExperimentEnabled;
   return variations::AssociateVariationParams(kClientSidePreviewsFieldTrial,
                                               kEnabled, params) &&
          base::FieldTrialList::CreateFieldTrial(kClientSidePreviewsFieldTrial,
                                                 kEnabled);
 }
 
+bool EnableBlackListParamsForTesting(const int history_length,
+                                     const int opt_out_threshold,

[tbansal1, 2016/09/14 17:17:30]

Why is this function needed? Can you not set the variation param using the test
framework provided already.
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/gtest.h?l=44...

[RyanSturm, 2016/09/14 18:36:43]

This uses internal strings that I don't need to expose outside of this class.

[tbansal1, 2016/09/14 20:17:50]

These are not really internal strings since they will be used outside of this
class (in the experiment configs on the server side). The goal typically is to
reduce the release binary size. So, I would still prefer if this is removed.

https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/gtest.h?l=44...

[RyanSturm, 2016/09/14 21:01:03]

Done.

+                                     const int duration_time_in_seconds) {
+  std::map<std::string, std::string> params;
+  params[kStoredHistoryLength] = base::IntToString(history_length);
+  params[kOptOutThreshold] = base::IntToString(opt_out_threshold);
+  params[kBlackListDuration] = base::IntToString(duration_time_in_seconds);
+  return variations::AssociateVariationParams(kClientSidePreviewsFieldTrial,
+                                              kEnabled, params) &&
+         base::FieldTrialList::CreateFieldTrial(kClientSidePreviewsFieldTrial,
+                                                kEnabled);
+}
+
 }  // namespace previews