Don't commit prefs::kStabilitySessionEndCompleted change immediately on chromeos

chrome/browser/metrics/metrics_service.cc

 // Copyright (c) 2012 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.
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 719 matching lines @@
 void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) {
   // If there wasn't a lot of action, maybe the computer was asleep, in which
   // case, the log transmissions should have stopped.  Here we start them up
   // again.
   if (!in_idle && idle_since_last_transmission_)
     StartSchedulerIfNecessary();
   idle_since_last_transmission_ = in_idle;
 }
 
 void MetricsService::RecordStartOfSessionEnd() {
-  LogCleanShutdown();
-  RecordBooleanPrefValue(prefs::kStabilitySessionEndCompleted, false);
+  RecordStabilitySessionEndCompleted(false);
 }
 
 void MetricsService::RecordCompletedSessionEnd() {
-  LogCleanShutdown();
-  RecordBooleanPrefValue(prefs::kStabilitySessionEndCompleted, true);
+  RecordStabilitySessionEndCompleted(true);
 }
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
 void MetricsService::OnAppEnterBackground() {
   scheduler_->Stop();
 
   MarkAppCleanShutdownAndCommit();
 
   // At this point, there's no way of knowing when the process will be
   // killed, so this has to be treated similar to a shutdown, closing and
@@ skipping 834 matching lines @@
   } else if (status == base::TERMINATION_STATUS_STILL_RUNNING) {
     UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.DisconnectedAlive",
                                was_extension_process ? 2 : 1);
     }
   }
 
 void MetricsService::LogRendererHang() {
   IncrementPrefValue(prefs::kStabilityRendererHangCount);
 }
 
+void MetricsService::RecordStabilitySessionEndCompleted(bool value) {
+  PrefService* pref = g_browser_process->local_state();
+  DCHECK(pref);
+  clean_shutdown_status_ = CLEANLY_SHUTDOWN;
+  pref->SetBoolean(prefs::kStabilitySessionEndCompleted, value);
+  RecordCurrentState(pref);

[Ilya Sherman, 2013/10/04 03:26:59]

Btw, why did you inline the call to RecordBooleanPrefVaue()?

[oshima, 2013/10/04 06:03:14]

Because it's calling RecordCurrentState(pref) inside, which results
in redundant call to RecordCurrentState(pref). (see
my comment in #5). 

Actually, it is no longer used after this change, so I removed it.

+  MarkAppCleanShutdownAndCommit();
+}
+
 bool MetricsService::UmaMetricsProperlyShutdown() {
   CHECK(clean_shutdown_status_ == CLEANLY_SHUTDOWN ||
         clean_shutdown_status_ == NEED_TO_SHUTDOWN);
   return clean_shutdown_status_ == CLEANLY_SHUTDOWN;
 }
 
-void MetricsService::LogCleanShutdown() {
-  // Redundant hack to write pref ASAP.
-  MarkAppCleanShutdownAndCommit();
-
-  // Redundant setting to assure that we always reset this value at shutdown
-  // (and that we don't use some alternate path, and not call LogCleanShutdown).

[Ilya Sherman, 2013/10/04 03:26:59]

What happened to this comment?  Is it no longer relevant?

[oshima, 2013/10/04 06:03:14]

Added back. Thank you for pointing out.

-  clean_shutdown_status_ = CLEANLY_SHUTDOWN;
-
-  RecordBooleanPrefValue(prefs::kStabilityExitedCleanly, true);
-}
-
 #if defined(OS_CHROMEOS)
 void MetricsService::LogChromeOSCrash(const std::string &crash_type) {
   if (crash_type == "user")
     IncrementPrefValue(prefs::kStabilityOtherUserCrashCount);
   else if (crash_type == "kernel")
     IncrementPrefValue(prefs::kStabilityKernelCrashCount);
   else if (crash_type == "uncleanshutdown")
     IncrementPrefValue(prefs::kStabilitySystemUncleanShutdownCount);
   else
     NOTREACHED() << "Unexpected Chrome OS crash type " << crash_type;
@@ skipping 163 matching lines @@
   if (local_state) {
     const PrefService::Preference* uma_pref =
         local_state->FindPreference(prefs::kMetricsReportingEnabled);
     if (uma_pref) {
       bool success = uma_pref->GetValue()->GetAsBoolean(&result);
       DCHECK(success);
     }
   }
   return result;
 }