Add EffectiveConnectionType enum to the system profile proto

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 <stddef.h>
 
 #include <utility>
 #include <vector>
@@ skipping 10 matching lines @@
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/path_service.h"
 #include "base/rand_util.h"
 #include "base/strings/string16.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_brand.h"
+#include "chrome/browser/io_thread.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #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/sampling_metrics_provider.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/metrics/time_ticks_experiment_win.h"
 #include "chrome/browser/safe_browsing/certificate_reporting_metrics_provider.h"
 #include "chrome/browser/sync/chrome_sync_client.h"
 #include "chrome/browser/ui/browser_otr_state.h"
@@ skipping 74 matching lines @@
 #include "third_party/crashpad/crashpad/client/crashpad_info.h"
 #endif
 
 #if !defined(OS_CHROMEOS)
 #include "chrome/browser/signin/chrome_signin_status_metrics_provider_delegate.h"
 #include "components/signin/core/browser/signin_status_metrics_provider.h"
 #endif  // !defined(OS_CHROMEOS)
 
 namespace {
 
+// Returns the network quality estimator. Runs on IO thread.
+net::NetworkQualityEstimator* GetNetworkQualityEstimatorOnIOThread(
+    IOThread* io_thread) {
+  if (!io_thread)

[Alexei Svitkine (slow), 2017/01/03 17:48:48]

In what cases would this be hit? Add a comment or remove if it's not possible.

[tbansal1, 2017/01/03 18:55:43]

Done.

+    return nullptr;
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  return io_thread->globals()->network_quality_estimator.get();
+}
+
 // This specifies the amount of time to wait for all renderers to send their
 // data.
 const int kMaxHistogramGatheringWaitDuration = 60000;  // 60 seconds.
 
 // Needs to be kept in sync with the writer in
 // third_party/crashpad/crashpad/handler/handler_main.cc.
 const char kCrashpadHistogramAllocatorName[] = "CrashpadMetrics";
 
 #if defined(OS_WIN) || defined(OS_MACOSX)
 // The stream type assigned to the minidump stream that holds the serialized
@@ skipping 141 matching lines @@
       plugin_metrics_provider_(nullptr),
 #endif
 #if defined(OS_WIN)
       google_update_metrics_provider_(nullptr),
       watcher_metrics_provider_(nullptr),
       antivirus_metrics_provider_(nullptr),
 #endif
       drive_metrics_provider_(nullptr),
       start_time_(base::TimeTicks::Now()),
       has_uploaded_profiler_data_(false),
+      network_metrics_provider_(nullptr),
       weak_ptr_factory_(this) {
   DCHECK(thread_checker_.CalledOnValidThread());
   RecordCommandLineMetrics();
   RegisterForNotifications();
 }
 
 ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
   DCHECK(thread_checker_.CalledOnValidThread());
   base::GlobalHistogramAllocator* allocator =
       base::GlobalHistogramAllocator::Get();
@@ skipping 267 matching lines @@
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new SubprocessMetricsProvider()));
 
   // Register metrics providers.
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
+
+  std::unique_ptr<metrics::NetworkMetricsProvider> network_metrics_provider(
+      new metrics::NetworkMetricsProvider(
+          content::BrowserThread::GetBlockingPool()));
+
+  network_metrics_provider_ = network_metrics_provider.get();
+
+  // Get the network quality estimator on the IO thread, and provide it to the
+  // |network_metrics_provider|.
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(GetNetworkQualityEstimatorOnIOThread,
+                 g_browser_process->io_thread()),
+      base::Bind(&ChromeMetricsServiceClient::ProvideNetworkQualityEstimator,
+                 weak_ptr_factory_.GetWeakPtr()));

[Alexei Svitkine (slow), 2017/01/03 17:48:48]

Can all of this be internally in NetworksMetricsProvider? Why does it have it be
in chrome_metrics_service_client.cc? Is it because of the IO thread reference?
If so, perhaps you can just pass in a Callback object to the relevant function
to NetworkMetricsProvider ctor and it does the rest with it?

[tbansal1, 2017/01/03 18:55:43]

Yes, IO thread is in chrome/browser, so IOThread or IOThread::Globals() can't be
referenced from components. That means I have to call
"io_thread->globals()->network_quality_estimator.get()" somewhere in
chrome/browser. Also, that call can only be done on IO thread. So, the only
option is to do the PostTask to IO thread here, and get the reference to NQE.
Probably not.

[asvitkine_google, 2017/01/03 19:09:50]

Just to be clear, I would like there to be as little code as possible in this
file - as adding more code like this doesn't scale well.

So let's find a solution that achieves that. One option is to have an interface
defined in network_metrics_provider.h that has functions to get the provider.
And then you can have a class implementing that interface in
chrome/browser/metrics and in this code you can create that object and pass it
to the network_metrics_provider ctor.

+
   metrics_service_->RegisterMetricsProvider(
-      std::unique_ptr<metrics::MetricsProvider>(
-          new metrics::NetworkMetricsProvider(
-              content::BrowserThread::GetBlockingPool())));
+      std::move(network_metrics_provider));
 
   // Currently, we configure OmniboxMetricsProvider to not log events to UMA
   // if there is a single incognito session visible. In the future, it may
   // be worth revisiting this to still log events from non-incognito sessions.
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(new OmniboxMetricsProvider(
           base::Bind(&chrome::IsIncognitoSessionActive))));
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new ChromeStabilityMetricsProvider(local_state)));
@@ skipping 307 matching lines @@
   }
 }
 
 void ChromeMetricsServiceClient::OnURLOpenedFromOmnibox(OmniboxLog* log) {
   metrics_service_->OnApplicationNotIdle();
 }
 
 bool ChromeMetricsServiceClient::IsUMACellularUploadLogicEnabled() {
   return metrics::IsCellularLogicEnabled();
 }
+
+void ChromeMetricsServiceClient::ProvideNetworkQualityEstimator(
+    net::NetworkQualityEstimator* network_quality_estimator) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // Provide the |network_quality_estimator| and the taskrunner for the thread
+  // on which |network_quality_estimator| runs to |network_metrics_provider_|.
+  network_metrics_provider_->ProvideNetworkQualityEstimator(
+      network_quality_estimator, content::BrowserThread::GetTaskRunnerForThread(
+                                     content::BrowserThread::IO));
+}