Adding FieldTrial support for Chrome Frame to allow measurement of startup time improvements from delaying Chrome shutdown.

The other half of http://crrev.com/124942

BUG=98506
TEST=NONE


Review URL: http://codereview.chromium.org/9600011

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

11 files changed, 132 insertions, 43 deletions

diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index c7f144f..147cdee 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -14,6 +14,7 @@
 #include "base/file_version_info.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/string_util.h"
@@ -34,12 +35,20 @@
 #include "chrome_frame/utils.h"
 #include "ui/base/ui_base_switches.h"
 
+namespace {
+
 #ifdef NDEBUG
 int64 kAutomationServerReasonableLaunchDelay = 1000;  // in milliseconds
 #else
 int64 kAutomationServerReasonableLaunchDelay = 1000 * 10;
 #endif
 
+const char kChromeShutdownDelaySeconds[] = "30";
+const char kWithDelayFieldTrialName[] = "WithShutdownDelay";
+const char kNoDelayFieldTrialName[] = "NoShutdownDelay";
+
+}  // namespace
+
 class ChromeFrameAutomationProxyImpl::TabProxyNotificationMessageFilter
     : public IPC::ChannelProxy::MessageFilter {
  public:
@@ -291,6 +300,10 @@ void AutomationProxyCacheEntry::CreateProxy(ChromeFrameLaunchParams* params,
     if (IsAccessibleMode())
       command_line->AppendSwitch(switches::kForceRendererAccessibility);
 
+    if (params->send_shutdown_delay_switch())
+      command_line->AppendSwitchASCII(switches::kChromeFrameShutdownDelay,
+                                      kChromeShutdownDelaySeconds);
+
     DVLOG(1) << "Profile path: " << params->profile_path().value();
     command_line->AppendSwitchPath(switches::kUserDataDir,
                                    params->profile_path());
@@ -335,9 +348,19 @@ void AutomationProxyCacheEntry::CreateProxy(ChromeFrameLaunchParams* params,
     if (launch_result_ == AUTOMATION_SUCCESS) {
       UMA_HISTOGRAM_TIMES(
           "ChromeFrame.AutomationServerLaunchSuccessTime", delta);
+      UMA_HISTOGRAM_TIMES(
+          base::FieldTrial::MakeName(
+              "ChromeFrame.AutomationServerLaunchSuccessTime",
+              "ChromeShutdownDelay"),
+          delta);
     } else {
       UMA_HISTOGRAM_TIMES(
           "ChromeFrame.AutomationServerLaunchFailedTime", delta);
+      UMA_HISTOGRAM_TIMES(
+          base::FieldTrial::MakeName(
+              "ChromeFrame.AutomationServerLaunchFailedTime",
+              "ChromeShutdownDelay"),
+          delta);
     }
 
     UMA_HISTOGRAM_CUSTOM_COUNTS("ChromeFrame.LaunchResult",
@@ -526,7 +549,9 @@ ChromeFrameAutomationClient::ChromeFrameAutomationClient()
       url_fetcher_(NULL),
       url_fetcher_flags_(PluginUrlRequestManager::NOT_THREADSAFE),
       navigate_after_initialization_(false),
-      route_all_top_level_navigations_(false) {
+      route_all_top_level_navigations_(false),
+      send_shutdown_delay_switch_(true) {
+  InitializeFieldTrials();
 }
 
 ChromeFrameAutomationClient::~ChromeFrameAutomationClient() {
@@ -665,7 +690,8 @@ bool ChromeFrameAutomationClient::InitiateNavigation(
       FilePath profile_path;
       chrome_launch_params_ = new ChromeFrameLaunchParams(parsed_url,
           referrer_gurl, profile_path, L"", SimpleResourceLoader::GetLanguage(),
-          false, false, route_all_top_level_navigations_);
+          false, false, route_all_top_level_navigations_,
+          send_shutdown_delay_switch_);
     } else {
       chrome_launch_params_->set_referrer(referrer_gurl);
       chrome_launch_params_->set_url(parsed_url);
@@ -1001,6 +1027,31 @@ bool ChromeFrameAutomationClient::ProcessUrlRequestMessage(TabProxy* tab,
   return true;
 }
 
+void ChromeFrameAutomationClient::InitializeFieldTrials() {
+  static base::FieldTrial* trial = NULL;
+  if (!trial) {
+    // Do one-time initialization of the field trial here.
+    // TODO(robertshield): End the field trial before March 7th 2013.
+    scoped_refptr<base::FieldTrial> new_trial = new base::FieldTrial(
+        "ChromeShutdownDelay", 1000, kWithDelayFieldTrialName, 2013, 3, 7);
+
+    // Be consistent for this client. Note that this will only have an effect
+    // once the client id is persisted. See http://crbug.com/117188
+    new_trial->UseOneTimeRandomization();
+
+    new_trial->AppendGroup(kNoDelayFieldTrialName, 500);    // 50% without.
+
+    trial = new_trial.get();
+  }
+
+  // Take action depending of which group we randomly land in.
+  if (trial->group_name() == kWithDelayFieldTrialName)
+    send_shutdown_delay_switch_ = true;
+  else
+    send_shutdown_delay_switch_ = false;
+
+}
+
 // These are invoked in channel's background thread.
 // Cannot call any method of the activex here since it is a STA kind of being.
 // By default we marshal the IPC message to the main/GUI thread and from there
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 0115157..26c9901 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -111,13 +111,15 @@ class ChromeFrameLaunchParams :  // NOLINT
                           const std::wstring& profile_name,
                           const std::wstring& language,
                           bool incognito, bool widget_mode,
-                          bool route_all_top_level_navigations)
+                          bool route_all_top_level_navigations,
+                          bool send_shutdown_delay_switch)
     : launch_timeout_(kCommandExecutionTimeout), url_(url),
       referrer_(referrer), profile_path_(profile_path),
       profile_name_(profile_name), language_(language),
       version_check_(true), incognito_mode_(incognito),
       is_widget_mode_(widget_mode),
-      route_all_top_level_navigations_(route_all_top_level_navigations) {
+      route_all_top_level_navigations_(route_all_top_level_navigations),
+      send_shutdown_delay_switch_(send_shutdown_delay_switch) {
   }
 
   ~ChromeFrameLaunchParams() {
@@ -184,6 +186,10 @@ class ChromeFrameLaunchParams :  // NOLINT
     return route_all_top_level_navigations_;
   }
 
+  bool send_shutdown_delay_switch() const {
+    return send_shutdown_delay_switch_;
+  }
+
  protected:
   int launch_timeout_;
   GURL url_;
@@ -195,6 +201,7 @@ class ChromeFrameLaunchParams :  // NOLINT
   bool incognito_mode_;
   bool is_widget_mode_;
   bool route_all_top_level_navigations_;
+  bool send_shutdown_delay_switch_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ChromeFrameLaunchParams);
@@ -366,10 +373,15 @@ class ChromeFrameAutomationClient
   void set_use_chrome_network(bool use_chrome_network) {
     use_chrome_network_ = use_chrome_network;
   }
+
   bool use_chrome_network() const {
     return use_chrome_network_;
   }
 
+  bool send_shutdown_delay_switch() const {
+    return send_shutdown_delay_switch_;
+  }
+
 #ifdef UNIT_TEST
   void set_proxy_factory(ProxyFactory* factory) {
     proxy_factory_ = factory;
@@ -399,11 +411,6 @@ class ChromeFrameAutomationClient
   // the website to put up a confirmation dialog on unload.
   void OnUnload(bool* should_unload);
 
-  void set_route_all_top_level_navigations(
-      bool route_all_top_level_navigations) {
-    route_all_top_level_navigations_ = route_all_top_level_navigations;
-  }
-
  protected:
   // ChromeFrameAutomationProxy::LaunchDelegate implementation.
   virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy,
@@ -432,6 +439,8 @@ class ChromeFrameAutomationClient
   }
 
  private:
+  void InitializeFieldTrials();
+
   void OnMessageReceivedUIThread(const IPC::Message& msg);
   void OnChannelErrorUIThread();
 
@@ -515,6 +524,10 @@ class ChromeFrameAutomationClient
   // page without chrome frame. Defaults to false.
   bool route_all_top_level_navigations_;
 
+  // Set to true if Chrome Frame should tell Chrome to delay shutdown after
+  // we break a connection. Currently used only as part of a field trial.
+  bool send_shutdown_delay_switch_;
+
   friend class BeginNavigateContext;
   friend class CreateExternalTabContext;
 };
diff --git a/chrome_frame/chrome_frame_plugin.h b/chrome_frame/chrome_frame_plugin.h
index a6066f9..e71cb2f 100644
--- a/chrome_frame/chrome_frame_plugin.h
+++ b/chrome_frame/chrome_frame_plugin.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -82,7 +82,8 @@ END_MSG_MAP()
     FilePath actual_profile_name = profile_path.BaseName();
     launch_params_ = new ChromeFrameLaunchParams(url, referrer, profile_path,
         actual_profile_name.value(), SimpleResourceLoader::GetLanguage(),
-        incognito_mode, is_widget_mode, route_all_top_level_navigations);
+        incognito_mode, is_widget_mode, route_all_top_level_navigations,
+        automation_client_->send_shutdown_delay_switch());
     return automation_client_->Initialize(this, launch_params_);
   }
 
diff --git a/chrome_frame/chrome_launcher.cc b/chrome_frame/chrome_launcher.cc
index e943c7f..f1a9733 100644
--- a/chrome_frame/chrome_launcher.cc
+++ b/chrome_frame/chrome_launcher.cc
@@ -20,6 +20,7 @@ namespace {
 const wchar_t* kAllowedSwitches[] = {
   L"automation-channel",
   L"chrome-frame",
+  L"chrome-frame-shutdown-delay",
   L"chrome-version",
   L"disable-background-mode",
   L"disable-popup-blocking",
diff --git a/chrome_frame/chrome_tab.cc b/chrome_frame/chrome_tab.cc
index 69e72f6..8634956 100644
--- a/chrome_frame/chrome_tab.cc
+++ b/chrome_frame/chrome_tab.cc
@@ -16,6 +16,7 @@
 #include "base/file_version_info.h"
 #include "base/logging.h"
 #include "base/logging_win.h"
+#include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
@@ -38,6 +39,7 @@
 #include "chrome_frame/chrome_protocol.h"
 #include "chrome_frame/dll_redirector.h"
 #include "chrome_frame/exception_barrier.h"
+#include "chrome_frame/metrics_service.h"
 #include "chrome_frame/resource.h"
 #include "chrome_frame/utils.h"
 #include "googleurl/src/url_util.h"
@@ -194,7 +196,7 @@ class ChromeTabModule : public CAtlDllModuleT<ChromeTabModule> {
 ChromeTabModule _AtlModule;
 
 base::AtExitManager* g_exit_manager = NULL;
-
+base::FieldTrialList* g_field_trial_list = NULL;
 
 HRESULT RefreshElevationPolicy() {
   const wchar_t kIEFrameDll[] = L"ieframe.dll";
@@ -858,14 +860,24 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
                                  NULL, 0)) {
       logging::LogEventProvider::Initialize(kChromeFrameProvider);
     }
+
+    // Initialize the field test infrastructure. Must be done somewhere that
+    // can only get called once. For Chrome Frame, that is here.
+    g_field_trial_list = new base::FieldTrialList(
+        MetricsService::GetClientID());
   } else if (reason == DLL_PROCESS_DETACH) {
+    delete g_field_trial_list;
+    g_field_trial_list = NULL;
+
     DllRedirector* dll_redirector = DllRedirector::GetInstance();
     DCHECK(dll_redirector);
-
     dll_redirector->UnregisterAsFirstCFModule();
+
     g_patch_helper.UnpatchIfNeeded();
+
     delete g_exit_manager;
     g_exit_manager = NULL;
+
     ShutdownCrashReporting();
   }
   return _AtlModule.DllMain(reason, reserved);
diff --git a/chrome_frame/metrics_service.cc b/chrome_frame/metrics_service.cc
index 0186b24..eefc3ee 100644
--- a/chrome_frame/metrics_service.cc
+++ b/chrome_frame/metrics_service.cc
@@ -50,6 +50,7 @@
 #include "third_party/bzip2/bzlib.h"
 #endif
 
+#include "base/string16.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/synchronization/lock.h"
@@ -81,6 +82,8 @@ static const int kMinMilliSecondsPerUMAUpload = 600000;
 base::LazyInstance<base::ThreadLocalPointer<MetricsService> >
     MetricsService::g_metrics_instance_ = LAZY_INSTANCE_INITIALIZER;
 
+std::string MetricsService::client_id_;
+
 base::Lock MetricsService::metrics_service_lock_;
 
 // Initialize histogram statistics gathering system.
@@ -295,12 +298,7 @@ void MetricsService::SetRecording(bool enabled) {
     return;
 
   if (enabled) {
-    if (client_id_.empty()) {
-      client_id_ = GenerateClientID();
-      // Save client id somewhere.
-    }
     StartRecording();
-
   } else {
     state_ = STOPPED;
   }
@@ -308,18 +306,24 @@ void MetricsService::SetRecording(bool enabled) {
 }
 
 // static
-std::string MetricsService::GenerateClientID() {
-  const int kGUIDSize = 39;
-
-  GUID guid;
-  HRESULT guid_result = CoCreateGuid(&guid);
-  DCHECK(SUCCEEDED(guid_result));
-
-  std::wstring guid_string;
-  int result = StringFromGUID2(guid,
-                               WriteInto(&guid_string, kGUIDSize), kGUIDSize);
-  DCHECK(result == kGUIDSize);
-  return WideToUTF8(guid_string.substr(1, guid_string.length() - 2));
+const std::string& MetricsService::GetClientID() {
+  // TODO(robertshield): Chrome Frame shouldn't generate a new ID on every run
+  // as this apparently breaks some assumptions during metric analysis.
+  // See http://crbug.com/117188
+  if (client_id_.empty()) {
+    const int kGUIDSize = 39;
+
+    GUID guid;
+    HRESULT guid_result = CoCreateGuid(&guid);
+    DCHECK(SUCCEEDED(guid_result));
+
+    string16 guid_string;
+    int result = StringFromGUID2(guid,
+                                 WriteInto(&guid_string, kGUIDSize), kGUIDSize);
+    DCHECK(result == kGUIDSize);
+    client_id_ = WideToUTF8(guid_string.substr(1, guid_string.length() - 2));
+  }
+  return client_id_;
 }
 
 // static
@@ -368,7 +372,8 @@ void MetricsService::StartRecording() {
 
   MetricsLogManager::LogType log_type = (state_ == INITIALIZED) ?
       MetricsLogManager::INITIAL_LOG : MetricsLogManager::ONGOING_LOG;
-  log_manager_.BeginLoggingWithLog(new MetricsLogBase(client_id_, session_id_,
+  log_manager_.BeginLoggingWithLog(new MetricsLogBase(GetClientID(),
+                                                      session_id_,
                                                       GetVersionString()),
                                    log_type);
   if (state_ == INITIALIZED)
diff --git a/chrome_frame/metrics_service.h b/chrome_frame/metrics_service.h
index bcf4026..dbd6afc 100644
--- a/chrome_frame/metrics_service.h
+++ b/chrome_frame/metrics_service.h
@@ -14,6 +14,7 @@
 #include "base/basictypes.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
@@ -35,6 +36,9 @@ class MetricsService : public MetricsServiceBase {
   // Set up client ID, session ID, etc.
   void InitializeMetricsState();
 
+  // Retrieves a client ID to use to identify self to metrics server.
+  static const std::string& GetClientID();
+
  private:
   MetricsService();
   virtual ~MetricsService();
@@ -70,9 +74,6 @@ class MetricsService : public MetricsServiceBase {
   // starting the timer is permitted).
   void HandleIdleSinceLastTransmission(bool in_idle);
 
-  // Generates a new client ID to use to identify self to metrics server.
-  static std::string GenerateClientID();
-
   // ChromeFrame UMA data is uploaded when this timer proc gets invoked.
   static void CALLBACK TransmissionTimerProc(HWND window, unsigned int message,
                                              unsigned int event_id,
@@ -128,7 +129,7 @@ class MetricsService : public MetricsServiceBase {
   std::wstring server_url_;
 
   // The identifier that's sent to the server with the log reports.
-  std::string client_id_;
+  static std::string client_id_;
 
   // A number that identifies the how many times the app has been launched.
   int session_id_;
diff --git a/chrome_frame/test/automation_client_mock.cc b/chrome_frame/test/automation_client_mock.cc
index 3b362c6..e24d05b 100644
--- a/chrome_frame/test/automation_client_mock.cc
+++ b/chrome_frame/test/automation_client_mock.cc
@@ -137,7 +137,7 @@ void CFACWithChrome::SetUp() {
   GURL empty;
   launch_params_ = new ChromeFrameLaunchParams(
       empty, empty, profile_path_, profile_path_.BaseName().value(), L"",
-      false, false, false);
+      false, false, false, false);
   launch_params_->set_version_check(false);
   launch_params_->set_launch_timeout(kSaneAutomationTimeoutMs);
 }
@@ -266,7 +266,7 @@ TEST_F(CFACMockTest, MockedCreateTabOk) {
   GURL empty;
   scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams(
       empty, empty, profile_path_, profile_path_.BaseName().value(), L"",
-      false, false, false));
+      false, false, false, false));
   clp->set_launch_timeout(timeout);
   clp->set_version_check(false);
   EXPECT_TRUE(client_->Initialize(&cfd_, clp));
@@ -299,7 +299,7 @@ TEST_F(CFACMockTest, MockedCreateTabFailed) {
   GURL empty;
   scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams(
       empty, empty, profile_path_, profile_path_.BaseName().value(), L"",
-      false, false, false));
+      false, false, false, false));
   clp->set_launch_timeout(timeout_);
   clp->set_version_check(false);
   EXPECT_TRUE(client_->Initialize(&cfd_, clp));
@@ -340,7 +340,7 @@ TEST_F(CFACMockTest, OnChannelError) {
   GURL empty;
   scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams(
       empty, empty, profile_path_, profile_path_.BaseName().value(), L"",
-      false, false, false));
+      false, false, false, false));
   clp->set_launch_timeout(1);  // Unneeded timeout, but can't be 0.
   clp->set_version_check(false);
 
@@ -460,7 +460,7 @@ TEST_F(CFACMockTest, NavigateTwiceAfterInitToSameUrl) {
   scoped_refptr<ChromeFrameLaunchParams> launch_params(
       new ChromeFrameLaunchParams(
           GURL("http://www.nonexistent.com"), empty, profile_path_,
-          profile_path_.BaseName().value(), L"", false, false, false));
+          profile_path_.BaseName().value(), L"", false, false, false, false));
   launch_params->set_launch_timeout(timeout);
   launch_params->set_version_check(false);
   EXPECT_TRUE(client_->Initialize(&cfd_, launch_params));
diff --git a/chrome_frame/test/chrome_frame_automation_mock.h b/chrome_frame/test/chrome_frame_automation_mock.h
index 3daae53..deb625c 100644
--- a/chrome_frame/test/chrome_frame_automation_mock.h
+++ b/chrome_frame/test/chrome_frame_automation_mock.h
@@ -49,7 +49,7 @@ class AutomationMockDelegate
     GURL empty;
     scoped_refptr<ChromeFrameLaunchParams> clp(
         new ChromeFrameLaunchParams(empty, empty, profile_path, profile_name,
-            language, incognito, is_widget_mode, false));
+            language, incognito, is_widget_mode, false, false));
     clp->set_launch_timeout(launch_timeout);
     clp->set_version_check(perform_version_check);
     automation_client_->Initialize(this, clp);
diff --git a/chrome_frame/test/proxy_factory_mock.cc b/chrome_frame/test/proxy_factory_mock.cc
index 1d0defd..4932b01 100644
--- a/chrome_frame/test/proxy_factory_mock.cc
+++ b/chrome_frame/test/proxy_factory_mock.cc
@@ -38,7 +38,7 @@ ChromeFrameLaunchParams* ProxyFactoryTest::MakeLaunchParams(
   ChromeFrameLaunchParams* params =
       new ChromeFrameLaunchParams(empty, empty, profile_path,
                                   profile_path.BaseName().value(), L"", false,
-                                  false, false);
+                                  false, false, false);
   params->set_launch_timeout(0);
   params->set_version_check(false);
   return params;
diff --git a/chrome_frame/test/run_all_unittests.cc b/chrome_frame/test/run_all_unittests.cc
index defc880..7e23640 100644
--- a/chrome_frame/test/run_all_unittests.cc
+++ b/chrome_frame/test/run_all_unittests.cc
@@ -5,6 +5,7 @@
 #include <atlbase.h>
 
 #include "base/command_line.h"
+#include "base/metrics/field_trial.h"
 #include "base/process_util.h"
 #include "base/test/test_suite.h"
 #include "base/threading/platform_thread.h"
@@ -48,6 +49,10 @@ int main(int argc, char **argv) {
 
   _set_purecall_handler(PureCall);
 
+  // Set up a FieldTrialList to keep any field trials we have going in
+  // Chrome Frame happy.
+  base::FieldTrialList field_trial_list("42");
+
   base::TestSuite test_suite(argc, argv);
 
   SetConfigBool(kChromeFrameHeadlessMode, true);