Add one-time randomization support for FieldTrial, and the ability to

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 1308 matching lines @@
   // watched.
   scoped_ptr<ThreadWatcherList> thread_watcher_list(new ThreadWatcherList());
 
   // Initialize the prefs of the local state.
   browser::RegisterLocalState(local_state);
 
   // Convert active labs into switches. Modifies the current command line.
   about_flags::ConvertFlagsToSwitches(local_state,
                                       CommandLine::ForCurrentProcess());
 
+  // Initialize FieldTrialList to support FieldTrials that use one-time
+  // randomization.  The metrics client ID has been initialized if needed
+  // (and only if metrics collection is enabled) by the call to
+  // browser::RegisterLocalState() above.

[Jói, 2011/04/21 20:23:00]

Whoops, my bad, that's not where the state actually gets created.  Ignore this
file for now, I'll upload an update once I've done this properly.

[jar (doing other things), 2011/04/21 22:10:02]

Perhaps the above comment means this... but to reitterate: There is (or should
be) a public call in metrics_services that will get or create an ID, if UMA is
enabled.  It is better to use that, and not export knowledge about where the
pref is stored (i.e., you're violating encapsulation by exporting infomation
like kMetricsClientID and pulling this directly from the prefs system... and
you're not allowing for a lazy construction, which means you won't run such
tests in the first run of a browser).

Also, be sure you call said metrics functions after we've initialized that
module, which will include decoding of whether UMA is enabled or not.  I think
that module is initialized as of line 1683 below.

If you want to get the "head start" and aren't worried about "first run"
execution of your field trial, then you should at least add a method to metrics
services which pulls the value out of a pref.  This way only the
metrics_services will be aware of the name of the stored pref (and it will be
easier to change said code in metrics services as we evolve).

[Jói, 2011/04/28 01:03:50]

Agreed, this was me thinking I had found a shortcut but I hadn't - and it was
sloppy not to encapsulate the access in a method on the metrics service.

The updated version of this just relies on the metrics service already being
initialized when you call GetClientId on it, which will return the empty string
if it's not running (e.g. if it's not enabled).  Note that this requires me to
move the call to InitializeMetrics quite a bit earlier in the BrowserMain
function; reviewing the various things that can affect whether InitializeMetrics
starts metrics recording or not, this seems OK to me, and looking at
MetricsService::Start it is already delaying the actual work, but please double
check if this looks OK to you.

+  std::string client_id = local_state->GetString(prefs::kMetricsClientID);
+  if (!client_id.empty())
+    base::FieldTrialList::EnableOneTimeRandomization(client_id);
+
   // Now the command line has been mutated based on about:flags, we can run some
   // field trials
   parts->SetupFieldTrials();
 
   // Now that all preferences have been registered, set the install date
   // for the uninstall metrics if this is our first run. This only actually
   // gets used if the user has metrics reporting enabled at uninstall time.
   int64 install_date =
       local_state->GetInt64(prefs::kUninstallMetricsInstallDate);
   if (install_date == 0) {
@@ skipping 548 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("BrowserMain", 0, 0);
   return result_code;
 }