Adding a previews IO-thread blacklist

components/previews/previews_experiments.cc

Index: components/previews/previews_experiments.cc
diff --git a/components/previews/previews_experiments.cc b/components/previews/previews_experiments.cc
index 35abce728d98247e689e9e0d1fdcf9072d672928..5181925f8892b7459161665abbd8a06b19dd4772 100644
--- a/components/previews/previews_experiments.cc
+++ b/components/previews/previews_experiments.cc
@@ -1,60 +1,115 @@
 // 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

[tbansal1, 2016/09/15 16:34:25]

s/The number of most recent/The maximum number of recent/

[RyanSturm, 2016/09/19 18:07:25]

Done.

+// 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";

[tbansal1, 2016/09/15 16:34:25]

Specify the units of duration both in the var name and the field trial param
name (i.e., change to "black_list_duration_seconds"). This makes it easier to
read the experiment config without refering to the chromium code.

Also, I would prefer if it is in days (instead of seconds). That would make it
easier to write/read the config. I do not think we would ever make the black
list duration shorter than a day. 

[RyanSturm, 2016/09/19 18:07:25]

Done.

+
 // The string that corresponds to enabled for the variation param experiments.
 const char kExperimentEnabled[] = "true";
 
+// In seconds. Domains are blacklisted for 30 days.
+constexpr int kDefaultBlackListDuration = 30 * 24 * 60 * 60;

[tbansal1, 2016/09/15 16:34:25]

may be change to kDefaultBlackListDurationDays = 30;
and then use FromDays() below.

[RyanSturm, 2016/09/19 18:07:25]

Done.

+
+// Returns the parameter value of |param| as a string. If there is no value for
+// |param|, returns an empty string.
+std::string ParamValue(const std::string& param) {
+  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())

[tbansal1, 2016/09/15 16:34:25]

replace the last three lines by a ternary operator.

[RyanSturm, 2016/09/19 18:07:25]

Done.

+    return std::string();
+  return it->second;
+}
+
 }  // namespace
 
-namespace previews {
+namespace params {
+
+size_t StoredHistoryLengthForBlackList() {
+  std::string param_value = ParamValue(kStoredHistoryLength);
+  size_t history_length;
+  if (!base::StringToSizeT(param_value, &history_length)) {
+    return 4;

[tbansal1, 2016/09/15 16:34:25]

Note that this returns "4" even if the Chrome is not part of field trial. Is
that okay?

[RyanSturm, 2016/09/19 18:07:25]

I believe so, the blacklist won't be used until previews are turned on, and when
they are turned on, either they use the default or they set their own. Either
way, the blacklist won't start storing until a preview is enabled.

+  }
+  return history_length;
+}
+
+int BlackListOptOutThreshold() {
+  std::string param_value = ParamValue(kOptOutThreshold);
+  int opt_out_threshold;
+  if (!base::StringToInt(param_value, &opt_out_threshold)) {
+    return 2;
+  }
+  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(kDefaultBlackListDuration);
+  }
+  return base::TimeDelta::FromSeconds(duration);
+}
+
+}  // namespace params
 
 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);
 }