Commit 597752e4 authored by Jun Cai's avatar Jun Cai Committed by Commit Bot

Reland: Reland: Network Service: Migrate NetworkMetricsProvider to NetworkConnectionTracker

The "Initial upload" patch of this CL is the same as:
https://chromium-review.googlesource.com/c/chromium/src/+/1320710

The above original CL was reverted at:
https://chromium-review.googlesource.com/c/chromium/src/+/1324104

The revert is caused by a browser test failure:
ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
and more details can be found in the following bug link:
https://bugs.chromium.org/p/chromium/issues/detail?id=902784

The browser test failure is fixed at:
https://chromium-review.googlesource.com/c/chromium/src/+/1336433

So this CL is exactly the same as the "Initial upload".

TBR=olivierrobin@chromium.org, michaelbai@chromium.org, jam@chromium.org, fdoray@chromium.org, asvitkine@chromium.org

Bug: 883121
Change-Id: I253764764b409e7c6d1522fd5e5d44927c3f3c60
Reviewed-on: https://chromium-review.googlesource.com/c/1323658
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: default avatarJun Cai <juncai@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608481}
parent c4fa8aef
......@@ -32,6 +32,7 @@
#include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h"
#include "components/metrics/metrics_state_manager.h"
#include "components/metrics/net/network_metrics_provider.h"
#include "components/metrics/ui/screen_info_metrics_provider.h"
#include "components/metrics/url_constants.h"
#include "components/metrics/version_utils.h"
......@@ -39,6 +40,7 @@
#include "components/version_info/android/channel_getter.h"
#include "components/version_info/version_info.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
namespace android_webview {
......@@ -167,7 +169,8 @@ void AwMetricsServiceClient::InitializeWithClientId() {
metrics_service_->RegisterMetricsProvider(
std::unique_ptr<metrics::MetricsProvider>(
new metrics::NetworkMetricsProvider));
new metrics::NetworkMetricsProvider(
content::CreateNetworkConnectionTrackerAsyncGetter())));
metrics_service_->RegisterMetricsProvider(
std::unique_ptr<metrics::MetricsProvider>(
......
......@@ -90,6 +90,7 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/histogram_fetcher.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/browser/notification_service.h"
#include "ppapi/buildflags/buildflags.h"
#include "printing/buildflags/buildflags.h"
......@@ -606,6 +607,7 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
metrics_service_->RegisterMetricsProvider(
std::make_unique<metrics::NetworkMetricsProvider>(
content::CreateNetworkConnectionTrackerAsyncGetter(),
std::make_unique<metrics::NetworkQualityEstimatorProviderImpl>()));
// Currently, we configure OmniboxMetricsProvider to not log events to UMA
......@@ -727,6 +729,7 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
void ChromeMetricsServiceClient::RegisterUKMProviders() {
ukm_service_->RegisterMetricsProvider(
std::make_unique<metrics::NetworkMetricsProvider>(
content::CreateNetworkConnectionTrackerAsyncGetter(),
std::make_unique<metrics::NetworkQualityEstimatorProviderImpl>()));
#if defined(OS_CHROMEOS)
......
......@@ -35,6 +35,7 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/histogram_fetcher.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/content_switches.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
......@@ -355,7 +356,8 @@ void CastMetricsServiceClient::Initialize() {
new ::metrics::ScreenInfoMetricsProvider));
}
metrics_service_->RegisterMetricsProvider(
std::make_unique<::metrics::NetworkMetricsProvider>());
std::make_unique<::metrics::NetworkMetricsProvider>(
content::CreateNetworkConnectionTrackerAsyncGetter()));
shell::CastBrowserProcess::GetInstance()->browser_client()->
RegisterMetricsProviders(metrics_service_.get());
......
......@@ -430,6 +430,10 @@ source_set("unit_tests") {
if (is_ios) {
sources -= [ "child_call_stack_profile_collector_unittest.cc" ]
}
if (!is_ios) {
deps += [ "//content/test:test_support" ]
}
}
# Convenience testing target
......
......@@ -33,7 +33,6 @@
#include "components/metrics/metrics_log_store.h"
#include "components/metrics/metrics_provider.h"
#include "components/metrics/metrics_reporting_service.h"
#include "components/metrics/net/network_metrics_provider.h"
#include "components/variations/synthetic_trial_registry.h"
class PrefService;
......
......@@ -4,6 +4,7 @@ include_rules = [
"+components/data_use_measurement/core",
"+components/encrypted_messages",
"+components/variations",
"+content/public/test/test_browser_thread_bundle.h",
"+net",
"+services/network/public/cpp",
"+services/network/test",
......
......@@ -55,10 +55,14 @@ ConvertEffectiveConnectionType(
}
NetworkMetricsProvider::NetworkMetricsProvider(
network::NetworkConnectionTrackerAsyncGetter
network_connection_tracker_async_getter,
std::unique_ptr<NetworkQualityEstimatorProvider>
network_quality_estimator_provider)
: connection_type_is_ambiguous_(false),
network_change_notifier_initialized_(false),
: network_connection_tracker_(nullptr),
connection_type_is_ambiguous_(false),
connection_type_(network::mojom::ConnectionType::CONNECTION_UNKNOWN),
network_connection_tracker_initialized_(false),
wifi_phy_layer_protocol_is_ambiguous_(false),
wifi_phy_layer_protocol_(net::WIFI_PHY_LAYER_PROTOCOL_UNKNOWN),
total_aborts_(0),
......@@ -69,11 +73,9 @@ NetworkMetricsProvider::NetworkMetricsProvider(
min_effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
max_effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
weak_ptr_factory_(this) {
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
connection_type_ = net::NetworkChangeNotifier::GetConnectionType();
if (connection_type_ != net::NetworkChangeNotifier::CONNECTION_UNKNOWN)
network_change_notifier_initialized_ = true;
network_connection_tracker_async_getter.Run(
base::BindOnce(&NetworkMetricsProvider::SetNetworkConnectionTracker,
weak_ptr_factory_.GetWeakPtr()));
ProbeWifiPHYLayerProtocol();
if (network_quality_estimator_provider_) {
......@@ -88,7 +90,21 @@ NetworkMetricsProvider::NetworkMetricsProvider(
NetworkMetricsProvider::~NetworkMetricsProvider() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
if (network_connection_tracker_)
network_connection_tracker_->RemoveNetworkConnectionObserver(this);
}
void NetworkMetricsProvider::SetNetworkConnectionTracker(
network::NetworkConnectionTracker* network_connection_tracker) {
DCHECK(network_connection_tracker);
network_connection_tracker_ = network_connection_tracker;
network_connection_tracker_->AddNetworkConnectionObserver(this);
network_connection_tracker_->GetConnectionType(
&connection_type_,
base::BindOnce(&NetworkMetricsProvider::OnConnectionChanged,
weak_ptr_factory_.GetWeakPtr()));
if (connection_type_ != network::mojom::ConnectionType::CONNECTION_UNKNOWN)
network_connection_tracker_initialized_ = true;
}
void NetworkMetricsProvider::ProvideCurrentSessionData(
......@@ -104,7 +120,7 @@ void NetworkMetricsProvider::ProvideSystemProfileMetrics(
SystemProfileProto* system_profile) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!connection_type_is_ambiguous_ ||
network_change_notifier_initialized_);
network_connection_tracker_initialized_);
SystemProfileProto::Network* network = system_profile->mutable_network();
network->set_connection_type_is_ambiguous(connection_type_is_ambiguous_);
network->set_connection_type(GetConnectionType());
......@@ -117,13 +133,21 @@ void NetworkMetricsProvider::ProvideSystemProfileMetrics(
network->set_max_effective_connection_type(
ConvertEffectiveConnectionType(max_effective_connection_type_));
// Note: We get the initial connection type when it becomes available and it
// is handled at SetNetworkConnectionTracker() when GetConnectionType() is
// called.
//
// Update the connection type. Note that this is necessary to set the network
// type to "none" if there is no network connection for an entire UMA logging
// window, since OnConnectionTypeChanged() ignores transitions to the "none"
// state.
connection_type_ = net::NetworkChangeNotifier::GetConnectionType();
if (connection_type_ != net::NetworkChangeNotifier::CONNECTION_UNKNOWN)
network_change_notifier_initialized_ = true;
// state, and that is ok since it just deals with the current known state.
if (network_connection_tracker_) {
network_connection_tracker_->GetConnectionType(&connection_type_,
base::DoNothing());
}
if (connection_type_ != network::mojom::ConnectionType::CONNECTION_UNKNOWN)
network_connection_tracker_initialized_ = true;
// Reset the "ambiguous" flags, since a new metrics log session has started.
connection_type_is_ambiguous_ = false;
wifi_phy_layer_protocol_is_ambiguous_ = false;
......@@ -146,8 +170,8 @@ void NetworkMetricsProvider::ProvideSystemProfileMetrics(
WriteWifiAccessPointProto(info, network);
}
void NetworkMetricsProvider::OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) {
void NetworkMetricsProvider::OnConnectionChanged(
network::mojom::ConnectionType type) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// To avoid reporting an ambiguous connection type for users on flaky
// connections, ignore transitions to the "none" state. Note that the
......@@ -155,26 +179,27 @@ void NetworkMetricsProvider::OnNetworkChanged(
// new UMA logging window begins, so users who genuinely transition to offline
// mode for an extended duration will still be at least partially represented
// in the metrics logs.
if (type == net::NetworkChangeNotifier::CONNECTION_NONE) {
network_change_notifier_initialized_ = true;
if (type == network::mojom::ConnectionType::CONNECTION_NONE) {
network_connection_tracker_initialized_ = true;
return;
}
DCHECK(network_change_notifier_initialized_ ||
connection_type_ == net::NetworkChangeNotifier::CONNECTION_UNKNOWN);
DCHECK(network_connection_tracker_initialized_ ||
connection_type_ ==
network::mojom::ConnectionType::CONNECTION_UNKNOWN);
if (type != connection_type_ &&
connection_type_ != net::NetworkChangeNotifier::CONNECTION_NONE &&
network_change_notifier_initialized_) {
// If |network_change_notifier_initialized_| is false, it implies that this
// is the first connection change callback received from network change
// notifier, and the previous connection type was CONNECTION_UNKNOWN. In
// that case, connection type should not be marked as ambiguous since there
// was no actual change in the connection type.
connection_type_ != network::mojom::ConnectionType::CONNECTION_NONE &&
network_connection_tracker_initialized_) {
// If |network_connection_tracker_initialized_| is false, it implies that
// this is the first connection change callback received from network
// connection tracker, and the previous connection type was
// CONNECTION_UNKNOWN. In that case, connection type should not be marked as
// ambiguous since there was no actual change in the connection type.
connection_type_is_ambiguous_ = true;
}
network_change_notifier_initialized_ = true;
network_connection_tracker_initialized_ = true;
connection_type_ = type;
ProbeWifiPHYLayerProtocol();
......@@ -184,21 +209,21 @@ SystemProfileProto::Network::ConnectionType
NetworkMetricsProvider::GetConnectionType() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
switch (connection_type_) {
case net::NetworkChangeNotifier::CONNECTION_NONE:
case network::mojom::ConnectionType::CONNECTION_NONE:
return SystemProfileProto::Network::CONNECTION_NONE;
case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
case network::mojom::ConnectionType::CONNECTION_UNKNOWN:
return SystemProfileProto::Network::CONNECTION_UNKNOWN;
case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
case network::mojom::ConnectionType::CONNECTION_ETHERNET:
return SystemProfileProto::Network::CONNECTION_ETHERNET;
case net::NetworkChangeNotifier::CONNECTION_WIFI:
case network::mojom::ConnectionType::CONNECTION_WIFI:
return SystemProfileProto::Network::CONNECTION_WIFI;
case net::NetworkChangeNotifier::CONNECTION_2G:
case network::mojom::ConnectionType::CONNECTION_2G:
return SystemProfileProto::Network::CONNECTION_2G;
case net::NetworkChangeNotifier::CONNECTION_3G:
case network::mojom::ConnectionType::CONNECTION_3G:
return SystemProfileProto::Network::CONNECTION_3G;
case net::NetworkChangeNotifier::CONNECTION_4G:
case network::mojom::ConnectionType::CONNECTION_4G:
return SystemProfileProto::Network::CONNECTION_4G;
case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
case network::mojom::ConnectionType::CONNECTION_BLUETOOTH:
return SystemProfileProto::Network::CONNECTION_BLUETOOTH;
}
NOTREACHED();
......
......@@ -16,9 +16,9 @@
#include "base/single_thread_task_runner.h"
#include "components/metrics/metrics_provider.h"
#include "components/metrics/net/wifi_access_point_info_provider.h"
#include "net/base/network_change_notifier.h"
#include "net/base/network_interfaces.h"
#include "net/nqe/effective_connection_type.h"
#include "services/network/public/cpp/network_connection_tracker.h"
#include "third_party/metrics_proto/system_profile.pb.h"
namespace metrics {
......@@ -27,11 +27,11 @@ SystemProfileProto::Network::EffectiveConnectionType
ConvertEffectiveConnectionType(
net::EffectiveConnectionType effective_connection_type);
// Registers as observer with net::NetworkChangeNotifier and
// network::NetworkQualityTracker to keep track of the network environment.
// Registers as observer with network::NetworkConnectionTracker and keeps track
// of the network environment.
class NetworkMetricsProvider
: public MetricsProvider,
public net::NetworkChangeNotifier::NetworkChangeObserver {
public network::NetworkConnectionTracker::NetworkConnectionObserver {
public:
// Class that provides |this| with the network quality estimator.
class NetworkQualityEstimatorProvider {
......@@ -54,7 +54,8 @@ class NetworkMetricsProvider
// Creates a NetworkMetricsProvider, where
// |network_quality_estimator_provider| should be set if it is useful to
// attach the quality of the network to the metrics report.
explicit NetworkMetricsProvider(
NetworkMetricsProvider(network::NetworkConnectionTrackerAsyncGetter
network_connection_tracker_async_getter,
std::unique_ptr<NetworkQualityEstimatorProvider>
network_quality_estimator_provider = nullptr);
~NetworkMetricsProvider() override;
......@@ -73,9 +74,8 @@ class NetworkMetricsProvider
ChromeUserMetricsExtension* uma_proto) override;
void ProvideSystemProfileMetrics(SystemProfileProto* system_profile) override;
// NetworkChangeObserver:
void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) override;
// NetworkConnectionObserver:
void OnConnectionChanged(network::mojom::ConnectionType type) override;
SystemProfileProto::Network::ConnectionType GetConnectionType() const;
SystemProfileProto::Network::WifiPHYLayerProtocol GetWifiPHYLayerProtocol()
......@@ -98,12 +98,24 @@ class NetworkMetricsProvider
void OnEffectiveConnectionTypeChanged(net::EffectiveConnectionType type);
// Used as a callback to be given to NetworkConnectionTracker async getter to
// set the |network_connection_tracker_|.
void SetNetworkConnectionTracker(
network::NetworkConnectionTracker* network_connection_tracker);
// Watches for network connection changes.
// This |network_connection_tracker_| raw pointer is not owned by this class.
// It is obtained from the global |g_network_connection_tracker| pointer in
// //content/public/browser/network_service_instance.cc and points to the same
// object.
network::NetworkConnectionTracker* network_connection_tracker_;
// True if |connection_type_| changed during the lifetime of the log.
bool connection_type_is_ambiguous_;
// The connection type according to net::NetworkChangeNotifier.
net::NetworkChangeNotifier::ConnectionType connection_type_;
// True if the network change notifier has been initialized.
bool network_change_notifier_initialized_;
// The connection type according to network::NetworkConnectionTracker.
network::mojom::ConnectionType connection_type_;
// True if the network connection tracker has been initialized.
bool network_connection_tracker_initialized_;
// True if |wifi_phy_layer_protocol_| changed during the lifetime of the log.
bool wifi_phy_layer_protocol_is_ambiguous_;
......
......@@ -10,9 +10,9 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "net/base/network_change_notifier.h"
#include "build/build_config.h"
#include "services/network/test/test_network_connection_tracker.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/metrics_proto/system_profile.pb.h"
......@@ -21,14 +21,26 @@
#include "chromeos/network/network_handler.h"
#endif // OS_CHROMEOS
#if defined(OS_IOS)
#include "base/test/scoped_task_environment.h"
#else // !defined(OS_IOS)
#include "content/public/test/test_browser_thread_bundle.h"
#endif // defined(OS_IOS)
namespace metrics {
class NetworkMetricsProviderTest : public testing::Test {
public:
protected:
NetworkMetricsProviderTest()
#if defined(OS_IOS)
: scoped_task_environment_(
base::test::ScopedTaskEnvironment::MainThreadType::IO) {
base::test::ScopedTaskEnvironment::MainThreadType::IO)
#else // !defined(OS_IOS)
: test_browser_thread_bundle_(
content::TestBrowserThreadBundle::IO_MAINLOOP)
#endif // defined(OS_IOS)
{
#if defined(OS_CHROMEOS)
chromeos::DBusThreadManager::Initialize();
chromeos::NetworkHandler::Initialize();
......@@ -36,13 +48,17 @@ class NetworkMetricsProviderTest : public testing::Test {
}
private:
#if defined(OS_IOS)
base::test::ScopedTaskEnvironment scoped_task_environment_;
#else // !defined(OS_IOS)
content::TestBrowserThreadBundle test_browser_thread_bundle_;
#endif // defined(OS_IOS)
};
// Verifies that the effective connection type is correctly set.
TEST_F(NetworkMetricsProviderTest, EffectiveConnectionType) {
SystemProfileProto system_profile;
NetworkMetricsProvider network_metrics_provider;
NetworkMetricsProvider network_metrics_provider(
network::TestNetworkConnectionTracker::CreateAsyncGetter());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
......@@ -51,6 +67,7 @@ TEST_F(NetworkMetricsProviderTest, EffectiveConnectionType) {
network_metrics_provider.min_effective_connection_type_);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
network_metrics_provider.max_effective_connection_type_);
SystemProfileProto system_profile;
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
system_profile.network().min_effective_connection_type());
......@@ -99,8 +116,8 @@ TEST_F(NetworkMetricsProviderTest, EffectiveConnectionType) {
// Verifies that the effective connection type is not set to UNKNOWN when there
// is a change in the connection type.
TEST_F(NetworkMetricsProviderTest, ECTAmbiguousOnConnectionTypeChange) {
SystemProfileProto system_profile;
NetworkMetricsProvider network_metrics_provider;
NetworkMetricsProvider network_metrics_provider(
network::TestNetworkConnectionTracker::CreateAsyncGetter());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
......@@ -121,6 +138,7 @@ TEST_F(NetworkMetricsProviderTest, ECTAmbiguousOnConnectionTypeChange) {
// There is no change in the connection type. Effective connection types
// should be reported as 2G.
SystemProfileProto system_profile;
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_2G,
system_profile.network().min_effective_connection_type());
......@@ -129,8 +147,8 @@ TEST_F(NetworkMetricsProviderTest, ECTAmbiguousOnConnectionTypeChange) {
// Even with change in the connection type, effective connection types
// should be reported as 2G.
network_metrics_provider.OnNetworkChanged(
net::NetworkChangeNotifier::CONNECTION_2G);
network_metrics_provider.OnConnectionChanged(
network::mojom::ConnectionType::CONNECTION_2G);
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
EXPECT_EQ(SystemProfileProto::Network::EFFECTIVE_CONNECTION_TYPE_2G,
system_profile.network().min_effective_connection_type());
......@@ -144,13 +162,14 @@ TEST_F(NetworkMetricsProviderTest, ECTNotAmbiguousOnUnknownOrOffline) {
for (net::EffectiveConnectionType force_ect :
{net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
net::EFFECTIVE_CONNECTION_TYPE_OFFLINE}) {
NetworkMetricsProvider network_metrics_provider;
NetworkMetricsProvider network_metrics_provider(
network::TestNetworkConnectionTracker::CreateAsyncGetter());
base::RunLoop().RunUntilIdle();
SystemProfileProto system_profile;
network_metrics_provider.OnEffectiveConnectionTypeChanged(
net::EFFECTIVE_CONNECTION_TYPE_2G);
SystemProfileProto system_profile;
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
network_metrics_provider.OnEffectiveConnectionTypeChanged(force_ect);
......@@ -173,46 +192,49 @@ TEST_F(NetworkMetricsProviderTest, ECTNotAmbiguousOnUnknownOrOffline) {
// Verifies that the connection type is ambiguous boolean is correctly set.
TEST_F(NetworkMetricsProviderTest, ConnectionTypeIsAmbiguous) {
SystemProfileProto system_profile;
NetworkMetricsProvider network_metrics_provider;
NetworkMetricsProvider network_metrics_provider(
network::TestNetworkConnectionTracker::CreateAsyncGetter());
EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_UNKNOWN,
EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_UNKNOWN,
network_metrics_provider.connection_type_);
EXPECT_FALSE(network_metrics_provider.connection_type_is_ambiguous_);
EXPECT_FALSE(network_metrics_provider.network_change_notifier_initialized_);
EXPECT_FALSE(
network_metrics_provider.network_connection_tracker_initialized_);
// When a connection type change callback is received, network change notifier
// should be marked as initialized.
network_metrics_provider.OnNetworkChanged(
net::NetworkChangeNotifier::CONNECTION_2G);
EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_2G,
network_metrics_provider.OnConnectionChanged(
network::mojom::ConnectionType::CONNECTION_2G);
EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_2G,
network_metrics_provider.connection_type_);
// Connection type should not be marked as ambiguous when a delayed connection
// type change callback is received due to delayed initialization of the
// network change notifier.
EXPECT_FALSE(network_metrics_provider.connection_type_is_ambiguous_);
EXPECT_TRUE(network_metrics_provider.network_change_notifier_initialized_);
EXPECT_TRUE(network_metrics_provider.network_connection_tracker_initialized_);
// On collection of the system profile, |connection_type_is_ambiguous_| should
// stay false, and |network_change_notifier_initialized_| should remain true.
// stay false, and |network_connection_tracker_initialized_| should remain
// true.
SystemProfileProto system_profile;
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
EXPECT_FALSE(network_metrics_provider.connection_type_is_ambiguous_);
EXPECT_TRUE(network_metrics_provider.network_change_notifier_initialized_);
EXPECT_TRUE(network_metrics_provider.network_connection_tracker_initialized_);
EXPECT_FALSE(system_profile.network().connection_type_is_ambiguous());
EXPECT_EQ(SystemProfileProto::Network::CONNECTION_2G,
system_profile.network().connection_type());
network_metrics_provider.OnNetworkChanged(
net::NetworkChangeNotifier::CONNECTION_3G);
network_metrics_provider.OnConnectionChanged(
network::mojom::ConnectionType::CONNECTION_3G);
EXPECT_TRUE(network_metrics_provider.connection_type_is_ambiguous_);
EXPECT_TRUE(network_metrics_provider.network_change_notifier_initialized_);
EXPECT_TRUE(network_metrics_provider.network_connection_tracker_initialized_);
// On collection of the system profile, |connection_type_is_ambiguous_| should
// be reset to false, and |network_change_notifier_initialized_| should remain
// true.
// be reset to false, and |network_connection_tracker_initialized_| should
// remain true.
network_metrics_provider.ProvideSystemProfileMetrics(&system_profile);
EXPECT_FALSE(network_metrics_provider.connection_type_is_ambiguous_);
EXPECT_TRUE(network_metrics_provider.network_change_notifier_initialized_);
EXPECT_TRUE(network_metrics_provider.network_connection_tracker_initialized_);
EXPECT_TRUE(system_profile.network().connection_type_is_ambiguous());
EXPECT_EQ(SystemProfileProto::Network::CONNECTION_3G,
system_profile.network().connection_type());
......
......@@ -26,7 +26,6 @@
#include "net/log/net_log_util.h"
#include "services/network/network_service.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/network_connection_tracker.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/mojom/net_log.mojom.h"
#include "services/network/public/mojom/network_change_manager.mojom.h"
......@@ -210,11 +209,32 @@ network::NetworkConnectionTracker* GetNetworkConnectionTracker() {
void GetNetworkConnectionTrackerFromUIThread(
base::OnceCallback<void(network::NetworkConnectionTracker*)> callback) {
// TODO(fdoray): Investigate why this is needed. The IO thread is supposed to
// be initialized by the time the UI thread starts running tasks.
//
// GetNetworkConnectionTracker() will call CreateNetworkServiceOnIO(). Here it
// makes sure the IO thread is running when CreateNetworkServiceOnIO() is
// called.
if (!content::BrowserThread::IsThreadInitialized(
content::BrowserThread::IO)) {
// IO thread is not yet initialized. Try again in the next message pump.
bool task_posted = base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&GetNetworkConnectionTrackerFromUIThread,
std::move(callback)));
DCHECK(task_posted);
return;
}
base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
base::BindOnce(&GetNetworkConnectionTracker), std::move(callback));
}
network::NetworkConnectionTrackerAsyncGetter
CreateNetworkConnectionTrackerAsyncGetter() {
return base::BindRepeating(&content::GetNetworkConnectionTrackerFromUIThread);
}
void SetNetworkConnectionTrackerForTesting(
network::NetworkConnectionTracker* network_connection_tracker) {
if (g_network_connection_tracker != network_connection_tracker) {
......
......@@ -10,13 +10,13 @@
#include "base/callback.h"
#include "base/callback_list.h"
#include "content/common/content_export.h"
#include "services/network/public/cpp/network_connection_tracker.h"
namespace net {
class NetworkChangeNotifier;
} // namespace net
namespace network {
class NetworkConnectionTracker;
class NetworkService;
namespace mojom {
class NetworkService;
......@@ -79,6 +79,10 @@ CONTENT_EXPORT network::NetworkConnectionTracker* GetNetworkConnectionTracker();
CONTENT_EXPORT void GetNetworkConnectionTrackerFromUIThread(
base::OnceCallback<void(network::NetworkConnectionTracker*)> callback);
// Helper method to create a NetworkConnectionTrackerAsyncGetter.
CONTENT_EXPORT network::NetworkConnectionTrackerAsyncGetter
CreateNetworkConnectionTrackerAsyncGetter();
// Sets the NetworkConnectionTracker instance to use. For testing only.
// Must be called on the UI thread. Must be called before the first call to
// GetNetworkConnectionTracker.
......
......@@ -67,6 +67,16 @@
#error "This file requires ARC support."
#endif
namespace {
void GetNetworkConnectionTrackerAsync(
base::OnceCallback<void(network::NetworkConnectionTracker*)> callback) {
std::move(callback).Run(
GetApplicationContext()->GetNetworkConnectionTracker());
}
} // namespace
IOSChromeMetricsServiceClient::IOSChromeMetricsServiceClient(
metrics::MetricsStateManager* state_manager)
: metrics_state_manager_(state_manager),
......@@ -196,7 +206,8 @@ void IOSChromeMetricsServiceClient::Initialize() {
// Register metrics providers.
metrics_service_->RegisterMetricsProvider(
std::make_unique<metrics::NetworkMetricsProvider>());
std::make_unique<metrics::NetworkMetricsProvider>(
base::BindRepeating(&GetNetworkConnectionTrackerAsync)));
// Currently, we configure OmniboxMetricsProvider to not log events to UMA
// if there is a single incognito session visible. In the future, it may
......
......@@ -26,6 +26,10 @@ namespace network {
class NetworkConnectionTracker;
using NetworkConnectionTrackerGetter =
base::RepeatingCallback<NetworkConnectionTracker*()>;
// Defines the type of a callback that can be used to asynchronously get a
// NetworkConnectionTracker instance.
using NetworkConnectionTrackerAsyncGetter = base::RepeatingCallback<void(
base::OnceCallback<void(NetworkConnectionTracker*)>)>;
// This class subscribes to network change events from
// network::mojom::NetworkChangeManager and propogates these notifications to
......
......@@ -4,7 +4,10 @@
#include "services/network/test/test_network_connection_tracker.h"
#include <utility>
#include "base/bind.h"
#include "base/callback.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/threading/sequenced_task_runner_handle.h"
......@@ -15,6 +18,19 @@ static TestNetworkConnectionTracker* g_test_network_connection_tracker_instance;
namespace {
using NetworkConnectionTrackerCallback =
base::OnceCallback<void(NetworkConnectionTracker*)>;
void GetInstanceAsync(NetworkConnectionTrackerCallback callback) {
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(
[](NetworkConnectionTrackerCallback callback) {
std::move(callback).Run(g_test_network_connection_tracker_instance);
},
std::move(callback)));
}
NetworkConnectionTracker* GetNonTestInstance() {
return TestNetworkConnectionTracker::GetInstance();
}
......@@ -38,6 +54,12 @@ NetworkConnectionTrackerGetter TestNetworkConnectionTracker::CreateGetter() {
return base::BindRepeating(&GetNonTestInstance);
}
// static
NetworkConnectionTrackerAsyncGetter
TestNetworkConnectionTracker::CreateAsyncGetter() {
return base::BindRepeating(&GetInstanceAsync);
}
TestNetworkConnectionTracker::TestNetworkConnectionTracker() {
if (g_test_network_connection_tracker_instance) {
LOG(WARNING) << "Creating more than one TestNetworkConnectionTracker";
......
......@@ -29,6 +29,10 @@ class TestNetworkConnectionTracker : public NetworkConnectionTracker {
// TestNetworkConnectionTracker instance when called.
static NetworkConnectionTrackerGetter CreateGetter();
// Creates a NetworkConnectionTrackerGetter that will asynchronously return
// the active TestNetworkConnectionTracker instance.
static NetworkConnectionTrackerAsyncGetter CreateAsyncGetter();
~TestNetworkConnectionTracker() override;
bool GetConnectionType(network::mojom::ConnectionType* type,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment