Adds PreviewsType version mechanism for clearing blacklist entries.

components/previews/core/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/core/previews_experiments.h"
 
 #include <map>
 #include <string>
 
 #include "base/logging.h"
 #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";
 
+// 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[] =
     "per_host_max_stored_history_length";
 
 // The maximum number of recent previews navigations the black list looks at to
 // determine if all previews navigations should be disallowed.
 const char kMaxStoredHistoryLengthHostIndifferent[] =
     "host_indifferent_max_stored_history_length";
 
@@ skipping 150 matching lines @@
 bool IsPreviewsTypeEnabled(PreviewsType type) {
   switch (type) {
     case PreviewsType::OFFLINE:
       return ParamValue(kOfflinePagesSlowNetwork) == kExperimentEnabled;
     default:
       NOTREACHED();
       return false;
   }
 }
 
+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;
   return variations::AssociateVariationParams(kClientSidePreviewsFieldTrial,
                                               kEnabled, params) &&
          base::FieldTrialList::CreateFieldTrial(kClientSidePreviewsFieldTrial,
                                                 kEnabled);
 }
 
 }  // namespace previews