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

Reland "Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker""

This is a reland of e00fddec

Needed to initialize variables in ChromeBrowserMainBrowserTest for asan/msan.
Verified this works with msan build.

Original change's description:
> Reland "Migrate ResourceRequestAllowedNotifier to NetworkConnectionTracker"
>
> This is a reland of a9ed46b7
>
> ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
> was flaky in the original change. Added logic in the test to wait for the
> connection type change.
>
> Only diffs from original are in chrome/browser/chrome_browser_main_browsertest.cc
>
> Original change's description:
> > 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/1180360
> > Reviewed-by: Eugene But <eugenebut@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> > Commit-Queue: Clark DuVall <cduvall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#584849}
>
> TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org
>
> Bug: 868021
> Change-Id: I5941b72474657159f0d4a1e6667fd77a3c475887
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Reviewed-on: https://chromium-review.googlesource.com/1185602
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585387}

TBR=eugenebut@chromium.org,rsesek@chromium.org,rmcelrath@chromium.org,jam@chromium.org

Bug: 868021, 876861
Change-Id: I46fccf072d0b3080603e97c73ff055ac7c45e723
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1187081
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: default avatarClark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585565}
parent f6d9ecb0
......@@ -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,44 +43,50 @@ 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);
};
class ChromeBrowserMainBrowserTest : public InProcessBrowserTest {
class ChromeBrowserMainBrowserTest
: public InProcessBrowserTest,
network::NetworkConnectionTracker::NetworkConnectionObserver {
public:
ChromeBrowserMainBrowserTest() = default;
~ChromeBrowserMainBrowserTest() override = default;
~ChromeBrowserMainBrowserTest() override {
content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(
this);
}
protected:
// InProcessBrowserTest:
......@@ -95,8 +108,35 @@ class ChromeBrowserMainBrowserTest : public InProcessBrowserTest {
chrome_browser_main_parts->AddParts(extra_parts_);
}
void SetUpOnMainThread() override {
content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
}
void WaitForConnectionType(network::mojom::ConnectionType type) {
if (connection_type_ == type)
return;
expected_connection_type_ = type;
run_loop_ = std::make_unique<base::RunLoop>();
run_loop_->Run();
}
ChromeBrowserMainExtraPartsNetFactoryInstaller* extra_parts_ = nullptr;
private:
// network::NetworkConnectionTracker::NetworkConnectionObserver:
void OnConnectionChanged(network::mojom::ConnectionType type) override {
connection_type_ = type;
if (expected_connection_type_ == connection_type_ && run_loop_)
run_loop_->Quit();
}
network::mojom::ConnectionType expected_connection_type_ =
network::mojom::ConnectionType::CONNECTION_UNKNOWN;
network::mojom::ConnectionType connection_type_ =
network::mojom::ConnectionType::CONNECTION_UNKNOWN;
std::unique_ptr<base::RunLoop> run_loop_;
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainBrowserTest);
};
......@@ -107,14 +147,11 @@ 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);
WaitForConnectionType(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