(1) ExternalMetrics has to destroy itself in file thread due to weakptr.

(2) MetricsService has to stop before destruction of CastBrowserProcess, which is referenced inside MetricsService's stopping code.

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

Cr-Commit-Position: refs/heads/master@{#310958}

Author: gfhuang

chromecast/browser/metrics/cast_metrics_service_client.cc

@@ -175,11 +175,13 @@ CastMetricsServiceClient::CastMetricsServiceClient(
     : io_task_runner_(io_task_runner),
       pref_service_(pref_service),
       cast_service_(NULL),
+      external_metrics_(NULL),
       metrics_service_loop_(base::MessageLoopProxy::current()),
       request_context_(request_context) {
 }
 
 CastMetricsServiceClient::~CastMetricsServiceClient() {
+  DCHECK(!external_metrics_);
 }
 
 void CastMetricsServiceClient::Initialize(CastService* cast_service) {
@@ -240,7 +242,7 @@ void CastMetricsServiceClient::Initialize(CastService* cast_service) {
   // Start external metrics collection, which feeds data from external
   // processes into the main external metrics.
 #if defined(OS_LINUX)
-  external_metrics_.reset(new ExternalMetrics(stability_provider));
+  external_metrics_ = new ExternalMetrics(stability_provider);
   external_metrics_->Start();
 #endif  // defined(OS_LINUX)
 }
@@ -250,6 +252,13 @@ void CastMetricsServiceClient::Finalize() {
   // Set clean_shutdown bit.
   metrics_service_->RecordCompletedSessionEnd();
 #endif  // !defined(OS_ANDROID)
+
+  // Stop metrics service cleanly before destructing CastMetricsServiceClient.
+#if defined(OS_LINUX)
+  external_metrics_->StopAndDestroy();
+  external_metrics_ = NULL;
+#endif  // defined(OS_LINUX)
+  metrics_service_->Stop();
 }
 
 bool CastMetricsServiceClient::IsReportingEnabled() {

chromecast/browser/metrics/cast_metrics_service_client.h

@@ -90,7 +90,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient {
   std::string client_id_;
 
 #if defined(OS_LINUX)
-  scoped_ptr<ExternalMetrics> external_metrics_;
+  ExternalMetrics* external_metrics_;
 #endif  // defined(OS_LINUX)
   const scoped_refptr<base::MessageLoopProxy> metrics_service_loop_;
   scoped_ptr< ::metrics::MetricsStateManager> metrics_state_manager_;

chromecast/browser/metrics/external_metrics.cc

@@ -58,7 +58,13 @@ ExternalMetrics::ExternalMetrics(
   DCHECK(stability_provider);
 }
 
-ExternalMetrics::~ExternalMetrics() {}
+ExternalMetrics::~ExternalMetrics() {
+}
+
+void ExternalMetrics::StopAndDestroy() {
+  content::BrowserThread::DeleteSoon(
+      content::BrowserThread::FILE, FROM_HERE, this);
+}
 
 void ExternalMetrics::Start() {
   ScheduleCollector();

chromecast/browser/metrics/external_metrics.h

@@ -24,13 +24,19 @@ class CastStabilityMetricsProvider;
 class ExternalMetrics {
  public:
   explicit ExternalMetrics(CastStabilityMetricsProvider* stability_provider);
-  ~ExternalMetrics();
 
   // Begins external data collection. Calls to RecordAction originate in the
   // File thread but are executed in the UI thread.
   void Start();
 
+  // Destroys itself in appropriate thread.
+  void StopAndDestroy();
+
  private:
+  friend class base::DeleteHelper<ExternalMetrics>;
+
+  ~ExternalMetrics();
+
   // The max length of a message (name-value pair, plus header)
   static const int kMetricsMessageMaxLength = 1024;  // be generous