Quantify initial stability report edge cases.

components/metrics/metrics_log.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 "components/metrics/metrics_log.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 99 matching lines @@
 #if defined(SYZYASAN)
   system_profile->set_is_asan_build(true);
 #endif
 }
 
 MetricsLog::~MetricsLog() {
 }
 
 // static
 void MetricsLog::RegisterPrefs(PrefRegistrySimple* registry) {
-  registry->RegisterIntegerPref(prefs::kStabilityLaunchCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityCrashCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityIncompleteSessionEndCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityLaunchCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityBreakpadRegistrationFail, 0);
   registry->RegisterIntegerPref(
       prefs::kStabilityBreakpadRegistrationSuccess, 0);
   registry->RegisterIntegerPref(prefs::kStabilityDebuggerPresent, 0);
   registry->RegisterIntegerPref(prefs::kStabilityDebuggerNotPresent, 0);
   registry->RegisterStringPref(prefs::kStabilitySavedSystemProfile,
                                std::string());
   registry->RegisterStringPref(prefs::kStabilitySavedSystemProfileHash,
                                std::string());
+  registry->RegisterIntegerPref(prefs::kStabilityDeferredCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityDiscardCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityVersionMismatchCount, 0);
 }
 
 // static
 uint64_t MetricsLog::Hash(const std::string& value) {
   uint64_t hash = base::HashMetricName(value);
 
   // The following log is VERY helpful when folks add some named histogram into
   // the code, but forgot to update the descriptive list of histograms.  When
   // that happens, all we get to see (server side) is a hash of the histogram
   // name.  We can then use this logging to find out what histogram name was
@@ skipping 58 matching lines @@
   for (size_t i = 0; i < metrics_providers.size(); ++i) {
     if (log_type() == INITIAL_STABILITY_LOG)
       metrics_providers[i]->ProvideInitialStabilityMetrics(system_profile);
     metrics_providers[i]->ProvideStabilityMetrics(system_profile);
   }
 
   // Omit some stats unless this is the initial stability log.
   if (log_type() != INITIAL_STABILITY_LOG)
     return;
 
+  SystemProfileProto::Stability* stability =
+      system_profile->mutable_stability();
+
+  // TODO(jar): The following are all optional in SystemProfileProto::Stability,
+  // so we *could* optimize them for values of zero (and not include them).
+
   int incomplete_shutdown_count =
       pref->GetInteger(prefs::kStabilityIncompleteSessionEndCount);
-  pref->SetInteger(prefs::kStabilityIncompleteSessionEndCount, 0);
+  if (incomplete_shutdown_count)

[Alexei Svitkine (slow), 2016/08/30 21:25:59]

Maybe break this out in a separate change? Seems useful on its own while we
continue discussing the other parts. (This is addressing the TODO(jar) right?
Remove the TODO then?)

[manzagop (departed), 2016/09/01 21:26:55]

Done. http://crrev.com/2294323002

+    pref->SetInteger(prefs::kStabilityIncompleteSessionEndCount, 0);
+  stability->set_incomplete_shutdown_count(incomplete_shutdown_count);
+
   int breakpad_registration_success_count =
       pref->GetInteger(prefs::kStabilityBreakpadRegistrationSuccess);
-  pref->SetInteger(prefs::kStabilityBreakpadRegistrationSuccess, 0);
+  if (breakpad_registration_success_count)
+    pref->SetInteger(prefs::kStabilityBreakpadRegistrationSuccess, 0);
+  stability->set_breakpad_registration_success_count(
+      breakpad_registration_success_count);
+
   int breakpad_registration_failure_count =
       pref->GetInteger(prefs::kStabilityBreakpadRegistrationFail);
-  pref->SetInteger(prefs::kStabilityBreakpadRegistrationFail, 0);
+  if (breakpad_registration_failure_count)
+    pref->SetInteger(prefs::kStabilityBreakpadRegistrationFail, 0);
+  stability->set_breakpad_registration_failure_count(
+      breakpad_registration_failure_count);
+
   int debugger_present_count =
       pref->GetInteger(prefs::kStabilityDebuggerPresent);
-  pref->SetInteger(prefs::kStabilityDebuggerPresent, 0);
+  if (debugger_present_count)
+    pref->SetInteger(prefs::kStabilityDebuggerPresent, 0);
+  stability->set_debugger_present_count(debugger_present_count);
+
   int debugger_not_present_count =
       pref->GetInteger(prefs::kStabilityDebuggerNotPresent);
-  pref->SetInteger(prefs::kStabilityDebuggerNotPresent, 0);
+  if (debugger_not_present_count)
+    pref->SetInteger(prefs::kStabilityDebuggerNotPresent, 0);
+  stability->set_debugger_not_present_count(debugger_not_present_count);
 
-  // TODO(jar): The following are all optional, so we *could* optimize them for
-  // values of zero (and not include them).
-  SystemProfileProto::Stability* stability =
-      system_profile->mutable_stability();
-  stability->set_incomplete_shutdown_count(incomplete_shutdown_count);
-  stability->set_breakpad_registration_success_count(
-      breakpad_registration_success_count);
-  stability->set_breakpad_registration_failure_count(
-      breakpad_registration_failure_count);
-  stability->set_debugger_present_count(debugger_present_count);
-  stability->set_debugger_not_present_count(debugger_not_present_count);
+  int deferred_count = pref->GetInteger(prefs::kStabilityDeferredCount);
+  if (deferred_count)
+    local_state_->SetInteger(prefs::kStabilityDeferredCount, 0);
+  stability->set_deferred_count(deferred_count);
+
+  int discard_count = local_state_->GetInteger(prefs::kStabilityDiscardCount);
+  if (discard_count)
+    local_state_->SetInteger(prefs::kStabilityDiscardCount, 0);
+  stability->set_discard_count(discard_count);
+
+  int version_mismatch_count =
+      local_state_->GetInteger(prefs::kStabilityVersionMismatchCount);
+  if (version_mismatch_count)
+    local_state_->SetInteger(prefs::kStabilityVersionMismatchCount, 0);
+  stability->set_version_mismatch_count(version_mismatch_count);
 }
 
 void MetricsLog::RecordGeneralMetrics(
     const std::vector<MetricsProvider*>& metrics_providers) {
   for (size_t i = 0; i < metrics_providers.size(); ++i)
     metrics_providers[i]->ProvideGeneralMetrics(uma_proto());
 }
 
 void MetricsLog::GetFieldTrialIds(
     std::vector<ActiveGroupId>* field_trial_ids) const {
@@ skipping 33 matching lines @@
 }
 
 // The server refuses data that doesn't have certain values.  crashcount and
 // launchcount are currently "required" in the "stability" group.
 // TODO(isherman): Stop writing these attributes specially once the migration to
 // protobufs is complete.
 void MetricsLog::WriteRequiredStabilityAttributes(PrefService* pref) {
   int launch_count = pref->GetInteger(prefs::kStabilityLaunchCount);
   pref->SetInteger(prefs::kStabilityLaunchCount, 0);
   int crash_count = pref->GetInteger(prefs::kStabilityCrashCount);
-  pref->SetInteger(prefs::kStabilityCrashCount, 0);
+  if (crash_count)
+    pref->SetInteger(prefs::kStabilityCrashCount, 0);
 
   SystemProfileProto::Stability* stability =
       uma_proto()->mutable_system_profile()->mutable_stability();
   stability->set_launch_count(launch_count);
   stability->set_crash_count(crash_count);
 }
 
 void MetricsLog::WriteRealtimeStabilityAttributes(
     PrefService* pref,
     base::TimeDelta incremental_uptime,
@@ skipping 71 matching lines @@
   cpu->set_num_cores(base::SysInfo::NumberOfProcessors());
 
   std::vector<ActiveGroupId> field_trial_ids;
   GetFieldTrialIds(&field_trial_ids);
   WriteFieldTrials(field_trial_ids, system_profile);
   WriteFieldTrials(synthetic_trials, system_profile);
 
   for (size_t i = 0; i < metrics_providers.size(); ++i)
     metrics_providers[i]->ProvideSystemProfileMetrics(system_profile);
 
-  std::string serialied_system_profile;
+  std::string serialized_system_profile;
   std::string base64_system_profile;
-  if (system_profile->SerializeToString(&serialied_system_profile)) {
-    base::Base64Encode(serialied_system_profile, &base64_system_profile);
+  if (system_profile->SerializeToString(&serialized_system_profile)) {
+    base::Base64Encode(serialized_system_profile, &base64_system_profile);
     PrefService* local_state = local_state_;
     local_state->SetString(prefs::kStabilitySavedSystemProfile,
                            base64_system_profile);
     local_state->SetString(prefs::kStabilitySavedSystemProfileHash,
-                           ComputeSHA1(serialied_system_profile));
+                           ComputeSHA1(serialized_system_profile));
   }
 }
 
-bool MetricsLog::LoadSavedEnvironmentFromPrefs() {
+bool MetricsLog::LoadSavedEnvironmentFromPrefs(std::string* app_version) {
+  DCHECK(app_version);
+
   PrefService* local_state = local_state_;
   const std::string base64_system_profile =
       local_state->GetString(prefs::kStabilitySavedSystemProfile);
   if (base64_system_profile.empty())
     return false;
 
   const std::string system_profile_hash =
       local_state->GetString(prefs::kStabilitySavedSystemProfileHash);
   local_state->ClearPref(prefs::kStabilitySavedSystemProfile);
   local_state->ClearPref(prefs::kStabilitySavedSystemProfileHash);
 
   SystemProfileProto* system_profile = uma_proto()->mutable_system_profile();
-  std::string serialied_system_profile;
-  return base::Base64Decode(base64_system_profile, &serialied_system_profile) &&
-         ComputeSHA1(serialied_system_profile) == system_profile_hash &&
-         system_profile->ParseFromString(serialied_system_profile);
+  std::string serialized_system_profile;
+  bool success =
+      base::Base64Decode(base64_system_profile, &serialized_system_profile) &&
+      ComputeSHA1(serialized_system_profile) == system_profile_hash &&
+      system_profile->ParseFromString(serialized_system_profile);
+  if (success)
+    *app_version = system_profile->app_version();
+  return success;
 }
 
 void MetricsLog::CloseLog() {
   DCHECK(!closed_);
   closed_ = true;
 }
 
 void MetricsLog::GetEncodedLog(std::string* encoded_log) {
   DCHECK(closed_);
   uma_proto_.SerializeToString(encoded_log);
 }
 
 }  // namespace metrics