Pass MetricsServiceClient into MetricsService, and create a simple stubbed...

Pass MetricsServiceClient into MetricsService, and create a simple stubbed TestMetricsServiceClient class.

BUG=374237,374235
TEST=compiles
R=asvitkine@chromium.org, blundell@chromium.org

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

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

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -33,8 +33,7 @@ metrics::SystemProfileProto::Channel AsProtobufChannel(
 
 }  // namespace
 
-ChromeMetricsServiceClient::ChromeMetricsServiceClient()
-    : MetricsServiceClient() {
+ChromeMetricsServiceClient::ChromeMetricsServiceClient() {
 }
 
 ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {

chrome/browser/metrics/chrome_metrics_service_client.h

@@ -12,7 +12,7 @@
 
 // ChromeMetricsServiceClient provides an implementation of MetricsServiceClient
 // that depends on chrome/.
-class ChromeMetricsServiceClient : metrics::MetricsServiceClient {
+class ChromeMetricsServiceClient : public metrics::MetricsServiceClient {
  public:
   ChromeMetricsServiceClient();
   virtual ~ChromeMetricsServiceClient();

chrome/browser/metrics/metrics_service.cc

@@ -458,10 +458,12 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
 #endif  // defined(OS_ANDROID)
 }
 
-MetricsService::MetricsService(metrics::MetricsStateManager* state_manager)
+MetricsService::MetricsService(metrics::MetricsStateManager* state_manager,
+                               metrics::MetricsServiceClient* client)
     : MetricsServiceBase(g_browser_process->local_state(),
                          kUploadLogAvoidRetransmitSize),
       state_manager_(state_manager),
+      client_(client),
       recording_active_(false),
       reporting_active_(false),
       test_mode_active_(false),
@@ -476,6 +478,7 @@ MetricsService::MetricsService(metrics::MetricsStateManager* state_manager)
       num_async_histogram_fetches_in_progress_(0) {
   DCHECK(IsSingleThreaded());
   DCHECK(state_manager_);
+  DCHECK(client_);
 
   BrowserChildProcessObserver::Add(this);
 }

chrome/browser/metrics/metrics_service.h

@@ -74,6 +74,7 @@ class MetricsPrivateGetIsCrashReportingEnabledFunction;
 }
 
 namespace metrics {
+class MetricsServiceClient;
 class MetricsStateManager;
 }
 
@@ -132,10 +133,12 @@ class MetricsService
     SHUTDOWN_COMPLETE = 700,
   };
 
-  // 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);
+  // Creates the MetricsService with the given |state_manager| and |client|.
+  // Does not take ownership of |state_manager| or |client|; instead stores a
+  // weak pointer to each. Caller should ensure that |state_manager| and
+  // |client| are valid for the lifetime of this class.
+  MetricsService(metrics::MetricsStateManager* state_manager,
+                 metrics::MetricsServiceClient* client);
   virtual ~MetricsService();
 
   // Initializes metrics recording state. Updates various bookkeeping values in
@@ -487,7 +490,11 @@ class MetricsService
 
   // Used to manage various metrics reporting state prefs, such as client id,
   // low entropy source and whether metrics reporting is enabled. Weak pointer.
-  metrics::MetricsStateManager* state_manager_;
+  metrics::MetricsStateManager* const state_manager_;
+
+  // Used to interact with the embedder. Weak pointer; must outlive |this|
+  // instance.
+  metrics::MetricsServiceClient* const client_;
 
   // Registered metrics providers.
   ScopedVector<metrics::MetricsProvider> metrics_providers_;

chrome/browser/metrics/metrics_service_unittest.cc

@@ -14,6 +14,7 @@
 #include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "components/metrics/metrics_service_observer.h"
+#include "components/metrics/test_metrics_service_client.h"
 #include "components/variations/metrics_util.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/webplugininfo.h"
@@ -31,8 +32,9 @@ using metrics::MetricsLogManager;
 
 class TestMetricsService : public MetricsService {
  public:
-  explicit TestMetricsService(metrics::MetricsStateManager* state_manager)
-      : MetricsService(state_manager) {
+  TestMetricsService(metrics::MetricsStateManager* state_manager,
+                     metrics::MetricsServiceClient* client)
+      : MetricsService(state_manager, client) {
   }
   virtual ~TestMetricsService() {}
 
@@ -177,7 +179,8 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCleanShutDown) {
   EnableMetricsReporting();
   GetLocalState()->SetBoolean(prefs::kStabilityExitedCleanly, true);
 
-  TestMetricsService service(GetMetricsStateManager());
+  metrics::TestMetricsServiceClient client;
+  TestMetricsService service(GetMetricsStateManager(), &client);
   service.InitializeMetricsRecordingState();
   // No initial stability log should be generated.
   EXPECT_FALSE(service.log_manager()->has_unsent_logs());
@@ -207,7 +210,8 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCrash) {
 
   GetLocalState()->SetBoolean(prefs::kStabilityExitedCleanly, false);
 
-  TestMetricsService service(GetMetricsStateManager());
+  metrics::TestMetricsServiceClient client;
+  TestMetricsService service(GetMetricsStateManager(), &client);
   service.InitializeMetricsRecordingState();
 
   // The initial stability log should be generated and persisted in unsent logs.
@@ -235,7 +239,8 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCrash) {
 }
 
 TEST_F(MetricsServiceTest, RegisterSyntheticTrial) {
-  MetricsService service(GetMetricsStateManager());
+  metrics::TestMetricsServiceClient client;
+  MetricsService service(GetMetricsStateManager(), &client);
 
   // Add two synthetic trials and confirm that they show up in the list.
   SyntheticTrialGroup trial1(metrics::HashName("TestTrial1"),
@@ -333,7 +338,8 @@ TEST_F(MetricsServiceTest, CrashReportingEnabled) {
 }
 
 TEST_F(MetricsServiceTest, MetricsServiceObserver) {
-  MetricsService service(GetMetricsStateManager());
+  metrics::TestMetricsServiceClient client;
+  MetricsService service(GetMetricsStateManager(), &client);
   TestMetricsServiceObserver observer1;
   TestMetricsServiceObserver observer2;
 

chrome/browser/metrics/metrics_services_manager.cc

@@ -19,8 +19,10 @@ MetricsServicesManager::~MetricsServicesManager() {
 
 MetricsService* MetricsServicesManager::GetMetricsService() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!metrics_service_)
-    metrics_service_.reset(new MetricsService(GetMetricsStateManager()));
+  if (!metrics_service_) {
+    metrics_service_.reset(
+      new MetricsService(GetMetricsStateManager(), &metrics_service_client_));
+  }
   return metrics_service_.get();
 }
 

chrome/browser/metrics/metrics_services_manager.h

@@ -8,6 +8,7 @@
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/thread_checker.h"
+#include "chrome/browser/metrics/chrome_metrics_service_client.h"
 
 class MetricsService;
 class PrefService;
@@ -54,6 +55,10 @@ class MetricsServicesManager {
   // MetricsStateManager which is passed as a parameter to service constructors.
   scoped_ptr<metrics::MetricsStateManager> metrics_state_manager_;
 
+  // Chrome embedder implementation of the MetricsServiceClient, which is passed
+  // as a parameter to the MetricsService.
+  ChromeMetricsServiceClient metrics_service_client_;
+
   // The MetricsService, used for UMA report uploads.
   scoped_ptr<MetricsService> metrics_service_;
 

chrome/chrome_tests_unit.gypi

@@ -18,10 +18,11 @@
         'common',
         '../base/base.gyp:base_prefs_test_support',
         '../base/base.gyp:test_support_base',
+        '../components/components.gyp:bookmarks_test_support',
+        '../components/components.gyp:metrics_test_support',
         '../components/components.gyp:password_manager_core_browser_test_support',
         '../components/components.gyp:signin_core_browser_test_support',
         '../components/components.gyp:sync_driver_test_support',
-        '../components/components.gyp:bookmarks_test_support',
         '../content/content.gyp:content_app_both',
         '../content/content_shell_and_tests.gyp:test_support_content',
         '../net/net.gyp:net',

components/metrics.gypi

@@ -60,6 +60,26 @@
       },
       'includes': [ '../build/protoc.gypi' ],
     },
+    {
+      # TODO(isherman): Remove all //chrome dependencies on this target, and
+      # merge the files in this target with components_unittests.
+      'target_name': 'metrics_test_support',
+      'type': 'static_library',
+      'include_dirs': [
+        '..',
+      ],
+      'dependencies': [
+        'component_metrics_proto',
+        'metrics',
+      ],
+      'export_dependent_settings': [
+        'component_metrics_proto',
+      ],
+      'sources': [
+        'metrics/test_metrics_service_client.cc',
+        'metrics/test_metrics_service_client.h',
+      ],
+    },
   ],
   'conditions': [
     ['chromeos==1', {

components/metrics/test_metrics_service_client.cc

+// 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 "components/metrics/test_metrics_service_client.h"
+
+namespace metrics {
+
+TestMetricsServiceClient::TestMetricsServiceClient() {
+}
+
+TestMetricsServiceClient::~TestMetricsServiceClient() {
+}
+
+void TestMetricsServiceClient::SetClientID(const std::string& client_id) {
+  client_id_ = client_id;
+}
+
+bool TestMetricsServiceClient::IsOffTheRecordSessionActive() {
+  return false;
+}
+
+std::string TestMetricsServiceClient::GetApplicationLocale() {
+  return "en-US";
+}
+
+bool TestMetricsServiceClient::GetBrand(std::string* brand_code) {
+  *brand_code = "BRAND_CODE";
+  return true;
+}
+
+SystemProfileProto::Channel TestMetricsServiceClient::GetChannel() {
+  return SystemProfileProto::CHANNEL_BETA;
+}
+
+std::string TestMetricsServiceClient::GetVersionString() {
+  return "5.0.322.0-64-devel";
+}
+
+}  // namespace metrics

components/metrics/test_metrics_service_client.h

+// 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 COMPONENTS_METRICS_TEST_METRICS_SERVICE_CLIENT_H_
+#define COMPONENTS_METRICS_TEST_METRICS_SERVICE_CLIENT_H_
+
+#include "components/metrics/metrics_service_client.h"
+
+namespace metrics {
+
+// A simple concrete implementation of the MetricsServiceClient interface, for
+// use in tests.
+class TestMetricsServiceClient : public MetricsServiceClient {
+ public:
+  TestMetricsServiceClient();
+  virtual ~TestMetricsServiceClient();
+
+  // MetricsServiceClient:
+  virtual void SetClientID(const std::string& client_id) OVERRIDE;
+  virtual bool IsOffTheRecordSessionActive() OVERRIDE;
+  virtual std::string GetApplicationLocale() OVERRIDE;
+  virtual bool GetBrand(std::string* brand_code) OVERRIDE;
+  virtual SystemProfileProto::Channel GetChannel() OVERRIDE;
+  virtual std::string GetVersionString() OVERRIDE;
+
+  const std::string& get_client_id() const { return client_id_; }
+
+ private:
+  std::string client_id_;
+};
+
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_TEST_METRICS_SERVICE_CLIENT_H_