Re-enable pre-read experiment as a finch field trial.

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 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 "chrome/browser/chrome_browser_main.h"
 
 #if defined(TOOLKIT_GTK)
 #include <gtk/gtk.h>
 #endif
 
@@ skipping 689 matching lines @@
   }
 #endif  // defined(OS_MACOSX) || defined(OS_WIN) || defined(OS_LINUX)
 
   // Record collected startup metrics.
   startup_metric_utils::OnBrowserStartupComplete(is_first_run);
 
   // Deletes self.
   new LoadCompleteListener();
 }
 
-// This code is specific to the Windows-only PreReadExperiment field-trial.
 void ChromeBrowserMainParts::RecordPreReadExperimentTime(const char* name,
                                                          base::TimeDelta time) {
   DCHECK(name != NULL);
 
+#if defined(OS_WIN)
+#if defined(GOOGLE_CHROME_BUILD)
+  // The pre-read experiment is Windows and Google Chrome specific.  Only
+  // initialize the field-trial if the experiment is running (environment
+  // variable is set, and valid).
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+  std::string pre_read_group;
+  if (env->GetVar(chrome::kPreReadEnvironmentVariable, &pre_read_group)) {

[Alexei Svitkine (slow), 2013/08/28 17:30:50]

We have a script that parses source code to extract field trial names and group
names, which is necessary to make their hashes known to the dashboard.

If you use a variable (rather than a string literal) for the group name here,
the script won't find those.

You can fix this by invoking those calls explicitly here, e.g.:

if (pre_read_group == "foo")
  base::FieldTrialList::CreateFieldTrial(kPreReadTrialName, "foo");
else if (pre_read_group == "bar")
  base::FieldTrialList::CreateFieldTrial(kPreReadTrialName, "bar");

... etc.

Or you can submit a CL to google3 to add your extra strings to the list the
script knows about that can't be extracted from source.

[Roger McFarlane (Chromium), 2013/08/29 15:28:37]

I've pulled this out to chrome_browser_field_trials_desktop.cc. PTAL?

+    static const char kPreReadTrialName[] = "BrowserPreReadExperiment";
+    base::FieldTrialList::CreateFieldTrial(kPreReadTrialName, pre_read_group);

[Alexei Svitkine (slow), 2013/08/28 17:30:50]

You need to call group() on returned FieldTrial* object, else the field trial
will be considered unused and not be reported in upstream metrics.

[Roger McFarlane (Chromium), 2013/08/29 15:28:37]

Having created a setup function for the experiment this should now be happening
implicitly, IIUC. Right?

+  }
+#endif
+#endif
+
   // This gets called with different histogram names, so we don't want to use
   // the UMA_HISTOGRAM_CUSTOM_TIMES macro--it uses a static variable, and the
   // first call wins.
   AddPreReadHistogramTime(name, time);
-
-#if defined(OS_WIN)
-#if defined(GOOGLE_CHROME_BUILD)
-  // The pre-read experiment is Windows and Google Chrome specific.
-  scoped_ptr<base::Environment> env(base::Environment::Create());
-
-  // Only record the sub-histogram result if the experiment is running
-  // (environment variable is set, and valid).
-  std::string pre_read_percentage;
-  if (env->GetVar(chrome::kPreReadEnvironmentVariable, &pre_read_percentage)) {
-    std::string uma_name(name);
-
-    // We want XP to record a separate histogram, as the loader on XP
-    // is very different from the Vista and Win7 loaders.
-    if (base::win::GetVersion() <= base::win::VERSION_XP)
-      uma_name += "_XP";
-
-    uma_name += "_PreRead_";
-    uma_name += pre_read_percentage;
-    AddPreReadHistogramTime(uma_name.c_str(), time);
-  }
-#endif
-#endif
 }
 
 // -----------------------------------------------------------------------------
 // TODO(viettrungluu): move more/rest of BrowserMain() into BrowserMainParts.
 
 #if defined(OS_WIN)
 #define DLLEXPORT __declspec(dllexport)
 
 // We use extern C for the prototype DLLEXPORT to avoid C++ name mangling.
 extern "C" {
@@ skipping 948 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif
 #endif
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }