Use of param for experiments and simplifications.

Instead of previously checking based on experiment group now it checks the param of the experiment.

Also some simplifications for connection type callback.

BUG=455847

Review URL: https://codereview.chromium.org/1074113004

Cr-Commit-Position: refs/heads/master@{#325905}

5 files changed, 31 insertions, 73 deletions

diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index 39354d1..589c129 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -11,7 +11,6 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
@@ -39,6 +38,7 @@
 #include "components/metrics/profiler/profiler_metrics_provider.h"
 #include "components/metrics/profiler/tracking_synchronizer.h"
 #include "components/metrics/url_constants.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/histogram_fetcher.h"
 #include "content/public/browser/notification_service.h"
@@ -89,15 +89,17 @@ const int kStandardUploadIntervalCellularSeconds = 15 * 60;  // Fifteen minutes.
 const int kStandardUploadIntervalSeconds = 30 * 60;  // Thirty minutes.
 #endif
 
-#if defined(OS_ANDROID) || defined(OS_IOS)
-// Returns true if the user is assigned to the experiment group for enabled
-// cellular uploads.
-bool IsCellularEnabledByExperiment() {
-  const std::string group_name =
-      base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload");
-  return group_name == "Enabled";
+// Returns true if current connection type is cellular and user is assigned to
+// experimental group for enabled cellular uploads.
+bool IsCellularLogicEnabled() {
+  if (variations::GetVariationParamValue("UMA_EnableCellularLogUpload",
+                                         "Enabled") != "true") {
+    return false;
+  }
+
+  return net::NetworkChangeNotifier::IsConnectionCellular(
+      net::NetworkChangeNotifier::GetConnectionType());
 }
-#endif
 
 }  // namespace
 
@@ -252,10 +254,7 @@ ChromeMetricsServiceClient::CreateUploader(
 
 base::TimeDelta ChromeMetricsServiceClient::GetStandardUploadInterval() {
 #if defined(OS_ANDROID) || defined(OS_IOS)
-  bool is_cellular = false;
-  cellular_callback_.Run(&is_cellular);
-
-  if (is_cellular && IsCellularEnabledByExperiment())
+  if (IsCellularLogicEnabled())
     return base::TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds);
 #endif
   return base::TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
@@ -288,11 +287,9 @@ void ChromeMetricsServiceClient::Initialize() {
       scoped_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
-  scoped_ptr<metrics::NetworkMetricsProvider> network_metrics_provider(
-      new metrics::NetworkMetricsProvider(
-          content::BrowserThread::GetBlockingPool()));
-  cellular_callback_ = network_metrics_provider->GetConnectionCallback();
-  metrics_service_->RegisterMetricsProvider(network_metrics_provider.Pass());
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(new metrics::NetworkMetricsProvider(
+          content::BrowserThread::GetBlockingPool())));
 
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new OmniboxMetricsProvider));
@@ -306,7 +303,7 @@ void ChromeMetricsServiceClient::Initialize() {
       scoped_ptr<metrics::MetricsProvider>(drive_metrics_provider_));
 
   profiler_metrics_provider_ =
-      new metrics::ProfilerMetricsProvider(cellular_callback_);
+      new metrics::ProfilerMetricsProvider(base::Bind(&IsCellularLogicEnabled));
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(profiler_metrics_provider_));
 
diff --git a/components/metrics/net/network_metrics_provider.cc b/components/metrics/net/network_metrics_provider.cc
index 4ec6574..e845cde 100644
--- a/components/metrics/net/network_metrics_provider.cc
+++ b/components/metrics/net/network_metrics_provider.cc
@@ -227,24 +227,4 @@ void NetworkMetricsProvider::WriteWifiAccessPointProto(
   }
 }
 
-bool NetworkMetricsProvider::IsCellularConnection() {
-  switch (GetConnectionType()) {
-    case SystemProfileProto_Network_ConnectionType_CONNECTION_2G:
-    case SystemProfileProto_Network_ConnectionType_CONNECTION_3G:
-    case SystemProfileProto_Network_ConnectionType_CONNECTION_4G:
-      return true;
-    default:
-      return false;
-  }
-}
-
-void NetworkMetricsProvider::GetIsCellularConnection(bool* is_cellular_out) {
-  *is_cellular_out = IsCellularConnection();
-}
-
-base::Callback<void(bool*)> NetworkMetricsProvider::GetConnectionCallback() {
-  return base::Bind(&NetworkMetricsProvider::GetIsCellularConnection,
-                 weak_ptr_factory_.GetWeakPtr());
-}
-
 }  // namespace metrics
diff --git a/components/metrics/net/network_metrics_provider.h b/components/metrics/net/network_metrics_provider.h
index ffdc017..412ef67 100644
--- a/components/metrics/net/network_metrics_provider.h
+++ b/components/metrics/net/network_metrics_provider.h
@@ -27,10 +27,6 @@ class NetworkMetricsProvider
   explicit NetworkMetricsProvider(base::TaskRunner* io_task_runner);
   ~NetworkMetricsProvider() override;
 
-  // Returns callback function bound to the weak pointer of the provider, which
-  // can be used to get whether current connection type is cellular.
-  base::Callback<void(bool*)> GetConnectionCallback();
-
  private:
   // MetricsProvider:
   void OnDidCreateMetricsLog() override;
@@ -56,13 +52,6 @@ class NetworkMetricsProvider
       const WifiAccessPointInfoProvider::WifiAccessPointInfo& info,
       SystemProfileProto::Network* network_proto);
 
-  // Returns true if the connection type is 2G, 3G, or 4G.
-  bool IsCellularConnection();
-
-  // Assigns the passed |is_cellular_out| parameter based on whether current
-  // network connection is cellular.
-  void GetIsCellularConnection(bool* is_cellular_out);
-
   // Task runner used for blocking file I/O.
   base::TaskRunner* io_task_runner_;
 
diff --git a/components/metrics/profiler/profiler_metrics_provider.cc b/components/metrics/profiler/profiler_metrics_provider.cc
index dadd28a..341f74e 100644
--- a/components/metrics/profiler/profiler_metrics_provider.cc
+++ b/components/metrics/profiler/profiler_metrics_provider.cc
@@ -8,10 +8,10 @@
 #include <string>
 #include <vector>
 
+#include "base/stl_util.h"
 #include "base/tracked_objects.h"
 #include "components/metrics/metrics_log.h"
 #include "components/nacl/common/nacl_process_type.h"
-#include "components/variations/variations_associated_data.h"
 
 namespace metrics {
 
@@ -102,21 +102,13 @@ void WriteProfilerData(
   }
 }
 
-// Returns true if the user is assigned to the experiment group for enabled
-// cellular uploads.
-bool IsCellularEnabledByExperiment() {
-  const std::string group_name =
-      base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload");
-  return group_name == "Enabled";
-}
-
 }  // namespace
 
 ProfilerMetricsProvider::ProfilerMetricsProvider() {
 }
 
 ProfilerMetricsProvider::ProfilerMetricsProvider(
-    const base::Callback<void(bool*)>& cellular_callback)
+    const base::Callback<bool(void)>& cellular_callback)
     : cellular_callback_(cellular_callback) {
 }
 
@@ -145,7 +137,7 @@ void ProfilerMetricsProvider::RecordProfilerData(
     base::TimeDelta phase_start,
     base::TimeDelta phase_finish,
     const ProfilerEvents& past_events) {
-  if (IsCellularConnection() && IsCellularEnabledByExperiment())
+  if (IsCellularLogicEnabled())
     return;
   if (tracked_objects::GetTimeSourceType() !=
       tracked_objects::TIME_SOURCE_TYPE_WALL_TIME) {
@@ -171,15 +163,13 @@ void ProfilerMetricsProvider::RecordProfilerData(
                     profiler_event);
 }
 
-bool ProfilerMetricsProvider::IsCellularConnection() {
-  bool is_cellular = false;
-// For android get current connection type from NetworkMetricsProvider if the
-// callback exists.
+bool ProfilerMetricsProvider::IsCellularLogicEnabled() {
+// For android get current connection type if the callback exists.
 #if defined(OS_ANDROID)
   if (!cellular_callback_.is_null())
-    cellular_callback_.Run(&is_cellular);
+    return cellular_callback_.Run();
 #endif
-  return is_cellular;
+  return false;
 }
 
 }  // namespace metrics
diff --git a/components/metrics/profiler/profiler_metrics_provider.h b/components/metrics/profiler/profiler_metrics_provider.h
index 7740cb80..e9843f8 100644
--- a/components/metrics/profiler/profiler_metrics_provider.h
+++ b/components/metrics/profiler/profiler_metrics_provider.h
@@ -23,7 +23,7 @@ namespace metrics {
 class ProfilerMetricsProvider : public MetricsProvider {
  public:
   explicit ProfilerMetricsProvider(
-      const base::Callback<void(bool*)>& cellular_callback);
+      const base::Callback<bool(void)>& cellular_callback);
   // Creates profiler metrics provider with a null callback.
   ProfilerMetricsProvider();
   ~ProfilerMetricsProvider() override;
@@ -43,9 +43,10 @@ class ProfilerMetricsProvider : public MetricsProvider {
       const ProfilerEvents& past_events);
 
  private:
-  // Returns whether current connection is cellular or not according to the
-  // callback.
-  bool IsCellularConnection();
+  // Returns true if current connection type is cellular and user is assigned to
+  // experimental group for enabled cellular uploads according to
+  // |cellular_callback_|.
+  bool IsCellularLogicEnabled();
 
   // Saved cache of generated Profiler event protos, to be copied into the UMA
   // proto when ProvideGeneralMetrics() is called. The map is from a profiling
@@ -53,8 +54,9 @@ class ProfilerMetricsProvider : public MetricsProvider {
   // profiling phase.
   std::map<int, ProfilerEventProto> profiler_events_cache_;
 
-  // Callback function used to get current network connection type.
-  base::Callback<void(bool*)> cellular_callback_;
+  // Returns true if current connection type is cellular and user is assigned to
+  // experimental group for enabled cellular uploads.
+  base::Callback<bool(void)> cellular_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfilerMetricsProvider);
 };