Make a pre-read A/B field-trial

chrome/browser/browser_main.cc

 // Copyright (c) 2011 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/browser_main.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 147 matching lines @@
 // TODO(port): several win-only methods have been pulled out of this, but
 // BrowserMain() as a whole needs to be broken apart so that it's usable by
 // other platforms. For now, it's just a stub. This is a serious work in
 // progress and should not be taken as an indication of a real refactoring.
 
 #if defined(OS_WIN)
 #include <windows.h>
 #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"
 #include "chrome/browser/browser_util_win.h"
 #include "chrome/browser/first_run/try_chrome_dialog_view.h"
 #include "chrome/browser/first_run/upgrade_util_win.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/rlz/rlz.h"
 #include "chrome/browser/ui/views/user_data_dir_dialog.h"
 #include "chrome/installer/util/helper.h"
@@ skipping 1841 matching lines @@
       upgrade_util::SaveLastModifiedTimeOfExe();
 #endif
 
       // Record now as the last successful chrome start.
       GoogleUpdateSettings::SetLastRunTime();
       // Call Recycle() here as late as possible, before going into the loop
       // because Start() will add things to it while creating the main window.
       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)
       // If we're running tests (ui_task is non-null), then we don't want to
       // call FetchLanguageListFromTranslateServer
       if (parameters.ui_task == NULL && translate_manager != NULL) {
         // TODO(willchan): Get rid of this after TranslateManager doesn't use
         // the default request context. http://crbug.com/89396.
         // This is necessary to force |default_request_context_| to be
@@ skipping 74 matching lines @@
 #if defined(OS_CHROMEOS)
   // To be precise, logout (browser shutdown) is not yet done, but the
   // remaining work is negligible, hence we say LogoutDone here.
   chromeos::BootTimesLoader::Get()->AddLogoutTimeMarker("LogoutDone",
                                                         false);
   chromeos::BootTimesLoader::Get()->WriteLogoutTimes();
 #endif
   TRACE_EVENT_END_ETW("BrowserMain", 0, 0);
   return result_code;
 }