From 1c211ec90162088008228ed983ecb476a2cb2cbe Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Thu, 31 May 2018 14:24:21 -0700 Subject: [PATCH] inspector: code cleanup Remove some unused code from the WS server implementation and switch to smart pointers where possible. PR-URL: https://github.com/nodejs/node/pull/21070 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- src/inspector_io.cc | 38 +-- src/inspector_io.h | 3 +- src/inspector_socket.cc | 23 +- src/inspector_socket.h | 5 +- src/inspector_socket_server.cc | 146 +++------ src/inspector_socket_server.h | 28 +- test/cctest/test_inspector_socket_server.cc | 323 +++++++++----------- 7 files changed, 229 insertions(+), 337 deletions(-) diff --git a/src/inspector_io.cc b/src/inspector_io.cc index 4db26bee273b9a..615fc975922fd9 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -109,8 +109,12 @@ class IoSessionDelegate : public InspectorSessionDelegate { // mostly session start, message received, and session end. class InspectorIoDelegate: public node::inspector::SocketServerDelegate { public: - InspectorIoDelegate(InspectorIo* io, const std::string& script_path, + InspectorIoDelegate(InspectorIo* io, const std::string& target_id, + const std::string& script_path, const std::string& script_name, bool wait); + ~InspectorIoDelegate() { + io_->ServerDone(); + } // Calls PostIncomingMessage() with appropriate InspectorAction: // kStartSession void StartSession(int session_id, const std::string& target_id) override; @@ -122,11 +126,8 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate { std::vector GetTargetIds() override; std::string GetTargetTitle(const std::string& id) override; std::string GetTargetUrl(const std::string& id) override; - void ServerDone() override { - io_->ServerDone(); - } - void AssignTransport(InspectorSocketServer* server) { + void AssignServer(InspectorSocketServer* server) override { server_ = server; } @@ -163,11 +164,11 @@ class DispatchMessagesTask : public v8::Task { InspectorIo::InspectorIo(Environment* env, v8::Platform* platform, const std::string& path, const DebugOptions& options, bool wait_for_connect) - : options_(options), thread_(), delegate_(nullptr), - state_(State::kNew), parent_env_(env), - thread_req_(), platform_(platform), + : options_(options), thread_(), state_(State::kNew), + parent_env_(env), thread_req_(), platform_(platform), dispatching_messages_(false), script_name_(path), - wait_for_connect_(wait_for_connect), port_(-1) { + wait_for_connect_(wait_for_connect), port_(-1), + id_(GenerateID()) { main_thread_req_ = new AsyncAndAgent({uv_async_t(), env->inspector_agent()}); CHECK_EQ(0, uv_async_init(env->event_loop(), &main_thread_req_->first, InspectorIo::MainThreadReqAsyncCb)); @@ -244,7 +245,7 @@ void InspectorIo::IoThreadAsyncCb(uv_async_t* async) { transport->TerminateConnections(); // Fallthrough case TransportAction::kStop: - transport->Stop(nullptr); + transport->Stop(); break; case TransportAction::kSendMessage: transport->Send(session_id, @@ -271,11 +272,11 @@ void InspectorIo::ThreadMain() { err = uv_async_init(&loop, &thread_req_, IoThreadAsyncCb); CHECK_EQ(err, 0); std::string script_path = ScriptPath(&loop, script_name_); - InspectorIoDelegate delegate(this, script_path, script_name_, - wait_for_connect_); - delegate_ = &delegate; - Transport server(&delegate, &loop, options_.host_name(), options_.port()); - delegate.AssignTransport(&server); + auto delegate = std::unique_ptr( + new InspectorIoDelegate(this, id_, script_path, script_name_, + wait_for_connect_)); + Transport server(std::move(delegate), &loop, options_.host_name(), + options_.port()); TransportAndIo queue_transport(&server, this); thread_req_.data = &queue_transport; if (!server.Start()) { @@ -291,8 +292,6 @@ void InspectorIo::ThreadMain() { uv_run(&loop, UV_RUN_DEFAULT); thread_req_.data = nullptr; CHECK_EQ(uv_loop_close(&loop), 0); - delegate.AssignTransport(nullptr); - delegate_ = nullptr; } template @@ -328,7 +327,7 @@ void InspectorIo::PostIncomingMessage(InspectorAction action, int session_id, } std::vector InspectorIo::GetTargetIds() const { - return delegate_ ? delegate_->GetTargetIds() : std::vector(); + return { id_ }; } TransportAction InspectorIo::Attach(int session_id) { @@ -416,6 +415,7 @@ bool InspectorIo::WaitForFrontendEvent() { } InspectorIoDelegate::InspectorIoDelegate(InspectorIo* io, + const std::string& target_id, const std::string& script_path, const std::string& script_name, bool wait) @@ -423,7 +423,7 @@ InspectorIoDelegate::InspectorIoDelegate(InspectorIo* io, session_id_(0), script_name_(script_name), script_path_(script_path), - target_id_(GenerateID()), + target_id_(target_id), waiting_(wait), server_(nullptr) { } diff --git a/src/inspector_io.h b/src/inspector_io.h index 05de2d17cd3bad..9d27fc557b7b25 100644 --- a/src/inspector_io.h +++ b/src/inspector_io.h @@ -134,7 +134,6 @@ class InspectorIo { // and receive a connection if wait_for_connect was requested. uv_sem_t thread_start_sem_; - InspectorIoDelegate* delegate_; State state_; node::Environment* parent_env_; @@ -161,6 +160,8 @@ class InspectorIo { const bool wait_for_connect_; int port_; std::unordered_map> sessions_; + // May be accessed from any thread + const std::string id_; friend class DispatchMessagesTask; friend class IoSessionDelegate; diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index 1350269cf2c4b0..dc36359b5c927c 100644 --- a/src/inspector_socket.cc +++ b/src/inspector_socket.cc @@ -22,7 +22,8 @@ namespace inspector { class TcpHolder { public: - using Pointer = std::unique_ptr; + static void DisconnectAndDispose(TcpHolder* holder); + using Pointer = DeleteFnPtr; static Pointer Accept(uv_stream_t* server, InspectorSocket::DelegatePointer delegate); @@ -41,7 +42,6 @@ class TcpHolder { static void OnClosed(uv_handle_t* handle); static void OnDataReceivedCb(uv_stream_t* stream, ssize_t nread, const uv_buf_t* buf); - static void DisconnectAndDispose(TcpHolder* holder); explicit TcpHolder(InspectorSocket::DelegatePointer delegate); ~TcpHolder() = default; void ReclaimUvBuf(const uv_buf_t* buf, ssize_t read); @@ -68,14 +68,10 @@ class ProtocolHandler { InspectorSocket* inspector() { return inspector_; } - - static void Shutdown(ProtocolHandler* handler) { - handler->Shutdown(); - } + virtual void Shutdown() = 0; protected: virtual ~ProtocolHandler() = default; - virtual void Shutdown() = 0; int WriteRaw(const std::vector& buffer, uv_write_cb write_cb); InspectorSocket::Delegate* delegate(); @@ -653,10 +649,10 @@ TcpHolder::Pointer TcpHolder::Accept( err = uv_read_start(tcp, allocate_buffer, OnDataReceivedCb); } if (err == 0) { - return { result, DisconnectAndDispose }; + return TcpHolder::Pointer(result); } else { delete result; - return { nullptr, nullptr }; + return nullptr; } } @@ -721,12 +717,13 @@ void TcpHolder::ReclaimUvBuf(const uv_buf_t* buf, ssize_t read) { delete[] buf->base; } -// Public interface -InspectorSocket::InspectorSocket() - : protocol_handler_(nullptr, ProtocolHandler::Shutdown) { } - InspectorSocket::~InspectorSocket() = default; +// static +void InspectorSocket::Shutdown(ProtocolHandler* handler) { + handler->Shutdown(); +} + // static InspectorSocket::Pointer InspectorSocket::Accept(uv_stream_t* server, DelegatePointer delegate) { diff --git a/src/inspector_socket.h b/src/inspector_socket.h index 81b894f95cb88f..ae49d78ff3452a 100644 --- a/src/inspector_socket.h +++ b/src/inspector_socket.h @@ -40,9 +40,10 @@ class InspectorSocket { std::string GetHost(); private: - InspectorSocket(); + static void Shutdown(ProtocolHandler*); + InspectorSocket() = default; - std::unique_ptr protocol_handler_; + DeleteFnPtr protocol_handler_; DISALLOW_COPY_AND_ASSIGN(InspectorSocket); }; diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc index 42082c9c8495ed..d5eb1b75c81ae0 100644 --- a/src/inspector_socket_server.cc +++ b/src/inspector_socket_server.cc @@ -156,43 +156,6 @@ std::string FormatWsAddress(const std::string& host, int port, return FormatAddress(FormatHostPort(host, port), target_id, include_protocol); } -class Closer { - public: - explicit Closer(InspectorSocketServer* server) : server_(server), - close_count_(0) { } - - void AddCallback(InspectorSocketServer::ServerCallback callback) { - if (callback == nullptr) - return; - callbacks_.insert(callback); - } - - void DecreaseExpectedCount() { - --close_count_; - NotifyIfDone(); - } - - void IncreaseExpectedCount() { - ++close_count_; - } - - void NotifyIfDone() { - if (close_count_ == 0) { - for (auto callback : callbacks_) { - callback(server_); - } - InspectorSocketServer* server = server_; - delete server->closer_; - server->closer_ = nullptr; - } - } - - private: - InspectorSocketServer* server_; - std::set callbacks_; - int close_count_; -}; - class SocketSession { public: SocketSession(InspectorSocketServer* server, int id, int server_port); @@ -250,45 +213,38 @@ class SocketSession { class ServerSocket { public: - static int Listen(InspectorSocketServer* inspector_server, - sockaddr* addr, uv_loop_t* loop); + explicit ServerSocket(InspectorSocketServer* server) + : tcp_socket_(uv_tcp_t()), server_(server) {} + int Listen(sockaddr* addr, uv_loop_t* loop); void Close() { - uv_close(reinterpret_cast(&tcp_socket_), - SocketClosedCallback); + uv_close(reinterpret_cast(&tcp_socket_), FreeOnCloseCallback); } int port() const { return port_; } private: - explicit ServerSocket(InspectorSocketServer* server) - : tcp_socket_(uv_tcp_t()), server_(server), port_(-1) {} template static ServerSocket* FromTcpSocket(UvHandle* socket) { return node::ContainerOf(&ServerSocket::tcp_socket_, reinterpret_cast(socket)); } static void SocketConnectedCallback(uv_stream_t* tcp_socket, int status); - static void SocketClosedCallback(uv_handle_t* tcp_socket); static void FreeOnCloseCallback(uv_handle_t* tcp_socket_) { delete FromTcpSocket(tcp_socket_); } int DetectPort(); + ~ServerSocket() = default; uv_tcp_t tcp_socket_; InspectorSocketServer* server_; - int port_; + int port_ = -1; }; -InspectorSocketServer::InspectorSocketServer(SocketServerDelegate* delegate, - uv_loop_t* loop, - const std::string& host, - int port, - FILE* out) : loop_(loop), - delegate_(delegate), - host_(host), - port_(port), - closer_(nullptr), - next_session_id_(0), - out_(out) { +InspectorSocketServer::InspectorSocketServer( + std::unique_ptr delegate, uv_loop_t* loop, + const std::string& host, int port, FILE* out) + : loop_(loop), delegate_(std::move(delegate)), host_(host), port_(port), + next_session_id_(0), out_(out) { + delegate_->AssignServer(this); state_ = ServerState::kNew; } @@ -328,7 +284,7 @@ void InspectorSocketServer::SessionTerminated(int session_id) { delegate_->GetTargetIds(), out_); } if (state_ == ServerState::kStopped) { - delegate_->ServerDone(); + delegate_.reset(); } } } @@ -389,7 +345,11 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket, } bool InspectorSocketServer::Start() { + CHECK_NE(delegate_, nullptr); CHECK_EQ(state_, ServerState::kNew); + std::unique_ptr delegate_holder; + // We will return it if startup is successful + delegate_.swap(delegate_holder); struct addrinfo hints; memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_NUMERICSERV; @@ -407,13 +367,13 @@ bool InspectorSocketServer::Start() { } for (addrinfo* address = req.addrinfo; address != nullptr; address = address->ai_next) { - err = ServerSocket::Listen(this, address->ai_addr, loop_); + auto server_socket = ServerSocketPtr(new ServerSocket(this)); + err = server_socket->Listen(address->ai_addr, loop_); + if (err == 0) + server_sockets_.push_back(std::move(server_socket)); } uv_freeaddrinfo(req.addrinfo); - if (!connected_sessions_.empty()) { - return true; - } // We only show error if we failed to start server on all addresses. We only // show one error, for the last address. if (server_sockets_.empty()) { @@ -424,6 +384,7 @@ bool InspectorSocketServer::Start() { } return false; } + delegate_.swap(delegate_holder); state_ = ServerState::kRunning; // getaddrinfo sorts the addresses, so the first port is most relevant. PrintDebuggerReadyMessage(host_, server_sockets_[0]->port(), @@ -431,17 +392,12 @@ bool InspectorSocketServer::Start() { return true; } -void InspectorSocketServer::Stop(ServerCallback cb) { +void InspectorSocketServer::Stop() { CHECK_EQ(state_, ServerState::kRunning); - if (closer_ == nullptr) { - closer_ = new Closer(this); - } - closer_->AddCallback(cb); - closer_->IncreaseExpectedCount(); - state_ = ServerState::kStopping; - for (ServerSocket* server_socket : server_sockets_) - server_socket->Close(); - closer_->NotifyIfDone(); + state_ = ServerState::kStopped; + server_sockets_.clear(); + if (done()) + delegate_.reset(); } void InspectorSocketServer::TerminateConnections() { @@ -455,28 +411,6 @@ bool InspectorSocketServer::TargetExists(const std::string& id) { return found != target_ids.end(); } -void InspectorSocketServer::ServerSocketListening(ServerSocket* server_socket) { - server_sockets_.push_back(server_socket); -} - -void InspectorSocketServer::ServerSocketClosed(ServerSocket* server_socket) { - CHECK_EQ(state_, ServerState::kStopping); - - server_sockets_.erase(std::remove(server_sockets_.begin(), - server_sockets_.end(), server_socket), - server_sockets_.end()); - if (!server_sockets_.empty()) - return; - - if (closer_ != nullptr) { - closer_->DecreaseExpectedCount(); - } - if (connected_sessions_.empty()) { - delegate_->ServerDone(); - } - state_ = ServerState::kStopped; -} - int InspectorSocketServer::Port() const { if (!server_sockets_.empty()) { return server_sockets_[0]->port(); @@ -525,6 +459,10 @@ void InspectorSocketServer::Send(int session_id, const std::string& message) { } } +void InspectorSocketServer::CloseServerSocket(ServerSocket* server) { + server->Close(); +} + // InspectorSession tracking SocketSession::SocketSession(InspectorSocketServer* server, int id, int server_port) @@ -569,11 +507,8 @@ int ServerSocket::DetectPort() { return err; } -// static -int ServerSocket::Listen(InspectorSocketServer* inspector_server, - sockaddr* addr, uv_loop_t* loop) { - ServerSocket* server_socket = new ServerSocket(inspector_server); - uv_tcp_t* server = &server_socket->tcp_socket_; +int ServerSocket::Listen(sockaddr* addr, uv_loop_t* loop) { + uv_tcp_t* server = &tcp_socket_; CHECK_EQ(0, uv_tcp_init(loop, server)); int err = uv_tcp_bind(server, addr, 0); if (err == 0) { @@ -582,12 +517,7 @@ int ServerSocket::Listen(InspectorSocketServer* inspector_server, ServerSocket::SocketConnectedCallback); } if (err == 0) { - err = server_socket->DetectPort(); - } - if (err == 0) { - inspector_server->ServerSocketListening(server_socket); - } else { - uv_close(reinterpret_cast(server), FreeOnCloseCallback); + err = DetectPort(); } return err; } @@ -601,13 +531,5 @@ void ServerSocket::SocketConnectedCallback(uv_stream_t* tcp_socket, server_socket->server_->Accept(server_socket->port_, tcp_socket); } } - -// static -void ServerSocket::SocketClosedCallback(uv_handle_t* tcp_socket) { - ServerSocket* server_socket = ServerSocket::FromTcpSocket(tcp_socket); - server_socket->server_->ServerSocketClosed(server_socket); - delete server_socket; -} - } // namespace inspector } // namespace node diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h index f6003c4c4b4d70..b8d98e13a26b62 100644 --- a/src/inspector_socket_server.h +++ b/src/inspector_socket_server.h @@ -16,19 +16,20 @@ namespace node { namespace inspector { -class Closer; +class InspectorSocketServer; class SocketSession; class ServerSocket; class SocketServerDelegate { public: + virtual void AssignServer(InspectorSocketServer* server) = 0; virtual void StartSession(int session_id, const std::string& target_id) = 0; virtual void EndSession(int session_id) = 0; virtual void MessageReceived(int session_id, const std::string& message) = 0; virtual std::vector GetTargetIds() = 0; virtual std::string GetTargetTitle(const std::string& id) = 0; virtual std::string GetTargetUrl(const std::string& id) = 0; - virtual void ServerDone() = 0; + virtual ~SocketServerDelegate() {} }; // HTTP Server, writes messages requested as TransportActions, and responds @@ -36,8 +37,7 @@ class SocketServerDelegate { class InspectorSocketServer { public: - using ServerCallback = void (*)(InspectorSocketServer*); - InspectorSocketServer(SocketServerDelegate* delegate, + InspectorSocketServer(std::unique_ptr delegate, uv_loop_t* loop, const std::string& host, int port, @@ -49,7 +49,7 @@ class InspectorSocketServer { // Called by the TransportAction sent with InspectorIo::Write(): // kKill and kStop - void Stop(ServerCallback callback); + void Stop(); // kSendMessage void Send(int session_id, const std::string& message); // kKill @@ -58,13 +58,8 @@ class InspectorSocketServer { void AcceptSession(int session_id); // kDeclineSession void DeclineSession(int session_id); - int Port() const; - // Server socket lifecycle. There may be multiple sockets - void ServerSocketListening(ServerSocket* server_socket); - void ServerSocketClosed(ServerSocket* server_socket); - // Session connection lifecycle void Accept(int server_port, uv_stream_t* server_socket); bool HandleGetRequest(int session_id, const std::string& host, @@ -76,27 +71,30 @@ class InspectorSocketServer { delegate_->MessageReceived(session_id, message); } SocketSession* Session(int session_id); + bool done() const { + return server_sockets_.empty() && connected_sessions_.empty(); + } private: + static void CloseServerSocket(ServerSocket*); + using ServerSocketPtr = DeleteFnPtr; + void SendListResponse(InspectorSocket* socket, const std::string& host, SocketSession* session); bool TargetExists(const std::string& id); enum class ServerState {kNew, kRunning, kStopping, kStopped}; uv_loop_t* loop_; - SocketServerDelegate* const delegate_; + std::unique_ptr delegate_; const std::string host_; int port_; std::string path_; - std::vector server_sockets_; - Closer* closer_; + std::vector server_sockets_; std::map>> connected_sessions_; int next_session_id_; FILE* out_; ServerState state_; - - friend class Closer; }; } // namespace inspector diff --git a/test/cctest/test_inspector_socket_server.cc b/test/cctest/test_inspector_socket_server.cc index 9023ad43c1c75c..60b7eefc5e997f 100644 --- a/test/cctest/test_inspector_socket_server.cc +++ b/test/cctest/test_inspector_socket_server.cc @@ -84,78 +84,6 @@ class InspectorSocketServerTest : public ::testing::Test { } }; -class TestInspectorServerDelegate : public SocketServerDelegate { - public: - TestInspectorServerDelegate() : connected(0), disconnected(0), done(false), - targets_({ MAIN_TARGET_ID, - UNCONNECTABLE_TARGET_ID }), - session_id_(0) {} - - void Connect(InspectorSocketServer* server) { - server_ = server; - } - - void StartSession(int session_id, const std::string& target_id) override { - buffer_.clear(); - CHECK_NE(targets_.end(), - std::find(targets_.begin(), targets_.end(), target_id)); - if (target_id == UNCONNECTABLE_TARGET_ID) { - server_->DeclineSession(session_id); - return; - } - connected++; - session_id_ = session_id; - server_->AcceptSession(session_id); - } - - void MessageReceived(int session_id, const std::string& message) override { - CHECK_EQ(session_id_, session_id); - buffer_.insert(buffer_.end(), message.begin(), message.end()); - } - - void EndSession(int session_id) override { - CHECK_EQ(session_id_, session_id); - disconnected++; - } - - std::vector GetTargetIds() override { - return targets_; - } - - std::string GetTargetTitle(const std::string& id) override { - return id + " Target Title"; - } - - std::string GetTargetUrl(const std::string& id) override { - return "file://" + id + "/script.js"; - } - - void Expect(const std::string& expects) { - SPIN_WHILE(buffer_.size() < expects.length()); - ASSERT_STREQ(std::string(buffer_.data(), expects.length()).c_str(), - expects.c_str()); - buffer_.erase(buffer_.begin(), buffer_.begin() + expects.length()); - } - - void Write(const std::string& message) { - server_->Send(session_id_, message); - } - - void ServerDone() override { - done = true; - } - - int connected; - int disconnected; - bool done; - - private: - const std::vector targets_; - InspectorSocketServer* server_; - int session_id_; - std::vector buffer_; -}; - class SocketWrapper { public: explicit SocketWrapper(uv_loop_t* loop) : closed_(false), @@ -312,76 +240,136 @@ class SocketWrapper { class ServerHolder { public: - template - ServerHolder(Delegate* delegate, uv_loop_t* loop, int port) - : ServerHolder(delegate, loop, HOST, port, nullptr) { } + ServerHolder(bool has_targets, uv_loop_t* loop, int port) + : ServerHolder(has_targets, loop, HOST, port, nullptr) { } - template - ServerHolder(Delegate* delegate, uv_loop_t* loop, const std::string host, - int port, FILE* out) - : closed(false), paused(false), - server_(delegate, loop, host, port, out) { - delegate->Connect(&server_); - } + ServerHolder(bool has_targets, uv_loop_t* loop, + const std::string host, int port, FILE* out); InspectorSocketServer* operator->() { - return &server_; + return server_.get(); } int port() { - return server_.Port(); + return server_->Port(); } - static void CloseCallback(InspectorSocketServer* server) { - ServerHolder* holder = node::ContainerOf(&ServerHolder::server_, server); - holder->closed = true; + bool done() { + return server_->done(); } - static void PausedCallback(InspectorSocketServer* server) { - ServerHolder* holder = node::ContainerOf(&ServerHolder::server_, server); - holder->paused = true; + void Connected() { + connected++; + } + + void Disconnected() { + disconnected++; + } + + void Done() { + delegate_done = true; + } + + void PrepareSession(int id) { + buffer_.clear(); + session_id_ = id; + } + + void Received(const std::string& message) { + buffer_.insert(buffer_.end(), message.begin(), message.end()); } - bool closed; - bool paused; + void Write(const std::string& message) { + server_->Send(session_id_, message); + } + + void Expect(const std::string& expects) { + SPIN_WHILE(buffer_.size() < expects.length()); + ASSERT_STREQ(std::string(buffer_.data(), expects.length()).c_str(), + expects.c_str()); + buffer_.erase(buffer_.begin(), buffer_.begin() + expects.length()); + } + + int connected = 0; + int disconnected = 0; + bool delegate_done = false; private: - InspectorSocketServer server_; + std::unique_ptr server_; + std::vector buffer_; + int session_id_; }; -class ServerDelegateNoTargets : public SocketServerDelegate { +class TestSocketServerDelegate : public SocketServerDelegate { public: - ServerDelegateNoTargets() : server_(nullptr) { } - void Connect(InspectorSocketServer* server) { } - void MessageReceived(int session_id, const std::string& message) override { } - void EndSession(int session_id) override { } + explicit TestSocketServerDelegate( + ServerHolder* server, + const std::vector& target_ids) + : harness_(server), + targets_(target_ids), + session_id_(0) {} + + ~TestSocketServerDelegate() { + harness_->Done(); + } + + void AssignServer(InspectorSocketServer* server) override { + server_ = server; + } void StartSession(int session_id, const std::string& target_id) override { - server_->DeclineSession(session_id); + session_id_ = session_id; + harness_->PrepareSession(session_id_); + CHECK_NE(targets_.end(), + std::find(targets_.begin(), targets_.end(), target_id)); + if (target_id == UNCONNECTABLE_TARGET_ID) { + server_->DeclineSession(session_id); + return; + } + harness_->Connected(); + server_->AcceptSession(session_id); } - std::vector GetTargetIds() override { - return std::vector(); + void MessageReceived(int session_id, const std::string& message) override { + CHECK_EQ(session_id_, session_id); + harness_->Received(message); } - std::string GetTargetTitle(const std::string& id) override { - return ""; + void EndSession(int session_id) override { + CHECK_EQ(session_id_, session_id); + harness_->Disconnected(); } - std::string GetTargetUrl(const std::string& id) override { - return ""; + std::vector GetTargetIds() override { + return targets_; } - void ServerDone() override { - done = true; + std::string GetTargetTitle(const std::string& id) override { + return id + " Target Title"; } - bool done = false; + std::string GetTargetUrl(const std::string& id) override { + return "file://" + id + "/script.js"; + } private: + ServerHolder* harness_; + const std::vector targets_; InspectorSocketServer* server_; + int session_id_; }; +ServerHolder::ServerHolder(bool has_targets, uv_loop_t* loop, + const std::string host, int port, FILE* out) { + std::vector targets; + if (has_targets) + targets = { MAIN_TARGET_ID, UNCONNECTABLE_TARGET_ID }; + std::unique_ptr delegate( + new TestSocketServerDelegate(this, targets)); + server_.reset( + new InspectorSocketServer(std::move(delegate), loop, host, port, out)); +} + static void TestHttpRequest(int port, const std::string& path, const std::string& expected_body) { SocketWrapper socket(&loop); @@ -402,8 +390,7 @@ static const std::string WsHandshakeRequest(const std::string& target_id) { TEST_F(InspectorSocketServerTest, InspectorSessions) { - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(true, &loop, 0); ASSERT_TRUE(server->Start()); SocketWrapper well_behaved_socket(&loop); @@ -412,18 +399,18 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { well_behaved_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID)); well_behaved_socket.Expect(WS_HANDSHAKE_RESPONSE); - EXPECT_EQ(1, delegate.connected); + EXPECT_EQ(1, server.connected); well_behaved_socket.Write("\x81\x84\x7F\xC2\x66\x31\x4E\xF0\x55\x05"); - delegate.Expect("1234"); - delegate.Write("5678"); + server.Expect("1234"); + server.Write("5678"); well_behaved_socket.Expect("\x81\x4" "5678"); well_behaved_socket.Write(CLIENT_CLOSE_FRAME); well_behaved_socket.Expect(SERVER_CLOSE_FRAME); - EXPECT_EQ(1, delegate.disconnected); + EXPECT_EQ(1, server.disconnected); well_behaved_socket.Close(); @@ -433,8 +420,8 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { declined_target_socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID)); declined_target_socket.Expect("HTTP/1.0 400 Bad Request"); declined_target_socket.ExpectEOF(); - EXPECT_EQ(1, delegate.connected); - EXPECT_EQ(1, delegate.disconnected); + EXPECT_EQ(1, server.connected); + EXPECT_EQ(1, server.disconnected); // Bogus target - start session callback should not even be invoked SocketWrapper bogus_target_socket(&loop); @@ -442,8 +429,8 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { bogus_target_socket.Write(WsHandshakeRequest("bogus_target")); bogus_target_socket.Expect("HTTP/1.0 400 Bad Request"); bogus_target_socket.ExpectEOF(); - EXPECT_EQ(1, delegate.connected); - EXPECT_EQ(1, delegate.disconnected); + EXPECT_EQ(1, server.connected); + EXPECT_EQ(1, server.disconnected); // Drop connection (no proper close frames) SocketWrapper dropped_connection_socket(&loop); @@ -451,13 +438,13 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { dropped_connection_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID)); dropped_connection_socket.Expect(WS_HANDSHAKE_RESPONSE); - EXPECT_EQ(2, delegate.connected); + EXPECT_EQ(2, server.connected); - delegate.Write("5678"); + server.Write("5678"); dropped_connection_socket.Expect("\x81\x4" "5678"); dropped_connection_socket.Close(); - SPIN_WHILE(delegate.disconnected < 2); + SPIN_WHILE(server.disconnected < 2); // Reconnect regular connection SocketWrapper stays_till_termination_socket(&loop); @@ -465,41 +452,38 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) { stays_till_termination_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID)); stays_till_termination_socket.Expect(WS_HANDSHAKE_RESPONSE); - SPIN_WHILE(3 != delegate.connected); + SPIN_WHILE(3 != server.connected); - delegate.Write("5678"); + server.Write("5678"); stays_till_termination_socket.Expect("\x81\x4" "5678"); stays_till_termination_socket .Write("\x81\x84\x7F\xC2\x66\x31\x4E\xF0\x55\x05"); - delegate.Expect("1234"); + server.Expect("1234"); - server->Stop(ServerHolder::CloseCallback); + server->Stop(); server->TerminateConnections(); stays_till_termination_socket.Write(CLIENT_CLOSE_FRAME); stays_till_termination_socket.Expect(SERVER_CLOSE_FRAME); - SPIN_WHILE(3 != delegate.disconnected); - - SPIN_WHILE(!server.closed); + SPIN_WHILE(3 != server.disconnected); + SPIN_WHILE(!server.done()); stays_till_termination_socket.ExpectEOF(); } TEST_F(InspectorSocketServerTest, ServerDoesNothing) { - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(true, &loop, 0); ASSERT_TRUE(server->Start()); - - server->Stop(ServerHolder::CloseCallback); + server->Stop(); server->TerminateConnections(); - SPIN_WHILE(!server.closed); - ASSERT_TRUE(delegate.done); + SPIN_WHILE(!server.done()); + ASSERT_TRUE(server.delegate_done); + SPIN_WHILE(uv_loop_alive(&loop)); } TEST_F(InspectorSocketServerTest, ServerWithoutTargets) { - ServerDelegateNoTargets delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(false, &loop, 0); ASSERT_TRUE(server->Start()); TestHttpRequest(server.port(), "/json/list", "[ ]"); TestHttpRequest(server.port(), "/json", "[ ]"); @@ -510,89 +494,81 @@ TEST_F(InspectorSocketServerTest, ServerWithoutTargets) { socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID)); socket.Expect("HTTP/1.0 400 Bad Request"); socket.ExpectEOF(); - server->Stop(ServerHolder::CloseCallback); + server->Stop(); server->TerminateConnections(); - SPIN_WHILE(!server.closed); + SPIN_WHILE(!server.done()); + SPIN_WHILE(uv_loop_alive(&loop)); } TEST_F(InspectorSocketServerTest, ServerCannotStart) { - ServerDelegateNoTargets delegate1, delegate2; - ServerHolder server1(&delegate1, &loop, 0); + ServerHolder server1(false, &loop, 0); ASSERT_TRUE(server1->Start()); - ServerHolder server2(&delegate2, &loop, server1.port()); + ServerHolder server2(false, &loop, server1.port()); ASSERT_FALSE(server2->Start()); - server1->Stop(ServerHolder::CloseCallback); + server1->Stop(); server1->TerminateConnections(); - SPIN_WHILE(!server1.closed); - ASSERT_TRUE(delegate1.done); + SPIN_WHILE(!server1.done()); + ASSERT_TRUE(server1.delegate_done); + SPIN_WHILE(uv_loop_alive(&loop)); } TEST_F(InspectorSocketServerTest, StoppingServerDoesNotKillConnections) { - ServerDelegateNoTargets delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(false, &loop, 0); ASSERT_TRUE(server->Start()); SocketWrapper socket1(&loop); socket1.Connect(HOST, server.port()); socket1.TestHttpRequest("/json/list", "[ ]"); - server->Stop(ServerHolder::CloseCallback); - SPIN_WHILE(!server.closed); + server->Stop(); socket1.TestHttpRequest("/json/list", "[ ]"); socket1.Close(); uv_run(&loop, UV_RUN_DEFAULT); - ASSERT_TRUE(delegate.done); + ASSERT_TRUE(server.delegate_done); } TEST_F(InspectorSocketServerTest, ClosingConnectionReportsDone) { - ServerDelegateNoTargets delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(false, &loop, 0); ASSERT_TRUE(server->Start()); SocketWrapper socket1(&loop); socket1.Connect(HOST, server.port()); socket1.TestHttpRequest("/json/list", "[ ]"); - server->Stop(ServerHolder::CloseCallback); - SPIN_WHILE(!server.closed); + server->Stop(); socket1.TestHttpRequest("/json/list", "[ ]"); socket1.Close(); uv_run(&loop, UV_RUN_DEFAULT); - ASSERT_TRUE(delegate.done); + ASSERT_TRUE(server.delegate_done); } TEST_F(InspectorSocketServerTest, ClosingSocketReportsDone) { - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(true, &loop, 0); ASSERT_TRUE(server->Start()); SocketWrapper socket1(&loop); socket1.Connect(HOST, server.port()); socket1.Write(WsHandshakeRequest(MAIN_TARGET_ID)); socket1.Expect(WS_HANDSHAKE_RESPONSE); - server->Stop(ServerHolder::CloseCallback); - SPIN_WHILE(!server.closed); - ASSERT_FALSE(delegate.done); + server->Stop(); + ASSERT_FALSE(server.delegate_done); socket1.Close(); - SPIN_WHILE(!delegate.done); + SPIN_WHILE(!server.delegate_done); } TEST_F(InspectorSocketServerTest, TerminatingSessionReportsDone) { - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, 0); + ServerHolder server(true, &loop, 0); ASSERT_TRUE(server->Start()); SocketWrapper socket1(&loop); socket1.Connect(HOST, server.port()); socket1.Write(WsHandshakeRequest(MAIN_TARGET_ID)); socket1.Expect(WS_HANDSHAKE_RESPONSE); - server->Stop(ServerHolder::CloseCallback); - SPIN_WHILE(!server.closed); - ASSERT_FALSE(delegate.done); + server->Stop(); + ASSERT_FALSE(server.delegate_done); server->TerminateConnections(); socket1.Expect(SERVER_CLOSE_FRAME); socket1.Write(CLIENT_CLOSE_FRAME); socket1.ExpectEOF(); - SPIN_WHILE(!delegate.done); + SPIN_WHILE(!server.delegate_done); } TEST_F(InspectorSocketServerTest, FailsToBindToNodejsHost) { - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, "nodejs.org", 80, nullptr); + ServerHolder server(true, &loop, "nodejs.org", 80, nullptr); ASSERT_FALSE(server->Start()); SPIN_WHILE(uv_loop_alive(&loop)); } @@ -619,17 +595,14 @@ TEST_F(InspectorSocketServerTest, BindsToIpV6) { fprintf(stderr, "No IPv6 network detected\n"); return; } - TestInspectorServerDelegate delegate; - ServerHolder server(&delegate, &loop, "::", 0, nullptr); + ServerHolder server(true, &loop, "::", 0, nullptr); ASSERT_TRUE(server->Start()); - SocketWrapper socket1(&loop); socket1.Connect("::1", server.port(), true); socket1.Write(WsHandshakeRequest(MAIN_TARGET_ID)); socket1.Expect(WS_HANDSHAKE_RESPONSE); - server->Stop(ServerHolder::CloseCallback); - SPIN_WHILE(!server.closed); - ASSERT_FALSE(delegate.done); + server->Stop(); + ASSERT_FALSE(server.delegate_done); socket1.Close(); - SPIN_WHILE(!delegate.done); + SPIN_WHILE(!server.delegate_done); }