Add a delay when unlocking WebSocket endpoints.

net/socket/websocket_endpoint_lock_manager.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_SOCKET_WEBSOCKET_ENDPOINT_LOCK_MANAGER_H_
 #define NET_SOCKET_WEBSOCKET_ENDPOINT_LOCK_MANAGER_H_
 
 #include <map>
 
 #include "base/containers/linked_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
+#include "base/time/time.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_export.h"
 #include "net/socket/websocket_transport_client_socket_pool.h"
 
 namespace net {
 
 class StreamSocket;
 
 class NET_EXPORT_PRIVATE WebSocketEndpointLockManager {

[mmenke, 2015/01/20 15:50:53]

Mind adding some class level documentation?  I've never heard of this class
before.

[Adam Rice, 2015/01/21 08:54:02]

Wow. Not only did I fail to document it here, I only mentioned it in passing in
the design doc.

I put in a comment of what it does. I'm not sure whether the implementation is
sufficiently interesting to justify adding notes about how it works.

  public:
   class NET_EXPORT_PRIVATE Waiter : public base::LinkNode<Waiter> {
    public:
     // If the node is in a list, removes it.
     virtual ~Waiter();
 
     virtual void GotEndpointLock() = 0;
   };
 
   static WebSocketEndpointLockManager* GetInstance();

[mmenke, 2015/01/20 15:50:53]

Unrelated to this CL, but should this really be a singleton?  Seems weird to be
leaking that information between profiles - don't see leaking that from
incognito as a big issue, but seems a little weird.

Also creates an implicit requirement that all URLRequestContexts that use
WebSockets live on a single thread, or random crashes may ensue.

[mmenke, 2015/01/20 17:46:21]

And to elaborte:  This is the case with Chrome, it's not currently the case with
Cronet, where every context has its own thread.  We may want to switch our
threading model at some point, but the reason shouldn't be because of random
non-threadsafe globals peppered throughout the code.

[Adam Rice, 2015/01/21 08:54:02]

Originally I didn't know what its lifetime should be or how to access it from
net/websockets. Now I'm pretty sure it belongs in HttpNetworkSession and can be
accessed via the URLRequestContext.

I have filed http://crbug.com/450518 to fix this.

 
   // Returns OK if lock was acquired immediately, ERR_IO_PENDING if not. If the
   // lock was not acquired, then |waiter->GotEndpointLock()| will be called when
   // it is. A Waiter automatically removes itself from the list of waiters when
   // its destructor is called.
   int LockEndpoint(const IPEndPoint& endpoint, Waiter* waiter);
 
   // Records the IPEndPoint associated with a particular socket. This is
   // necessary because TCPClientSocket refuses to return the PeerAddress after
   // the connection is disconnected. The association will be forgotten when
   // UnlockSocket() or UnlockEndpoint() is called. The |socket| pointer must not
   // be deleted between the call to RememberSocket() and the call to
   // UnlockSocket().
   void RememberSocket(StreamSocket* socket, const IPEndPoint& endpoint);
 
-  // Releases the lock on the endpoint that was associated with |socket| by
-  // RememberSocket(). If appropriate, triggers the next socket connection.
-  // Should be called exactly once for each |socket| that was passed to
-  // RememberSocket(). Does nothing if UnlockEndpoint() has been called since
+  // Removes the socket association that was recorded by RememberSocket(), then
+  // asynchronously releases the lock on the endpoint after a delay. If
+  // appropriate, calls |waiter->GetEndpointLock()| when the lock is
+  // released. Should be called exactly once for each |socket| that was passed
+  // to RememberSocket(). Does nothing if UnlockEndpoint() has been called since
   // the call to RememberSocket().
   void UnlockSocket(StreamSocket* socket);
 
-  // Releases the lock on |endpoint|. Does nothing if |endpoint| is not locked.
-  // Removes any socket association that was recorded with RememberSocket(). If
-  // appropriate, calls |waiter->GotEndpointLock()|.
+  // Asynchronously releases the lock on |endpoint| after a delay. Does nothing
+  // if |endpoint| is not locked.  Removes any socket association that was
+  // recorded with RememberSocket(). If appropriate, calls
+  // |waiter->GotEndpointLock()| when the lock is released.
   void UnlockEndpoint(const IPEndPoint& endpoint);
 
   // Checks that |lock_info_map_| and |socket_lock_info_map_| are empty. For
   // tests.
   bool IsEmpty() const;
 
+  // Changes the value of the unlock delay. Returns the previous value of the
+  // delay.
+  base::TimeDelta SetUnlockDelayForTesting(base::TimeDelta new_delay);
+
  private:
   struct LockInfo {
     typedef base::LinkedList<Waiter> WaiterQueue;
 
     LockInfo();
     ~LockInfo();
 
     // This object must be copyable to be placed in the map, but it cannot be
     // copied after |queue| has been assigned to.
     LockInfo(const LockInfo& rhs);
@@ skipping 16 matching lines @@
 
   // SocketLockInfoMap requires std::map iterator semantics for LockInfoMap
   // (ie. that the iterator will remain valid as long as the entry is not
   // deleted).
   typedef std::map<IPEndPoint, LockInfo> LockInfoMap;
   typedef std::map<StreamSocket*, LockInfoMap::iterator> SocketLockInfoMap;
 
   WebSocketEndpointLockManager();
   ~WebSocketEndpointLockManager();
 
-  void UnlockEndpointByIterator(LockInfoMap::iterator lock_info_it);
+  void UnlockEndpointAfterDelay(const IPEndPoint& endpoint);
+  void DelayedUnlockEndpoint(const IPEndPoint& endpoint);
   void EraseSocket(LockInfoMap::iterator lock_info_it);
 
   // If an entry is present in the map for a particular endpoint, then that
   // endpoint is locked. If LockInfo.queue is non-empty, then one or more
   // Waiters are waiting for the lock.
   LockInfoMap lock_info_map_;
 
   // Store sockets remembered by RememberSocket() and not yet unlocked by
   // UnlockSocket() or UnlockEndpoint(). Every entry in this map always
   // references a live entry in lock_info_map_, and the LockInfo::socket member
   // is non-NULL if and only if there is an entry in this map for the socket.
   SocketLockInfoMap socket_lock_info_map_;
 
+  // Time to wait between a call to Unlock* and actually unlocking the socket.
+  base::TimeDelta unlock_delay_;
+
+  // Number of sockets currently pending unlock.
+  size_t pending_unlock_count_;
+
+  // The messsage loop holding the unlock delay callback may outlive this
+  // object.
+  base::WeakPtrFactory<WebSocketEndpointLockManager> weak_factory_;
+
   friend struct DefaultSingletonTraits<WebSocketEndpointLockManager>;
 
   DISALLOW_COPY_AND_ASSIGN(WebSocketEndpointLockManager);
 };
 
 }  // namespace net
 
 #endif  // NET_SOCKET_WEBSOCKET_ENDPOINT_LOCK_MANAGER_H_