Create A/B test of SDCH

To do this, I needed to add the feature that ALL FieldTrials that are
established in the browser process are forwarded and established in
the corresponding renderer processes.  This then allows both DNS impact,
 as well as SDCH inmpact (and any other field tests) to be studied
at the same time in a single binary.

This checkin also establishes a pattern that when we're doing A/B tests
via a histogram such as RequestToFinish, that we produce names for
all groups, rather than leaving one group as the "default" or "empty postfix"
group.  This is critical for naming various sub-groups when a multitude
of tests are taking place at the same time.

BUG=15479

r=mbelshe
Review URL: http://codereview.chromium.org/150087

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19595 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 207 insertions, 103 deletions

diff --git a/base/field_trial.cc b/base/field_trial.cc
index fc12f31..549822f 100644
--- a/base/field_trial.cc
+++ b/base/field_trial.cc
@@ -14,7 +14,10 @@ using base::Time;
 const int FieldTrial::kNotParticipating = -1;
 
 // static
-const char FieldTrial::kPersistentStringSeparator('/');
+const int FieldTrial::kAllRemainingProbability = -2;
+
+// static
+const char FieldTrialList::kPersistentStringSeparator('/');
 
 //------------------------------------------------------------------------------
 // FieldTrial methods and members.
@@ -53,40 +56,6 @@ std::string FieldTrial::MakeName(const std::string& name_prefix,
   return big_string.append(FieldTrialList::FindFullName(trial_name));
 }
 
-std::string FieldTrial::MakePersistentString() const {
-  DCHECK_EQ(name_.find(kPersistentStringSeparator), std::string::npos);
-  DCHECK_EQ(group_name_.find(kPersistentStringSeparator), std::string::npos);
-
-  std::string persistent(name_);
-  persistent = persistent.append(1, kPersistentStringSeparator);
-  persistent = persistent.append(group_name_);
-  return persistent;
-}
-
-// static
-FieldTrial* FieldTrial::RestorePersistentString(const std::string &persistent) {
-  size_t split_point = persistent.find(kPersistentStringSeparator);
-  if (std::string::npos == split_point)
-    return NULL;  // Bogus string.
-  std::string new_name(persistent, 0, split_point);
-  std::string new_group_name(persistent, split_point + 1);
-  if (new_name.empty() || new_group_name.empty())
-    return NULL;  // Incomplete string.
-
-  FieldTrial *field_trial;
-  field_trial = FieldTrialList::Find(new_name);
-  if (field_trial) {
-    // In single process mode, we may have already created the field trial.
-    if (field_trial->group_name_ != new_group_name)
-      return NULL;  // Conflicting group name :-(.
-  } else {
-    const int kTotalProbability = 100;
-    field_trial = new FieldTrial(new_name, kTotalProbability);
-    field_trial->AppendGroup(new_group_name, kTotalProbability);
-  }
-  return field_trial;
-}
-
 //------------------------------------------------------------------------------
 // FieldTrialList methods and members.
 
@@ -151,3 +120,58 @@ FieldTrial* FieldTrialList::PreLockedFind(const std::string& name) {
     return NULL;
   return it->second;
 }
+
+// static
+void FieldTrialList::StatesToString(std::string* output) {
+  if (!global_)
+    return;
+  DCHECK(output->empty());
+  for (RegistrationList::iterator it = global_->registered_.begin();
+       it != global_->registered_.end(); ++it) {
+    const std::string name = it->first;
+    const std::string group_name = it->second->group_name();
+    if (group_name.empty())
+      continue;  // No definitive winner in this trial.
+    DCHECK_EQ(name.find(kPersistentStringSeparator), std::string::npos);
+    DCHECK_EQ(group_name.find(kPersistentStringSeparator), std::string::npos);
+    output->append(name);
+    output->append(1, kPersistentStringSeparator);
+    output->append(group_name);
+    output->append(1, kPersistentStringSeparator);
+  }
+}
+
+// static
+bool FieldTrialList::StringAugmentsState(const std::string& prior_state) {
+  DCHECK(global_);
+  if (prior_state.empty() || !global_)
+    return true;
+
+  size_t next_item = 0;
+  while (next_item < prior_state.length()) {
+    size_t name_end = prior_state.find(kPersistentStringSeparator, next_item);
+    if (name_end == prior_state.npos || next_item == name_end)
+      return false;
+    size_t group_name_end = prior_state.find(kPersistentStringSeparator,
+                                             name_end + 1);
+    if (group_name_end == prior_state.npos || name_end + 1 == group_name_end)
+      return false;
+    std::string name(prior_state, next_item, name_end - next_item);
+    std::string group_name(prior_state, name_end + 1,
+                           group_name_end - name_end - 1);
+    next_item = group_name_end + 1;
+
+    FieldTrial *field_trial(FieldTrialList::Find(name));
+    if (field_trial) {
+      // In single process mode, we may have already created the field trial.
+      if (field_trial->group_name() != group_name)
+        return false;
+      continue;
+    }
+    const int kTotalProbability = 100;
+    field_trial = new FieldTrial(name, kTotalProbability);
+    field_trial->AppendGroup(group_name, kTotalProbability);
+  }
+  return true;
+}
+
diff --git a/base/field_trial.h b/base/field_trial.h
index 39a5bc7..6d3c101 100644
--- a/base/field_trial.h
+++ b/base/field_trial.h
@@ -72,14 +72,16 @@
 
 class FieldTrial : public base::RefCounted<FieldTrial> {
  public:
-  static const int kNotParticipating;
+  typedef int Probability;  // Probability type for being selected in a trial.
 
-  // Define a separator charactor to use when creating a persistent form of an
-  // instance.  This is intended for use as a command line argument, passed to a
-  // second process to mimic our state (i.e., provide the same group name).
-  static const char kPersistentStringSeparator;  // Currently a slash.
+  // A return value to indicate that a given instance has not yet had a group
+  // assignment (and hence is not yet participating in the trial).
+  static const int kNotParticipating;
 
-  typedef int Probability;  // Use scaled up probability.
+  // Provide an easy way to assign all remaining probability to a group.  Note
+  // that this will force an instance to participate, and make it illegal to
+  // attempt to probabalistically add any other groups to the trial.
+  static const Probability kAllRemainingProbability;
 
   // The name is used to register the instance with the FieldTrialList class,
   // and can be used to find the trial (only one trial can be present for each
@@ -111,18 +113,6 @@ class FieldTrial : public base::RefCounted<FieldTrial> {
   static std::string MakeName(const std::string& name_prefix,
                               const std::string& trial_name);
 
-  // Create a persistent representation of the instance that could be resurected
-  // in another process.  This allows randomization to be done in one process,
-  // and secondary processes can by synchronized on the result.
-  // The resulting string contains only the name, the trial name, and a "/"
-  // separator.
-  std::string MakePersistentString() const;
-
-  // Using a string created by MakePersistentString(), construct a new instance
-  // that has the same state as the original instance.  Currently only the
-  // group_name_ and name_ are restored.
-  static FieldTrial* RestorePersistentString(const std::string &persistent);
-
  private:
   // The name of the field trial, as can be found via the FieldTrialList.
   // This is empty of the trial is not in the experiment.
@@ -159,6 +149,11 @@ class FieldTrial : public base::RefCounted<FieldTrial> {
 // Only one instance of this class exists.
 class FieldTrialList {
  public:
+  // Define a separator charactor to use when creating a persistent form of an
+  // instance.  This is intended for use as a command line argument, passed to a
+  // second process to mimic our state (i.e., provide the same group name).
+  static const char kPersistentStringSeparator;  // Currently a slash.
+
   // This singleton holds the global list of registered FieldTrials.
   FieldTrialList();
   // Destructor Release()'s references to all registered FieldTrial instances.
@@ -176,6 +171,21 @@ class FieldTrialList {
 
   static std::string FindFullName(const std::string& name);
 
+  // Create a persistent representation of all FieldTrial instances for
+  // resurrection in another process.  This allows randomization to be done in
+  // one process, and secondary processes can by synchronized on the result.
+  // The resulting string contains only the names, the trial name, and a "/"
+  // separator.
+  static void StatesToString(std::string* output);
+
+  // Use a previously generated state string (re: StatesToString()) augment the
+  // current list of field tests to include the supplied tests, and using a 100%
+  // probability for each test, force them to have the same group string.  This
+  // is commonly used in a sub-process, to carry randomly selected state in a
+  // parent process into this sub-process.
+  //  Currently only the group_name_ and name_ are restored.
+  static bool StringAugmentsState(const std::string& prior_state);
+
   // The time of construction of the global map is recorded in a static variable
   // and is commonly used by experiments to identify the time since the start
   // of the application.  In some experiments it may be useful to discount
@@ -185,6 +195,7 @@ class FieldTrialList {
     if (global_)
       return global_->application_start_time_;
     // For testing purposes only, or when we don't yet have a start time.
+    // TODO(jar): Switch to TimeTicks
     return base::Time::Now();
   }
 
@@ -192,10 +203,14 @@ class FieldTrialList {
   // Helper function should be called only while holding lock_.
   FieldTrial* PreLockedFind(const std::string& name);
 
+  // A map from FieldTrial names to the actual instances.
   typedef std::map<std::string, FieldTrial*> RegistrationList;
 
   static FieldTrialList* global_;  // The singleton of this class.
 
+  // A helper value made availabel to users, that shows when the FieldTrialList
+  // was initialized.  Note that this is a singleton instance, and hence is a
+  // good approximation to the start of the process.
   base::Time application_start_time_;
 
   // Lock for access to registered_.
@@ -206,3 +221,4 @@ class FieldTrialList {
 };
 
 #endif  // BASE_FIELD_TRIAL_H_
+
diff --git a/base/field_trial_unittest.cc b/base/field_trial_unittest.cc
index f678b0b..59d8e15 100644
--- a/base/field_trial_unittest.cc
+++ b/base/field_trial_unittest.cc
@@ -116,42 +116,64 @@ TEST_F(FieldTrialTest, OneWinner) {
 }
 
 TEST_F(FieldTrialTest, Save) {
+  std::string save_string;
+
   FieldTrial* trial = new FieldTrial("Some name", 10);
   // There is no winner yet, so no textual group name is associated with trial.
   EXPECT_EQ(trial->group_name(), "");
-  EXPECT_EQ(trial->MakePersistentString(), "Some name/");
+  FieldTrialList::StatesToString(&save_string);
+  EXPECT_EQ(save_string, "");
+  save_string.clear();
 
   // Create a winning group.
   trial->AppendGroup("Winner", 10);
-  EXPECT_EQ(trial->MakePersistentString(), "Some name/Winner");
+  FieldTrialList::StatesToString(&save_string);
+  EXPECT_EQ(save_string, "Some name/Winner/");
+  save_string.clear();
+
+  // Create a second trial and winning group.
+  FieldTrial* trial2 = new FieldTrial("xxx", 10);
+  trial2->AppendGroup("yyyy", 10);
+
+  FieldTrialList::StatesToString(&save_string);
+  // We assume names are alphabetized... though this is not critical.
+  EXPECT_EQ(save_string, "Some name/Winner/xxx/yyyy/");
 }
 
 TEST_F(FieldTrialTest, Restore) {
-  FieldTrial* trial = FieldTrial::RestorePersistentString("Some name/winner");
-  EXPECT_EQ(trial->group_name(), "winner");
-  EXPECT_EQ(trial->name(), "Some name");
-}
+  EXPECT_EQ(NULL, FieldTrialList::Find("Some_name"));
+  EXPECT_EQ(NULL, FieldTrialList::Find("xxx"));
 
-TEST_F(FieldTrialTest, BogusRestore) {
-  const FieldTrial *trial = FieldTrial::RestorePersistentString("MissingSlash");
-  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+  FieldTrialList::StringAugmentsState("Some_name/Winner/xxx/yyyy/");
+
+  FieldTrial* trial = FieldTrialList::Find("Some_name");
+  ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
+  EXPECT_EQ(trial->group_name(), "Winner");
+  EXPECT_EQ(trial->name(), "Some_name");
 
-  trial = FieldTrial::RestorePersistentString("MissingGroupName/");
-  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+  trial = FieldTrialList::Find("xxx");
+  ASSERT_NE(static_cast<FieldTrial*>(NULL), trial);
+  EXPECT_EQ(trial->group_name(), "yyyy");
+  EXPECT_EQ(trial->name(), "xxx");
+}
 
-  trial = FieldTrial::RestorePersistentString("/MissingName");
-  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+TEST_F(FieldTrialTest, BogusRestore) {
+  EXPECT_FALSE(FieldTrialList::StringAugmentsState("MissingSlash"));
+  EXPECT_FALSE(FieldTrialList::StringAugmentsState("MissingGroupName/"));
+  EXPECT_FALSE(FieldTrialList::StringAugmentsState("MissingFinalSlash/gname"));
+  EXPECT_FALSE(FieldTrialList::StringAugmentsState("/noname, only group/"));
 }
 
 TEST_F(FieldTrialTest, DuplicateRestore) {
   FieldTrial* trial = new FieldTrial("Some name", 10);
   trial->AppendGroup("Winner", 10);
-  EXPECT_EQ(trial->MakePersistentString(), "Some name/Winner");
+  std::string save_string;
+  FieldTrialList::StatesToString(&save_string);
+  EXPECT_EQ("Some name/Winner/", save_string);
 
   // It is OK if we redundantly specify a winner.
-  EXPECT_EQ(trial, FieldTrial::RestorePersistentString("Some name/Winner"));
+  EXPECT_TRUE(FieldTrialList::StringAugmentsState(save_string));
 
   // But it is an error to try to change to a different winner.
-  EXPECT_EQ(FieldTrial::RestorePersistentString("Some name/Loser"),
-            static_cast<FieldTrial *>(NULL));
+  EXPECT_FALSE(FieldTrialList::StringAugmentsState("Some name/Loser/"));
 }
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 91bc5e4..5a52734 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -690,15 +690,34 @@ int BrowserMain(const MainFunctionParams& parameters) {
   PluginService::GetInstance()->SetChromePluginDataDir(profile->GetPath());
 
   // Prepare for memory caching of SDCH dictionaries.
-  SdchManager sdch_manager;  // Construct singleton database.
-  sdch_manager.set_sdch_fetcher(new SdchDictionaryFetcher);
-  // Use default of "" so that all domains are supported.
+  // Perform A/B test to measure global impact of SDCH support.
+  // Set up a field trial to see what disabling SDCH does to latency of page
+  // layout globally.
+  FieldTrial::Probability kSDCH_DIVISOR = 100;
+  FieldTrial::Probability kSDCH_PROBABILITY_PER_GROUP = 50;  // 50% probability.
+  scoped_refptr<FieldTrial> sdch_trial =
+      new FieldTrial("GlobalSdch", kSDCH_DIVISOR);
+
+  bool need_to_init_sdch = true;
   std::string switch_domain("");
   if (parsed_command_line.HasSwitch(switches::kSdchFilter)) {
     switch_domain =
         WideToASCII(parsed_command_line.GetSwitchValue(switches::kSdchFilter));
+  } else {
+    sdch_trial->AppendGroup("_global_disable_sdch",
+                            kSDCH_PROBABILITY_PER_GROUP);
+    int sdch_enabled = sdch_trial->AppendGroup("_global_enable_sdch",
+                                               kSDCH_PROBABILITY_PER_GROUP);
+    need_to_init_sdch = (sdch_enabled == sdch_trial->group());
+  }
+
+  scoped_ptr<SdchManager> sdch_manager;  // Singleton database.
+  if (need_to_init_sdch) {
+    sdch_manager.reset(new SdchManager);
+    sdch_manager->set_sdch_fetcher(new SdchDictionaryFetcher);
+    // Use default of "" so that all domains are supported.
+    sdch_manager->EnableSdchSupport(switch_domain);
   }
-  sdch_manager.EnableSdchSupport(switch_domain);
 
   MetricsService* metrics = NULL;
   if (!parsed_command_line.HasSwitch(switches::kDisableMetrics)) {
diff --git a/chrome/browser/net/dns_global.cc b/chrome/browser/net/dns_global.cc
index 68cfad3..d18ea45 100644
--- a/chrome/browser/net/dns_global.cc
+++ b/chrome/browser/net/dns_global.cc
@@ -581,12 +581,15 @@ DnsPrefetcherInit::DnsPrefetcherInit(PrefService* user_prefs,
   int parallel_4_prefetch = trial_->AppendGroup("_parallel_4_prefetch",
                                                 kProbabilityPerGroup);
   // Set congestion detection at 500ms, rather than the 1 second default.
-  int max_500ms_prefetch = trial_->AppendGroup("_max_500ms_prefetch_queue",
+  int max_500ms_prefetch = trial_->AppendGroup("_max_500ms_queue_prefetch",
                                                kProbabilityPerGroup);
   // Set congestion detection at 2 seconds instead of the 1 second default.
-  int max_2s_prefetch = trial_->AppendGroup("_max_2s_prefetch_queue",
+  int max_2s_prefetch = trial_->AppendGroup("_max_2s_queue_prefetch",
                                             kProbabilityPerGroup);
 
+  trial_->AppendGroup("_default_enabled_prefetch",
+      FieldTrial::kAllRemainingProbability);
+
   if (trial_->group() != disabled_prefetch) {
     // Initialize the DNS prefetch system.
 
diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h
index b4af6f4..608dba8 100644
--- a/chrome/browser/net/dns_master.h
+++ b/chrome/browser/net/dns_master.h
@@ -99,6 +99,9 @@ class DnsMaster : public base::RefCountedThreadSafe<DnsMaster> {
   // values into the current referrer list.
   void DeserializeReferrers(const ListValue& referral_list);
 
+  // For unit test code only.
+  size_t max_concurrent_lookups() const { return max_concurrent_lookups_; }
+
  private:
   FRIEND_TEST(DnsMasterTest, BenefitLookupTest);
   FRIEND_TEST(DnsMasterTest, ShutdownWhenResolutionIsPendingTest);
diff --git a/chrome/browser/net/dns_master_unittest.cc b/chrome/browser/net/dns_master_unittest.cc
index 921b43b..c2b3d27 100644
--- a/chrome/browser/net/dns_master_unittest.cc
+++ b/chrome/browser/net/dns_master_unittest.cc
@@ -285,7 +285,7 @@ TEST_F(DnsMasterTest, SingleLookupTest) {
   EXPECT_GT(testing_master->peak_pending_lookups(), names.size() / 2);
   EXPECT_LE(testing_master->peak_pending_lookups(), names.size());
   EXPECT_LE(testing_master->peak_pending_lookups(),
-            DnsPrefetcherInit::kMaxConcurrentLookups);
+            testing_master->max_concurrent_lookups());
 
   testing_master->Shutdown();
 }
@@ -340,7 +340,7 @@ TEST_F(DnsMasterTest, ConcurrentLookupTest) {
   EXPECT_GT(testing_master->peak_pending_lookups(), names.size() / 2);
   EXPECT_LE(testing_master->peak_pending_lookups(), names.size());
   EXPECT_LE(testing_master->peak_pending_lookups(),
-            DnsPrefetcherInit::kMaxConcurrentLookups);
+            testing_master->max_concurrent_lookups());
 
   testing_master->Shutdown();
 }
@@ -366,7 +366,7 @@ TEST_F(DnsMasterTest, DISABLED_MassiveConcurrentLookupTest) {
 
   EXPECT_LE(testing_master->peak_pending_lookups(), names.size());
   EXPECT_LE(testing_master->peak_pending_lookups(),
-            DnsPrefetcherInit::kMaxConcurrentLookups);
+            testing_master->max_concurrent_lookups());
 
   testing_master->Shutdown();
 }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index fac595f..03c961a 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -311,15 +311,14 @@ bool BrowserRenderProcessHost::Init() {
   const std::string locale = g_browser_process->GetApplicationLocale();
   cmd_line.AppendSwitchWithValue(switches::kLang, ASCIIToWide(locale));
 
-  // If we run a FieldTrial that we want to pass to the renderer, this is where
-  // the SINGULAR trial name and value should be specified.  Note that only one
-  // such flag should be passed/set in the renderer command line.
-
-  // Today we're watching the impact of DNS on some page load times.
-  FieldTrial* field_trial = FieldTrialList::Find("DnsImpact");
-  if (field_trial && (field_trial->group() != FieldTrial::kNotParticipating))
+  // If we run FieldTrials, we want to pass to their state to the renderer so
+  // that it can act in accordance with each state, or record histograms
+  // relating to the FieldTrial states.
+  std::string field_trial_states;
+  FieldTrialList::StatesToString(&field_trial_states);
+  if (!field_trial_states.empty())
     cmd_line.AppendSwitchWithValue(switches::kForceFieldTestNameAndValue,
-        ASCIIToWide(field_trial->MakePersistentString()));
+        ASCIIToWide(field_trial_states));
 
 #if defined(OS_POSIX)
   const bool has_cmd_prefix =
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index cd9da9e..be103f9 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -473,16 +473,16 @@ const wchar_t kDisableAudio[]                  = L"disable-audio";
 // is cleaned up and playback testing completed.
 const wchar_t kSimpleDataSource[]              = L"simple-data-source";
 
-// Some field tests may be performed in the browser, and the randomly selected
-// outcome needs to be propogated to the renderer to appropriately modify the
-// histogram names that will be tested.  This command line argument is only
-// parsed by the renderer, and consists of a field test name, and a forced
-// selection of an outcome.  For example, if a field test "DnsImpact" has
-// selected "_disabled_prefetch" as a current test, then the render should be
-// passed the command line:
-//      force-fieldtest=DnsImpact/_disabled_prefetch
-// The renderer will then force said named field test to exist, and will force
-// the selected outcome to have the indicated text value.
+// Some field tests may rendomized in the browser, and the randomly selected
+// outcome needs to be propogated to the renderer.  For instance, this is used
+// to modify histograms recorded in the renderer, or to get the renderer to
+// also set of its state (initialize, or not initialize components) to match the
+// experiment(s).
+// The argument is a string-ized list of experiment names, and the associated
+// value that was randomly selected.  In the recent implementetaion, the
+// persistent representation generated by field_trial.cc and later decoded, is a
+// list of name and value pairs, separated by slashes. See field trial.cc for
+// current details.
 const wchar_t kForceFieldTestNameAndValue[]    = L"force-fieldtest";
 
 // Allows the new tab page resource to be loaded from a local HTML file. This
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 2fad6fb..5b33bd2 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1326,7 +1326,7 @@ void RenderView::DidReceiveTitle(WebView* webview,
 void RenderView::DidFinishLoadForFrame(WebView* webview, WebFrame* frame) {
   WebDataSource* ds = frame->GetDataSource();
   NavigationState* navigation_state = NavigationState::FromDataSource(ds);
-  // TODO(darin): It should not be possible for navigation_state to be null 
+  // TODO(darin): It should not be possible for navigation_state to be null
   // here!
   if (navigation_state)
     navigation_state->set_finish_load_time(Time::Now());
@@ -1344,7 +1344,7 @@ void RenderView::DidFinishDocumentLoadForFrame(WebView* webview,
                                                WebFrame* frame) {
   WebDataSource* ds = frame->GetDataSource();
   NavigationState* navigation_state = NavigationState::FromDataSource(ds);
-  // TODO(darin): It should not be possible for navigation_state to be null 
+  // TODO(darin): It should not be possible for navigation_state to be null
   // here!
   if (navigation_state)
     navigation_state->set_finish_document_load_time(Time::Now());
@@ -2894,10 +2894,28 @@ void RenderView::DumpLoadHistograms() const {
 
   UMA_HISTOGRAM_MEDIUM_TIMES("Renderer4.BeginToCommit", commit - begin);
   UMA_HISTOGRAM_MEDIUM_TIMES("Renderer4.BeginToFinishDoc", finish_doc - begin);
+
+  static const TimeDelta kBeginToFinishMin(TimeDelta::FromMilliseconds(10));
+  static const TimeDelta kBeginToFinishMax(TimeDelta::FromMinutes(10));
+  static const size_t kBeginToFinishBucketCount(100);
+
+  UMA_HISTOGRAM_CUSTOM_TIMES("Renderer4.BeginToFinish",
+      finish - begin, kBeginToFinishMin,
+      kBeginToFinishMax, kBeginToFinishBucketCount);
+
+  DCHECK(FieldTrialList::Find("DnsImpact") &&
+         !FieldTrialList::Find("DnsImpact")->group_name().empty());
   UMA_HISTOGRAM_CUSTOM_TIMES(
       FieldTrial::MakeName("Renderer4.BeginToFinish", "DnsImpact").data(),
-      finish - begin, TimeDelta::FromMilliseconds(10),
-      TimeDelta::FromMinutes(10), 100);
+      finish - begin, kBeginToFinishMin,
+      kBeginToFinishMax, kBeginToFinishBucketCount);
+
+  DCHECK(FieldTrialList::Find("GlobalSdch") &&
+         !FieldTrialList::Find("GlobalSdch")->group_name().empty());
+  UMA_HISTOGRAM_CUSTOM_TIMES(
+      FieldTrial::MakeName("Renderer4.BeginToFinish", "GlobalSdch").data(),
+      finish - begin, kBeginToFinishMin,
+      kBeginToFinishMax, kBeginToFinishBucketCount);
 
   UMA_HISTOGRAM_MEDIUM_TIMES("Renderer4.CommitToFinish", finish - commit);
 
diff --git a/chrome/renderer/renderer_main.cc b/chrome/renderer/renderer_main.cc
index 55ca339..2dfe061 100644
--- a/chrome/renderer/renderer_main.cc
+++ b/chrome/renderer/renderer_main.cc
@@ -110,8 +110,8 @@ int RendererMain(const MainFunctionParams& parameters) {
   if (parsed_command_line.HasSwitch(switches::kForceFieldTestNameAndValue)) {
     std::string persistent(WideToASCII(parsed_command_line.GetSwitchValue(
         switches::kForceFieldTestNameAndValue)));
-    FieldTrial *field_trial = FieldTrial::RestorePersistentString(persistent);
-    DCHECK(field_trial);
+    bool ret = field_trial.StringAugmentsState(persistent);
+    DCHECK(ret);
   }
 
   {