[chromecast] ExternalMetrics has to destroy itself in file thread.

chromecast/browser/metrics/cast_metrics_service_client.cc

Index: chromecast/browser/metrics/cast_metrics_service_client.cc
diff --git a/chromecast/browser/metrics/cast_metrics_service_client.cc b/chromecast/browser/metrics/cast_metrics_service_client.cc
index 32b4c15130e4bdfcc73da854ac4bc92783b34123..9357a2b843c385ba6dd8d477badf024ea623e6e3 100644
--- a/chromecast/browser/metrics/cast_metrics_service_client.cc
+++ b/chromecast/browser/metrics/cast_metrics_service_client.cc
@@ -250,6 +250,12 @@ 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)

[lcwu1, 2015/01/10 00:36:39]

In our case, !OS_ANDROID is equal to OS_LINUX, so you should be able to move the
following code to the block above (i.e. below line 251).

[gunsch, 2015/01/10 00:42:23]

I'd prefer not---RecordCompletedSessionEnd is actually declared with
!OS_ANDROID, and ExternalMetrics is guarded with OS_LINUX due to Linux-specific
dependencies.

I agree that for us those two are equivalent, but it seems like an unnecessary
assumption to make.

[gfhuang, 2015/01/10 00:58:30]

Acknowledged.

+  external_metrics_.release()->StopAndSelfDestroy();

[gunsch, 2015/01/10 00:31:46]

Thought: this mechanic is sort of strange. I think it would make more sense to
do the following:

1) Make ~ExternalMetrics private (since no one should be calling it)
2) Make CastMetricsServiceClient have a ExternalMetrics* instead of a
scoped_ptr<ExternalMetrics>

what do you think?

[gfhuang, 2015/01/10 00:58:30]

Done.

[gfhuang, 2015/01/10 01:17:30]

Note that I have to revert back to use PostTask instead of DeleteSoon though,
since dtor is private now.

+#endif  // defined(OS_LINUX)
+  metrics_service_->Stop();
 }
 
 bool CastMetricsServiceClient::IsReportingEnabled() {