Add UpdateCurrentAppInfo() interface to record info about current app, including

chromecast/base/metrics/cast_metrics_helper.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 "chromecast/base/metrics/cast_metrics_helper.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop_proxy.h"
@@ skipping 50 matching lines @@
 CastMetricsHelper::~CastMetricsHelper() {
   DCHECK_EQ(g_instance, this);
   g_instance = NULL;
 }
 
 void CastMetricsHelper::TagAppStart(const std::string& arg_app_name) {
   MAKE_SURE_THREAD(TagAppStart, arg_app_name);
   app_name_ = arg_app_name;
   app_start_time_ = base::TimeTicks::Now();
   new_startup_time_ = true;
+  // Clear app info
+  app_id_ = "";

[byungchul, 2014/12/10 05:39:41]

app_id_.clear();

[gunsch, 2014/12/10 16:54:18]

Alternately, you could use UpdateCurrentAppInfo("", "", "") --- this would
prevent the two from accidentally getting out of sync in the future, since any
API change would be forced to be updated

[gfhuang, 2014/12/10 22:02:41]

Done.

+  session_id_ = "";

[byungchul, 2014/12/10 05:39:41]

ditto

+  sdk_version_ = "";

[byungchul, 2014/12/10 05:39:40]

ditto

+}
+
+void CastMetricsHelper::UpdateCurrentAppInfo(const std::string& app_id,
+                                             const std::string& session_id,
+                                             const std::string& sdk_version) {
+  MAKE_SURE_THREAD(UpdateCurrentAppInfo, app_id, session_id, sdk_version);
+  app_id_ = app_id;
+  session_id_ = session_id;
+  sdk_version_ = sdk_version;
 }
 
 void CastMetricsHelper::LogMediaPlay() {
-  RecordSimpleAction(GetMetricsNameWithAppName("MediaPlay", ""));
+  RecordSimpleAction(GetMetricsNameWithAppInfo("MediaPlay"));
 }
 
 void CastMetricsHelper::LogMediaPause() {
-  RecordSimpleAction(GetMetricsNameWithAppName("MediaPause", ""));
+  RecordSimpleAction(GetMetricsNameWithAppInfo("MediaPause"));
 }
 
 void CastMetricsHelper::LogTimeToDisplayVideo() {
   if (!new_startup_time_) {  // For faster check.
     return;
   }
   MAKE_SURE_THREAD(LogTimeToDisplayVideo);
   new_startup_time_ = false;
   base::TimeDelta launch_time = base::TimeTicks::Now() - app_start_time_;
   const std::string uma_name(GetMetricsNameWithAppName("Startup",
@@ skipping 90 matching lines @@
     metrics_name.append(app_name_);
   }
   if (!suffix.empty()) {
     if (!metrics_name.empty())
       metrics_name.push_back('.');
     metrics_name.append(suffix);
   }
   return metrics_name;
 }
 
+std::string CastMetricsHelper::GetMetricsNameWithAppInfo(
+    const std::string& action_name) const {
+  DCHECK(message_loop_proxy_->BelongsToCurrentThread());
+  return action_name + "#" + app_id_ + "#" + session_id_ + "#" + sdk_version_;

[byungchul, 2014/12/10 05:39:40]

Please define a constant for delimiter, '#' for somewhere to parse it.

[gunsch, 2014/12/10 16:54:18]

Aside: I saw that you're parsing this in a different location in an internal CL.
Why not have the logic for both ways (serializing and deserializing) in this
class, and pass in/return a struct with all three args? Then this awkward
behavior is at least self-contained.

[gfhuang, 2014/12/10 22:02:41]

Done.

+}
+
 void CastMetricsHelper::SetMetricsSink(MetricsSink* delegate) {
   MAKE_SURE_THREAD(SetMetricsSink, delegate);
   metrics_sink_ = delegate;
 }
 
 void CastMetricsHelper::SetRecordActionCallback(
       const RecordActionCallback& callback) {
   DCHECK(message_loop_proxy_->BelongsToCurrentThread());
   default_record_action_callback_ = callback;
 }
@@ skipping 38 matching lines @@
     const base::TimeDelta& value) {
   // Follow UMA_HISTOGRAM_MEDIUM_TIMES definition.
   LogTimeHistogramEvent(name, value,
                         base::TimeDelta::FromMilliseconds(10),
                         base::TimeDelta::FromMinutes(3),
                         50);
 }
 
 }  // namespace metrics
 }  // namespace chromecast