Decouple DevTools from socket implementation.

net/base/stream_listen_socket.h

 // Copyright (c) 2012 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.
 
 // Stream-based listen socket implementation that handles reading and writing
 // to the socket, but does not handle creating the socket nor connecting
 // sockets, which are handled by subclasses on creation and in Accept,
 // respectively.
 
 // StreamListenSocket handles IO asynchronously in the specified MessageLoop.
@@ skipping 114 matching lines @@
   void PauseReads();
   void ResumeReads();
 
   const SOCKET socket_;
   bool reads_paused_;
   bool has_pending_reads_;
 
   DISALLOW_COPY_AND_ASSIGN(StreamListenSocket);
 };
 
+// Abstract factory that must be subclassed for each subclass of
+// StreamListenSocket.
+class StreamListenSocketFactory {
+public:

[pfeldman, 2012/05/14 11:12:00]

Space before public is missing.

[Philippe, 2012/05/14 12:36:52]

I've been fighting against my VIM on that for a couple months. It seems that it
won this time :)

+  virtual ~StreamListenSocketFactory() {}
+
+  // Returns a new instance of StreamListenSocket or NULL if an error occurred.
+  virtual scoped_refptr<StreamListenSocket> CreateAndListen(

[pfeldman, 2012/05/14 11:12:00]

Why scoped_refptr?

[Philippe, 2012/05/14 12:36:52]

Only for consistency with the existing CreateAndListen() methods. I'm not a big
fan of shared semantic by default. I would have used a unique_ptr or scoped_ptr
in Chromium with move semantic emulation through .Pass(). The client could still
release the unique pointer and use a shared pointer if he wants to.
Initially the CreateAndListen() methods were returning a raw pointer and had a
comment about ownership. I changed that recently while addressing Matt's
comments. I agreed at that time and still agree that scoped_refptr is better
than a raw pointer. I didn't really consider the use of scoped_ptr at that time
since the client code was using a scoped_refptr anyway.

I don't know either if the use of scoped_ptr with move emulation in place of
unique_ptr is well established in the Chromium codebase. I like it but it might
be confusing for some users since it behavior differs from std::scoped_ptr
(which is not movable if I remember correctly).

What do you think?

[mmenke, 2012/05/14 20:45:19]

The advantage of explicitly returning a scoped_refptr is it provides an extra
level of defense against storing in a scoped_ptr instead.

Another way to do that (That should be done here, and I always forgot to mention
in reviews) is to make the destructor private, and friend the scoped_refptr
class.

scoped_ptr.Pass() is a pretty recent addition to Chrome, but it's seeing fast
adoption by some engineers...Not so much by others.

[Philippe, 2012/05/15 14:39:09]

I made TCPListenSocket's destructor private and added the friend declaration.
Good idea.

+      StreamListenSocket::Delegate* delegate) = 0;
+};
+
 }  // namespace net
 
 #endif  // NET_BASE_STREAM_LISTEN_SOCKET_H_