Add simple cache backend experiment hidden behind a flag.

chrome/browser/chrome_browser_field_trials.cc

Index: chrome/browser/chrome_browser_field_trials.cc
diff --git a/chrome/browser/chrome_browser_field_trials.cc b/chrome/browser/chrome_browser_field_trials.cc
index 1c9a854e5c034f3c4c1a199cb271b27e3dab09aa..6a1765926dd0789aa314dfc482e43117662b9439 100644
--- a/chrome/browser/chrome_browser_field_trials.cc
+++ b/chrome/browser/chrome_browser_field_trials.cc
@@ -59,6 +59,7 @@ ChromeBrowserFieldTrials::~ChromeBrowserFieldTrials() {
 void ChromeBrowserFieldTrials::SetupFieldTrials(
     const base::Time& install_time) {
   chrome_variations::SetupUniformityFieldTrials(install_time);
+  SetUpSimpleCacheFieldTrial();
 #if !defined(OS_ANDROID)
   SetupDesktopFieldTrials();
 #endif  // defined(OS_ANDROID)
@@ -191,6 +192,42 @@ void ChromeBrowserFieldTrials::DisableShowProfileSwitcherTrialIfNecessary() {
   }
 }
 
+// Sets up the experiment. The actual cache backend choice is made in the net/
+// internals by looking at the experiment state.
+void ChromeBrowserFieldTrials::SetUpSimpleCacheFieldTrial() {
+  if (parsed_command_line_.HasSwitch(switches::kUseSimpleCacheBackend)) {

[rvargas (doing something else), 2013/03/15 18:20:16]

It is much simpler to remove the command line argument and use the field trial
override (so basically there's no extra logic here).

The user would do something like
--force-fieldtrials="SimpleCacheTrial/Yes/"

to select the simple backend.

This also means that the probability of enabling the simple backend is hardcoded
to 0. (for platforms with full Finch support).

[SteveT, 2013/03/15 18:30:41]

This is true, but there are a couple small catches with forced:

(1) You can't control it from about:flags, so it really has to be someone
running Chrome from the command line.
(2) It still marks you as being in the experiment group, so we'll start sending
stats with your behaviour. I think this is OK because your experiment is pretty
invisible to the user, but in most cases we want to avoid people who know they
have the feature on from sending experimental stats (hence my suggestion of only
creating the FieldTrial if no flag is explicitly set).

+    std::string opt_value = parsed_command_line_.GetSwitchValueASCII(

[SteveT, 2013/03/15 17:32:49]

nit: const std::string

[pasko-google - do not use, 2013/03/15 18:27:52]

Done.

+        switches::kUseSimpleCacheBackend);
+    if (LowerCaseEqualsASCII(opt_value, "off")) {
+      // This is the default.
+      return;
+    }
+    const base::FieldTrial::Probability kDivisor = 100;
+    scoped_refptr<base::FieldTrial> trial(
+        base::FieldTrialList::FactoryGetFieldTrial("SimpleCacheTrial", kDivisor,
+                                                   "No", 2013, 12, 31, NULL));
+    trial->UseOneTimeRandomization();
+    if (LowerCaseEqualsASCII(opt_value, "on")) {
+      trial->AppendGroup("Yes", 100);
+      return;
+    }
+#if defined(OS_ANDROID)
+    if (LowerCaseEqualsASCII(opt_value, "experiment")) {
+      // TODO(pasko): Make this the default on Android when the simple cache

[rvargas (doing something else), 2013/03/15 18:20:16]

That would also mean that there's no need to special case Android until we are
ready to run actual experiments.

[pasko-google - do not use, 2013/03/15 19:05:30]

I just wanted to be clear about the intentions. I tried the simpler version in
the style of "only what we need right now", but it created legitimate
confusions. So I went closer to something that can go into release with minor
changes in probabilities.

I can remove it now since everyone seems to have a good idea of what is
happening.

+      // adds a few more necessary features. Also adjust the probability.
+      base::FieldTrial::Probability simple_cache_probability = 1;

[SteveT, 2013/03/15 17:32:49]

nit: const base::FieldTrial::Probability

[pasko-google - do not use, 2013/03/15 18:27:52]

Done.

[pasko-google - do not use, 2013/03/15 18:27:52]

Done.

+      trial->AppendGroup("Yes", simple_cache_probability);
+      trial->AppendGroup("Control", simple_cache_probability);

[SteveT, 2013/03/15 17:32:49]

For now, you're going to want to call

trial->group()

here to ensure your experiment is actually registered. Later, when you access
the field trial state (likely using FieldTrialList::FindFullName rather than
FieldTrial::group), you can remove this line.

[pasko-google - do not use, 2013/03/15 18:27:52]

Done.

+      return;
+    }
+    CHECK(0) << "Only values (on|off|experiment) are supported for option "

[rvargas (doing something else), 2013/03/15 18:20:16]

Please don't crash release builds based on a command line... and avoid messages
on checks (they just increase the executable size)

[pasko-google - do not use, 2013/03/15 19:05:30]

My argument: there is value in failing early on incorrect command-line
arguments. Otherwise people often are doing not what they think they are doing.
We have a few cases with this in net/ that killed quite a lot of my time
previously. I think the binary size cost justifies the time spent on
understanding why the code does not work as intended.

[rvargas (doing something else), 2013/03/15 19:16:25]

Not convinced. The most common mistake is not to forget "on" or "off", but to
forget the name of the flag, and a check here doesn't help at all. This is a
pattern that we don't use, and crashing the product doesn't help random people
that want to do something with the command line. The bar for adding crash
statements is much higher.

Binary size is a real problem, and checks/dchecks don't provide an opportunity
to see the message anyway. If someone is looking for it, a comment by the check
works better.

[pasko-google - do not use, 2013/03/15 21:30:57]

I still have the opinion that the message overhead is small compared to the
overall experiment overhead. Though this is all minor, don't want to spend too
much time on it.

Done.

+             << switches::kUseSimpleCacheBackend << ".";

[SteveT, 2013/03/15 17:32:49]

Question: There is no NOTREACHED() equivalent for Release builds?

[pasko-google - do not use, 2013/03/15 18:27:52]

I did not find one :(

+#endif
+    CHECK(0) << "Only values (on|off) are supported for option "
+             << switches::kUseSimpleCacheBackend << ".";
+  }
+}
+
 void ChromeBrowserFieldTrials::SetUpCacheSensitivityAnalysisFieldTrial() {
   const base::FieldTrial::Probability kDivisor = 100;