Add reprioritization to SSLConnectJobs
This change allows the priority of an SSLConnectJob to be changed. If
the reprioritization occurs while the job is establishing its underlying
connection, the priority change is also passed down to the lower-level
socket pool.
Bug: 166689
Change-Id: I87d1e536846443901cc628423f4615c3ffa7542a
Reviewed-on: https://chromium-review.googlesource.com/c/1327423
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606876}diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 164b9f3..19984825 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -2017,11 +2017,13 @@
std::unique_ptr<StreamSocket> socket,
ClientSocketHandle* handle,
const SocketTag& socket_tag,
- CompletionOnceCallback callback)
+ CompletionOnceCallback callback,
+ RequestPriority priority)
: socket_(std::move(socket)),
handle_(handle),
socket_tag_(socket_tag),
- user_callback_(std::move(callback)) {}
+ user_callback_(std::move(callback)),
+ priority_(priority) {}
MockTransportClientSocketPool::MockConnectJob::~MockConnectJob() = default;
@@ -2107,8 +2109,8 @@
std::unique_ptr<StreamSocket> socket =
client_socket_factory_->CreateTransportClientSocket(
AddressList(), NULL, net_log.net_log(), NetLogSource());
- MockConnectJob* job = new MockConnectJob(std::move(socket), handle,
- socket_tag, std::move(callback));
+ MockConnectJob* job = new MockConnectJob(
+ std::move(socket), handle, socket_tag, std::move(callback), priority);
job_list_.push_back(base::WrapUnique(job));
handle->set_pool_id(1);
return job->Connect();
@@ -2117,7 +2119,13 @@
void MockTransportClientSocketPool::SetPriority(const std::string& group_name,
ClientSocketHandle* handle,
RequestPriority priority) {
- // TODO: Implement.
+ for (auto& job : job_list_) {
+ if (job->handle() == handle) {
+ job->set_priority(priority);
+ return;
+ }
+ }
+ NOTREACHED();
}
void MockTransportClientSocketPool::CancelRequest(const std::string& group_name,
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index b7cabb2..033a04a 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -1177,12 +1177,18 @@
MockConnectJob(std::unique_ptr<StreamSocket> socket,
ClientSocketHandle* handle,
const SocketTag& socket_tag,
- CompletionOnceCallback callback);
+ CompletionOnceCallback callback,
+ RequestPriority priority);
~MockConnectJob();
int Connect();
bool CancelHandle(const ClientSocketHandle* handle);
+ ClientSocketHandle* handle() const { return handle_; }
+
+ RequestPriority priority() const { return priority_; }
+ void set_priority(RequestPriority priority) { priority_ = priority; }
+
private:
void OnConnect(int rv);
@@ -1190,6 +1196,7 @@
ClientSocketHandle* handle_;
const SocketTag socket_tag_;
CompletionOnceCallback user_callback_;
+ RequestPriority priority_;
DISALLOW_COPY_AND_ASSIGN(MockConnectJob);
};
@@ -1203,6 +1210,11 @@
RequestPriority last_request_priority() const {
return last_request_priority_;
}
+
+ const std::vector<std::unique_ptr<MockConnectJob>>& requests() const {
+ return job_list_;
+ }
+
int release_count() const { return release_count_; }
int cancel_count() const { return cancel_count_; }
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index 83d78bc6..af09b976 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -429,6 +429,11 @@
return DoLoop(OK);
}
+void SSLConnectJob::ChangePriorityInternal(RequestPriority priority) {
+ if (transport_socket_handle_)
+ transport_socket_handle_->SetPriority(priority);
+}
+
SSLClientSocketPool::SSLConnectJobFactory::SSLConnectJobFactory(
TransportClientSocketPool* transport_pool,
SOCKSClientSocketPool* socks_pool,
diff --git a/net/socket/ssl_client_socket_pool.h b/net/socket/ssl_client_socket_pool.h
index 4041761..6ef474be 100644
--- a/net/socket/ssl_client_socket_pool.h
+++ b/net/socket/ssl_client_socket_pool.h
@@ -151,6 +151,8 @@
// Otherwise, it returns a net error code.
int ConnectInternal() override;
+ void ChangePriorityInternal(RequestPriority priority) override;
+
scoped_refptr<SSLSocketParams> params_;
TransportClientSocketPool* const transport_pool_;
SOCKSClientSocketPool* const socks_pool_;
diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index 1cc25c5..07fb01f 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -285,6 +285,7 @@
EXPECT_THAT(rv, IsOk());
EXPECT_TRUE(handle.is_initialized());
EXPECT_TRUE(handle.socket());
+ EXPECT_EQ(MEDIUM, transport_socket_pool_.requests()[0]->priority());
TestLoadTimingInfo(handle);
EXPECT_EQ(0u, handle.connection_attempts().size());
}
@@ -310,10 +311,40 @@
ClientSocketPool::RespectLimits::ENABLED,
callback.callback(), pool_.get(), NetLogWithSource()));
EXPECT_EQ(priority, transport_socket_pool_.last_request_priority());
+ EXPECT_EQ(priority, transport_socket_pool_.requests()[i]->priority());
handle.socket()->Disconnect();
}
}
+// Test that the SSLConnectJob passes priority changes down to the transport
+// socket pool (for the DIRECT case).
+TEST_F(SSLClientSocketPoolTest, SetSocketRequestPriorityDirect) {
+ StaticSocketDataProvider data;
+ socket_factory_.AddSocketDataProvider(&data);
+ SSLSocketDataProvider ssl(ASYNC, OK);
+ socket_factory_.AddSSLSocketDataProvider(&ssl);
+
+ CreatePool(true /* tcp pool */, false, false);
+ scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_DIRECT);
+
+ ClientSocketHandle handle;
+ TestCompletionCallback callback;
+ int rv = handle.Init(kGroupName, params, MEDIUM, SocketTag(),
+ ClientSocketPool::RespectLimits::ENABLED,
+ callback.callback(), pool_.get(), NetLogWithSource());
+ EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+ EXPECT_FALSE(handle.is_initialized());
+ EXPECT_FALSE(handle.socket());
+ EXPECT_EQ(MEDIUM, transport_socket_pool_.requests()[0]->priority());
+
+ pool_->SetPriority(kGroupName, &handle, LOWEST);
+ EXPECT_EQ(LOWEST, transport_socket_pool_.requests()[0]->priority());
+
+ EXPECT_THAT(callback.WaitForResult(), IsOk());
+ EXPECT_TRUE(handle.is_initialized());
+ EXPECT_TRUE(handle.socket());
+}
+
TEST_F(SSLClientSocketPoolTest, BasicDirectAsync) {
StaticSocketDataProvider data;
socket_factory_.AddSocketDataProvider(&data);
@@ -524,6 +555,36 @@
ClientSocketPool::RespectLimits::ENABLED,
callback.callback(), pool_.get(), NetLogWithSource()));
EXPECT_EQ(HIGHEST, transport_socket_pool_.last_request_priority());
+ EXPECT_EQ(HIGHEST, transport_socket_pool_.requests()[0]->priority());
+}
+
+// Test that the SSLConnectJob passes priority changes down to the transport
+// socket pool (for the SOCKS_PROXY case).
+TEST_F(SSLClientSocketPoolTest, SetTransportPrioritySOCKS) {
+ StaticSocketDataProvider data;
+ socket_factory_.AddSocketDataProvider(&data);
+ SSLSocketDataProvider ssl(ASYNC, OK);
+ socket_factory_.AddSSLSocketDataProvider(&ssl);
+
+ CreatePool(false, true /* http proxy pool */, true /* socks pool */);
+ scoped_refptr<SSLSocketParams> params = SSLParams(ProxyServer::SCHEME_SOCKS5);
+
+ ClientSocketHandle handle;
+ TestCompletionCallback callback;
+ int rv = handle.Init(kGroupName, params, MEDIUM, SocketTag(),
+ ClientSocketPool::RespectLimits::ENABLED,
+ callback.callback(), pool_.get(), NetLogWithSource());
+ EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+ EXPECT_FALSE(handle.is_initialized());
+ EXPECT_FALSE(handle.socket());
+ EXPECT_EQ(MEDIUM, transport_socket_pool_.requests()[0]->priority());
+
+ pool_->SetPriority(kGroupName, &handle, LOWEST);
+ EXPECT_EQ(LOWEST, transport_socket_pool_.requests()[0]->priority());
+
+ EXPECT_THAT(callback.WaitForResult(), IsOk());
+ EXPECT_TRUE(handle.is_initialized());
+ EXPECT_TRUE(handle.socket());
}
TEST_F(SSLClientSocketPoolTest, SOCKSBasicAsync) {
@@ -656,8 +717,13 @@
ClientSocketPool::RespectLimits::ENABLED,
callback.callback(), pool_.get(), NetLogWithSource()));
EXPECT_EQ(HIGHEST, transport_socket_pool_.last_request_priority());
+ EXPECT_EQ(HIGHEST, transport_socket_pool_.requests()[0]->priority());
}
+// TODO(chlily): Test that the SSLConnectJob passes priority changes down to the
+// transport socket pool (for the HTTP_PROXY case), once change priority is
+// implemented for HttpProxyClientSocketPool.
+
TEST_F(SSLClientSocketPoolTest, HttpProxyBasicAsync) {
MockWrite writes[] = {
MockWrite(
