Facilitate a FieldTrial in the renderer

I added a command line for the renderer that accepts a FieldTrial
name and value, and forces that value to be activated in the
renderer.  As a result, any FieldTrial setting that is specified
by the browser process can be set (forced) in the renderer
process.  Such settings can then be used to establish names
of histograms, which means all processes can work in sync
on a single field trial (and generate data).  This should
allow A/B tests to be run that modulate the page load times.

Dave: Please review/confirm that you are happy with the changes to
render_view.cc.  Note that all I did was change the names and limits
for the histograms (they now go up to 3 minutes).  The MakeName()
allows me to get an A/B test of the impact of DNS pre-resolution.

Mike: Please review the code for passing along switch settings.

r=davemoore,mbelshe
Review URL: http://codereview.chromium.org/115525

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

8 files changed, 162 insertions, 13 deletions

diff --git a/base/field_trial.cc b/base/field_trial.cc
index d0085e6..fc12f31 100644
--- a/base/field_trial.cc
+++ b/base/field_trial.cc
@@ -13,6 +13,9 @@ using base::Time;
 // static
 const int FieldTrial::kNotParticipating = -1;
 
+// static
+const char FieldTrial::kPersistentStringSeparator('/');
+
 //------------------------------------------------------------------------------
 // FieldTrial methods and members.
 
@@ -50,6 +53,40 @@ 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.
 
@@ -75,6 +112,9 @@ FieldTrialList::~FieldTrialList() {
 
 // static
 void FieldTrialList::Register(FieldTrial* trial) {
+  DCHECK(global_);
+  if (!global_)
+    return;
   AutoLock auto_lock(global_->lock_);
   DCHECK(!global_->PreLockedFind(trial->name()));
   trial->AddRef();
diff --git a/base/field_trial.h b/base/field_trial.h
index 4d04d30..83cbcf6 100644
--- a/base/field_trial.h
+++ b/base/field_trial.h
@@ -75,6 +75,11 @@ class FieldTrial : public base::RefCounted<FieldTrial> {
  public:
   static const int kNotParticipating;
 
+  // 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.
+
   typedef int Probability;  // Use scaled up probability.
 
   // The name is used to register the instance with the FieldTrialList class,
@@ -107,6 +112,18 @@ 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.
diff --git a/base/field_trial_unittest.cc b/base/field_trial_unittest.cc
index 6c818e5..f678b0b 100644
--- a/base/field_trial_unittest.cc
+++ b/base/field_trial_unittest.cc
@@ -114,3 +114,44 @@ TEST_F(FieldTrialTest, OneWinner) {
   EXPECT_EQ(trial->group(), winner_index);
   EXPECT_EQ(winner_name, trial->group_name());
 }
+
+TEST_F(FieldTrialTest, Save) {
+  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/");
+
+  // Create a winning group.
+  trial->AppendGroup("Winner", 10);
+  EXPECT_EQ(trial->MakePersistentString(), "Some name/Winner");
+}
+
+TEST_F(FieldTrialTest, Restore) {
+  FieldTrial* trial = FieldTrial::RestorePersistentString("Some name/winner");
+  EXPECT_EQ(trial->group_name(), "winner");
+  EXPECT_EQ(trial->name(), "Some name");
+}
+
+TEST_F(FieldTrialTest, BogusRestore) {
+  const FieldTrial *trial = FieldTrial::RestorePersistentString("MissingSlash");
+  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+
+  trial = FieldTrial::RestorePersistentString("MissingGroupName/");
+  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+
+  trial = FieldTrial::RestorePersistentString("/MissingName");
+  EXPECT_EQ(trial, static_cast<FieldTrial *>(NULL));
+}
+
+TEST_F(FieldTrialTest, DuplicateRestore) {
+  FieldTrial* trial = new FieldTrial("Some name", 10);
+  trial->AppendGroup("Winner", 10);
+  EXPECT_EQ(trial->MakePersistentString(), "Some name/Winner");
+
+  // It is OK if we redundantly specify a winner.
+  EXPECT_EQ(trial, FieldTrial::RestorePersistentString("Some name/Winner"));
+
+  // But it is an error to try to change to a different winner.
+  EXPECT_EQ(FieldTrial::RestorePersistentString("Some name/Loser"),
+            static_cast<FieldTrial *>(NULL));
+}
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index e8b608d..fa0c54c 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -16,6 +16,7 @@
 #include "app/win_util.h"
 #endif
 #include "base/command_line.h"
+#include "base/field_trial.h"
 #include "base/linked_ptr.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -281,6 +282,16 @@ bool BrowserRenderProcessHost::Init() {
   const std::wstring locale = g_browser_process->GetApplicationLocale();
   cmd_line.AppendSwitchWithValue(switches::kLang, 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))
+    cmd_line.AppendSwitchWithValue(switches::kForceFieldTestNameAndValue,
+        ASCIIToWide(field_trial->MakePersistentString()));
+
 #if defined(OS_POSIX)
   if (browser_command_line.HasSwitch(switches::kRendererCmdPrefix)) {
     // launch the renderer child with some prefix (usually "gdb --args")
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index f7e495b..e93abe7 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -454,6 +454,18 @@ 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.
+const wchar_t kForceFieldTestNameAndValue[]    = L"force-fieldtest";
+
 // Enables the prototype of the next version of the New Tab page.
 const wchar_t kNewNewTabPage[]                 = L"new-new-tab-page";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 324e4a4..34745f189 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -173,6 +173,8 @@ extern const wchar_t kEnableOmnibox2[];
 extern const wchar_t kDisableAudio[];
 extern const wchar_t kSimpleDataSource[];
 
+extern const wchar_t kForceFieldTestNameAndValue[];
+
 extern const wchar_t kNewNewTabPage[];
 
 }  // namespace switches
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 8a311af..33a25de 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -15,6 +15,7 @@
 #include "app/resource_bundle.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/field_trial.h"
 #include "base/gfx/png_encoder.h"
 #include "base/gfx/native_widget_types.h"
 #include "base/string_piece.h"
@@ -249,7 +250,7 @@ RenderView* RenderView::Create(
 void RenderView::SetNextPageID(int32 next_page_id) {
   // This method should only be called during process startup, and the given
   // page id had better not exceed our current next page id!
-  DCHECK(next_page_id_ == 1);
+  DCHECK_EQ(next_page_id_, 1);
   DCHECK(next_page_id >= next_page_id_);
   next_page_id_ = next_page_id;
 }
@@ -534,7 +535,7 @@ void RenderView::PrintPage(const ViewMsg_PrintPage_Params& params,
 
   // Get the size of the compiled EMF.
   unsigned buf_size = emf.GetDataSize();
-  DCHECK(buf_size > 128);
+  DCHECK_GT(buf_size, 128u);
   ViewHostMsg_DidPrintPage_Params page_params;
   page_params.data_size = 0;
   page_params.emf_data_handle = NULL;
@@ -1532,7 +1533,7 @@ void RenderView::DocumentElementAvailable(WebFrame* frame) {
     frame->GrantUniversalAccess();
 
   // Tell extensions to self-register their js contexts.
-  // TODO:(rafaelw): This is kind of gross. We need a way to call through
+  // TODO(rafaelw): This is kind of gross. We need a way to call through
   // the glue layer to retrieve the current v8::Context.
   if (frame->GetURL().SchemeIs(chrome::kExtensionScheme))
     ExtensionProcessBindings::RegisterExtensionContext(frame);
@@ -3009,19 +3010,33 @@ void RenderView::DumpLoadHistograms() const {
 
   // Client side redirects will have no request time
   if (request_time.ToInternalValue() != 0) {
-    UMA_HISTOGRAM_TIMES("Renderer.All.RequestToStart", request_to_start);
-    UMA_HISTOGRAM_TIMES("Renderer.All.RequestToFinish", request_to_finish);
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        FieldTrial::MakeName("Renderer2.RequestToStart", "DnsImpact").data(),
+        request_to_start);
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        FieldTrial::MakeName("Renderer2.RequestToFinish", "DnsImpact").data(),
+        request_to_finish);
     if (request_to_first_layout.ToInternalValue() >= 0) {
-      UMA_HISTOGRAM_TIMES(
-        "Renderer.All.RequestToFirstLayout", request_to_first_layout);
+      UMA_HISTOGRAM_MEDIUM_TIMES(
+          FieldTrial::MakeName("Renderer2.RequestToFirstLayout",
+                               "DnsImpact").data(),
+          request_to_first_layout);
     }
   }
-  UMA_HISTOGRAM_TIMES("Renderer.All.StartToFinishDoc", start_to_finish_doc);
-  UMA_HISTOGRAM_TIMES("Renderer.All.FinishDocToFinish", finish_doc_to_finish);
-  UMA_HISTOGRAM_TIMES("Renderer.All.StartToFinish", start_to_finish);
+  UMA_HISTOGRAM_MEDIUM_TIMES(
+      FieldTrial::MakeName("Renderer2.StartToFinishDoc", "DnsImpact").data(),
+      start_to_finish_doc);
+  UMA_HISTOGRAM_MEDIUM_TIMES(
+      FieldTrial::MakeName("Renderer2.FinishDocToFinish", "DnsImpact").data(),
+      finish_doc_to_finish);
+  UMA_HISTOGRAM_MEDIUM_TIMES(
+      FieldTrial::MakeName("Renderer2.StartToFinish", "DnsImpact").data(),
+      start_to_finish);
   if (start_to_first_layout.ToInternalValue() >= 0) {
-    UMA_HISTOGRAM_TIMES(
-      "Renderer.All.StartToFirstLayout", start_to_first_layout);
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        FieldTrial::MakeName("Renderer2.StartToFirstLayout",
+                             "DnsImpact").data(),
+        start_to_first_layout);
   }
 }
 
diff --git a/chrome/renderer/renderer_main.cc b/chrome/renderer/renderer_main.cc
index 74b5334..fc4d291 100644
--- a/chrome/renderer/renderer_main.cc
+++ b/chrome/renderer/renderer_main.cc
@@ -5,6 +5,7 @@
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/command_line.h"
+#include "base/field_trial.h"
 #include "base/histogram.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
@@ -88,12 +89,22 @@ int RendererMain(const MainFunctionParams& parameters) {
   platform.InitSandboxTests(no_sandbox);
 
   // Initialize histogram statistics gathering system.
-  // Don't create StatisticsRecorde in the single process mode.
+  // Don't create StatisticsRecorder in the single process mode.
   scoped_ptr<StatisticsRecorder> statistics;
   if (!StatisticsRecorder::WasStarted()) {
     statistics.reset(new StatisticsRecorder());
   }
 
+  // Initialize statistical testing infrastructure.
+  FieldTrialList field_trial;
+  // Ensure any field trials in browser are reflected into renderer.
+  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);
+  }
+
   {
     RenderProcess render_process;
     bool run_loop = true;