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}

net/socket/socket_test_util.cc

@@ -2017,11 +2017,13 @@ MockTransportClientSocketPool::MockConnectJob::MockConnectJob(
     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 @@ int MockTransportClientSocketPool::RequestSocket(
   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 @@ int MockTransportClientSocketPool::RequestSocket(
 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,

net/socket/socket_test_util.h

@@ -1177,12 +1177,18 @@ class MockTransportClientSocketPool : public TransportClientSocketPool {
     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 @@ class MockTransportClientSocketPool : public TransportClientSocketPool {
     ClientSocketHandle* handle_;
     const SocketTag socket_tag_;
     CompletionOnceCallback user_callback_;
+    RequestPriority priority_;
 
     DISALLOW_COPY_AND_ASSIGN(MockConnectJob);
   };
@@ -1203,6 +1210,11 @@ class MockTransportClientSocketPool : public TransportClientSocketPool {
   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_; }
 

net/socket/ssl_client_socket_pool.cc

@@ -429,6 +429,11 @@ int SSLConnectJob::ConnectInternal() {
   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,

net/socket/ssl_client_socket_pool.h

@@ -151,6 +151,8 @@ class SSLConnectJob : public ConnectJob {
   // 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_;

net/socket/ssl_client_socket_pool_unittest.cc

@@ -285,6 +285,7 @@ TEST_F(SSLClientSocketPoolTest, BasicDirect) {
   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 @@ TEST_F(SSLClientSocketPoolTest, SetSocketRequestPriorityOnInitDirect) {
                         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 @@ TEST_F(SSLClientSocketPoolTest, SetTransportPriorityOnInitSOCKS) {
                         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 @@ TEST_F(SSLClientSocketPoolTest, SetTransportPriorityOnInitHTTP) {
                         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(