Adds PreviewsType version mechanism for clearing blacklist entries.

components/previews/core/previews_experiments.cc

Index: components/previews/core/previews_experiments.cc
diff --git a/components/previews/core/previews_experiments.cc b/components/previews/core/previews_experiments.cc
index df82562047688db80168d1a5b2f118a1c217dce1..007a1ba0296e5b48a26a5e4294c9eaa6845f4e71 100644
--- a/components/previews/core/previews_experiments.cc
+++ b/components/previews/core/previews_experiments.cc
@@ -25,6 +25,9 @@ const char kEnabled[] = "Enabled";
 // Allow offline pages to show for prohibitively slow networks.
 const char kOfflinePagesSlowNetwork[] = "show_offline_pages";
 
+// Version of treatment for showing offline pages, if enabled.
+const char kOfflinePagesSlowNetworkVersion[] = "show_offline_pages.version";
+
 // The maximum number of recent previews navigations the black list looks at to
 // determine if a host is blacklisted.
 const char kMaxStoredHistoryLengthPerHost[] =
@@ -195,6 +198,35 @@ bool IsPreviewsTypeEnabled(PreviewsType type) {
   }
 }
 
+int GetPreviewsTypeVersion(PreviewsType type) {
+  DCHECK(IsPreviewsTypeEnabled(type));
+  int version = 0;  // default
+  switch (type) {
+    case PreviewsType::OFFLINE:
+      base::StringToInt(ParamValue(kOfflinePagesSlowNetworkVersion), &version);
+      return version;
+    // List remaining enum cases vs. default to catch when new one is added.
+    case PreviewsType::NONE:
+      break;
+    case PreviewsType::LAST:

[RyanSturm, 2017/02/08 23:51:00]

nit: change "case PreviewsType::LAST: break;" to "default: NOTREACHED(); return
-1;" and get rid of things after the switch.

There could be a unittest around this behavior so that calling
GetPreviewsTypeVersion(PreviewsType::LAST - 1) fails if this switch isn't
updated when a new enum value is added.

[dougarnett, 2017/02/09 17:46:59]

Tarun asked for explicit cases instead of default (which I had) so let's get you
two to agree on approach.

[tbansal1, 2017/02/09 17:48:58]

ryansturm@: What are the advantages of using tests vs. using compile time
checks. Also, see a relevant discussion here:
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/switch...

[RyanSturm, 2017/02/09 17:54:38]

If this will cause GCC to complain when a new value is added, I'm all for it. I
thought that this would need to run with the new value to cause an error.

[dougarnett, 2017/02/13 17:56:17]

Here is example compile time error from warning: error: enumeration value
'SOME_NEW_ENUM_VALUE' not handled in switch [-Werror=switch]

+      break;
+  }
+  NOTREACHED();
+  return -1;
+}
+
+std::unique_ptr<std::vector<std::pair<PreviewsType, int>>>
+GetEnabledPreviews() {
+  std::unique_ptr<std::vector<std::pair<PreviewsType, int>>> enabled_previews(
+      new std::vector<std::pair<PreviewsType, int>>());
+
+  if (IsPreviewsTypeEnabled(PreviewsType::OFFLINE)) {

[bengr, 2017/02/09 17:33:46]

I'd add the loop between NONE and LAST now, even though there's only one type.
One less thing to fix when we add another type.

[dougarnett, 2017/02/13 17:56:17]

Done.

+    enabled_previews->push_back(
+        {PreviewsType::OFFLINE, GetPreviewsTypeVersion(PreviewsType::OFFLINE)});
+  }
+  return enabled_previews;
+}
+
 bool EnableOfflinePreviewsForTesting() {
   std::map<std::string, std::string> params;
   params[kOfflinePagesSlowNetwork] = kExperimentEnabled;