Make a pre-read A/B field-trial

chrome/browser/browser_main.cc

Index: chrome/browser/browser_main.cc
===================================================================
--- chrome/browser/browser_main.cc	(revision 95298)
+++ chrome/browser/browser_main.cc	(working copy)
@@ -165,6 +165,7 @@
 #include <commctrl.h>
 #include <shellapi.h>
 
+#include "base/win/registry.h"  // For PreReadExperiment field-trial.
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/browser_trial.h"
@@ -2026,9 +2027,26 @@
       if (pool)
         pool->Recycle();
 
+      base::TimeDelta elapsed_from_browser_open_start =
+          base::TimeTicks::Now() - browser_open_start;
       UMA_HISTOGRAM_MEDIUM_TIMES("Startup.BrowserOpenTabs",
-                                 base::TimeTicks::Now() - browser_open_start);
+                                 elapsed_from_browser_open_start);
 
+#if defined(OS_WIN)
+      // On Windows, use the result of the PreReadExperiment coin-toss to
+      // output a sub-histogram.
+      base::win::RegKey key(HKEY_CURRENT_USER, L"Software\\Google\\ChromeFrame",

[Sigurður Ásgeirsson, 2011/08/04 13:35:32]

as we discussed, we'd want to test the PreRead key as well, so that we can
correctly bucket chrome.dlls that are loaded by older chrome.exes.

[chrisha, 2011/08/04 14:48:40]

As we discussed, there's the potential for an impedance mismatch between
chrome.exe and chrome.dll with this approach. If an experiment-enabled
chrome.exe runs and sets PreReadExperiment, and then chrome.dll is launched by
an experiment-disabled chrome.exe, then chrome.dll will still report experiment
results even though it hasn't taken place. This can causes problems in the
leading and trailing edges of our experiment, as chrome.dll tends to update
faster and sooner than chrome.exe.

I've moved to using the registry to make the coin-toss result persistent, but I
communicate that result via side channel directly from the running chrome.exe to
the running chrome.dll (using environment variables). This ensures that
experiment results are only reported when the experiment runs.

+                            KEY_QUERY_VALUE);
+      DWORD pre_read = 1;
+      const char* uma_name =
+          "Startup.BrowserOpenTabs_PreReadEnabled";
+      if (key.Valid())
+        key.ReadValueDW(L"PreReadExperiment", &pre_read);
+      if (pre_read == 0)
+        uma_name = "Startup.BrowserOpenTabs_PreReadDisabled";

[jar (doing other things), 2011/08/03 21:24:34]

personal style nit:

This logic is correct and OK.  My preference to avoid typos is to list the
prefix once, and then optionally add one of several postfix strings.  For
example:
char* uma_name = "Startup.BrowserOpenTabs_PreRead";
uma_name += (pre_read == 0): "Disabled" : "Enabled".;

[chrisha, 2011/08/04 14:48:40]

Done.

+      UMA_HISTOGRAM_MEDIUM_TIMES(uma_name, elapsed_from_browser_open_start);

[jar (doing other things), 2011/08/03 21:24:34]

When performing detailed experiments, I've often found it valuable to have a
full 100 buckets.  I'd also sadly predict that you want a longer cutoff in your
tail (you'll get too many samples in your top bucket otherwise).  
I'd suggest:
UMA_HISTOGRAM_CUSTOM_TIMES(uma_name, elapsed_from_browser_open_start,
base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1), 100)

[chrisha, 2011/08/04 14:48:40]

Done.

+#endif
+
       // TODO(mad): Move this call in a proper place on CrOS.
       // http://crosbug.com/17687
 #if !defined(OS_CHROMEOS)