Commit a9ed46b7 authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker

A getter is used for NetworkConnectionTracker because some services that
use ResourceRequestAllowedNotifier are initialized early in browser
startup (e.g. VariationsService), and only perform the initialization
of ResourceRequestAllowedNotifier later on the UI thread. The getter
allows us to run get the connection tracker at that point so we don't get
DCHECKs about being on the UI thread when running
content::GetNetworkConnectionTracker().

This also moves the NetworkConnectionTracker in ios/ from BrowserState to
ApplicationContext, which is available everywhere. It also matches non-IOS
usage more closely, since we have it as a global there.

Bug: 868021
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I130c6b47feb90f0f7f0776ccc65666414a1ae802
Reviewed-on: https://chromium-review.googlesource.com/1180360Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarRobbie McElrath <rmcelrath@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584849}
parent fff5908b
......@@ -14,8 +14,15 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "components/variations/service/variations_service.h"
#include "components/variations/variations_switches.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/service_manager_connection.h"
#include "content/public/common/service_names.mojom.h"
#include "content/public/test/browser_test_utils.h"
#include "net/base/mock_network_change_notifier.h"
#include "net/base/network_change_notifier_factory.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/mojom/network_service_test.mojom.h"
#include "services/service_manager/public/cpp/connector.h"
// Friend of ChromeBrowserMainPartsTestApi to poke at internal state.
class ChromeBrowserMainPartsTestApi {
......@@ -36,37 +43,38 @@ class ChromeBrowserMainPartsTestApi {
namespace {
// Simulates a network connection change.
void SimulateNetworkChange(network::mojom::ConnectionType type) {
if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
!content::IsNetworkServiceRunningInProcess()) {
network::mojom::NetworkServiceTestPtr network_service_test;
content::ServiceManagerConnection::GetForProcess()
->GetConnector()
->BindInterface(content::mojom::kNetworkServiceName,
&network_service_test);
base::RunLoop run_loop;
network_service_test->SimulateNetworkChange(type, run_loop.QuitClosure());
run_loop.Run();
return;
}
net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
net::NetworkChangeNotifier::ConnectionType(type));
}
// ChromeBrowserMainExtraParts used to install a MockNetworkChangeNotifier.
class ChromeBrowserMainExtraPartsNetFactoryInstaller
: public ChromeBrowserMainExtraParts {
public:
ChromeBrowserMainExtraPartsNetFactoryInstaller() = default;
~ChromeBrowserMainExtraPartsNetFactoryInstaller() override {
// |network_change_notifier_| needs to be destroyed before |net_installer_|.
network_change_notifier_.reset();
}
net::test::MockNetworkChangeNotifier* network_change_notifier() {
return network_change_notifier_.get();
}
// ChromeBrowserMainExtraParts:
void PreEarlyInitialization() override {}
void PostMainMessageLoopStart() override {
ASSERT_TRUE(net::NetworkChangeNotifier::HasNetworkChangeNotifier());
net_installer_ =
std::make_unique<net::NetworkChangeNotifier::DisableForTest>();
network_change_notifier_ =
std::make_unique<net::test::MockNetworkChangeNotifier>();
network_change_notifier_->SetConnectionType(
net::NetworkChangeNotifier::CONNECTION_NONE);
void ServiceManagerConnectionStarted(
content::ServiceManagerConnection* connection) override {
SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_NONE);
}
private:
std::unique_ptr<net::test::MockNetworkChangeNotifier>
network_change_notifier_;
std::unique_ptr<net::NetworkChangeNotifier::DisableForTest> net_installer_;
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsNetFactoryInstaller);
};
......@@ -107,14 +115,10 @@ IN_PROC_BROWSER_TEST_F(ChromeBrowserMainBrowserTest,
const int initial_request_count =
g_browser_process->variations_service()->request_count();
ASSERT_TRUE(extra_parts_);
extra_parts_->network_change_notifier()->SetConnectionType(
net::NetworkChangeNotifier::CONNECTION_WIFI);
net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
net::NetworkChangeNotifier::CONNECTION_WIFI);
SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_WIFI);
// NotifyObserversOfNetworkChangeForTests uses PostTask, so run the loop until
// idle to ensure VariationsService processes the network change.
base::RunLoop run_loop;
run_loop.RunUntilIdle();
base::RunLoop().RunUntilIdle();
const int final_request_count =
g_browser_process->variations_service()->request_count();
EXPECT_EQ(initial_request_count + 1, final_request_count);
......
......@@ -28,6 +28,7 @@
#include "components/variations/variations_associated_data.h"
#include "components/version_info/version_info.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#if defined(OS_ANDROID)
......@@ -240,7 +241,8 @@ ChromeMetricsServicesManagerClient::CreateVariationsService() {
return variations::VariationsService::Create(
std::make_unique<ChromeVariationsServiceClient>(), local_state_,
GetMetricsStateManager(), switches::kDisableBackgroundNetworking,
chrome_variations::CreateUIStringOverrider());
chrome_variations::CreateUIStringOverrider(),
base::BindOnce(&content::GetNetworkConnectionTracker));
}
std::unique_ptr<metrics::MetricsServiceClient>
......
......@@ -14,6 +14,7 @@
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/service_manager_connection.h"
#include "services/data_decoder/public/cpp/safe_json_parser.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
......@@ -89,7 +90,8 @@ PluginsResourceService::PluginsResourceService(PrefService* local_state)
base::Bind(data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()
->GetConnector()),
kPluginResourceServiceTrafficAnnotation) {}
kPluginResourceServiceTrafficAnnotation,
base::BindOnce(&content::GetNetworkConnectionTracker)) {}
void PluginsResourceService::Init() {
const base::DictionaryValue* metadata =
......
......@@ -474,7 +474,8 @@ class TranslateManagerRenderViewHostTest
// Clears the translate script so it is fetched every time and sets the
// expiration delay to a large value by default (in case it was zeroed in a
// previous test).
TranslateService::InitializeForTesting();
TranslateService::InitializeForTesting(
network::mojom::ConnectionType::CONNECTION_WIFI);
translate::TranslateDownloadManager* download_manager =
translate::TranslateDownloadManager::GetInstance();
download_manager->ClearTranslateScriptForTesting();
......
......@@ -19,6 +19,7 @@
#include "components/prefs/pref_service.h"
#include "components/translate/core/browser/translate_download_manager.h"
#include "components/translate/core/browser/translate_manager.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/url_constants.h"
#include "url/gurl.h"
......@@ -29,14 +30,15 @@
namespace {
// The singleton instance of TranslateService.
TranslateService* g_translate_service = NULL;
TranslateService* g_translate_service = nullptr;
}
TranslateService::TranslateService()
: resource_request_allowed_notifier_(
g_browser_process->local_state(),
switches::kDisableBackgroundNetworking) {
resource_request_allowed_notifier_.Init(this);
switches::kDisableBackgroundNetworking,
base::BindOnce(&content::GetNetworkConnectionTracker)) {
resource_request_allowed_notifier_.Init(this, true /* leaky */);
}
TranslateService::~TranslateService() {}
......@@ -75,14 +77,18 @@ void TranslateService::Shutdown(bool cleanup_pending_fetcher) {
}
// static
void TranslateService::InitializeForTesting() {
void TranslateService::InitializeForTesting(
network::mojom::ConnectionType type) {
if (!g_translate_service) {
TranslateService::Initialize();
translate::TranslateManager::SetIgnoreMissingKeyForTesting(true);
} else {
translate::TranslateDownloadManager::GetInstance()->ResetForTesting();
g_translate_service->OnResourceRequestsAllowed();
}
g_translate_service->resource_request_allowed_notifier_
.SetConnectionTypeForTesting(type);
g_translate_service->OnResourceRequestsAllowed();
}
// static
......
......@@ -27,7 +27,7 @@ class TranslateService
// Initializes the TranslateService in a way that it can be initialized
// multiple times in a unit test suite (once for each test). Should be paired
// with ShutdownForTesting at the end of the test.
static void InitializeForTesting();
static void InitializeForTesting(network::mojom::ConnectionType type);
// Shuts down the TranslateService at the end of a test in a way that the next
// test can initialize and use the service.
......
......@@ -10,6 +10,7 @@
#include "components/prefs/testing_pref_service.h"
#include "components/translate/core/browser/translate_download_manager.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
......@@ -49,7 +50,9 @@ TEST(TranslateServiceTest, CheckTranslatableURL) {
// Tests that download and history URLs are not translatable.
TEST(TranslateServiceTest, DownloadsAndHistoryNotTranslated) {
TranslateService::InitializeForTesting();
content::TestBrowserThreadBundle thread_bundle;
TranslateService::InitializeForTesting(
network::mojom::ConnectionType::CONNECTION_WIFI);
EXPECT_FALSE(
TranslateService::IsTranslatableURL(GURL(chrome::kChromeUIDownloadsURL)));
EXPECT_FALSE(
......
......@@ -419,12 +419,15 @@ std::unique_ptr<VariationsService> VariationsService::Create(
PrefService* local_state,
metrics::MetricsStateManager* state_manager,
const char* disable_network_switch,
const UIStringOverrider& ui_string_overrider) {
const UIStringOverrider& ui_string_overrider,
web_resource::ResourceRequestAllowedNotifier::NetworkConnectionTrackerGetter
network_connection_tracker_getter) {
std::unique_ptr<VariationsService> result;
result.reset(new VariationsService(
std::move(client),
std::make_unique<web_resource::ResourceRequestAllowedNotifier>(
local_state, disable_network_switch),
local_state, disable_network_switch,
std::move(network_connection_tracker_getter)),
local_state, state_manager, ui_string_overrider));
return result;
}
......@@ -478,8 +481,7 @@ void VariationsService::InitResourceRequestedAllowedNotifier() {
// ResourceRequestAllowedNotifier does not install an observer if there is no
// NetworkChangeNotifier, which results in never being notified of changes to
// network status.
DCHECK(net::NetworkChangeNotifier::HasNetworkChangeNotifier());
resource_request_allowed_notifier_->Init(this);
resource_request_allowed_notifier_->Init(this, false /* leaky */);
}
bool VariationsService::DoFetchFromURL(const GURL& url, bool is_http_retry) {
......
......@@ -147,7 +147,9 @@ class VariationsService
PrefService* local_state,
metrics::MetricsStateManager* state_manager,
const char* disable_network_switch,
const UIStringOverrider& ui_string_overrider);
const UIStringOverrider& ui_string_overrider,
web_resource::ResourceRequestAllowedNotifier::
NetworkConnectionTrackerGetter network_connection_tracker_getter);
// Enables fetching the seed for testing, even for unofficial builds. This
// should be used along with overriding |DoActualFetch| or using
......
......@@ -36,6 +36,7 @@ static_library("test_support") {
deps = [
":web_resource",
"//base",
"//services/network/public/cpp",
]
}
......
......@@ -10,25 +10,39 @@ namespace web_resource {
ResourceRequestAllowedNotifier::ResourceRequestAllowedNotifier(
PrefService* local_state,
const char* disable_network_switch)
const char* disable_network_switch,
NetworkConnectionTrackerGetter network_connection_tracker_getter)
: disable_network_switch_(disable_network_switch),
local_state_(local_state),
observer_requested_permission_(false),
waiting_for_user_to_accept_eula_(false),
observer_(nullptr) {
}
observer_(nullptr),
network_connection_tracker_getter_(
std::move(network_connection_tracker_getter)),
weak_factory_(this) {}
ResourceRequestAllowedNotifier::~ResourceRequestAllowedNotifier() {
if (observer_)
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
network_connection_tracker_->RemoveNetworkConnectionObserver(this);
}
void ResourceRequestAllowedNotifier::Init(Observer* observer) {
void ResourceRequestAllowedNotifier::Init(Observer* observer, bool leaky) {
DCHECK(!observer_);
DCHECK(observer);
observer_ = observer;
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
DCHECK(network_connection_tracker_getter_);
network_connection_tracker_ =
std::move(network_connection_tracker_getter_).Run();
if (leaky)
network_connection_tracker_->AddLeakyNetworkConnectionObserver(this);
else
network_connection_tracker_->AddNetworkConnectionObserver(this);
network_connection_tracker_->GetConnectionType(
&connection_type_,
base::BindOnce(&ResourceRequestAllowedNotifier::SetConnectionType,
weak_factory_.GetWeakPtr()));
eula_notifier_.reset(CreateEulaNotifier());
if (eula_notifier_) {
......@@ -48,8 +62,10 @@ ResourceRequestAllowedNotifier::GetResourceRequestsAllowedState() {
// The observer requested permission. Return the current criteria state and
// set a flag to remind this class to notify the observer once the criteria
// is met.
observer_requested_permission_ = waiting_for_user_to_accept_eula_ ||
net::NetworkChangeNotifier::IsOffline();
observer_requested_permission_ =
waiting_for_user_to_accept_eula_ ||
connection_type_ == network::mojom::ConnectionType::CONNECTION_NONE ||
!connection_initialized_;
if (!observer_requested_permission_)
return ALLOWED;
return waiting_for_user_to_accept_eula_ ? DISALLOWED_EULA_NOT_ACCEPTED :
......@@ -69,6 +85,11 @@ void ResourceRequestAllowedNotifier::SetObserverRequestedForTesting(
observer_requested_permission_ = requested;
}
void ResourceRequestAllowedNotifier::SetConnectionTypeForTesting(
network::mojom::ConnectionType type) {
SetConnectionType(type);
}
void ResourceRequestAllowedNotifier::MaybeNotifyObserver() {
// Need to ensure that all criteria are met before notifying observers.
if (observer_requested_permission_ && ResourceRequestsAllowed()) {
......@@ -93,9 +114,10 @@ void ResourceRequestAllowedNotifier::OnEulaAccepted() {
MaybeNotifyObserver();
}
void ResourceRequestAllowedNotifier::OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) {
if (type != net::NetworkChangeNotifier::CONNECTION_NONE) {
void ResourceRequestAllowedNotifier::OnConnectionChanged(
network::mojom::ConnectionType type) {
SetConnectionType(type);
if (type != network::mojom::ConnectionType::CONNECTION_NONE) {
DVLOG(1) << "Network came online.";
// MaybeNotifyObserver() internally guarantees that it will only notify the
// observer if it's currently waiting for the network to come online.
......@@ -103,4 +125,10 @@ void ResourceRequestAllowedNotifier::OnNetworkChanged(
}
}
void ResourceRequestAllowedNotifier::SetConnectionType(
network::mojom::ConnectionType connection_type) {
connection_initialized_ = true;
connection_type_ = connection_type;
}
} // namespace web_resource
......@@ -9,7 +9,7 @@
#include "base/macros.h"
#include "components/web_resource/eula_accepted_notifier.h"
#include "net/base/network_change_notifier.h"
#include "services/network/public/cpp/network_connection_tracker.h"
class PrefService;
......@@ -37,7 +37,7 @@ namespace web_resource {
// global instance.
class ResourceRequestAllowedNotifier
: public EulaAcceptedNotifier::Observer,
public net::NetworkChangeNotifier::NetworkChangeObserver {
public network::NetworkConnectionTracker::NetworkConnectionObserver {
public:
// Observes resource request allowed state changes.
class Observer {
......@@ -54,20 +54,26 @@ class ResourceRequestAllowedNotifier
DISALLOWED_COMMAND_LINE_DISABLED,
};
using NetworkConnectionTrackerGetter =
base::OnceCallback<network::NetworkConnectionTracker*()>;
// Creates a new ResourceRequestAllowedNotifier.
// |local_state| is the PrefService to observe.
// |disable_network_switch| is the command line switch to disable network
// activity. It is expected to outlive the ResourceRequestAllowedNotifier and
// may be null.
ResourceRequestAllowedNotifier(PrefService* local_state,
const char* disable_network_switch);
ResourceRequestAllowedNotifier(
PrefService* local_state,
const char* disable_network_switch,
NetworkConnectionTrackerGetter network_connection_tracker_getter);
~ResourceRequestAllowedNotifier() override;
// Sets |observer| as the service to be notified by this instance, and
// performs initial checks on the criteria. |observer| may not be null.
// This is to be called immediately after construction of an instance of
// ResourceRequestAllowedNotifier to pass it the interested service.
void Init(Observer* observer);
// ResourceRequestAllowedNotifier to pass it the interested service. Set
// |leaky| to true if this class will not be destructed before shutdown.
void Init(Observer* observer, bool leaky);
// Returns whether resource requests are allowed, per the various criteria.
// If not, this call will set some flags so it knows to notify the observer
......@@ -82,6 +88,8 @@ class ResourceRequestAllowedNotifier
void SetWaitingForEulaForTesting(bool waiting);
void SetObserverRequestedForTesting(bool requested);
void SetConnectionTypeForTesting(
network::mojom::ConnectionType connection_type);
protected:
// Notifies the observer if all criteria needed for resource requests are met.
......@@ -96,9 +104,10 @@ class ResourceRequestAllowedNotifier
// EulaAcceptedNotifier::Observer overrides:
void OnEulaAccepted() override;
// net::NetworkChangeNotifier::NetworkChangeObserver overrides:
void OnNetworkChanged(
net::NetworkChangeNotifier::ConnectionType type) override;
// network::NetworkConnectionTracker::NetworkConnectionObserver overrides:
void OnConnectionChanged(network::mojom::ConnectionType type) override;
void SetConnectionType(network::mojom::ConnectionType connection_type);
// Name of the command line switch to disable the network activity.
const char* disable_network_switch_;
......@@ -120,6 +129,14 @@ class ResourceRequestAllowedNotifier
// Observing service interested in request permissions.
Observer* observer_;
NetworkConnectionTrackerGetter network_connection_tracker_getter_;
network::NetworkConnectionTracker* network_connection_tracker_ = nullptr;
network::mojom::ConnectionType connection_type_ =
network::mojom::ConnectionType::CONNECTION_UNKNOWN;
bool connection_initialized_ = false;
base::WeakPtrFactory<ResourceRequestAllowedNotifier> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(ResourceRequestAllowedNotifier);
};
......
......@@ -6,11 +6,19 @@
namespace web_resource {
TestRequestAllowedNotifier::TestRequestAllowedNotifier(PrefService* local_state)
: ResourceRequestAllowedNotifier(local_state, nullptr),
TestRequestAllowedNotifier::TestRequestAllowedNotifier(
PrefService* local_state,
network::NetworkConnectionTracker* network_connection_tracker)
: ResourceRequestAllowedNotifier(
local_state,
nullptr,
base::BindOnce(
[](network::NetworkConnectionTracker* tracker) {
return tracker;
},
network_connection_tracker)),
override_requests_allowed_(false),
requests_allowed_(true) {
}
requests_allowed_(true) {}
TestRequestAllowedNotifier::~TestRequestAllowedNotifier() {
}
......@@ -19,7 +27,7 @@ void TestRequestAllowedNotifier::InitWithEulaAcceptNotifier(
Observer* observer,
std::unique_ptr<EulaAcceptedNotifier> eula_notifier) {
test_eula_notifier_.swap(eula_notifier);
Init(observer);
Init(observer, false /* leaky */);
}
void TestRequestAllowedNotifier::SetRequestsAllowedOverride(bool allowed) {
......
......@@ -25,7 +25,9 @@ namespace web_resource {
// it to return.
class TestRequestAllowedNotifier : public ResourceRequestAllowedNotifier {
public:
explicit TestRequestAllowedNotifier(PrefService* local_state);
TestRequestAllowedNotifier(
PrefService* local_state,
network::NetworkConnectionTracker* network_connection_tracker);
~TestRequestAllowedNotifier() override;
// A version of |Init()| that accepts a custom EulaAcceptedNotifier.
......
......@@ -44,10 +44,14 @@ WebResourceService::WebResourceService(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const char* disable_network_switch,
const ParseJSONCallback& parse_json_callback,
const net::NetworkTrafficAnnotationTag& traffic_annotation)
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ResourceRequestAllowedNotifier::NetworkConnectionTrackerGetter
network_connection_tracker_getter)
: prefs_(prefs),
resource_request_allowed_notifier_(
new ResourceRequestAllowedNotifier(prefs, disable_network_switch)),
resource_request_allowed_notifier_(new ResourceRequestAllowedNotifier(
prefs,
disable_network_switch,
std::move(network_connection_tracker_getter))),
fetch_scheduled_(false),
in_fetch_(false),
web_resource_server_(web_resource_server),
......@@ -59,7 +63,7 @@ WebResourceService::WebResourceService(
parse_json_callback_(parse_json_callback),
traffic_annotation_(traffic_annotation),
weak_ptr_factory_(this) {
resource_request_allowed_notifier_->Init(this);
resource_request_allowed_notifier_->Init(this, false /* leaky */);
DCHECK(prefs);
}
......@@ -114,7 +118,7 @@ void WebResourceService::ScheduleFetch(int64_t delay_ms) {
void WebResourceService::SetResourceRequestAllowedNotifier(
std::unique_ptr<ResourceRequestAllowedNotifier> notifier) {
resource_request_allowed_notifier_ = std::move(notifier);
resource_request_allowed_notifier_->Init(this);
resource_request_allowed_notifier_->Init(this, false /* leaky */);
}
bool WebResourceService::GetFetchScheduled() const {
......
......@@ -55,7 +55,9 @@ class WebResourceService : public ResourceRequestAllowedNotifier::Observer {
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const char* disable_network_switch,
const ParseJSONCallback& parse_json_callback,
const net::NetworkTrafficAnnotationTag& traffic_annotation);
const net::NetworkTrafficAnnotationTag& traffic_annotation,
ResourceRequestAllowedNotifier::NetworkConnectionTrackerGetter
network_connection_tracker_getter);
~WebResourceService() override;
......
......@@ -15,6 +15,7 @@
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_network_connection_tracker.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -29,9 +30,18 @@ namespace web_resource {
class TestResourceRequestAllowedNotifier
: public ResourceRequestAllowedNotifier {
public:
TestResourceRequestAllowedNotifier(PrefService* prefs,
const char* disable_network_switch)
: ResourceRequestAllowedNotifier(prefs, disable_network_switch) {}
TestResourceRequestAllowedNotifier(
PrefService* prefs,
const char* disable_network_switch,
network::NetworkConnectionTracker* network_connection_tracker)
: ResourceRequestAllowedNotifier(
prefs,
disable_network_switch,
base::BindOnce(
[](network::NetworkConnectionTracker* tracker) {
return tracker;
},
network_connection_tracker)) {}
ResourceRequestAllowedNotifier::State GetResourceRequestsAllowedState()
override {
......@@ -61,7 +71,8 @@ class TestWebResourceService : public WebResourceService {
int cache_update_delay_ms,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
const char* disable_network_switch,
const ParseJSONCallback& parse_json_callback)
const ParseJSONCallback& parse_json_callback,
network::NetworkConnectionTracker* network_connection_tracker)
: WebResourceService(prefs,
web_resource_server,
application_locale,
......@@ -71,14 +82,21 @@ class TestWebResourceService : public WebResourceService {
url_loader_factory,
disable_network_switch,
parse_json_callback,
TRAFFIC_ANNOTATION_FOR_TESTS){};
TRAFFIC_ANNOTATION_FOR_TESTS,
base::BindOnce(
[](network::NetworkConnectionTracker* tracker) {
return tracker;
},
network_connection_tracker)){};
void Unpack(const base::DictionaryValue& parsed_json) override {}
};
class WebResourceServiceTest : public testing::Test {
public:
WebResourceServiceTest() {}
WebResourceServiceTest()
: network_tracker_(true,
network::mojom::ConnectionType::CONNECTION_UNKNOWN) {}
void SetUp() override {
test_shared_loader_factory_ =
......@@ -89,10 +107,12 @@ class WebResourceServiceTest : public testing::Test {
test_web_resource_service_.reset(new TestWebResourceService(
local_state_.get(), GURL(kTestUrl), "", kCacheUpdatePath.c_str(), 100,
5000, test_shared_loader_factory_, nullptr,
base::Bind(web_resource::WebResourceServiceTest::Parse)));
base::BindRepeating(web_resource::WebResourceServiceTest::Parse),
&network_tracker_));
error_message_ = "";
TestResourceRequestAllowedNotifier* notifier =
new TestResourceRequestAllowedNotifier(local_state_.get(), nullptr);
new TestResourceRequestAllowedNotifier(local_state_.get(), nullptr,
&network_tracker_);
notifier->SetState(ResourceRequestAllowedNotifier::ALLOWED);
test_web_resource_service_->SetResourceRequestAllowedNotifier(
std::unique_ptr<ResourceRequestAllowedNotifier>(notifier));
......@@ -132,6 +152,7 @@ class WebResourceServiceTest : public testing::Test {
network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
std::unique_ptr<TestingPrefServiceSimple> local_state_;
network::TestNetworkConnectionTracker network_tracker_;
std::unique_ptr<TestWebResourceService> test_web_resource_service_;
};
......
......@@ -210,6 +210,7 @@ source_set("browser_impl") {
"//ios/web/public/app",
"//net",
"//rlz/buildflags",
"//services/network:network_service",
"//ui/base",
]
......
......@@ -106,6 +106,7 @@ include_rules = [
"+rlz/buildflags",
"+services/identity/public",
"+services/metrics/public",
"+services/network/network_change_manager.h",
"+services/network/public/mojom",
"+services/network/public/cpp",
"+services/service_manager/public",
......
......@@ -39,6 +39,7 @@ class ChromeNetLog;
}
namespace network {
class NetworkConnectionTracker;
class SharedURLLoaderFactory;
namespace mojom {
class NetworkContext;
......@@ -139,6 +140,9 @@ class ApplicationContext {
virtual component_updater::ComponentUpdateService*
GetComponentUpdateService() = 0;
// Returns the NetworkConnectionTracker instance for this ApplicationContext.
virtual network::NetworkConnectionTracker* GetNetworkConnectionTracker() = 0;
protected:
// Sets the global ApplicationContext instance.
static void SetApplicationContext(ApplicationContext* context);
......
......@@ -55,6 +55,8 @@
#include "net/socket/client_socket_pool_manager.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/network/network_change_manager.h"
#include "services/network/public/cpp/network_connection_tracker.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
namespace {
......@@ -79,6 +81,13 @@ void RequestProxyResolvingSocketFactory(
app_context, std::move(request)));
}
// Passed to NetworkConnectionTracker to bind a NetworkChangeManagerRequest.
void BindNetworkChangeManagerRequest(
network::NetworkChangeManager* network_change_manager,
network::mojom::NetworkChangeManagerRequest request) {
network_change_manager->AddRequest(std::move(request));
}
} // namespace
ApplicationContextImpl::ApplicationContextImpl(
......@@ -345,6 +354,21 @@ ApplicationContextImpl::GetComponentUpdateService() {
return component_updater_.get();
}
network::NetworkConnectionTracker*
ApplicationContextImpl::GetNetworkConnectionTracker() {
if (!network_connection_tracker_) {
if (!network_change_manager_) {
network_change_manager_ =
std::make_unique<network::NetworkChangeManager>(nullptr);
}
network_connection_tracker_ =
std::make_unique<network::NetworkConnectionTracker>(base::BindRepeating(
&BindNetworkChangeManagerRequest,
base::Unretained(network_change_manager_.get())));
}
return network_connection_tracker_.get();
}
void ApplicationContextImpl::SetApplicationLocale(const std::string& locale) {
DCHECK(thread_checker_.CalledOnValidThread());
application_locale_ = locale;
......
......@@ -21,6 +21,7 @@ class SequencedTaskRunner;
}
namespace network {
class NetworkChangeManager;
class WeakWrapperSharedURLLoaderFactory;
}
......@@ -68,6 +69,7 @@ class ApplicationContextImpl : public ApplicationContext {
gcm::GCMDriver* GetGCMDriver() override;
component_updater::ComponentUpdateService* GetComponentUpdateService()
override;
network::NetworkConnectionTracker* GetNetworkConnectionTracker() override;
private:
// Sets the locale used by the application.
......@@ -102,6 +104,10 @@ class ApplicationContextImpl : public ApplicationContext {
// Created on the UI thread, destroyed on the IO thread.
std::unique_ptr<web::NetworkContextOwner> network_context_owner_;
std::unique_ptr<network::NetworkChangeManager> network_change_manager_;
std::unique_ptr<network::NetworkConnectionTracker>
network_connection_tracker_;
bool was_last_shutdown_clean_;
DISALLOW_COPY_AND_ASSIGN(ApplicationContextImpl);
......
......@@ -17,6 +17,7 @@ source_set("google") {
"//components/google/core/browser",
"//components/keyed_service/ios",
"//components/prefs",
"//ios/chrome/browser",
"//ios/chrome/browser/browser_state",
"//ios/public/provider/chrome/browser",
"//ios/public/provider/chrome/browser/distribution",
......
......@@ -10,6 +10,7 @@
#include "components/google/core/browser/google_url_tracker.h"
#include "components/keyed_service/ios/browser_state_dependency_manager.h"
#include "components/prefs/pref_service.h"
#include "ios/chrome/browser/application_context.h"
#include "ios/chrome/browser/browser_state/browser_state_otr_helper.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/google/google_url_tracker_client_impl.h"
......@@ -49,7 +50,8 @@ std::unique_ptr<KeyedService> GoogleURLTrackerFactory::BuildServiceInstanceFor(
return std::make_unique<GoogleURLTracker>(
base::WrapUnique(new GoogleURLTrackerClientImpl(browser_state)),
GoogleURLTracker::NORMAL_MODE, context->GetNetworkConnectionTracker());
GoogleURLTracker::NORMAL_MODE,
GetApplicationContext()->GetNetworkConnectionTracker());
}
web::BrowserState* GoogleURLTrackerFactory::GetBrowserStateToUse(
......
......@@ -76,7 +76,9 @@ IOSChromeMetricsServicesManagerClient::CreateVariationsService() {
return variations::VariationsService::Create(
std::make_unique<IOSChromeVariationsServiceClient>(), local_state_,
GetMetricsStateManager(), "dummy-disable-background-switch",
::CreateUIStringOverrider());
::CreateUIStringOverrider(),
base::BindOnce(&ApplicationContext::GetNetworkConnectionTracker,
base::Unretained(GetApplicationContext())));
}
std::unique_ptr<metrics::MetricsServiceClient>
......
......@@ -19,8 +19,10 @@ TranslateServiceIOS* g_translate_service = nullptr;
TranslateServiceIOS::TranslateServiceIOS()
: resource_request_allowed_notifier_(
GetApplicationContext()->GetLocalState(),
nullptr) {
resource_request_allowed_notifier_.Init(this);
nullptr,
base::BindOnce(&ApplicationContext::GetNetworkConnectionTracker,
base::Unretained(GetApplicationContext()))) {
resource_request_allowed_notifier_.Init(this, false /* leaky */);
}
TranslateServiceIOS::~TranslateServiceIOS() {
......
......@@ -59,6 +59,7 @@ class TestingApplicationContext : public ApplicationContext {
gcm::GCMDriver* GetGCMDriver() override;
component_updater::ComponentUpdateService* GetComponentUpdateService()
override;
network::NetworkConnectionTracker* GetNetworkConnectionTracker() override;
private:
base::ThreadChecker thread_checker_;
......
......@@ -179,3 +179,9 @@ TestingApplicationContext::GetComponentUpdateService() {
DCHECK(thread_checker_.CalledOnValidThread());
return nullptr;
}
network::NetworkConnectionTracker*
TestingApplicationContext::GetNetworkConnectionTracker() {
DCHECK(thread_checker_.CalledOnValidThread());
return nullptr;
}
......@@ -5,7 +5,6 @@ include_rules = [
"+ios/web",
"+mojo/public",
"+net",
"+services/network/network_change_manager.h",
"+services/network/network_context.h",
"+services/network/public/cpp",
"+services/network/public/mojom",
......
......@@ -23,9 +23,7 @@
#include "mojo/public/cpp/bindings/interface_request.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_context_getter_observer.h"
#include "services/network/network_change_manager.h"
#include "services/network/network_context.h"
#include "services/network/public/cpp/network_connection_tracker.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/service_manager/public/mojom/service.mojom.h"
......@@ -109,13 +107,6 @@ class BrowserStateServiceManagerConnectionHolder
DISALLOW_COPY_AND_ASSIGN(BrowserStateServiceManagerConnectionHolder);
};
// Passed to NetworkConnectionTracker to bind a NetworkChangeManagerRequest.
void BindNetworkChangeManagerRequest(
network::NetworkChangeManager* network_change_manager,
network::mojom::NetworkChangeManagerRequest request) {
network_change_manager->AddRequest(std::move(request));
}
} // namespace
// static
......@@ -198,19 +189,6 @@ network::mojom::CookieManager* BrowserState::GetCookieManager() {
return cookie_manager_.get();
}
network::NetworkConnectionTracker* BrowserState::GetNetworkConnectionTracker() {
if (!network_connection_tracker_) {
DCHECK(!network_change_manager_);
network_change_manager_ =
std::make_unique<network::NetworkChangeManager>(nullptr);
network_connection_tracker_ =
std::make_unique<network::NetworkConnectionTracker>(base::BindRepeating(
&BindNetworkChangeManagerRequest,
base::Unretained(network_change_manager_.get())));
}
return network_connection_tracker_.get();
}
void BrowserState::GetProxyResolvingSocketFactory(
network::mojom::ProxyResolvingSocketFactoryRequest request) {
CreateNetworkContextIfNecessary();
......
......@@ -23,8 +23,6 @@ class URLRequestContextGetter;
}
namespace network {
class NetworkChangeManager;
class NetworkConnectionTracker;
class SharedURLLoaderFactory;
class WeakWrapperSharedURLLoaderFactory;
} // namespace network
......@@ -70,9 +68,6 @@ class BrowserState : public base::SupportsUserData {
// Returns a CookieManager that is backed by GetRequestContext.
network::mojom::CookieManager* GetCookieManager();
// Returns the NetworkConnectionTracker instance for this BrowserState.
network::NetworkConnectionTracker* GetNetworkConnectionTracker();
// Binds a ProxyResolvingSocketFactory request to NetworkContext.
void GetProxyResolvingSocketFactory(
network::mojom::ProxyResolvingSocketFactoryRequest request);
......@@ -134,12 +129,6 @@ class BrowserState : public base::SupportsUserData {
shared_url_loader_factory_;
network::mojom::NetworkContextPtr network_context_;
// Acts as a proxy between the NetworkChangeNotifier and
// NetworkConnectionTracker.
std::unique_ptr<network::NetworkChangeManager> network_change_manager_;
std::unique_ptr<network::NetworkConnectionTracker>
network_connection_tracker_;
// Owns the network::NetworkContext that backs |url_loader_factory_|. Created
// on the UI thread, destroyed on the IO thread.
std::unique_ptr<NetworkContextOwner> network_context_owner_;
......
......@@ -246,6 +246,7 @@ ios_web_view_deps = [
"//components/language/core/browser",
"//components/language/core/common",
"//components/net_log",
"//services/network:network_service",
"//components/password_manager/core/browser",
"//components/password_manager/core/browser/form_parsing:form_parsing",
"//components/password_manager/core/common",
......
......@@ -34,6 +34,7 @@ include_rules = [
"+ios/web_view",
"+net",
"+services/identity/public/cpp",
"+services/network/network_change_manager.h",
"+services/network/public/cpp",
"+services/network/public/mojom",
"+third_party/ocmock",
......
......@@ -20,10 +20,22 @@
#include "ios/web_view/cwv_web_view_features.h"
#include "ios/web_view/internal/app/web_view_io_thread.h"
#include "net/socket/client_socket_pool_manager.h"
#include "services/network/network_change_manager.h"
#include "services/network/public/cpp/network_connection_tracker.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "ui/base/l10n/l10n_util_mac.h"
namespace ios_web_view {
namespace {
// Passed to NetworkConnectionTracker to bind a NetworkChangeManagerRequest.
void BindNetworkChangeManagerRequest(
network::NetworkChangeManager* network_change_manager,
network::mojom::NetworkChangeManagerRequest request) {
network_change_manager->AddRequest(std::move(request));
}
} // namespace
ApplicationContext* ApplicationContext::GetInstance() {
return base::Singleton<ApplicationContext>::get();
......@@ -137,6 +149,21 @@ network::mojom::NetworkContext* ApplicationContext::GetSystemNetworkContext() {
return network_context_.get();
}
network::NetworkConnectionTracker*
ApplicationContext::GetNetworkConnectionTracker() {
if (!network_connection_tracker_) {
if (!network_change_manager_) {
network_change_manager_ =
std::make_unique<network::NetworkChangeManager>(nullptr);
}
network_connection_tracker_ =
std::make_unique<network::NetworkConnectionTracker>(base::BindRepeating(
&BindNetworkChangeManagerRequest,
base::Unretained(network_change_manager_.get())));
}
return network_connection_tracker_.get();
}
const std::string& ApplicationContext::GetApplicationLocale() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!application_locale_.empty());
......
......@@ -23,6 +23,8 @@ class URLRequestContextGetter;
}
namespace network {
class NetworkChangeManager;
class NetworkConnectionTracker;
class SharedURLLoaderFactory;
class WeakWrapperSharedURLLoaderFactory;
namespace mojom {
......@@ -54,6 +56,9 @@ class ApplicationContext {
scoped_refptr<network::SharedURLLoaderFactory> GetSharedURLLoaderFactory();
network::mojom::NetworkContext* GetSystemNetworkContext();
// Returns the NetworkConnectionTracker instance for this ApplicationContext.
network::NetworkConnectionTracker* GetNetworkConnectionTracker();
// Gets the locale used by the application.
const std::string& GetApplicationLocale();
......@@ -98,6 +103,10 @@ class ApplicationContext {
// Created on the UI thread, destroyed on the IO thread.
std::unique_ptr<web::NetworkContextOwner> network_context_owner_;
std::unique_ptr<network::NetworkChangeManager> network_change_manager_;
std::unique_ptr<network::NetworkConnectionTracker>
network_connection_tracker_;
DISALLOW_COPY_AND_ASSIGN(ApplicationContext);
};
......
......@@ -15,8 +15,10 @@ WebViewTranslateService::TranslateRequestsAllowedListener::
TranslateRequestsAllowedListener()
: resource_request_allowed_notifier_(
ios_web_view::ApplicationContext::GetInstance()->GetLocalState(),
/*disable_network_switch=*/nullptr) {
resource_request_allowed_notifier_.Init(this);
/*disable_network_switch=*/nullptr,
base::BindOnce(&ApplicationContext::GetNetworkConnectionTracker,
base::Unretained(ApplicationContext::GetInstance()))) {
resource_request_allowed_notifier_.Init(this, /*leaky=*/false);
}
WebViewTranslateService::TranslateRequestsAllowedListener::
......
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