When an idle socket is added back to a socket pool, check for stalled jobs in lower pools

net/socket/client_socket_handle.cc

Index: net/socket/client_socket_handle.cc
===================================================================
--- net/socket/client_socket_handle.cc	(revision 218769)
+++ net/socket/client_socket_handle.cc	(working copy)
@@ -18,7 +18,7 @@
 ClientSocketHandle::ClientSocketHandle()
     : is_initialized_(false),
       pool_(NULL),
-      layered_pool_(NULL),
+      higher_pool_(NULL),
       is_reused_(false),
       callback_(base::Bind(&ClientSocketHandle::OnIOComplete,
                            base::Unretained(this))),
@@ -53,10 +53,8 @@
   group_name_.clear();
   is_reused_ = false;
   user_callback_.Reset();
-  if (layered_pool_) {
-    pool_->RemoveLayeredPool(layered_pool_);
-    layered_pool_ = NULL;
-  }
+  if (higher_pool_)
+    RemoveHigherLayeredPool(higher_pool_);
   pool_ = NULL;
   idle_time_ = base::TimeDelta();
   init_time_ = base::TimeTicks();
@@ -82,24 +80,27 @@
 }
 
 bool ClientSocketHandle::IsPoolStalled() const {
+  if (!pool_)
+    return false;
   return pool_->IsStalled();
 }
 
-void ClientSocketHandle::AddLayeredPool(LayeredPool* layered_pool) {
-  CHECK(layered_pool);
-  CHECK(!layered_pool_);
+void ClientSocketHandle::AddHigherLayeredPool(HigherLayeredPool* higher_pool) {
+  CHECK(higher_pool);

[akalin, 2013/08/21 22:36:31]

can you add comments in the .h file saying that at most one HLP can be added to
the handle at a time?

[mmenke, 2013/08/22 14:35:32]

Done.

+  CHECK(!higher_pool_);
   if (pool_) {

[akalin, 2013/08/21 22:36:31]

calling this function when pool_ is NULL seems like an error to me. Is it? If
so, change this into a check maybe?

[mmenke, 2013/08/22 14:35:32]

CreateFakeSpdySessionHelper in spdy_test_util_common.cc creates
ClientSocketHandles used for SPDY tests without SocketPools, and also results in
calling this function.  Think changing that is beyond the scope of this CL.

I've added a TODO about changing that.

-    pool_->AddLayeredPool(layered_pool);
-    layered_pool_ = layered_pool;
+    pool_->AddHigherLayeredPool(higher_pool);
+    higher_pool_ = higher_pool;
   }
 }
 
-void ClientSocketHandle::RemoveLayeredPool(LayeredPool* layered_pool) {
-  CHECK(layered_pool);
-  CHECK(layered_pool_);
+void ClientSocketHandle::RemoveHigherLayeredPool(

[akalin, 2013/08/21 22:36:31]

may as well add a comment in the .h file saying that the passed-in pool must
match the one passed in to AddHigherLayeredPool().

[mmenke, 2013/08/22 14:35:32]

Done.

+    HigherLayeredPool* higher_pool) {
+  CHECK(higher_pool_);
+  CHECK_EQ(higher_pool_, higher_pool);
   if (pool_) {

[akalin, 2013/08/21 22:36:31]

same comment re. pool_ being NULL

[mmenke, 2013/08/22 14:35:32]

While I believe this code will be skipped in the case mentioned above, I think
the two should match, to just in case when/where RemoveHigherLayeredPool is
called changes.

-    pool_->RemoveLayeredPool(layered_pool);
-    layered_pool_ = NULL;
+    pool_->RemoveHigherLayeredPool(higher_pool);
+    higher_pool_ = NULL;
   }
 }