(Correctly) acquire real-time UMA stats for aggregation

Properly nest XML so existing back-end code can grok the data.
Include plugin crash stats as well.

Correct bug where default time for interlog transmission is set (in case
the server does not provide a value).

bug=1564631

r=evanm,hunar
Review URL: http://codereview.chromium.org/17068

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@7599 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 46 insertions, 20 deletions

diff --git a/chrome/browser/metrics_log.cc b/chrome/browser/metrics_log.cc
index 60851bf..f366412 100644
--- a/chrome/browser/metrics_log.cc
+++ b/chrome/browser/metrics_log.cc
@@ -311,9 +311,18 @@ void MetricsLog::RecordIncrementalStabilityElements() {
   PrefService* pref = g_browser_process->local_state();
   DCHECK(pref);
 
-  OPEN_ELEMENT_FOR_SCOPE("stability");
-  WriteRequiredStabilityElements(pref);
-  WriteRealtimeStabilityElements(pref);
+  OPEN_ELEMENT_FOR_SCOPE("profile");
+  WriteCommonEventAttributes();
+
+  WriteInstallElement();  // Supply appversion.
+
+  {
+    OPEN_ELEMENT_FOR_SCOPE("stability");  // Minimal set of stability elements.
+    WriteRequiredStabilityAttributes(pref);
+    WriteRealtimeStabilityAttributes(pref);
+
+    WritePluginStabilityElements(pref);
+  }
 }
 
 void MetricsLog::WriteStabilityElement() {
@@ -327,8 +336,8 @@ void MetricsLog::WriteStabilityElement() {
   //       sent, but that's true for all the metrics.
 
   OPEN_ELEMENT_FOR_SCOPE("stability");
-  WriteRequiredStabilityElements(pref);
-  WriteRealtimeStabilityElements(pref);
+  WriteRequiredStabilityAttributes(pref);
+  WriteRealtimeStabilityAttributes(pref);
 
   // TODO(jar): The following are all optional, so we *could* optimize them for
   // values of zero (and not include them).
@@ -356,7 +365,11 @@ void MetricsLog::WriteStabilityElement() {
                  WideToUTF8(pref->GetString(prefs::kStabilityUptimeSec)));
   pref->SetString(prefs::kStabilityUptimeSec, L"0");
 
-  // Now log plugin stability info
+  WritePluginStabilityElements(pref);
+}
+
+void MetricsLog::WritePluginStabilityElements(PrefService* pref) {
+  // Now log plugin stability info.
   const ListValue* plugin_stats_list = pref->GetList(
       prefs::kStabilityPluginStats);
   if (plugin_stats_list) {
@@ -397,7 +410,7 @@ void MetricsLog::WriteStabilityElement() {
   }
 }
 
-void MetricsLog::WriteRequiredStabilityElements(PrefService* pref) {
+void MetricsLog::WriteRequiredStabilityAttributes(PrefService* pref) {
   // The server refuses data that doesn't have certain values.  crashcount and
   // launchcount are currently "required" in the "stability" group.
   WriteIntAttribute("launchcount",
@@ -408,7 +421,7 @@ void MetricsLog::WriteRequiredStabilityElements(PrefService* pref) {
   pref->SetInteger(prefs::kStabilityCrashCount, 0);
 }
 
-void MetricsLog::WriteRealtimeStabilityElements(PrefService* pref) {
+void MetricsLog::WriteRealtimeStabilityAttributes(PrefService* pref) {
   // Update the stats which are critical for real-time stability monitoring.
   // Since these are "optional," only list ones that are non-zero, as the counts
   // are aggergated (summed) server side.
@@ -452,6 +465,13 @@ void MetricsLog::WritePluginList(
   }
 }
 
+void MetricsLog::WriteInstallElement() {
+  OPEN_ELEMENT_FOR_SCOPE("install");
+  WriteAttribute("installdate", GetInstallDate());
+  WriteIntAttribute("buildid", 0);  // We're using appversion instead.
+  WriteAttribute("appversion", GetVersionString());
+}
+
 void MetricsLog::RecordEnvironment(
          const std::vector<WebPluginInfo>& plugin_list,
          const DictionaryValue* profile_metrics) {
@@ -462,12 +482,7 @@ void MetricsLog::RecordEnvironment(
   OPEN_ELEMENT_FOR_SCOPE("profile");
   WriteCommonEventAttributes();
 
-  {
-    OPEN_ELEMENT_FOR_SCOPE("install");
-    WriteAttribute("installdate", GetInstallDate());
-    WriteIntAttribute("buildid", 0);  // We're using appversion instead.
-    WriteAttribute("appversion", GetVersionString());
-  }
+  WriteInstallElement();
 
   WritePluginList(plugin_list);
 
diff --git a/chrome/browser/metrics_log.h b/chrome/browser/metrics_log.h
index 84faabe..63ac23d 100644
--- a/chrome/browser/metrics_log.h
+++ b/chrome/browser/metrics_log.h
@@ -157,17 +157,24 @@ class MetricsLog {
   // NOTE: Has the side-effect of clearing those counts.
   void WriteStabilityElement();
 
-  // Within the stability group, write required elements.
-  void WriteRequiredStabilityElements(PrefService* pref);
-  // Within the stability group, write elements that need to be updated asap
+  // Within stability group, write plugin crash stats.
+  void WritePluginStabilityElements(PrefService* pref);
+
+  // Within the stability group, write required attributes.
+  void WriteRequiredStabilityAttributes(PrefService* pref);
+
+  // Within the stability group, write attributes that need to be updated asap
   // and can't be delayed until the user decides to restart chromium.
   // Delaying these stats would bias metrics away from happy long lived
   // chromium processes (ones that don't crash, and keep on running).
-  void WriteRealtimeStabilityElements(PrefService* pref);
+  void WriteRealtimeStabilityAttributes(PrefService* pref);
 
   // Writes the list of installed plugins.
   void WritePluginList(const std::vector<WebPluginInfo>& plugin_list);
 
+  // Within the profile group, write basic install info including appversion.
+  void WriteInstallElement();
+
   // Writes all profile metrics. This invokes WriteProfileMetrics for each key
   // in all_profiles_metrics that starts with kProfilePrefix.
   void WriteAllProfilesMetrics(const DictionaryValue& all_profiles_metrics);
diff --git a/chrome/browser/metrics_service.cc b/chrome/browser/metrics_service.cc
index acc9933..7a11a3c 100644
--- a/chrome/browser/metrics_service.cc
+++ b/chrome/browser/metrics_service.cc
@@ -680,7 +680,7 @@ void MetricsService::StopRecording(MetricsLog** log) {
   }
 
   // Put incremental data (histogram deltas, and realtime stats deltas) at the
-  // end of every log transmission.
+  // end of all log transmissions (initial log handles this separately).
   // Don't bother if we're going to discard current_log_.
   if (log) {
     current_log_->RecordIncrementalStabilityElements();
@@ -1193,6 +1193,10 @@ void MetricsService::OnURLFetchComplete(const URLFetcher* source,
       local_state->ScheduleSavePersistentPrefs(
           g_browser_process->file_thread());
 
+    // Provide a default (free of exponetial backoff, other varances) in case
+    // the server does not specify a value.
+    interlog_duration_ = TimeDelta::FromSeconds(kMinSecondsPerLog);
+
     GetSettingsFromResponseData(data);
     // Override server specified interlog delay if there are unsent logs to
     // transmit.
diff --git a/chrome/browser/metrics_service.h b/chrome/browser/metrics_service.h
index 137b534..a7072c5 100644
--- a/chrome/browser/metrics_service.h
+++ b/chrome/browser/metrics_service.h
@@ -163,7 +163,7 @@ class MetricsService : public NotificationObserver,
   // Called to start recording user experience metrics.
   // Constructs a new, empty current_log_.
   void StartRecording();
-  
+
   // Called to stop recording user experience metrics.  The caller takes
   // ownership of the resulting MetricsLog object via the log parameter,
   // or passes in NULL to indicate that the log should simply be deleted.