Refactored adopting a socket for devtools into cleaner, more idiomatic code.

headless/lib/browser/headless_devtools.cc

Index: headless/lib/browser/headless_devtools.cc
diff --git a/headless/lib/browser/headless_devtools.cc b/headless/lib/browser/headless_devtools.cc
index 960600793d903f0cb93a4b6c6f01cefb2c79d153..acade013fc2e29fecd35ade4f7fe80a398e34393 100644
--- a/headless/lib/browser/headless_devtools.cc
+++ b/headless/lib/browser/headless_devtools.cc
@@ -26,60 +26,74 @@ namespace {
 const int kBackLog = 10;
 
 class TCPServerSocketFactory : public content::DevToolsSocketFactory {
+ private:
+  std::unique_ptr<net::ServerSocket> CreateForTethering(
+      std::string* out_name) override {
+    return nullptr;
+  }
+};
+
+class TCPEndpointServerSocketFactory : public TCPServerSocketFactory {
  public:
-  explicit TCPServerSocketFactory(const net::IPEndPoint& endpoint)
-      : endpoint_(endpoint), socket_fd_(0) {
+  explicit TCPEndpointServerSocketFactory(const net::IPEndPoint& endpoint)
+      : endpoint_(endpoint) {
     DCHECK(endpoint_.address().IsValid());
   }
 
-  explicit TCPServerSocketFactory(const size_t socket_fd)
+ private:
+  // content::DevToolsSocketFactory implementation:
+  std::unique_ptr<net::ServerSocket> CreateForHttpServer() override {
+    std::unique_ptr<net::ServerSocket> socket(
+        new net::TCPServerSocket(nullptr, net::NetLogSource()));
+    if (socket->Listen(endpoint_, kBackLog) != net::OK)
+      return std::unique_ptr<net::ServerSocket>();
+    return socket;
+  }
+
+  net::IPEndPoint endpoint_;
+
+  DISALLOW_COPY_AND_ASSIGN(TCPEndpointServerSocketFactory);
+};
+
+#if defined(OS_POSIX)
+class TCPAdoptServerSocketFactory : public TCPServerSocketFactory {
+ public:
+  explicit TCPAdoptServerSocketFactory(const size_t socket_fd)
       : socket_fd_(socket_fd) {}
 
  private:
   // content::DevToolsSocketFactory implementation:
   std::unique_ptr<net::ServerSocket> CreateForHttpServer() override {
-    if (!socket_fd_) {
-      std::unique_ptr<net::ServerSocket> socket(
-          new net::TCPServerSocket(nullptr, net::NetLogSource()));
-      if (socket->Listen(endpoint_, kBackLog) != net::OK)
-        return std::unique_ptr<net::ServerSocket>();
-      return socket;
-    }
-#if defined(OS_POSIX)
     std::unique_ptr<net::TCPServerSocket> tsock(
         new net::TCPServerSocket(nullptr, net::NetLogSource()));
     if (tsock->AdoptSocket(socket_fd_) != net::OK) {
       LOG(ERROR) << "Failed to adopt open socket";
       return std::unique_ptr<net::ServerSocket>();
     }
-    return std::unique_ptr<net::ServerSocket>(tsock.release());
-#else
-    LOG(ERROR) << "Can't inherit an open socket on non-Posix systems";
-    return std::unique_ptr<net::ServerSocket>();
-#endif
-  }
-
-  std::unique_ptr<net::ServerSocket> CreateForTethering(
-      std::string* out_name) override {
-    return nullptr;
+    // Note that we assume that the socket is already listening, so we don't
+    // call Listen.
+    return std::unique_ptr<net::ServerSocket>(std::move(tsock));
   }
 
-  net::IPEndPoint endpoint_;
   size_t socket_fd_;
 
-  DISALLOW_COPY_AND_ASSIGN(TCPServerSocketFactory);
+  DISALLOW_COPY_AND_ASSIGN(TCPAdoptServerSocketFactory);
 };
-
+#endif

[Sami, 2017/05/16 12:46:52]

nit: Please add // defined(OS_POSIX) to make it easier to see which block this
belongs to.

 }  // namespace
 
 void StartLocalDevToolsHttpHandler(HeadlessBrowser::Options* options) {
   std::unique_ptr<content::DevToolsSocketFactory> socket_factory;
   if (options->devtools_socket_fd == 0) {
     const net::IPEndPoint& endpoint = options->devtools_endpoint;
-    socket_factory.reset(new TCPServerSocketFactory(endpoint));
+    socket_factory.reset(new TCPEndpointServerSocketFactory(endpoint));
   } else {
+#if defined(OS_POSIX)
     const uint16_t socket_fd = options->devtools_socket_fd;
-    socket_factory.reset(new TCPServerSocketFactory(socket_fd));
+    socket_factory.reset(new TCPAdoptServerSocketFactory(socket_fd));
+#else
+    LOG(ERROR) << "Can't inherit an open socket on non-Posix systems";
+#endif
   }
   content::DevToolsAgentHost::StartRemoteDebuggingServer(
       std::move(socket_factory), std::string(),