Introduce a MetricsServicesManager class.

This class now owns MetricsService, VariationsService
and RapporService as well as MetricsStateManager.

The motivation is to be able to pass MetricsStateManager
to VariationsService (in a followup CL), so that it
can be used for simulation.

BUG=315807

Review URL: https://codereview.chromium.org/263093003

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

8 files changed, 166 insertions, 50 deletions

diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 935c016..1189149 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -44,8 +44,8 @@
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/metrics/metrics_services_manager.h"
 #include "chrome/browser/metrics/thread_watcher.h"
-#include "chrome/browser/metrics/variations/variations_service.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/crl_set_fetcher.h"
 #include "chrome/browser/net/sdch_dictionary_fetcher.h"
@@ -75,7 +75,6 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "components/policy/core/common/policy_service.h"
-#include "components/rappor/rappor_service.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "content/public/browser/browser_thread.h"
@@ -143,8 +142,7 @@ using content::ResourceDispatcherHost;
 BrowserProcessImpl::BrowserProcessImpl(
     base::SequencedTaskRunner* local_state_task_runner,
     const CommandLine& command_line)
-    : created_metrics_service_(false),
-      created_watchdog_thread_(false),
+    : created_watchdog_thread_(false),
       created_browser_policy_connector_(false),
       created_profile_manager_(false),
       created_local_state_(false),
@@ -205,15 +203,13 @@ void BrowserProcessImpl::StartTearDown() {
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(&SdchDictionaryFetcher::Shutdown));
 
-  // We need to destroy the MetricsService, RapporService, VariationsService,
-  // IntranetRedirectDetector, PromoResourceService, and SafeBrowsing
-  // ClientSideDetectionService (owned by the SafeBrowsingService) before the
-  // io_thread_ gets destroyed, since their destructors can call the URLFetcher
-  // destructor, which does a PostDelayedTask operation on the IO thread. (The
-  // IO thread will handle that URLFetcher operation before going away.)
-  metrics_service_.reset();
-  rappor_service_.reset();
-  variations_service_.reset();
+  // We need to destroy the MetricsServicesManager, IntranetRedirectDetector,
+  // PromoResourceService, and SafeBrowsing ClientSideDetectionService (owned by
+  // the SafeBrowsingService) before the io_thread_ gets destroyed, since their
+  // destructors can call the URLFetcher destructor, which does a
+  // PostDelayedTask operation on the IO thread. (The IO thread will handle that
+  // URLFetcher operation before going away.)
+  metrics_services_manager_.reset();
   intranet_redirect_detector_.reset();
 #if defined(FULL_SAFE_BROWSING) || defined(MOBILE_SAFE_BROWSING)
   if (safe_browsing_service_.get())
@@ -412,16 +408,12 @@ void BrowserProcessImpl::EndSession() {
 
 MetricsService* BrowserProcessImpl::metrics_service() {
   DCHECK(CalledOnValidThread());
-  if (!created_metrics_service_)
-    CreateMetricsService();
-  return metrics_service_.get();
+  return GetMetricsServicesManager()->GetMetricsService();
 }
 
 rappor::RapporService* BrowserProcessImpl::rappor_service() {
   DCHECK(CalledOnValidThread());
-  if (!rappor_service_.get())
-    rappor_service_.reset(new rappor::RapporService());
-  return rappor_service_.get();
+  return GetMetricsServicesManager()->GetRapporService();
 }
 
 IOThread* BrowserProcessImpl::io_thread() {
@@ -459,11 +451,7 @@ net::URLRequestContextGetter* BrowserProcessImpl::system_request_context() {
 
 chrome_variations::VariationsService* BrowserProcessImpl::variations_service() {
   DCHECK(CalledOnValidThread());
-  if (!variations_service_.get()) {
-    variations_service_.reset(
-        chrome_variations::VariationsService::Create(local_state()));
-  }
-  return variations_service_.get();
+  return GetMetricsServicesManager()->GetVariationsService();
 }
 
 BrowserProcessPlatformPart* BrowserProcessImpl::platform_part() {
@@ -784,13 +772,6 @@ void BrowserProcessImpl::ResourceDispatcherHostCreated() {
   ApplyAllowCrossOriginAuthPromptPolicy();
 }
 
-void BrowserProcessImpl::CreateMetricsService() {
-  DCHECK(!created_metrics_service_ && metrics_service_.get() == NULL);
-  created_metrics_service_ = true;
-
-  metrics_service_.reset(new MetricsService);
-}
-
 void BrowserProcessImpl::CreateWatchdogThread() {
   DCHECK(!created_watchdog_thread_ && watchdog_thread_.get() == NULL);
   created_watchdog_thread_ = true;
@@ -979,6 +960,13 @@ void BrowserProcessImpl::CreateSafeBrowsingService() {
 #endif
 }
 
+MetricsServicesManager* BrowserProcessImpl::GetMetricsServicesManager() {
+  DCHECK(CalledOnValidThread());
+  if (!metrics_services_manager_)
+    metrics_services_manager_.reset(new MetricsServicesManager(local_state()));
+  return metrics_services_manager_.get();
+}
+
 void BrowserProcessImpl::ApplyDefaultBrowserPolicy() {
   if (local_state()->GetBoolean(prefs::kDefaultBrowserSettingEnabled)) {
     scoped_refptr<ShellIntegration::DefaultWebClientWorker>
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index dd64ee7..75258d0 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -23,6 +23,7 @@
 
 class ChromeNetLog;
 class ChromeResourceDispatcherHostDelegate;
+class MetricsServicesManager;
 class RemoteDebuggingServer;
 class PrefRegistrySimple;
 class PromoResourceService;
@@ -135,7 +136,6 @@ class BrowserProcessImpl : public BrowserProcess,
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
  private:
-  void CreateMetricsService();
   void CreateWatchdogThread();
   void CreateProfileManager();
   void CreateLocalState();
@@ -151,14 +151,13 @@ class BrowserProcessImpl : public BrowserProcess,
   void CreateStatusTray();
   void CreateBackgroundModeManager();
 
+  MetricsServicesManager* GetMetricsServicesManager();
+
   void ApplyAllowCrossOriginAuthPromptPolicy();
   void ApplyDefaultBrowserPolicy();
   void ApplyMetricsReportingPolicy();
 
-  bool created_metrics_service_;
-  scoped_ptr<MetricsService> metrics_service_;
-
-  scoped_ptr<rappor::RapporService> rappor_service_;
+  scoped_ptr<MetricsServicesManager> metrics_services_manager_;
 
   scoped_ptr<IOThread> io_thread_;
 
@@ -206,8 +205,6 @@ class BrowserProcessImpl : public BrowserProcess,
 
   scoped_ptr<printing::BackgroundPrintingManager> background_printing_manager_;
 
-  scoped_ptr<chrome_variations::VariationsService> variations_service_;
-
   // Manager for desktop notification UI.
   bool created_notification_ui_manager_;
   scoped_ptr<NotificationUIManager> notification_ui_manager_;
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 8f56fa1..d27ca1b 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -462,9 +462,8 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
 #endif  // defined(OS_ANDROID)
 }
 
-MetricsService::MetricsService()
-    : state_manager_(metrics::MetricsStateManager::Create(
-        g_browser_process->local_state())),
+MetricsService::MetricsService(metrics::MetricsStateManager* state_manager)
+    : state_manager_(state_manager),
       recording_active_(false),
       reporting_active_(false),
       test_mode_active_(false),
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 1499ddf..731706b 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -127,7 +127,10 @@ class MetricsService
     SHUTDOWN_COMPLETE = 700,
   };
 
-  MetricsService();
+  // Creates the MetricsService with the given |state_manager|. Does not take
+  // ownership of |state_manager|, instead stores a weak pointer to it. Caller
+  // should ensure that |state_manager| is valid for the lifetime of this class.
+  explicit MetricsService(metrics::MetricsStateManager* state_manager);
   virtual ~MetricsService();
 
   // Initializes metrics recording state. Updates various bookkeeping values in
@@ -472,8 +475,8 @@ class MetricsService
       std::vector<chrome_variations::ActiveGroupId>* synthetic_trials);
 
   // Used to manage various metrics reporting state prefs, such as client id,
-  // low entropy source and whether metrics reporting is enabled.
-  scoped_ptr<metrics::MetricsStateManager> state_manager_;
+  // low entropy source and whether metrics reporting is enabled. Weak pointer.
+  metrics::MetricsStateManager* state_manager_;
 
   base::ActionCallback action_callback_;
 
diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc
index b42d789..801659e 100644
--- a/chrome/browser/metrics/metrics_service_unittest.cc
+++ b/chrome/browser/metrics/metrics_service_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/threading/platform_thread.h"
+#include "chrome/browser/metrics/metrics_state_manager.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
@@ -29,7 +30,9 @@ using metrics::MetricsLogManager;
 
 class TestMetricsService : public MetricsService {
  public:
-  TestMetricsService() {}
+  explicit TestMetricsService(metrics::MetricsStateManager* state_manager)
+      : MetricsService(state_manager) {
+  }
   virtual ~TestMetricsService() {}
 
   MetricsLogManager* log_manager() {
@@ -85,13 +88,19 @@ class TestMetricsLog : public MetricsLog {
 class MetricsServiceTest : public testing::Test {
  public:
   MetricsServiceTest()
-      : testing_local_state_(TestingBrowserProcess::GetGlobal()) {
+      : testing_local_state_(TestingBrowserProcess::GetGlobal()),
+        metrics_state_manager_(metrics::MetricsStateManager::Create(
+            GetLocalState())) {
   }
 
   virtual ~MetricsServiceTest() {
     MetricsService::SetExecutionPhase(MetricsService::UNINITIALIZED_PHASE);
   }
 
+  metrics::MetricsStateManager* GetMetricsStateManager() {
+    return metrics_state_manager_.get();
+  }
+
   PrefService* GetLocalState() {
     return testing_local_state_.Get();
   }
@@ -131,6 +140,7 @@ class MetricsServiceTest : public testing::Test {
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   ScopedTestingLocalState testing_local_state_;
+  scoped_ptr<metrics::MetricsStateManager> metrics_state_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(MetricsServiceTest);
 };
@@ -150,7 +160,7 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCleanShutDown) {
   EnableMetricsReporting();
   GetLocalState()->SetBoolean(prefs::kStabilityExitedCleanly, true);
 
-  TestMetricsService service;
+  TestMetricsService service(GetMetricsStateManager());
   service.InitializeMetricsRecordingState();
   // No initial stability log should be generated.
   EXPECT_FALSE(service.log_manager()->has_unsent_logs());
@@ -179,7 +189,7 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCrash) {
 
   GetLocalState()->SetBoolean(prefs::kStabilityExitedCleanly, false);
 
-  TestMetricsService service;
+  TestMetricsService service(GetMetricsStateManager());
   service.InitializeMetricsRecordingState();
 
   // The initial stability log should be generated and persisted in unsent logs.
@@ -207,7 +217,7 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCrash) {
 }
 
 TEST_F(MetricsServiceTest, RegisterSyntheticTrial) {
-  MetricsService service;
+  MetricsService service(GetMetricsStateManager());
 
   // Add two synthetic trials and confirm that they show up in the list.
   SyntheticTrialGroup trial1(metrics::HashName("TestTrial1"),
@@ -237,7 +247,6 @@ TEST_F(MetricsServiceTest, RegisterSyntheticTrial) {
   WaitUntilTimeChanges(begin_log_time);
 
   // Change the group for the first trial after the log started.
-  // TODO(asvitkine): Assumption that this is > than BeginLoggingWithLog() time.
   SyntheticTrialGroup trial3(metrics::HashName("TestTrial1"),
                              metrics::HashName("Group2"));
   service.RegisterSyntheticFieldTrial(trial3);
diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc
new file mode 100644
index 0000000..95d0ac9
--- /dev/null
+++ b/chrome/browser/metrics/metrics_services_manager.cc
@@ -0,0 +1,49 @@
+// Copyright 2014 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/metrics/metrics_services_manager.h"
+
+#include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/metrics/metrics_state_manager.h"
+#include "chrome/browser/metrics/variations/variations_service.h"
+#include "components/rappor/rappor_service.h"
+
+MetricsServicesManager::MetricsServicesManager(PrefService* local_state)
+    : local_state_(local_state) {
+  DCHECK(local_state);
+}
+
+MetricsServicesManager::~MetricsServicesManager() {
+}
+
+MetricsService* MetricsServicesManager::GetMetricsService() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!metrics_service_)
+    metrics_service_.reset(new MetricsService(GetMetricsStateManager()));
+  return metrics_service_.get();
+}
+
+rappor::RapporService* MetricsServicesManager::GetRapporService() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!rappor_service_)
+    rappor_service_.reset(new rappor::RapporService);
+  return rappor_service_.get();
+}
+
+chrome_variations::VariationsService*
+MetricsServicesManager::GetVariationsService() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!variations_service_) {
+    variations_service_.reset(
+        chrome_variations::VariationsService::Create(local_state_));
+  }
+  return variations_service_.get();
+}
+
+metrics::MetricsStateManager* MetricsServicesManager::GetMetricsStateManager() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (!metrics_state_manager_)
+    metrics_state_manager_ = metrics::MetricsStateManager::Create(local_state_);
+  return metrics_state_manager_.get();
+}
diff --git a/chrome/browser/metrics/metrics_services_manager.h b/chrome/browser/metrics/metrics_services_manager.h
new file mode 100644
index 0000000..66d0c2a
--- /dev/null
+++ b/chrome/browser/metrics/metrics_services_manager.h
@@ -0,0 +1,69 @@
+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_METRICS_METRICS_SERVICES_MANAGER_H_
+#define CHROME_BROWSER_METRICS_METRICS_SERVICES_MANAGER_H_
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/thread_checker.h"
+
+class MetricsService;
+class PrefService;
+
+namespace metrics {
+class MetricsStateManager;
+}
+
+namespace rappor {
+class RapporService;
+}
+
+namespace chrome_variations {
+class VariationsService;
+}
+
+// MetricsServicesManager is a helper class that has ownership over the various
+// metrics-related services in Chrome: MetricsService, RapporService and
+// VariationsService.
+class MetricsServicesManager {
+ public:
+  // Creates the MetricsServicesManager with the |local_state| prefs service.
+  explicit MetricsServicesManager(PrefService* local_state);
+  virtual ~MetricsServicesManager();
+
+  // Returns the MetricsService, creating it if it hasn't been created yet.
+  MetricsService* GetMetricsService();
+
+  // Returns the GetRapporService, creating it if it hasn't been created yet.
+  rappor::RapporService* GetRapporService();
+
+  // Returns the VariationsService, creating it if it hasn't been created yet.
+  chrome_variations::VariationsService* GetVariationsService();
+
+ private:
+  metrics::MetricsStateManager* GetMetricsStateManager();
+
+  // Ensures that all functions are called from the same thread.
+  base::ThreadChecker thread_checker_;
+
+  // Weak pointer to the local state prefs store.
+  PrefService* local_state_;
+
+  // MetricsStateManager which is passed as a parameter to service constructors.
+  scoped_ptr<metrics::MetricsStateManager> metrics_state_manager_;
+
+  // The MetricsService, used for UMA report uploads.
+  scoped_ptr<MetricsService> metrics_service_;
+
+  // The RapporService, for RAPPOR metric uploads.
+  scoped_ptr<rappor::RapporService> rappor_service_;
+
+  // The VariationsService, for server-side experiments infrastructure.
+  scoped_ptr<chrome_variations::VariationsService> variations_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(MetricsServicesManager);
+};
+
+#endif  // CHROME_BROWSER_METRICS_METRICS_SERVICES_MANAGER_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 2ca7810..849dcff 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1219,6 +1219,8 @@
         'browser/metrics/metrics_service_android.cc',
         'browser/metrics/metrics_service.cc',
         'browser/metrics/metrics_service.h',
+        'browser/metrics/metrics_services_manager.cc',
+        'browser/metrics/metrics_services_manager.h',
         'browser/metrics/metrics_state_manager.cc',
         'browser/metrics/metrics_state_manager.h',
         'browser/metrics/perf_provider_chromeos.cc',