summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorisherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-29 20:40:50 +0000
committerisherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-29 20:40:50 +0000
commit037642f6e9dd120f52f832b8a9a7aa3fcb8c4800 (patch)
treeb5af5a6c1446a64df104ea641474b4d2ed8d0581
parent5ed6888645a91d566c82f72b2fc418a2e127ff01 (diff)
downloadchromium_src-037642f6e9dd120f52f832b8a9a7aa3fcb8c4800.zip
chromium_src-037642f6e9dd120f52f832b8a9a7aa3fcb8c4800.tar.gz
chromium_src-037642f6e9dd120f52f832b8a9a7aa3fcb8c4800.tar.bz2
[Metrics] Fix memory ownership model: Client now owns service, rather than being a peer.
g_browser_process owns MetricsServicesManager, which owns the MetricsServiceClient, which owns the MetricsService. BUG=375248 TEST=none R=asvitkine@chromium.org, blundell@chromium.org Review URL: https://codereview.chromium.org/299913002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273579 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/browser/metrics/chrome_metrics_service_client.cc23
-rw-r--r--chrome/browser/metrics/chrome_metrics_service_client.h23
-rw-r--r--chrome/browser/metrics/metrics_services_manager.cc13
-rw-r--r--chrome/browser/metrics/metrics_services_manager.h15
4 files changed, 46 insertions, 28 deletions
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index ecb7dc7..832f7a6 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -84,9 +84,9 @@ class MetricsMemoryDetails : public MemoryDetails {
} // namespace
-ChromeMetricsServiceClient::ChromeMetricsServiceClient()
- : service_(NULL),
- waiting_for_collect_final_metrics_step_(false),
+ChromeMetricsServiceClient::ChromeMetricsServiceClient(
+ metrics::MetricsStateManager* state_manager)
+ : waiting_for_collect_final_metrics_step_(false),
num_async_histogram_fetches_in_progress_(0),
weak_ptr_factory_(this) {
DCHECK(thread_checker_.CalledOnValidThread());
@@ -102,6 +102,19 @@ ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
DCHECK(thread_checker_.CalledOnValidThread());
}
+// static
+scoped_ptr<ChromeMetricsServiceClient> ChromeMetricsServiceClient::Create(
+ metrics::MetricsStateManager* state_manager,
+ PrefService* local_state) {
+ // Perform two-phase initialization so that |client->metrics_service_| only
+ // receives pointers to fully constructed objects.
+ scoped_ptr<ChromeMetricsServiceClient> client(
+ new ChromeMetricsServiceClient(state_manager));
+ client->metrics_service_.reset(
+ new MetricsService(state_manager, client.get(), local_state));
+ return client.Pass();
+}
+
void ChromeMetricsServiceClient::SetClientID(const std::string& client_id) {
crash_keys::SetClientID(client_id);
}
@@ -288,9 +301,7 @@ void ChromeMetricsServiceClient::Observe(
case content::NOTIFICATION_LOAD_START:
case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
case content::NOTIFICATION_RENDER_WIDGET_HOST_HANG:
- // TODO(isherman): Remove this NULL check: http://crbug.com/375248
- if (service_)
- service_->OnApplicationNotIdle();
+ metrics_service_->OnApplicationNotIdle();
break;
default:
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h
index 3d1c26b..a7dcace 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.h
+++ b/chrome/browser/metrics/chrome_metrics_service_client.h
@@ -9,6 +9,7 @@
#include "base/basictypes.h"
#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "chrome/browser/metrics/network_stats_uploader.h"
@@ -18,14 +19,22 @@
class MetricsService;
+namespace metrics {
+class MetricsStateManager;
+}
+
// ChromeMetricsServiceClient provides an implementation of MetricsServiceClient
// that depends on chrome/.
class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
public content::NotificationObserver {
public:
- ChromeMetricsServiceClient();
virtual ~ChromeMetricsServiceClient();
+ // Factory function.
+ static scoped_ptr<ChromeMetricsServiceClient> Create(
+ metrics::MetricsStateManager* state_manager,
+ PrefService* local_state);
+
// metrics::MetricsServiceClient:
virtual void SetClientID(const std::string& client_id) OVERRIDE;
virtual bool IsOffTheRecordSessionActive() OVERRIDE;
@@ -39,12 +48,12 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
virtual void CollectFinalMetrics(const base::Closure& done_callback)
OVERRIDE;
- // Stores a weak pointer to the given |service|.
- // TODO(isherman): Fix the memory ownership model so that this method is not
- // needed: http://crbug.com/375248
- void set_service(MetricsService* service) { service_ = service; }
+ MetricsService* metrics_service() { return metrics_service_.get(); }
private:
+ explicit ChromeMetricsServiceClient(
+ metrics::MetricsStateManager* state_manager);
+
// Callbacks for various stages of final log info collection. Do not call
// these directly.
void OnMemoryDetailCollectionDone();
@@ -72,8 +81,8 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
base::ThreadChecker thread_checker_;
- // The MetricsService that |this| is a client of. Weak pointer.
- MetricsService* service_;
+ // The MetricsService that |this| is a client of.
+ scoped_ptr<MetricsService> metrics_service_;
content::NotificationRegistrar registrar_;
diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc
index 9820ffb..c560dcd 100644
--- a/chrome/browser/metrics/metrics_services_manager.cc
+++ b/chrome/browser/metrics/metrics_services_manager.cc
@@ -6,6 +6,7 @@
#include "base/command_line.h"
#include "base/prefs/pref_service.h"
+#include "chrome/browser/metrics/chrome_metrics_service_client.h"
#include "chrome/browser/metrics/extensions_metrics_provider.h"
#include "chrome/browser/metrics/metrics_service.h"
#include "chrome/browser/metrics/metrics_state_manager.h"
@@ -28,15 +29,15 @@ MetricsServicesManager::~MetricsServicesManager() {
MetricsService* MetricsServicesManager::GetMetricsService() {
DCHECK(thread_checker_.CalledOnValidThread());
- if (!metrics_service_) {
- metrics_service_.reset(new MetricsService(
- GetMetricsStateManager(), &metrics_service_client_, local_state_));
- metrics_service_client_.set_service(metrics_service_.get());
- metrics_service_->RegisterMetricsProvider(
+ if (!metrics_service_client_) {
+ metrics_service_client_ =
+ ChromeMetricsServiceClient::Create(GetMetricsStateManager(),
+ local_state_);
+ metrics_service_client_->metrics_service()->RegisterMetricsProvider(
scoped_ptr<metrics::MetricsProvider>(
new ExtensionsMetricsProvider(GetMetricsStateManager())));
}
- return metrics_service_.get();
+ return metrics_service_client_->metrics_service();
}
rappor::RapporService* MetricsServicesManager::GetRapporService() {
diff --git a/chrome/browser/metrics/metrics_services_manager.h b/chrome/browser/metrics/metrics_services_manager.h
index 538ae6b..6e16b16 100644
--- a/chrome/browser/metrics/metrics_services_manager.h
+++ b/chrome/browser/metrics/metrics_services_manager.h
@@ -8,8 +8,8 @@
#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 ChromeMetricsServiceClient;
class MetricsService;
class PrefService;
@@ -26,8 +26,8 @@ class VariationsService;
}
// MetricsServicesManager is a helper class that has ownership over the various
-// metrics-related services in Chrome: MetricsService, RapporService and
-// VariationsService.
+// metrics-related services in Chrome: MetricsService (via its client),
+// RapporService and VariationsService.
class MetricsServicesManager {
public:
// Creates the MetricsServicesManager with the |local_state| prefs service.
@@ -58,12 +58,9 @@ 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 embedder implementation of the MetricsServiceClient. Owns the
+ // MetricsService.
+ scoped_ptr<ChromeMetricsServiceClient> metrics_service_client_;
// The RapporService, for RAPPOR metric uploads.
scoped_ptr<rappor::RapporService> rappor_service_;
generated by cgit v1.1 at 2025-03-05 06:13:55 +0000