Reset metrics if cellular uploads logic should be applied.

chrome/browser/metrics/chrome_metrics_service_client.cc

Index: chrome/browser/metrics/chrome_metrics_service_client.cc
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index 2b6e1e1885a1aca2e24a7eb3cb54a3cad530145d..fe8d51653ae706d537088803a46aa6ad6e9867ef 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -101,6 +101,18 @@ bool IsCellularLogicEnabled() {
       net::NetworkChangeNotifier::GetConnectionType());
 }
 
+#if defined(OS_ANDROID)
+// Checks whether it is the first time that cellular uploads logic should be
+// enabled based on whether the the preference for that logic is initialized.
+// This should happen only once as the used preference will be initialized
+// afterwards in |UmaSessionStats.java|.
+bool needReset() {

[Alexei Svitkine (slow), 2015/05/12 16:40:00]

Please use a better name.

Capitalize first letter, but also NeedReset() isn't a good name - please make it
more descriptive, e.g. ShouldClearSavedMetrics().

[gayane -on leave until 09-2017, 2015/05/13 13:56:12]

Done.

+  PrefService* local_state = g_browser_process->local_state();
+  return !local_state->HasPrefPath(prefs::kMetricsReportingEnabled) &&
+         variations::GetVariationParamValue("UMA_EnableCellularLogUpload",
+                                            "Enabled") == "true";
+}
+#endif
 }  // namespace

[Alexei Svitkine (slow), 2015/05/12 16:40:00]

Nit: add a blank line after this.

[gayane -on leave until 09-2017, 2015/05/13 13:56:12]

Done.

 
 ChromeMetricsServiceClient::ChromeMetricsServiceClient(
@@ -279,6 +291,16 @@ void ChromeMetricsServiceClient::LogPluginLoadingError(
 }
 
 void ChromeMetricsServiceClient::Initialize() {
+// Reset metrics reports if it is the first time cellular upload logic should
+// apply to avoid sudden bulk uploads. It needs to be done before initializing
+// metrics service so that metrics log manager is initialized correctly.
+#if defined(OS_ANDROID)

[Alexei Svitkine (slow), 2015/05/12 16:40:00]

Instead of having the ifdefs in 3 places, how about making the function return
false on non-Android platforms, so that you can remove the ifdefs from 2 of the
places.

[gayane -on leave until 09-2017, 2015/05/13 13:56:12]

Done.

+  if (needReset()) {
+    local_state->ClearPref(metrics::prefs::kMetricsInitialLogs);
+    local_state->ClearPref(metrics::prefs::kMetricsOngoingLogs);
+  }
+#endif
+
   metrics_service_.reset(new metrics::MetricsService(
       metrics_state_manager_, this, g_browser_process->local_state()));
 
@@ -363,6 +385,15 @@ void ChromeMetricsServiceClient::Initialize() {
       scoped_ptr<metrics::MetricsProvider>(
           SigninStatusMetricsProvider::CreateInstance()));
 #endif  // !defined(OS_CHROMEOS) && !defined(OS_IOS)
+
+// Reset stability metrics if it is the first time cellular upload logic should
+// apply to avoid sudden bulk uploads. It needs to be done after all providers
+// are registered.
+#if defined(OS_ANDROID)
+  if (needReset()) {

[Alexei Svitkine (slow), 2015/05/12 16:40:00]

Nit: No {}'s

[gayane -on leave until 09-2017, 2015/05/13 13:56:12]

Done.

+    metrics_service_->ClearSavedStabilityMetrics();
+  }
+#endif
 }
 
 void ChromeMetricsServiceClient::OnInitTaskGotHardwareClass() {