[Chromecast] Moves persistent UMA client ID handling to common code.

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 d3f2f595ca8374ae490e09eec28bd821f7e5fed9..443775b450a9906e7bd8c7e3f79b6679317a4890 100644
--- a/chromecast/browser/metrics/cast_metrics_service_client.cc
+++ b/chromecast/browser/metrics/cast_metrics_service_client.cc
@@ -22,6 +22,7 @@
 #include "chromecast/public/cast_sys_info.h"
 #include "components/metrics/client_info.h"
 #include "components/metrics/gpu/gpu_metrics_provider.h"
+#include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_provider.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics/metrics_state_manager.h"
@@ -94,7 +95,8 @@ void CastMetricsServiceClient::SetMetricsClientId(
     const std::string& client_id) {
   client_id_ = client_id;
   LOG(INFO) << "Metrics client ID set: " << client_id;
-  PlatformSetClientID(cast_service_, client_id);
+  // Ensure |pref_service_| always gets new |client_id| value.
+  pref_service_->SetString(::metrics::prefs::kMetricsClientID, client_id);

[gfhuang, 2015/08/19 15:48:27]

why this is needed?

we don't want to write to ::metrics::prefs::kMetricsClientID directory.
that's the whole idea of Load/SaveClientInfo

[gunsch, 2015/08/19 15:52:50]

Sort of a safety mechanism to replace PlatformSetClientID, but I think it
_should_ already be safely set by metrics code. I can remove it.

 }
 
 void CastMetricsServiceClient::OnRecordingDisabled() {
@@ -110,6 +112,7 @@ void CastMetricsServiceClient::StoreClientInfo(
 
 scoped_ptr< ::metrics::ClientInfo> CastMetricsServiceClient::LoadClientInfo() {
   scoped_ptr< ::metrics::ClientInfo> client_info(new ::metrics::ClientInfo);
+  client_info_loaded_ = true;
 
   // kMetricsIsNewClientID would be missing if either the device was just
   // FDR'ed, or it is on pre-v1.2 build.
@@ -125,16 +128,16 @@ scoped_ptr< ::metrics::ClientInfo> CastMetricsServiceClient::LoadClientInfo() {
     // else the device was just FDR'ed, pass through.
   }
 
-  const std::string client_id(GetPlatformClientID(cast_service_));
-  if (!client_id.empty() && base::IsValidGUID(client_id)) {
-    client_info->client_id = client_id;
+  if (!platform_client_id_.empty() && base::IsValidGUID(platform_client_id_)) {
+    client_info->client_id = platform_client_id_;
     return client_info.Pass();
   } else {
-    if (client_id.empty()) {
+    if (platform_client_id_.empty()) {
       LOG(WARNING) << "Empty client id from platform,"
                    << " assuming this is the first boot up of a new device.";
     } else {
-      LOG(ERROR) << "Invalid client id " << client_id << " from platform.";
+      LOG(ERROR) << "Invalid client id from platform: " << platform_client_id_
+             << " from platform.";
     }
     return scoped_ptr< ::metrics::ClientInfo>();
   }
@@ -258,6 +261,7 @@ CastMetricsServiceClient::CastMetricsServiceClient(
     : io_task_runner_(io_task_runner),
       pref_service_(pref_service),
       cast_service_(nullptr),
+      client_info_loaded_(false),
 #if defined(OS_LINUX)
       external_metrics_(nullptr),
       platform_metrics_(nullptr),
@@ -273,6 +277,13 @@ CastMetricsServiceClient::~CastMetricsServiceClient() {
 #endif  // defined(OS_LINUX)
 }
 
+void CastMetricsServiceClient::SetPersistedClientId(
+    const std::string& client_id) {
+  DCHECK(!client_info_loaded_)
+      << "Persisted client ID must be set before client info is loaded.";
+  platform_client_id_ = client_id;
+}
+
 void CastMetricsServiceClient::Initialize(CastService* cast_service) {
   DCHECK(cast_service);
   DCHECK(!cast_service_);