Experiment to disable Prerendering with fast wind-down

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index f97d9cef68fe32ccad83161c5424530b0f6eaf48..6f7fb7e498bd23aa074e99665c60566d47695075 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -18,6 +18,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/sys_info.h"
@@ -611,6 +612,33 @@ bool PrerenderManager::IsNoStatePrefetch() {
   return GetMode() == PRERENDER_MODE_NOSTATE_PREFETCH;
 }
 
+bool PrerenderManager::IsPrerenderSilenceExperiment() {
+  // The group name should contain expiration time formatted as:
+  //   "ExperimentYes_expires_YYYY-MM-DDTHH:MM:SSZ".
+  std::string group_name =
+      base::FieldTrialList::FindFullName("PrerenderSilence");
+  const char kExperimentPrefix[] = "ExperimentYes";
+  if (!base::StartsWith(group_name, kExperimentPrefix,
+                        base::CompareCase::INSENSITIVE_ASCII)) {
+    return false;
+  }
+  const char kExperimentPrefixWithExpiration[] = "ExperimentYes_expires_";
+  if (!base::StartsWith(group_name, kExperimentPrefixWithExpiration,
+                        base::CompareCase::INSENSITIVE_ASCII)) {
+    // Without expiration day in the group name, behave as a normal experiment,
+    // i.e. sticky to the Chrome session.
+    return true;

[mattcary, 2016/08/25 16:15:21]

Not to belabor the point, but won't we turn in the experiment on unconditionally
if we have a flag like "ExperimentYes_until_2016-11-20"? That seems a little
permissive. I was thinking it would be safer to do something like:
if (group_name == kExperimentPrefix) return true;
else if (!StartsWith(kExperimentPrefixWithExperiation))  return false;

[pasko, 2016/08/25 16:25:39]

That's because we commonly like suffixing experiment groups: "ExperimentYes1",
"ExperimentYes2" etc. In case we wanted to see on Beta, say, how much noise
there is in the number of crashes per client. Not sure it is applicable here,
just following the common practice to allow suffixes.

+  }
+  base::Time expiration_time;
+  if (!base::Time::FromString(
+          group_name.c_str() + (arraysize(kExperimentPrefixWithExpiration) - 1),
+          &expiration_time)) {

[mmenke, 2016/08/25 19:22:50]

Why is this needed?  Experiments can have their expiration time on the server.

[pasko, 2016/08/26 11:22:12]

Server-based experiments are sticky until Chrome is restarted. By encoding an
additional expiration time here we can cut off the experiment faster. Not sure
it really affects a large volume of users, but estimating the number was not
easy. Also I am one of the users who does not like surprises to take effect for
a long time, and restarts Chrome once every 3 weeks (huge mistake, I know), so
this final straw convinced me that this extra small piece of code is handy to
provide a fast experiment wind down.

There are a few details in the doc linked from the bug.

[mmenke, 2016/08/26 16:44:47]

I remain extremely skeptical this serves any useful purpose, but not going to
fight over it.

[pasko, 2016/08/26 17:39:31]

Thank you. I am somewhat skeptical too, but figuring out how much skeptical I
should be was not worth it.

+    DLOG(ERROR) << "Could not parse expiration date in group: " << group_name;
+    return false;
+  }
+  return GetCurrentTime() < expiration_time;
+}
+
 bool PrerenderManager::IsWebContentsPrerendering(
     const WebContents* web_contents,
     Origin* origin) const {
@@ -882,6 +910,9 @@ std::unique_ptr<PrerenderHandle> PrerenderManager::AddPrerender(
     origin = ORIGIN_GWS_PRERENDER;
   }
 
+  if (origin != ORIGIN_OFFLINE && IsPrerenderSilenceExperiment())
+    return nullptr;
+
   GURL url = url_arg;
   GURL alias_url;
   if (IsControlGroup() && MaybeGetQueryStringBasedAliasURL(url, &alias_url))