Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(338)

Unified Diff: headless/lib/browser/headless_devtools.cc

Issue 2882393002: Refactored adopting a socket for devtools into cleaner, more idiomatic code. (Closed)
Patch Set: Formatting per reviewer comment Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..d69b55768cf1b5cae215102c5bb8eae8bf1673f7 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,45 @@ 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.
+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(),
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698