Skip to content

Commit

Permalink
Remove kNetworkService usages in chrome/
Browse files Browse the repository at this point in the history
Now that the Network Service is launched on stable, we can remove the
non-NS path.

Bug: 934009
Change-Id: I588236e9da61b5b6590de207bc9721822d92453b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662638
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670581}
  • Loading branch information
robbiemc authored and Commit Bot committed Jun 19, 2019
1 parent 79aa369 commit f6c3f96
Show file tree
Hide file tree
Showing 25 changed files with 68 additions and 241 deletions.
4 changes: 0 additions & 4 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@
#include "printing/buildflags/buildflags.h"
#include "services/image_annotation/public/mojom/constants.mojom.h"
#include "services/image_annotation/public/mojom/image_annotation.mojom.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/cpp/resource_request.h"
Expand Down Expand Up @@ -5015,9 +5014,6 @@ void ChromeContentBrowserClient::WillCreateWebSocket(

void ChromeContentBrowserClient::OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;

PrefService* local_state;
if (g_browser_process) {
DCHECK(g_browser_process->local_state());
Expand Down
21 changes: 3 additions & 18 deletions chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "services/network/public/cpp/features.h"

using content::BrowserThread;

Expand Down Expand Up @@ -67,11 +66,6 @@ void ChromeDataUseMeasurement::CreateInstance(PrefService* local_state) {

DCHECK(!g_chrome_data_use_measurement);

// Do not create when NetworkService is disabled, since data use of URLLoader
// is reported via the network delegate callbacks.
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;

g_chrome_data_use_measurement = new ChromeDataUseMeasurement(
content::GetNetworkConnectionTracker(), local_state);
}
Expand Down Expand Up @@ -105,7 +99,6 @@ void ChromeDataUseMeasurement::ReportNetworkServiceDataUse(
int64_t recv_bytes,
int64_t sent_bytes) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
// Negative byte numbres is not a critical problem (i.e. should have no security implications) but
// is not expected. TODO(rajendrant): remove these DCHECKs or consider using uint in Mojo instead.
DCHECK_GE(recv_bytes, 0);
Expand Down Expand Up @@ -160,19 +153,11 @@ void ChromeDataUseMeasurement::UpdateMetricsUsagePrefs(
int64_t total_bytes,
bool is_cellular,
bool is_metrics_service_usage) {
PrefService* local_state;
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
local_state = local_state_;
} else {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
local_state = g_browser_process->local_state();
}
DCHECK(local_state);

DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(local_state_);
metrics::DataUseTracker::UpdateMetricsUsagePrefs(
base::saturated_cast<int>(total_bytes), is_cellular,
is_metrics_service_usage, local_state);
is_metrics_service_usage, local_state_);
}

} // namespace data_use_measurement
19 changes: 8 additions & 11 deletions chrome/browser/download/chrome_download_manager_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
#if BUILDFLAG(ENABLE_OFFLINE_PAGES)
#include "chrome/browser/offline_pages/offline_page_utils.h"
#include "components/offline_pages/core/client_namespace_constants.h"
#include "services/network/public/cpp/features.h"
#endif

using content::BrowserThread;
Expand Down Expand Up @@ -584,16 +583,14 @@ bool ChromeDownloadManagerDelegate::InterceptDownloadIfApplicable(
int64_t content_length,
content::WebContents* web_contents) {
#if BUILDFLAG(ENABLE_OFFLINE_PAGES)
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (offline_pages::OfflinePageUtils::CanDownloadAsOfflinePage(url,
mime_type)) {
offline_pages::OfflinePageUtils::ScheduleDownload(
web_contents, offline_pages::kDownloadNamespace, url,
offline_pages::OfflinePageUtils::DownloadUIActionFlags::ALL,
request_origin);
return true;
}
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (offline_pages::OfflinePageUtils::CanDownloadAsOfflinePage(url,
mime_type)) {
offline_pages::OfflinePageUtils::ScheduleDownload(
web_contents, offline_pages::kDownloadNamespace, url,
offline_pages::OfflinePageUtils::DownloadUIActionFlags::ALL,
request_origin);
return true;
}
#endif
return false;
Expand Down
8 changes: 0 additions & 8 deletions chrome/browser/extensions/active_tab_permission_granter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/user_script.h"
#include "services/network/public/cpp/features.h"
#include "url/gurl.h"

namespace extensions {
Expand Down Expand Up @@ -97,13 +96,6 @@ bool ShouldGrantActiveTabOrPrompt(const Extension* extension,

void UpdateTabSpecificCorsOriginAccessLists(const ExtensionId& extension_id,
ProcessManager* process_manager) {
// TODO(crbug.com/736308): In OOR-CORS mode, activeTab permissions are
// supported only when NetworkService is enabled. Revisit here if the legacy
// path needs to support it. Even so, probably we won't have per-factory
// lists, but share the per-profile lists in the legacy path.
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;

const std::set<content::RenderFrameHost*>& extension_hosts =
process_manager->GetRenderFrameHostsForExtension(extension_id);
for (auto* host : extension_hosts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include "base/feature_list.h"
#include "base/task/post_task.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/extension_function_test_utils.h"
#include "chrome/browser/extensions/extension_service_test_base.h"
Expand All @@ -30,7 +29,6 @@
#include "content/public/test/web_contents_tester.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_test_util.h"
#include "services/network/public/cpp/features.h"

namespace extensions {

Expand Down Expand Up @@ -82,16 +80,11 @@ class SafeBrowsingPrivateApiUnitTest : public ExtensionServiceTestBase {

std::unique_ptr<TestBrowserWindow> browser_window_;
std::unique_ptr<Browser> browser_;
base::test::ScopedFeatureList feature_list_;

DISALLOW_COPY_AND_ASSIGN(SafeBrowsingPrivateApiUnitTest);
};

void SafeBrowsingPrivateApiUnitTest::SetUp() {
// Need to use the network service path because we've removed the
// URLRequestContext path in src/chrome and unit_tests currently are running
// with network service disabled. https://crbug.com/966633
feature_list_.InitAndEnableFeature(network::features::kNetworkService);
ExtensionServiceTestBase::SetUp();
InitializeEmptyExtensionService();
content::BrowserSideNavigationSetUp();
Expand Down
40 changes: 5 additions & 35 deletions chrome/browser/extensions/api/web_request/web_request_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_filter.h"
#include "net/url_request/url_request_interceptor.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/test/test_url_loader_client.h"
Expand Down Expand Up @@ -1455,14 +1454,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebSocketRequestAuthRequired) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(StartWebSocketServer(net::GetWebSocketTestDataDirectory(), true));
// Test expectations differ with the Network Service because of the way
// mojo onError is handled.
const char* network_service_arg =
base::FeatureList::IsEnabled(network::features::kNetworkService)
? "NetworkServiceEnabled"
: "NetworkServiceDisabled";
ASSERT_TRUE(RunExtensionSubtestWithArg(
"webrequest", "test_websocket_auth.html", network_service_arg))
ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_websocket_auth.html"))
<< message_;
}

Expand All @@ -1480,9 +1472,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebSocketRequestOnWorker) {
// http://crbug.com/878574.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebSocketConnectionErrorBeforeChannelRequest) {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;

InstallWebRequestExtension("extension");

network::mojom::WebSocketPtr web_socket;
Expand Down Expand Up @@ -1746,11 +1735,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, InitiatorAccessRequired) {

IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestApiClearsBindingOnFirstListener) {
// Skip if network service is disabled since the proxy is not used. Also skip
// if the proxy is forced since the bindings will never be cleared in that
// case.
if (!base::FeatureList::IsEnabled(network::features::kNetworkService) ||
base::FeatureList::IsEnabled(
// Skip if the proxy is forced since the bindings will never be cleared in
// that case.
if (base::FeatureList::IsEnabled(
extensions_features::kForceWebRequestProxyForTest)) {
return;
}
Expand Down Expand Up @@ -1779,9 +1766,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
// Regression test for http://crbug.com/878366.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestApiDoesNotCrashOnErrorAfterProfileDestroyed) {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;

ASSERT_TRUE(StartEmbeddedTestServer());

// Create a profile that will be destroyed later.
Expand Down Expand Up @@ -2052,21 +2036,7 @@ IN_PROC_BROWSER_TEST_F(LocalNTPInterceptionWebRequestAPITest,

// Ensure that devtools frontend requests are hidden from the webRequest API.
IN_PROC_BROWSER_TEST_F(DevToolsFrontendInWebRequestApiTest, HiddenRequests) {
// Test expectations differ with the Network Service because of the way
// request interception is done for the test. In the legacy networking path a
// URLRequestMockHTTPJob is used, which does not generate
// |onBeforeHeadersSent| events. With the Network Service enabled, requests
// issued to HTTP URLs by these tests look like real HTTP requests and
// therefore do generate |onBeforeHeadersSent| events.
//
// These tests adjust their expectations accordingly based on whether or not
// the Network Service is enabled.
const char* network_service_arg =
base::FeatureList::IsEnabled(network::features::kNetworkService)
? "NetworkServiceEnabled"
: "NetworkServiceDisabled";
ASSERT_TRUE(RunExtensionSubtestWithArg("webrequest", "test_devtools.html",
network_service_arg))
ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_devtools.html"))
<< message_;
}

Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/extensions/permissions_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#include "extensions/common/manifest_handlers/permissions_parser.h"
#include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h"
#include "services/network/public/cpp/features.h"

using content::RenderProcessHost;
using extensions::permissions_api_helpers::PackPermissionSet;
Expand Down Expand Up @@ -181,10 +180,6 @@ void PermissionsUpdater::NetworkPermissionsUpdateHelper::UpdatePermissions(
CreateCorsOriginAccessAllowList(
*extension,
PermissionsData::EffectiveHostPermissionsMode::kOmitTabSpecific);
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
ExtensionsClient::Get()->AddOriginAccessPermissions(*extension, true,
&allow_list);
}

NetworkPermissionsUpdateHelper* helper = new NetworkPermissionsUpdateHelper(
browser_context,
Expand Down Expand Up @@ -226,10 +221,6 @@ void PermissionsUpdater::NetworkPermissionsUpdateHelper::
CreateCorsOriginAccessAllowList(
*extension,
PermissionsData::EffectiveHostPermissionsMode::kOmitTabSpecific);
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
ExtensionsClient::Get()->AddOriginAccessPermissions(*extension, true,
&allow_list);
}
browser_context->SetCorsOriginAccessListForOrigin(
url::Origin::Create(extension->url()), std::move(allow_list),
CreateCorsOriginAccessBlockList(*extension), barrier_closure);
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/media/router/media_router_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "content/public/common/content_features.h"
#include "crypto/random.h"
#include "extensions/buildflags/buildflags.h"
#include "services/network/public/cpp/features.h"
#include "ui/base/buildflags.h"

#if defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
Expand Down Expand Up @@ -135,8 +134,7 @@ bool ShouldUseMirroringService() {
return (base::FeatureList::IsEnabled(
mirroring::features::kMirroringService) ||
base::FeatureList::IsEnabled(kCastMediaRouteProvider)) &&
base::FeatureList::IsEnabled(features::kAudioServiceAudioStreams) &&
base::FeatureList::IsEnabled(network::features::kNetworkService);
base::FeatureList::IsEnabled(features::kAudioServiceAudioStreams);
}

#endif // !defined(OS_ANDROID)
Expand Down
18 changes: 2 additions & 16 deletions chrome/browser/net/system_network_context_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
#include "net/third_party/uri_template/uri_template.h"
#include "services/network/network_service.h"
#include "services/network/public/cpp/cross_thread_shared_url_loader_factory_info.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/mojom/host_resolver.mojom.h"
#include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
Expand Down Expand Up @@ -282,12 +281,6 @@ class SystemNetworkContextManager::URLLoaderFactoryForSystem
};

network::mojom::NetworkContext* SystemNetworkContextManager::GetContext() {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// SetUp should already have been called.
DCHECK(io_thread_network_context_);
return io_thread_network_context_.get();
}

if (!network_service_network_context_ ||
network_service_network_context_.encountered_error()) {
// This should call into OnNetworkServiceCreated(), which will re-create
Expand Down Expand Up @@ -483,8 +476,6 @@ void SystemNetworkContextManager::RegisterPrefs(PrefRegistrySimple* registry) {

void SystemNetworkContextManager::OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return;
// Disable QUIC globally, if needed.
if (!is_quic_allowed_)
network_service->DisableQuic();
Expand Down Expand Up @@ -693,13 +684,8 @@ void SystemNetworkContextManager::FlushProxyConfigMonitorForTesting() {
}

void SystemNetworkContextManager::FlushNetworkInterfaceForTesting() {
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
DCHECK(io_thread_network_context_);
io_thread_network_context_.FlushForTesting();
} else {
DCHECK(network_service_network_context_);
network_service_network_context_.FlushForTesting();
}
DCHECK(network_service_network_context_);
network_service_network_context_.FlushForTesting();
if (url_loader_factory_)
url_loader_factory_.FlushForTesting();
}
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/net/system_network_context_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,6 @@ class SystemNetworkContextManager {
// enabled. nullptr, otherwise.
network::mojom::NetworkContextPtr network_service_network_context_;

// This is a NetworkContext that wraps the IOThread's SystemURLRequestContext.
// Always initialized in SetUp, but it's only returned by Context() when the
// network service is disabled.
network::mojom::NetworkContextPtr io_thread_network_context_;

// URLLoaderFactory backed by the NetworkContext returned by GetContext(), so
// consumers don't all need to create their own factory.
scoped_refptr<URLLoaderFactoryForSystem> shared_url_loader_factory_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "services/network/public/cpp/features.h"
#include "url/gurl.h"

DataSaverSiteBreakdownMetricsObserver::DataSaverSiteBreakdownMetricsObserver() =
Expand Down Expand Up @@ -80,18 +79,16 @@ void DataSaverSiteBreakdownMetricsObserver::OnResourceDataUseObserved(
received_data_length,
received_data_length + data_reduction_proxy_bytes_saved,
committed_host_);
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// TODO(rajendrant): Fix the |request_type| and |mime_type| sent below or
// remove the respective histograms.
data_reduction_proxy_settings->data_reduction_proxy_service()
->UpdateContentLengths(
received_data_length,
received_data_length + data_reduction_proxy_bytes_saved,
data_reduction_proxy_settings->IsDataReductionProxyEnabled(),
data_reduction_proxy::VIA_DATA_REDUCTION_PROXY,
std::string() /* mime_type */, true /*is_user_traffic*/,
data_use_measurement::DataUseUserData::OTHER, 0);
}
// TODO(rajendrant): Fix the |request_type| and |mime_type| sent below or
// remove the respective histograms.
data_reduction_proxy_settings->data_reduction_proxy_service()
->UpdateContentLengths(
received_data_length,
received_data_length + data_reduction_proxy_bytes_saved,
data_reduction_proxy_settings->IsDataReductionProxyEnabled(),
data_reduction_proxy::VIA_DATA_REDUCTION_PROXY,
std::string() /* mime_type */, true /*is_user_traffic*/,
data_use_measurement::DataUseUserData::OTHER, 0);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/metrics/sparse_histogram.h"
#include "chrome/browser/data_use_measurement/chrome_data_use_measurement.h"
#include "content/public/browser/navigation_handle.h"
#include "services/network/public/cpp/features.h"

DataUseMetricsObserver::DataUseMetricsObserver() = default;

Expand All @@ -18,8 +17,6 @@ page_load_metrics::PageLoadMetricsObserver::ObservePolicy
DataUseMetricsObserver::OnCommit(content::NavigationHandle* navigation_handle,
ukm::SourceId source_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
return STOP_OBSERVING;
return CONTINUE_OBSERVING;
}

Expand Down
Loading

0 comments on commit f6c3f96

Please sign in to comment.