Add EffectiveConnectionType enum to the system profile proto

Value of the EffectiveConnectionType enum is provided by the
NetworkMetricsProvider.

A new class EffectiveConnectionTypeObserver has been added which listens
to the changes in the EffectiveConnectionType, and lives on the same
thread as the NetworkQualityEstimator.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=677158

Review-Url: https://codereview.chromium.org/2605553002
Cr-Commit-Position: refs/heads/master@{#442821}

chrome/browser/BUILD.gn

@@ -603,6 +603,8 @@ split_static_library("browser") {
     "metrics/metrics_memory_details.h",
     "metrics/metrics_reporting_state.cc",
     "metrics/metrics_reporting_state.h",
+    "metrics/network_quality_estimator_provider_impl.cc",
+    "metrics/network_quality_estimator_provider_impl.h",
     "metrics/perf/perf_provider_chromeos.cc",
     "metrics/perf/perf_provider_chromeos.h",
     "metrics/sampling_metrics_provider.cc",

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -32,6 +32,7 @@
 #include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
 #include "chrome/browser/metrics/https_engagement_metrics_provider.h"
 #include "chrome/browser/metrics/metrics_reporting_state.h"
+#include "chrome/browser/metrics/network_quality_estimator_provider_impl.h"
 #include "chrome/browser/metrics/sampling_metrics_provider.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/metrics/time_ticks_experiment_win.h"
@@ -571,10 +572,12 @@ void ChromeMetricsServiceClient::Initialize() {
       std::unique_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
+
   metrics_service_->RegisterMetricsProvider(
-      std::unique_ptr<metrics::MetricsProvider>(
-          new metrics::NetworkMetricsProvider(
-              content::BrowserThread::GetBlockingPool())));
+      base::MakeUnique<metrics::NetworkMetricsProvider>(
+          base::MakeUnique<metrics::NetworkQualityEstimatorProviderImpl>(
+              g_browser_process->io_thread()),
+          content::BrowserThread::GetBlockingPool()));
 
   // Currently, we configure OmniboxMetricsProvider to not log events to UMA
   // if there is a single incognito session visible. In the future, it may

chrome/browser/metrics/network_quality_estimator_provider_impl.cc

+// Copyright 2017 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/network_quality_estimator_provider_impl.h"
+
+#include "chrome/browser/io_thread.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace net {
+class NetworkQualityEstimator;
+}
+
+namespace metrics {
+
+NetworkQualityEstimatorProviderImpl::NetworkQualityEstimatorProviderImpl(
+    IOThread* io_thread)
+    : io_thread_(io_thread) {
+  DCHECK(io_thread_);
+}
+
+NetworkQualityEstimatorProviderImpl::~NetworkQualityEstimatorProviderImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+scoped_refptr<base::SequencedTaskRunner>
+NetworkQualityEstimatorProviderImpl::GetTaskRunner() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // |this| is constructed on UI thread, but must be used on the IO thread.
+  thread_checker_.DetachFromThread();
+  return content::BrowserThread::GetTaskRunnerForThread(
+      content::BrowserThread::IO);
+}
+
+net::NetworkQualityEstimator*
+NetworkQualityEstimatorProviderImpl::GetNetworkQualityEstimator() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  return io_thread_->globals()->network_quality_estimator.get();
+}
+
+}  // namespace metrics

chrome/browser/metrics/network_quality_estimator_provider_impl.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_METRICS_NETWORK_QUALITY_ESTIMATOR_PROVIDER_IMPL_H_
+#define CHROME_BROWSER_METRICS_NETWORK_QUALITY_ESTIMATOR_PROVIDER_IMPL_H_
+
+#include "base/macros.h"
+#include "base/threading/thread_checker.h"
+#include "components/metrics/net/network_metrics_provider.h"
+
+class IOThread;
+
+namespace metrics {
+
+// Implements NetworkMetricsProvider::NetworkQualityEstimatorProvider. Provides
+// NetworkQualityEstimator by querying the IOThread.
+class NetworkQualityEstimatorProviderImpl
+    : public NetworkMetricsProvider::NetworkQualityEstimatorProvider {
+ public:
+  explicit NetworkQualityEstimatorProviderImpl(IOThread* io_thread);
+  ~NetworkQualityEstimatorProviderImpl() override;
+
+ private:
+  // NetworkMetricsProvider::NetworkQualityEstimatorProvider:
+  scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() override;
+  net::NetworkQualityEstimator* GetNetworkQualityEstimator() override;
+
+  IOThread* io_thread_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimatorProviderImpl);
+};
+
+}  // namespace metrics
+
+#endif  // CHROME_BROWSER_METRICS_NETWORK_QUALITY_ESTIMATOR_PROVIDER_IMPL_H_

components/metrics/BUILD.gn

@@ -325,6 +325,7 @@ source_set("unit_tests") {
     "metrics_service_unittest.cc",
     "metrics_state_manager_unittest.cc",
     "net/net_metrics_log_uploader_unittest.cc",
+    "net/network_metrics_provider_unittest.cc",
     "persisted_logs_unittest.cc",
     "profiler/profiler_metrics_provider_unittest.cc",
     "profiler/tracking_synchronizer_unittest.cc",
@@ -358,7 +359,10 @@ source_set("unit_tests") {
   }
 
   if (is_chromeos) {
-    deps += [ "leak_detector:unit_tests" ]
+    deps += [
+      "leak_detector:unit_tests",
+      "//chromeos",
+    ]
   }
 
   # iOS is not supported by the profiler and the ios-simulator bot chokes on

components/metrics/net/DEPS

 include_rules = [
+  "+chromeos/dbus",
   "+chromeos/network",
   "+components/data_use_measurement/core",
   "+components/variations",

components/metrics/net/network_metrics_provider.h

@@ -10,11 +10,17 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_base.h"
+#include "base/threading/thread_checker.h"
 #include "components/metrics/metrics_provider.h"
 #include "components/metrics/net/wifi_access_point_info_provider.h"
 #include "components/metrics/proto/system_profile.pb.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/network_interfaces.h"
+#include "net/nqe/effective_connection_type.h"
+
+namespace net {
+class NetworkQualityEstimator;
+}
 
 namespace metrics {
 
@@ -24,12 +30,44 @@ class NetworkMetricsProvider
     : public MetricsProvider,
       public net::NetworkChangeNotifier::ConnectionTypeObserver {
  public:
+  // Class that provides |this| with the network quality estimator.
+  class NetworkQualityEstimatorProvider {
+   public:
+    virtual ~NetworkQualityEstimatorProvider() {}
+
+    // Returns the network quality estimator. May be nullptr.
+    virtual net::NetworkQualityEstimator* GetNetworkQualityEstimator() = 0;
+
+    // Returns the task runner on which |this| should be used and destroyed.
+    virtual scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() = 0;
+
+   protected:
+    NetworkQualityEstimatorProvider() {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimatorProvider);
+  };
+
   // Creates a NetworkMetricsProvider, where |io_task_runner| is used to post
   // network info collection tasks.
   explicit NetworkMetricsProvider(base::TaskRunner* io_task_runner);
+
+  // Creates a NetworkMetricsProvider, where |io_task_runner| is used to post
+  // network info collection tasks.  |network_quality_estimator_provider|
+  // should be set if it is useful to attach the quality of the network to the
+  // metrics report.
+  NetworkMetricsProvider(std::unique_ptr<NetworkQualityEstimatorProvider>
+                             network_quality_estimator_provider,
+                         base::TaskRunner* io_task_runner);
+
   ~NetworkMetricsProvider() override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(NetworkMetricsProviderTest, EffectiveConnectionType);
+
+  // Listens to the changes in the effective conection type.
+  class EffectiveConnectionTypeObserver;
+
   // MetricsProvider:
   void ProvideGeneralMetrics(ChromeUserMetricsExtension* uma_proto) override;
   void ProvideSystemProfileMetrics(SystemProfileProto* system_profile) override;
@@ -41,6 +79,8 @@ class NetworkMetricsProvider
   SystemProfileProto::Network::ConnectionType GetConnectionType() const;
   SystemProfileProto::Network::WifiPHYLayerProtocol GetWifiPHYLayerProtocol()
       const;
+  SystemProfileProto::Network::EffectiveConnectionType
+  GetEffectiveConnectionType() const;
 
   // Posts a call to net::GetWifiPHYLayerProtocol on the blocking pool.
   void ProbeWifiPHYLayerProtocol();
@@ -57,6 +97,10 @@ class NetworkMetricsProvider
   // Logs metrics that are functions of other metrics being uploaded.
   void LogAggregatedMetrics();
 
+  // Notifies |this| that the effective connection type of the current network
+  // has changed to |type|.
+  void OnEffectiveConnectionTypeChanged(net::EffectiveConnectionType type);
+
   // Task runner used for blocking file I/O.
   base::TaskRunner* io_task_runner_;
 
@@ -79,6 +123,28 @@ class NetworkMetricsProvider
   base::HistogramBase::Count total_aborts_;
   base::HistogramBase::Count total_codes_;
 
+  // Provides the network quality estimator. May be null.
+  std::unique_ptr<NetworkQualityEstimatorProvider>
+      network_quality_estimator_provider_;
+
+  // Listens to the changes in the effective connection type. Initialized and
+  // destroyed using |network_quality_task_runner_|. May be null.
+  std::unique_ptr<EffectiveConnectionTypeObserver>
+      effective_connection_type_observer_;
+
+  // Task runner using which |effective_connection_type_observer_| is
+  // initialized and destroyed. May be null.
+  scoped_refptr<base::SequencedTaskRunner> network_quality_task_runner_;
+
+  // Last known effective connection type.
+  net::EffectiveConnectionType effective_connection_type_;
+
+  // True if |effective_connection_type_| changed during the lifetime of the
+  // log.
+  bool effective_connection_type_is_ambiguous_;
+
+  base::ThreadChecker thread_checker_;
+
   base::WeakPtrFactory<NetworkMetricsProvider> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkMetricsProvider);

components/metrics/net/network_metrics_provider_unittest.cc

+// Copyright 2017 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/net/network_metrics_provider.h"
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/metrics/proto/system_profile.pb.h"
+#include "net/base/network_change_notifier.h"
+#include "net/nqe/network_quality_estimator_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/network/network_handler.h"
+#endif  // OS_CHROMEOS
+
+namespace metrics {
+
+namespace {
+
+class TestNetworkQualityEstimatorProvider
+    : public NetworkMetricsProvider::NetworkQualityEstimatorProvider {
+ public:
+  explicit TestNetworkQualityEstimatorProvider(
+      net::TestNetworkQualityEstimator* estimator)
+      : estimator_(estimator) {}
+  ~TestNetworkQualityEstimatorProvider() override {}
+
+ private:
+  // NetworkMetricsProvider::NetworkQualityEstimatorProvider:
+  scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() override {
+    return base::ThreadTaskRunnerHandle::Get();
+  }
+
+  net::NetworkQualityEstimator* GetNetworkQualityEstimator() override {
+    return estimator_;
+  }
+
+  net::TestNetworkQualityEstimator* estimator_;
+  DISALLOW_COPY_AND_ASSIGN(TestNetworkQualityEstimatorProvider);
+};
+
+}  // namespace
+
+// Verifies that the effective connection type is correctly set.
+TEST(NetworkMetricsProviderTest, EffectiveConnectionType) {
+  base::MessageLoop loop(base::MessageLoop::TYPE_IO);
+
+#if defined(OS_CHROMEOS)
+  chromeos::DBusThreadManager::Initialize();
+  chromeos::NetworkHandler::Initialize();
+#endif  // OS_CHROMEOS
+
+  net::TestNetworkQualityEstimator estimator;
+  std::unique_ptr<NetworkMetricsProvider::NetworkQualityEstimatorProvider>
+      estimator_provider(base::WrapUnique(
+          new TestNetworkQualityEstimatorProvider(&estimator)));
+  SystemProfileProto system_profile;
+  NetworkMetricsProvider network_metrics_provider(
+      std::move(estimator_provider), base::ThreadTaskRunnerHandle::Get().get());
+
+  EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
+            network_metrics_provider.effective_connection_type_);
+  network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
+            system_profile.network().effective_connection_type());
+
+  // Set RTT so that the effective connection type is computed as 2G.
+  estimator.set_recent_http_rtt(base::TimeDelta::FromMilliseconds(1500));
+  estimator.set_start_time_null_http_rtt(
+      base::TimeDelta::FromMilliseconds(1500));
+
+  EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
+            network_metrics_provider.effective_connection_type_);
+  // Running a request would cause the effective connection type to be computed
+  // as 2G, and observers to be notified.
+  estimator.RunOneRequest();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_2G,
+            network_metrics_provider.effective_connection_type_);
+  network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_2G,
+            system_profile.network().effective_connection_type());
+
+  // Set RTT so that the effective connection type is computed as SLOW_2G.
+  estimator.set_recent_http_rtt(base::TimeDelta::FromMilliseconds(3000));
+  estimator.set_start_time_null_http_rtt(
+      base::TimeDelta::FromMilliseconds(3000));
+  // Running a request would cause the effective connection type to be computed
+  // as SLOW_2G, and observers to be notified.
+  estimator.RunOneRequest();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
+            network_metrics_provider.effective_connection_type_);
+  network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
+  // Effective connection type is set to ambiguous since the effective
+  // connection type changed from 2G to SLOW_2G during the lifetime of the log.
+  EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_AMBIGUOUS,
+            system_profile.network().effective_connection_type());
+
+  // Getting the system profile again should return the actual effective
+  // connection type since the effective connection type did not change during
+  // the lifetime of the log.
+  network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
+            system_profile.network().effective_connection_type());
+}
+
+}  // namespace metrics
\ No newline at end of file

components/metrics/proto/system_profile.proto

@@ -281,6 +281,7 @@ message SystemProfileProto {
   optional Hardware hardware = 6;
 
   // Information about the network connection.
+  // Next tag: 7
   message Network {
     // Set to true if connection_type changed during the lifetime of the log.
     optional bool connection_type_is_ambiguous = 1;
@@ -366,6 +367,27 @@ message SystemProfileProto {
     }
     // Information of the wireless AP that device is connected to.
     optional WifiAccessPoint access_point_info = 5;
+
+    // Derived from net::NetworkQualityEstimator::EffectiveConnectionType
+    // translated through NetworkMetricsProvider::GetConnectionType.
+    enum EffectiveConnectionType {
+      EFFECTIVE_CONNECTION_TYPE_UNKNOWN = 0;
+      // Specifies that the connection_type changed during the lifetime of the
+      // log.
+      EFFECTIVE_CONNECTION_TYPE_AMBIGUOUS = 1;
+      EFFECTIVE_CONNECTION_TYPE_OFFLINE = 2;
+      EFFECTIVE_CONNECTION_TYPE_SLOW_2G = 3;
+      EFFECTIVE_CONNECTION_TYPE_2G = 4;
+      EFFECTIVE_CONNECTION_TYPE_3G = 5;
+      EFFECTIVE_CONNECTION_TYPE_4G = 6;
+    }
+    // The connection type according to net::NetworkQualityEstimator.
+    // EffectiveConnectionType is the connection type whose typical performance
+    // is most similar to the measured performance of the network in use. In
+    // many cases, the "effective" connection type and the actual type of
+    // connection in use are the same, but often a network connection performs
+    // significantly differently, usually worse, from its expected capabilities.
+    optional EffectiveConnectionType effective_connection_type = 6;
   }
   optional Network network = 13;