Reset metrics if cellular uploads logic should be applied.

chrome/browser/metrics/chrome_metrics_service_client.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 "chrome/browser/metrics/chrome_metrics_service_client.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 83 matching lines @@
 bool IsCellularLogicEnabled() {
   if (variations::GetVariationParamValue("UMA_EnableCellularLogUpload",
                                          "Enabled") != "true") {
     return false;
   }
 
   return net::NetworkChangeNotifier::IsConnectionCellular(
       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(
     metrics::MetricsStateManager* state_manager)
     : metrics_state_manager_(state_manager),
       chromeos_metrics_provider_(nullptr),
       waiting_for_collect_final_metrics_step_(false),
       num_async_histogram_fetches_in_progress_(0),
       profiler_metrics_provider_(nullptr),
 #if defined(ENABLE_PLUGINS)
       plugin_metrics_provider_(nullptr),
@@ skipping 157 matching lines @@
 void ChromeMetricsServiceClient::LogPluginLoadingError(
     const base::FilePath& plugin_path) {
 #if defined(ENABLE_PLUGINS)
   plugin_metrics_provider_->LogPluginLoadingError(plugin_path);
 #else
   NOTREACHED();
 #endif  // defined(ENABLE_PLUGINS)
 }
 
 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()));
 
   // Register metrics providers.
 #if defined(ENABLE_EXTENSIONS)
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
   metrics_service_->RegisterMetricsProvider(
@@ skipping 64 matching lines @@
       new SigninStatusMetricsProviderChromeOS;
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(signin_metrics_provider_cros));
 #endif  // defined(OS_CHROMEOS)
 
 #if !defined(OS_CHROMEOS) && !defined(OS_IOS)
   metrics_service_->RegisterMetricsProvider(
       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() {
   const base::Closure got_plugin_info_callback =
       base::Bind(&ChromeMetricsServiceClient::OnInitTaskGotPluginInfo,
                  weak_ptr_factory_.GetWeakPtr());
 
 #if defined(ENABLE_PLUGINS)
   plugin_metrics_provider_->GetPluginInformation(got_plugin_info_callback);
 #else
@@ skipping 149 matching lines @@
     case content::NOTIFICATION_LOAD_START:
     case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
     case content::NOTIFICATION_RENDER_WIDGET_HOST_HANG:
       metrics_service_->OnApplicationNotIdle();
       break;
 
     default:
       NOTREACHED();
   }
 }