Commit 2a4e2ac1 authored by Robbie McElrath's avatar Robbie McElrath Committed by Commit Bot

Migrate download::NetworkStatusListenerImpl to NetworkConnectionTracker

This migrates NetworkStatusListenerImpl from using
net::NetworkChangeNotifier to content::NetworkConnectionTracker, which
works with the network service enabled.

Bug: 868011
Change-Id: I98d8644c128174b7b887c76dc40ad2725f052182
Reviewed-on: https://chromium-review.googlesource.com/1161200Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583835}
parent a4095e87
......@@ -28,6 +28,7 @@
#include "components/offline_pages/buildflags/buildflags.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/browser/storage_partition.h"
#if defined(OS_ANDROID)
......@@ -85,8 +86,8 @@ KeyedService* DownloadServiceFactory::BuildServiceInstanceFor(
content::BrowserThread::IO);
return download::BuildInMemoryDownloadService(
context, std::move(clients), base::FilePath(), blob_context_getter,
io_task_runner);
context, std::move(clients), content::GetNetworkConnectionTracker(),
base::FilePath(), blob_context_getter, io_task_runner);
} else {
// Build download service for normal profile.
base::FilePath storage_dir;
......@@ -106,9 +107,9 @@ KeyedService* DownloadServiceFactory::BuildServiceInstanceFor(
task_scheduler = std::make_unique<DownloadTaskSchedulerImpl>(context);
#endif
return download::BuildDownloadService(context, std::move(clients),
storage_dir, background_task_runner,
std::move(task_scheduler));
return download::BuildDownloadService(
context, std::move(clients), content::GetNetworkConnectionTracker(),
storage_dir, background_task_runner, std::move(task_scheduler));
}
}
......
......@@ -46,6 +46,7 @@ DownloadService* CreateDownloadServiceInternal(
std::unique_ptr<Store> store,
std::unique_ptr<TaskScheduler> task_scheduler,
std::unique_ptr<FileMonitor> file_monitor,
network::NetworkConnectionTracker* network_connection_tracker,
const base::FilePath& files_storage_dir) {
auto client_set = std::make_unique<ClientSet>(std::move(clients));
auto model = std::make_unique<ModelImpl>(std::move(store));
......@@ -60,7 +61,7 @@ DownloadService* CreateDownloadServiceInternal(
auto device_status_listener = std::make_unique<DeviceStatusListener>(
config->network_startup_delay, config->network_change_delay,
std::move(battery_listener));
std::move(battery_listener), network_connection_tracker);
NavigationMonitor* navigation_monitor =
NavigationMonitorFactory::GetForBrowserContext(browser_context);
auto scheduler = std::make_unique<SchedulerImpl>(
......@@ -81,6 +82,7 @@ DownloadService* CreateDownloadServiceInternal(
DownloadService* BuildDownloadService(
content::BrowserContext* browser_context,
std::unique_ptr<DownloadClientMap> clients,
network::NetworkConnectionTracker* network_connection_tracker,
const base::FilePath& storage_dir,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
std::unique_ptr<TaskScheduler> task_scheduler) {
......@@ -103,13 +105,14 @@ DownloadService* BuildDownloadService(
return CreateDownloadServiceInternal(
browser_context, std::move(clients), std::move(config), std::move(driver),
std::move(store), std::move(task_scheduler), std::move(file_monitor),
files_storage_dir);
network_connection_tracker, files_storage_dir);
}
// Create download service for incognito mode without any database or file IO.
DownloadService* BuildInMemoryDownloadService(
content::BrowserContext* browser_context,
std::unique_ptr<DownloadClientMap> clients,
network::NetworkConnectionTracker* network_connection_tracker,
const base::FilePath& storage_dir,
BlobTaskProxy::BlobContextGetter blob_context_getter,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
......@@ -134,7 +137,7 @@ DownloadService* BuildInMemoryDownloadService(
return CreateDownloadServiceInternal(
browser_context, std::move(clients), std::move(config), std::move(driver),
std::move(store), std::move(task_scheduler), std::move(file_monitor),
files_storage_dir);
network_connection_tracker, files_storage_dir);
}
} // namespace download
......@@ -18,6 +18,10 @@ namespace content {
class BrowserContext;
} // namespace content
namespace network {
class NetworkConnectionTracker;
}
namespace download {
class DownloadService;
......@@ -36,6 +40,7 @@ class TaskScheduler;
DownloadService* BuildDownloadService(
content::BrowserContext* browser_context,
std::unique_ptr<DownloadClientMap> clients,
network::NetworkConnectionTracker* network_connection_tracker,
const base::FilePath& storage_dir,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
std::unique_ptr<TaskScheduler> task_scheduler);
......@@ -45,6 +50,7 @@ DownloadService* BuildDownloadService(
DownloadService* BuildInMemoryDownloadService(
content::BrowserContext* browser_context,
std::unique_ptr<DownloadClientMap> clients,
network::NetworkConnectionTracker* network_connection_tracker,
const base::FilePath& storage_dir,
BlobTaskProxy::BlobContextGetter blob_context_getter,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
......
......@@ -18,7 +18,7 @@ void NetworkStatusListenerAndroid::NotifyNetworkChange(
const base::android::JavaRef<jobject>& jobj,
jint connectionType) {
DCHECK(observer_);
using ConnectionType = net::NetworkChangeNotifier::ConnectionType;
using ConnectionType = network::mojom::ConnectionType;
ConnectionType connection_type = static_cast<ConnectionType>(connectionType);
observer_->OnNetworkChanged(connection_type);
}
......@@ -39,13 +39,12 @@ void NetworkStatusListenerAndroid::Stop() {
base::android::AttachCurrentThread(), java_obj_);
}
net::NetworkChangeNotifier::ConnectionType
network::mojom::ConnectionType
NetworkStatusListenerAndroid::GetConnectionType() {
int connection_type =
Java_NetworkStatusListenerAndroid_getCurrentConnectionType(
base::android::AttachCurrentThread(), java_obj_);
return static_cast<net::NetworkChangeNotifier::ConnectionType>(
connection_type);
return static_cast<network::mojom::ConnectionType>(connection_type);
}
} // namespace download
......@@ -25,7 +25,7 @@ class NetworkStatusListenerAndroid : public NetworkStatusListener {
// NetworkStatusListener implementation.
void Start(NetworkStatusListener::Observer* observer) override;
void Stop() override;
net::NetworkChangeNotifier::ConnectionType GetConnectionType() override;
network::mojom::ConnectionType GetConnectionType() override;
void NotifyNetworkChange(JNIEnv* env,
const base::android::JavaRef<jobject>& jobj,
......
......@@ -23,18 +23,18 @@ BatteryStatus ToBatteryStatus(bool on_battery_power) {
}
// Converts a ConnectionType to NetworkStatus.
NetworkStatus ToNetworkStatus(net::NetworkChangeNotifier::ConnectionType type) {
NetworkStatus ToNetworkStatus(network::mojom::ConnectionType type) {
switch (type) {
case net::NetworkChangeNotifier::CONNECTION_ETHERNET:
case net::NetworkChangeNotifier::CONNECTION_WIFI:
case network::mojom::ConnectionType::CONNECTION_ETHERNET:
case network::mojom::ConnectionType::CONNECTION_WIFI:
return NetworkStatus::UNMETERED;
case net::NetworkChangeNotifier::CONNECTION_2G:
case net::NetworkChangeNotifier::CONNECTION_3G:
case net::NetworkChangeNotifier::CONNECTION_4G:
case network::mojom::ConnectionType::CONNECTION_2G:
case network::mojom::ConnectionType::CONNECTION_3G:
case network::mojom::ConnectionType::CONNECTION_4G:
return NetworkStatus::METERED;
case net::NetworkChangeNotifier::CONNECTION_UNKNOWN:
case net::NetworkChangeNotifier::CONNECTION_NONE:
case net::NetworkChangeNotifier::CONNECTION_BLUETOOTH:
case network::mojom::ConnectionType::CONNECTION_UNKNOWN:
case network::mojom::ConnectionType::CONNECTION_NONE:
case network::mojom::ConnectionType::CONNECTION_BLUETOOTH:
return NetworkStatus::DISCONNECTED;
}
NOTREACHED();
......@@ -97,13 +97,18 @@ void BatteryStatusListener::OnPowerStateChange(bool on_battery_power) {
DeviceStatusListener::DeviceStatusListener(
const base::TimeDelta& startup_delay,
const base::TimeDelta& online_delay,
std::unique_ptr<BatteryStatusListener> battery_listener)
std::unique_ptr<BatteryStatusListener> battery_listener,
network::NetworkConnectionTracker* network_connection_tracker)
: observer_(nullptr),
listening_(false),
is_valid_state_(false),
startup_delay_(startup_delay),
online_delay_(online_delay),
battery_listener_(std::move(battery_listener)) {}
#if !defined(OS_ANDROID)
network_connection_tracker_(network_connection_tracker),
#endif
battery_listener_(std::move(battery_listener)) {
}
DeviceStatusListener::~DeviceStatusListener() {
Stop();
......@@ -170,7 +175,7 @@ void DeviceStatusListener::Stop() {
}
void DeviceStatusListener::OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) {
network::mojom::ConnectionType type) {
pending_network_status_ = ToNetworkStatus(type);
if (pending_network_status_ == status_.network_status) {
......@@ -221,7 +226,8 @@ void DeviceStatusListener::BuildNetworkStatusListener() {
#if defined(OS_ANDROID)
network_listener_ = std::make_unique<NetworkStatusListenerAndroid>();
#else
network_listener_ = std::make_unique<NetworkStatusListenerImpl>();
network_listener_ =
std::make_unique<NetworkStatusListenerImpl>(network_connection_tracker_);
#endif
}
......
......@@ -9,9 +9,10 @@
#include "base/power_monitor/power_observer.h"
#include "base/timer/timer.h"
#include "build/build_config.h"
#include "components/download/internal/background_service/scheduler/device_status.h"
#include "components/download/internal/background_service/scheduler/network_status_listener.h"
#include "net/base/network_change_notifier.h"
#include "services/network/public/cpp/network_connection_tracker.h"
namespace download {
......@@ -73,10 +74,11 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
virtual void OnDeviceStatusChanged(const DeviceStatus& device_status) = 0;
};
explicit DeviceStatusListener(
DeviceStatusListener(
const base::TimeDelta& startup_delay,
const base::TimeDelta& online_delay,
std::unique_ptr<BatteryStatusListener> battery_listener);
std::unique_ptr<BatteryStatusListener> battery_listener,
network::NetworkConnectionTracker* network_connection_tracker);
~DeviceStatusListener() override;
bool is_valid_state() { return is_valid_state_; }
......@@ -94,6 +96,9 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
// Creates the instance of |network_listener_|, visible for testing.
virtual void BuildNetworkStatusListener();
// NetworkStatusListener::Observer implementation. Visible for testing.
void OnNetworkChanged(network::mojom::ConnectionType type) override;
// Used to listen to network connectivity changes.
std::unique_ptr<NetworkStatusListener> network_listener_;
......@@ -112,10 +117,6 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
// Start after a delay to wait for potential network stack setup.
void StartAfterDelay();
// NetworkStatusListener::Observer implementation.
void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) override;
// BatteryStatusListener::Observer implementation.
void OnPowerStateChange(bool on_battery_power) override;
......@@ -137,6 +138,11 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
// Pending network status used to update the current network status.
NetworkStatus pending_network_status_ = NetworkStatus::DISCONNECTED;
#if !defined(OS_ANDROID)
// Used to listen for network connection changes.
network::NetworkConnectionTracker* network_connection_tracker_;
#endif
// Used to listen to battery status.
std::unique_ptr<BatteryStatusListener> battery_listener_;
......
......@@ -10,6 +10,7 @@
#include "base/run_loop.h"
#include "base/test/power_monitor_test_base.h"
#include "components/download/internal/background_service/scheduler/network_status_listener.h"
#include "services/network/test/test_network_connection_tracker.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -17,7 +18,7 @@ using testing::_;
using testing::InSequence;
using ConnectionTypeObserver =
net::NetworkChangeNotifier::ConnectionTypeObserver;
using ConnectionType = net::NetworkChangeNotifier::ConnectionType;
using ConnectionType = network::mojom::ConnectionType;
namespace download {
namespace {
......@@ -30,29 +31,6 @@ MATCHER_P(BatteryStatusEqual, value, "") {
return arg.battery_status == value;
}
// NetworkChangeNotifier that can change network type in tests.
class TestNetworkChangeNotifier : public net::NetworkChangeNotifier {
public:
TestNetworkChangeNotifier()
: conn_type_(ConnectionType::CONNECTION_UNKNOWN) {}
// net::NetworkChangeNotifier implementation.
ConnectionType GetCurrentConnectionType() const override {
return conn_type_;
}
// Changes the network type.
void ChangeNetworkType(ConnectionType type) {
conn_type_ = type;
net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(type);
}
private:
ConnectionType conn_type_;
DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
};
class MockObserver : public DeviceStatusListener::Observer {
public:
MOCK_METHOD1(OnDeviceStatusChanged, void(const DeviceStatus&));
......@@ -79,18 +57,30 @@ class TestBatteryStatusListener : public BatteryStatusListener {
class TestDeviceStatusListener : public DeviceStatusListener {
public:
explicit TestDeviceStatusListener(
std::unique_ptr<TestBatteryStatusListener> battery_listener)
std::unique_ptr<TestBatteryStatusListener> battery_listener,
network::NetworkConnectionTracker* network_connection_tracker)
: DeviceStatusListener(base::TimeDelta(),
base::TimeDelta(),
std::move(battery_listener)) {}
std::move(battery_listener),
network_connection_tracker),
network_connection_tracker_(network_connection_tracker) {}
void BuildNetworkStatusListener() override {
network_listener_ = std::make_unique<NetworkStatusListenerImpl>();
network_listener_ = std::make_unique<NetworkStatusListenerImpl>(
network_connection_tracker_);
}
private:
network::NetworkConnectionTracker* network_connection_tracker_;
friend class DeviceStatusListenerTest;
};
class DeviceStatusListenerTest : public testing::Test {
public:
DeviceStatusListenerTest()
: network_connection_tracker_(true, ConnectionType::CONNECTION_UNKNOWN) {}
void SetUp() override {
auto power_source = std::make_unique<base::PowerMonitorTestSource>();
power_source_ = power_source.get();
......@@ -99,8 +89,8 @@ class DeviceStatusListenerTest : public testing::Test {
auto battery_listener = std::make_unique<TestBatteryStatusListener>();
test_battery_listener_ = battery_listener.get();
listener_ =
std::make_unique<TestDeviceStatusListener>(std::move(battery_listener));
listener_ = std::make_unique<TestDeviceStatusListener>(
std::move(battery_listener), &network_connection_tracker_);
}
void TearDown() override { listener_.reset(); }
......@@ -122,15 +112,14 @@ class DeviceStatusListenerTest : public testing::Test {
// Simulates a network change call, the event will be broadcasted
// asynchronously.
void ChangeNetworkType(ConnectionType type) {
test_network_notifier_.ChangeNetworkType(type);
network_connection_tracker_.SetConnectionType(type);
}
// Simulates a network change call, the event will be sent to client
// immediately.
void ChangeNetworkTypeImmediately(ConnectionType type) {
DCHECK(listener_);
static_cast<NetworkStatusListener::Observer*>(listener_.get())
->OnNetworkChanged(type);
listener_->OnNetworkChanged(type);
}
// Simulates a battery change call.
......@@ -148,7 +137,7 @@ class DeviceStatusListenerTest : public testing::Test {
// Needed for network change notifier and power monitor.
base::MessageLoop message_loop_;
TestNetworkChangeNotifier test_network_notifier_;
network::TestNetworkConnectionTracker network_connection_tracker_;
std::unique_ptr<base::PowerMonitor> power_monitor_;
base::PowerMonitorTestSource* power_source_;
TestBatteryStatusListener* test_battery_listener_;
......
......@@ -4,6 +4,8 @@
#include "components/download/internal/background_service/scheduler/network_status_listener.h"
#include "services/network/public/cpp/network_connection_tracker.h"
namespace download {
NetworkStatusListener::NetworkStatusListener() = default;
......@@ -16,28 +18,34 @@ void NetworkStatusListener::Stop() {
observer_ = nullptr;
}
NetworkStatusListenerImpl::NetworkStatusListenerImpl() = default;
NetworkStatusListenerImpl::NetworkStatusListenerImpl(
network::NetworkConnectionTracker* network_connection_tracker)
: network_connection_tracker_(network_connection_tracker),
connection_type_(network::mojom::ConnectionType::CONNECTION_UNKNOWN) {}
NetworkStatusListenerImpl::~NetworkStatusListenerImpl() = default;
void NetworkStatusListenerImpl::Start(
NetworkStatusListener::Observer* observer) {
NetworkStatusListener::Start(observer);
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
network_connection_tracker_->AddNetworkConnectionObserver(this);
network_connection_tracker_->GetConnectionType(
&connection_type_,
base::BindOnce(&NetworkStatusListenerImpl::OnConnectionChanged,
base::Unretained(this)));
}
void NetworkStatusListenerImpl::Stop() {
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
network_connection_tracker_->RemoveNetworkConnectionObserver(this);
NetworkStatusListener::Stop();
}
net::NetworkChangeNotifier::ConnectionType
NetworkStatusListenerImpl::GetConnectionType() {
return net::NetworkChangeNotifier::GetConnectionType();
network::mojom::ConnectionType NetworkStatusListenerImpl::GetConnectionType() {
return connection_type_;
}
void NetworkStatusListenerImpl::OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) {
void NetworkStatusListenerImpl::OnConnectionChanged(
network::mojom::ConnectionType type) {
DCHECK(observer_);
observer_->OnNetworkChanged(type);
}
......
......@@ -5,7 +5,7 @@
#ifndef COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_H_
#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_H_
#include "net/base/network_change_notifier.h"
#include "services/network/public/cpp/network_connection_tracker.h"
namespace download {
......@@ -18,8 +18,7 @@ class NetworkStatusListener {
// Observer to receive network connection type change notifications.
class Observer {
public:
virtual void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) = 0;
virtual void OnNetworkChanged(network::mojom::ConnectionType type) = 0;
protected:
virtual ~Observer() {}
......@@ -32,7 +31,7 @@ class NetworkStatusListener {
virtual void Stop();
// Gets the current connection type.
virtual net::NetworkChangeNotifier::ConnectionType GetConnectionType() = 0;
virtual network::mojom::ConnectionType GetConnectionType() = 0;
virtual ~NetworkStatusListener() {}
......@@ -43,31 +42,34 @@ class NetworkStatusListener {
Observer* observer_ = nullptr;
// The current network status.
net::NetworkChangeNotifier::ConnectionType network_status_ =
net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE;
network::mojom::ConnectionType network_status_ =
network::mojom::ConnectionType::CONNECTION_NONE;
private:
DISALLOW_COPY_AND_ASSIGN(NetworkStatusListener);
};
// Default implementation of NetworkStatusListener using
// net::NetworkChangeNotifier to listen to connectivity changes.
// NetworkConnectionTracker to listen to connectivity changes.
class NetworkStatusListenerImpl
: public net::NetworkChangeNotifier::NetworkChangeObserver,
: public network::NetworkConnectionTracker::NetworkConnectionObserver,
public NetworkStatusListener {
public:
NetworkStatusListenerImpl();
explicit NetworkStatusListenerImpl(
network::NetworkConnectionTracker* network_connection_tracker);
~NetworkStatusListenerImpl() override;
// NetworkStatusListener implementation.
void Start(NetworkStatusListener::Observer* observer) override;
void Stop() override;
net::NetworkChangeNotifier::ConnectionType GetConnectionType() override;
network::mojom::ConnectionType GetConnectionType() override;
private:
// net::NetworkChangeNotifier::NetworkChangeObserver implementation.
void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) override;
// network::NetworkConnectionTracker::NetworkConnectionObserver.
void OnConnectionChanged(network::mojom::ConnectionType type) override;
network::NetworkConnectionTracker* network_connection_tracker_;
network::mojom::ConnectionType connection_type_;
DISALLOW_COPY_AND_ASSIGN(NetworkStatusListenerImpl);
};
......
......@@ -43,6 +43,7 @@ source_set("test_support") {
deps = [
"//components/download/internal/background_service:internal",
"//services/network:test_support",
"//services/network/public/cpp",
]
}
......@@ -24,7 +24,11 @@ class FakeBatteryStatusListener : public BatteryStatusListener {
TestDeviceStatusListener::TestDeviceStatusListener()
: DeviceStatusListener(base::TimeDelta(), /* startup_delay */
base::TimeDelta(), /* online_delay */
std::make_unique<FakeBatteryStatusListener>()),
std::make_unique<FakeBatteryStatusListener>(),
&test_network_connection_tracker_),
test_network_connection_tracker_(
true,
network::mojom::ConnectionType::CONNECTION_UNKNOWN),
weak_ptr_factory_(this) {}
TestDeviceStatusListener::~TestDeviceStatusListener() {
......
......@@ -7,6 +7,7 @@
#include "base/memory/weak_ptr.h"
#include "components/download/internal/background_service/scheduler/device_status_listener.h"
#include "services/network/test/test_network_connection_tracker.h"
namespace download {
namespace test {
......@@ -32,6 +33,7 @@ class TestDeviceStatusListener : public DeviceStatusListener {
private:
void StartAfterDelay();
network::TestNetworkConnectionTracker test_network_connection_tracker_;
base::WeakPtrFactory<TestDeviceStatusListener> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(TestDeviceStatusListener);
......
......@@ -17,6 +17,7 @@
#include "content/public/browser/background_fetch_response.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/network/public/cpp/resource_request_body.h"
#include "ui/gfx/geometry/size.h"
......@@ -206,7 +207,8 @@ void LayoutTestBackgroundFetchDelegate::DownloadUrl(
download_service_ =
base::WrapUnique(download::BuildInMemoryDownloadService(
browser_context_, std::move(clients), base::FilePath(),
browser_context_, std::move(clients),
GetNetworkConnectionTracker(), base::FilePath(),
BrowserContext::GetBlobStorageContext(browser_context_),
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)));
}
......
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