Fix SimpleCache field trial creation.

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 7e90ba8697adc7c624adb9c148535e3862f43fff..fcc132e7d6a97d5239a3f51b0c09a36bb03d86f9 100644
--- a/chrome/browser/chrome_browser_field_trials.cc
+++ b/chrome/browser/chrome_browser_field_trials.cc
@@ -204,15 +204,15 @@ void ChromeBrowserFieldTrials::SetUpSimpleCacheFieldTrial() {
   if (parsed_command_line_.HasSwitch(switches::kUseSimpleCacheBackend)) {
     const std::string opt_value = parsed_command_line_.GetSwitchValueASCII(
         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, "off")) {

[SteveT, 2013/04/17 15:41:02]

Couple things here:

(1) Appending a new group with the same name as the default name is something
that we avoid doing. If you want to force your experiment to be disabled, we
instead recommend appending Yes with weight 0 (which makes the default group 
No" get chosen with 100% probability).

(2) However, it looks like you're dealing with the case where the user is aware
of SimpleCacheBackend, which means that if they opt in or out explicitly, we
don't want them in an experiment that returns data about the feature.

You should instead use a pattern where:

If the user opts in explicitly, tell the feature to be turned on and DON'T make
a FieldTrial.

If the user opts out explicitly, tell the feature to be turned off and DON'T
make a Field Trial.

If the user does not opt in or out, create the field trial with the experimental
weight that you want (kSimpleCacheProbability = 1 on ANDROID, 100 otherwise).

Does this make sense/work for you?

[Alexei Svitkine (slow), 2013/04/17 15:46:52]

Agree with Steve on all points.

Also, I'd like to expand on one aspect - I'm not sure if you're ever planning to
do a server-side config for this trial, but if you are, you should also make
sure to not query the state of the field trial (i.e. not use
FieldTrialList::FindFullName()) if the user explicitly opted in or opted out of
the trial.

[gavinp, 2013/04/17 16:19:42]

There's two considerations that cause me to want everyone in the field trial

(1) We use the field trial name, with FindFullName() to control the feature in
net::. Currently net:: doesn't take command line options or global options, so
this has been the pattern. But, for now, this is what we are doing. This came up
in the initial review of this code.

(2) I'd really really like data on explicit opt in vs opt out, especially since
this feature has no field trial running (no experiments approval yet (hint hint
odean)), and it was absolutely amazing using the field trial to see the results
for the dev channel opt ins already.

So I made an upload that calls the groups "ExplicitFoo" and "ExperimentFoo". Is
this better?

[gavinp, 2013/04/17 16:19:42]

Alexei,

Oh no! Right now FindFullName() is exactly how we turn the cache on and off. Is
there an alternative?

+      trial->AppendGroup("No", 100);
+      return;
+    }
     if (LowerCaseEqualsASCII(opt_value, "on")) {
       trial->AppendGroup("Yes", 100);
       return;