Schedule HTTP requests to SPDY proxies via Resource Scheduler
Enable the experiment to schedule HTTP requests to SPDY proxies
via resource scheduler. The experiment has been running on Stable
M-67+ for some time. Results here: http://shortn/_xRuL16I5De,
and they show slight page load performance improvement.
With the experiment disabled, HTTP requests to SPDY proxies
would bypass scheduling enforced by resource scheduler, and
sunh requests would be immediately dispatched on the network.
With the experiment enabled (as well as with this CL), the
HTTP requests to SPDY proxies are also subject to scheduling.
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I8e11e454addde53a6196d8af728bc0e5a4ecc789
Bug: 838253
Reviewed-on: https://chromium-review.googlesource.com/1137982
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575839}
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index fc3ec1d6..73c656ee 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -599,13 +599,6 @@
network::ResourceResponse* response) {
ResourceRequestInfoImpl* info = loader->GetRequestInfo();
net::URLRequest* request = loader->request();
- if (request->was_fetched_via_proxy() &&
- request->was_fetched_via_spdy() &&
- request->url().SchemeIs(url::kHttpScheme)) {
- scheduler_->OnReceivedSpdyProxiedHttpResponse(
- info->GetChildID(), info->GetRouteID());
- }
-
if (delegate_)
delegate_->OnResponseStarted(request, info->GetContext(), response);
}
diff --git a/services/network/resource_scheduler.cc b/services/network/resource_scheduler.cc
index 7de5c546..65fd1535 100644
--- a/services/network/resource_scheduler.cc
+++ b/services/network/resource_scheduler.cc
@@ -45,12 +45,6 @@
const base::Feature kPrioritySupportedRequestsDelayable{
"PrioritySupportedRequestsDelayable", base::FEATURE_DISABLED_BY_DEFAULT};
-// When kSpdyProxiesRequestsDelayable is enabled, HTTP requests fetched from
-// a SPDY/QUIC/H2 proxies can be delayed by the ResourceScheduler just as
-// HTTP/1.1 resources are.
-const base::Feature kSpdyProxiesRequestsDelayable{
- "SpdyProxiesRequestsDelayable", base::FEATURE_DISABLED_BY_DEFAULT};
-
// When enabled, low-priority H2 and QUIC requests are throttled, but only
// when the parser is in head.
const base::Feature kHeadPrioritySupportedRequestsDelayable{
@@ -385,10 +379,7 @@
public:
Client(const net::NetworkQualityEstimator* const network_quality_estimator,
ResourceScheduler* resource_scheduler)
- : spdy_proxy_requests_delayble_(
- base::FeatureList::IsEnabled(kSpdyProxiesRequestsDelayable)),
- deprecated_is_loaded_(false),
- using_spdy_proxy_(false),
+ : deprecated_is_loaded_(false),
in_flight_delayable_count_(0),
total_layout_blocking_count_(0),
num_skipped_scans_due_to_scheduled_start_(0),
@@ -472,18 +463,6 @@
UpdateParamsForNetworkQuality();
}
- void OnReceivedSpdyProxiedHttpResponse() {
- // If the requests to SPDY/H2/QUIC proxies are delayable, then return
- // immediately.
- if (spdy_proxy_requests_delayble_)
- return;
-
- if (!using_spdy_proxy_) {
- using_spdy_proxy_ = true;
- LoadAnyStartablePendingRequests(RequestStartTrigger::SPDY_PROXY_DETECTED);
- }
- }
-
void ReprioritizeRequest(ScheduledResourceRequestImpl* request,
RequestPriorityParams old_priority_params,
RequestPriorityParams new_priority_params) {
@@ -805,9 +784,6 @@
->SupportsRequestPriority(scheme_host_port);
if (!priority_delayable) {
- if (using_spdy_proxy_ && url_request.url().SchemeIs(url::kHttpScheme))
- return ShouldStartOrYieldRequest(request);
-
// TODO(willchan): We should really improve this algorithm as described in
// https://crbug.com/164101. Also, theoretically we should not count a
// request-priority capable request against the delayable requests limit.
@@ -955,16 +931,11 @@
}
}
- // True if requests to SPDY/H2/QUIC proxies can be delayed by the
- // ResourceScheduler just as HTTP/1.1 resources are.
- const bool spdy_proxy_requests_delayble_;
-
bool deprecated_is_loaded_;
// Tracks if the main HTML parser has reached the body which marks the end of
// layout-blocking resources.
// This is disabled and the is always true when kRendererSideResourceScheduler
// is enabled.
- bool using_spdy_proxy_;
RequestQueue pending_requests_;
RequestSet in_flight_requests_;
// The number of delayable in-flight requests.
@@ -1124,19 +1095,6 @@
client->DeprecatedOnNavigate();
}
-void ResourceScheduler::OnReceivedSpdyProxiedHttpResponse(int child_id,
- int route_id) {
- DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- ClientId client_id = MakeClientId(child_id, route_id);
-
- ClientMap::iterator client_it = client_map_.find(client_id);
- if (client_it == client_map_.end())
- return;
-
- Client* client = client_it->second.get();
- client->OnReceivedSpdyProxiedHttpResponse();
-}
-
bool ResourceScheduler::DeprecatedHasLoadingClients() const {
for (const auto& client : client_map_) {
if (!client.second->deprecated_is_loaded())
diff --git a/services/network/resource_scheduler.h b/services/network/resource_scheduler.h
index 5d1bd9a4..39500f2 100644
--- a/services/network/resource_scheduler.h
+++ b/services/network/resource_scheduler.h
@@ -119,10 +119,6 @@
// Signals from the IO thread:
- // Called when we received a response to a http request that was served
- // from a proxy using SPDY.
- void OnReceivedSpdyProxiedHttpResponse(int child_id, int route_id);
-
// Client functions:
// Returns true if at least one client is currently loading.
diff --git a/services/network/resource_scheduler_client.cc b/services/network/resource_scheduler_client.cc
index 38098de..751e6824 100644
--- a/services/network/resource_scheduler_client.cc
+++ b/services/network/resource_scheduler_client.cc
@@ -38,8 +38,5 @@
resource_scheduler_->ReprioritizeRequest(request, new_priority,
intra_priority_value);
}
-void ResourceSchedulerClient::OnReceivedSpdyProxiedHttpResponse() {
- resource_scheduler_->OnReceivedSpdyProxiedHttpResponse(child_id_, route_id_);
-}
} // namespace network
diff --git a/services/network/resource_scheduler_client.h b/services/network/resource_scheduler_client.h
index beefc2ec2..e88e393 100644
--- a/services/network/resource_scheduler_client.h
+++ b/services/network/resource_scheduler_client.h
@@ -38,7 +38,6 @@
void ReprioritizeRequest(net::URLRequest* request,
net::RequestPriority new_priority,
int intra_priority_value);
- void OnReceivedSpdyProxiedHttpResponse();
private:
friend class base::RefCounted<ResourceSchedulerClient>;
diff --git a/services/network/resource_scheduler_unittest.cc b/services/network/resource_scheduler_unittest.cc
index 08e651d..bf1d9263 100644
--- a/services/network/resource_scheduler_unittest.cc
+++ b/services/network/resource_scheduler_unittest.cc
@@ -733,96 +733,6 @@
EXPECT_TRUE(request->started());
}
-TEST_F(ResourceSchedulerTest, MaxRequestsPerHostForSpdyProxyWhenNotDelayable) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine("",
- kPrioritySupportedRequestsDelayable);
-
- InitializeScheduler();
-
- // Add more than max-per-host low-priority requests.
- std::vector<std::unique_ptr<TestRequest>> requests;
- for (size_t i = 0; i < kMaxNumDelayableRequestsPerHostPerClient + 1; ++i)
- requests.push_back(NewRequest("http://host/low", net::LOWEST));
-
- // Now the scheduler realizes these requests are for a spdy proxy.
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
-
- // No throttling.
- for (const auto& request : requests)
- EXPECT_TRUE(request->started());
-}
-
-TEST_F(ResourceSchedulerTest, MaxRequestsPerHostForSpdyProxyWhenDelayable) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine(
- kPrioritySupportedRequestsDelayable,
- kHeadPrioritySupportedRequestsDelayable);
-
- InitializeScheduler();
-
- // Add more than max-per-host low-priority requests.
- std::vector<std::unique_ptr<TestRequest>> requests;
- for (size_t i = 0; i < kMaxNumDelayableRequestsPerHostPerClient + 1; ++i)
- requests.push_back(NewRequest("http://host/low", net::LOWEST));
-
- // Now the scheduler realizes these requests are for a spdy proxy.
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
-
- // Only kMaxNumDelayableRequestsPerHostPerClient in body.
- for (size_t i = 0; i < requests.size(); ++i) {
- if (i < kMaxNumDelayableRequestsPerHostPerClient)
- EXPECT_TRUE(requests[i]->started());
- else
- EXPECT_FALSE(requests[i]->started());
- }
-}
-
-TEST_F(ResourceSchedulerTest, MaxRequestsPerHostForSpdyProxyWhenHeadDelayable) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine(
- kHeadPrioritySupportedRequestsDelayable,
- kPrioritySupportedRequestsDelayable);
-
- InitializeScheduler();
-
- // Add more than max-per-host low-priority requests.
- std::vector<std::unique_ptr<TestRequest>> requests;
- for (size_t i = 0; i < kMaxNumDelayableRequestsPerHostPerClient + 1; ++i)
- requests.push_back(NewRequest("http://host/low", net::LOWEST));
-
- // Now the scheduler realizes these requests are for a spdy proxy.
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
-
- // No throttling.
- for (const auto& request : requests)
- EXPECT_TRUE(request->started());
-}
-
-TEST_F(ResourceSchedulerTest, ThrottlesHeadForSpdyProxyWhenHeadDelayable) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine(
- kHeadPrioritySupportedRequestsDelayable,
- kPrioritySupportedRequestsDelayable);
-
- InitializeScheduler();
-
- // Add more than max-per-host low-priority requests.
- std::vector<std::unique_ptr<TestRequest>> requests;
- for (size_t i = 0; i < kMaxNumDelayableRequestsPerHostPerClient + 1; ++i)
- requests.push_back(NewRequest("http://host/low", net::LOWEST));
-
- // Now the scheduler realizes these requests are for a spdy proxy.
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
-
- // No throttling.
- for (const auto& request : requests)
- EXPECT_TRUE(request->started());
-}
TEST_F(ResourceSchedulerTest, BackgroundRequestStartsImmediately) {
const int route_id = 0; // Indicates a background request.
@@ -1097,39 +1007,8 @@
std::unique_ptr<TestRequest> request(
NewRequest("http://host/req", net::IDLE));
EXPECT_FALSE(request->started());
-
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(request->started());
-
- std::unique_ptr<TestRequest> after(
- NewRequest("http://host/after", net::IDLE));
- EXPECT_TRUE(after->started());
}
-TEST_F(ResourceSchedulerTest, SpdyProxyDelayable) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine(kPrioritySupportedRequestsDelayable,
- "");
- InitializeScheduler();
- SetMaxDelayableRequests(1);
-
- std::unique_ptr<TestRequest> high(
- NewRequest("http://host/high", net::HIGHEST));
- std::unique_ptr<TestRequest> low(NewRequest("http://host/low", net::LOWEST));
-
- std::unique_ptr<TestRequest> request(
- NewRequest("http://host/req", net::IDLE));
- EXPECT_FALSE(request->started());
-
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
- EXPECT_FALSE(request->started());
-
- std::unique_ptr<TestRequest> after(
- NewRequest("http://host/after", net::IDLE));
- EXPECT_FALSE(after->started());
-}
TEST_F(ResourceSchedulerTest, NewSpdyHostInDelayableRequests) {
base::test::ScopedFeatureList scoped_feature_list;
@@ -1692,68 +1571,6 @@
1);
}
-TEST_F(ResourceSchedulerTest,
- SpdyProxiesRequestsDelayableFeatureEnabled_DelaysSpdyProxyRequests) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine("SpdyProxiesRequestsDelayable", "");
- InitializeScheduler();
- SetMaxDelayableRequests(1);
-
- std::unique_ptr<TestRequest> high(
- NewRequest("http://host/high", net::HIGHEST));
- std::unique_ptr<TestRequest> low(NewRequest("http://host/low", net::LOWEST));
-
- std::unique_ptr<TestRequest> request(
- NewRequest("http://host/req", net::IDLE));
- EXPECT_FALSE(request->started());
-
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
- EXPECT_FALSE(request->started());
-
- // Low priority requests are not started even though the page is loaded from
- // an H2 proxy.
- std::unique_ptr<TestRequest> after(
- NewRequest("http://host/after", net::IDLE));
- EXPECT_FALSE(after->started());
-
- // High priority requests should still be scheduled immediately.
- std::unique_ptr<TestRequest> high_2(
- NewRequest("http://host/high", net::HIGHEST));
- EXPECT_TRUE(high_2->started());
-}
-
-TEST_F(
- ResourceSchedulerTest,
- SpdyProxiesRequestsDelayableFeatureDisabled_SpdyProxyRequestsScheduledImmediately) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitFromCommandLine("", "SpdyProxiesRequestsDelayable");
- InitializeScheduler();
- SetMaxDelayableRequests(1);
-
- std::unique_ptr<TestRequest> high(
- NewRequest("http://host/high", net::HIGHEST));
- std::unique_ptr<TestRequest> low(NewRequest("http://host/low", net::LOWEST));
-
- std::unique_ptr<TestRequest> request(
- NewRequest("http://host/req", net::IDLE));
- EXPECT_FALSE(request->started());
-
- scheduler()->OnReceivedSpdyProxiedHttpResponse(kChildId, kRouteId);
- base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(request->started());
-
- // Low priority requests are started since the page is loaded from an H2
- // proxy.
- std::unique_ptr<TestRequest> after(
- NewRequest("http://host/after", net::IDLE));
- EXPECT_TRUE(after->started());
-
- std::unique_ptr<TestRequest> high_2(
- NewRequest("http://host/high", net::HIGHEST));
- EXPECT_TRUE(high_2->started());
-}
-
TEST_F(ResourceSchedulerTest, SchedulerEnabled) {
SetMaxDelayableRequests(1);
std::unique_ptr<TestRequest> high(
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index 797c07a..9df659f 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -644,12 +644,6 @@
// Do not account header bytes when reporting received body bytes to client.
reported_total_encoded_bytes_ = url_request_->GetTotalReceivedBytes();
- if (resource_scheduler_client_ && url_request->was_fetched_via_proxy() &&
- url_request->was_fetched_via_spdy() &&
- url_request->url().SchemeIs(url::kHttpScheme)) {
- resource_scheduler_client_->OnReceivedSpdyProxiedHttpResponse();
- }
-
if (upload_progress_tracker_) {
upload_progress_tracker_->OnUploadCompleted();
upload_progress_tracker_ = nullptr;