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..6d460c1537673f5c7b03ccbac1b70de20c45d9a7 100644
--- a/headless/lib/browser/headless_devtools.cc
+++ b/headless/lib/browser/headless_devtools.cc
@@ -25,38 +25,50 @@ namespace {
 
 const int kBackLog = 10;
 
-class TCPServerSocketFactory : public content::DevToolsSocketFactory {
+class TCPEndpointServerSocketFactory : public content::DevToolsSocketFactory {
  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:
+  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;
+  }
+
+  std::unique_ptr<net::ServerSocket> CreateForTethering(
+      std::string* out_name) override {
+    return nullptr;
+  }
+
+  net::IPEndPoint endpoint_;
+
+  DISALLOW_COPY_AND_ASSIGN(TCPEndpointServerSocketFactory);
+};
+
+#if defined(OS_POSIX)
+class TCPAdoptServerSocketFactory : public content::DevToolsSocketFactory {
+ public:
+  // Construct a factory to use an already-open, already-listening socket.
+  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
+    // Note that we assume that the socket is already listening, so unlike
+    // TCPEndpointServerSocketFactory, we don't call Listen.
+    return std::unique_ptr<net::ServerSocket>(std::move(tsock));
   }
 
   std::unique_ptr<net::ServerSocket> CreateForTethering(
@@ -64,22 +76,44 @@ class TCPServerSocketFactory : public content::DevToolsSocketFactory {
     return nullptr;
   }
 
-  net::IPEndPoint endpoint_;
   size_t socket_fd_;
 
-  DISALLOW_COPY_AND_ASSIGN(TCPServerSocketFactory);
+  DISALLOW_COPY_AND_ASSIGN(TCPAdoptServerSocketFactory);
 };
+#else   // defined(OS_POSIX)
+// Placeholder class to use when a socket_fd is passed in on non-Posix.

[mcgreevy, 2017/05/24 01:23:19]

Can you prepend a blank line here?

[Raul Vera, 2017/05/24 03:00:06]

Done.

+class DummyTCPServerSocketFactory : public content::DevToolsSocketFactory {
+ public:
+  explicit DummyTCPServerSocketFactory() {}
+
+ private:
+  std::unique_ptr<net::ServerSocket> CreateForHttpServer() override {
+    return nullptr;
+  }
 
+  std::unique_ptr<net::ServerSocket> CreateForTethering(
+      std::string* out_name) override {
+    return nullptr;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(DummyTCPServerSocketFactory);
+};
+#endif  // defined(OS_POSIX)
 }  // 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";
+    socket_factory.reset(new DummyTCPServerSocketFactory());
+#endif
   }
   content::DevToolsAgentHost::StartRemoteDebuggingServer(
       std::move(socket_factory), std::string(),