Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: code cleanup #21070

Merged
merged 1 commit into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -122,11 +126,8 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
std::vector<std::string> 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;
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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,
Expand All @@ -271,11 +272,11 @@ void InspectorIo::ThreadMain() {
err = uv_async_init(&loop, &thread_req_, IoThreadAsyncCb<Transport>);
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<InspectorIoDelegate>(
new InspectorIoDelegate(this, id_, script_path, script_name_,
wait_for_connect_));
Transport server(std::move(delegate), &loop, options_.host_name(),
options_.port());
TransportAndIo<Transport> queue_transport(&server, this);
thread_req_.data = &queue_transport;
if (!server.Start()) {
Expand All @@ -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 <typename ActionType>
Expand Down Expand Up @@ -328,7 +327,7 @@ void InspectorIo::PostIncomingMessage(InspectorAction action, int session_id,
}

std::vector<std::string> InspectorIo::GetTargetIds() const {
return delegate_ ? delegate_->GetTargetIds() : std::vector<std::string>();
return { id_ };
}

TransportAction InspectorIo::Attach(int session_id) {
Expand Down Expand Up @@ -416,14 +415,15 @@ bool InspectorIo::WaitForFrontendEvent() {
}

InspectorIoDelegate::InspectorIoDelegate(InspectorIo* io,
const std::string& target_id,
const std::string& script_path,
const std::string& script_name,
bool wait)
: io_(io),
session_id_(0),
script_name_(script_name),
script_path_(script_path),
target_id_(GenerateID()),
target_id_(target_id),
waiting_(wait),
server_(nullptr) { }

Expand Down
3 changes: 2 additions & 1 deletion src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand All @@ -161,6 +160,8 @@ class InspectorIo {
const bool wait_for_connect_;
int port_;
std::unordered_map<int, std::unique_ptr<InspectorSession>> sessions_;
// May be accessed from any thread
const std::string id_;

friend class DispatchMessagesTask;
friend class IoSessionDelegate;
Expand Down
23 changes: 10 additions & 13 deletions src/inspector_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ namespace inspector {

class TcpHolder {
public:
using Pointer = std::unique_ptr<TcpHolder, void(*)(TcpHolder*)>;
static void DisconnectAndDispose(TcpHolder* holder);
using Pointer = DeleteFnPtr<TcpHolder, DisconnectAndDispose>;

static Pointer Accept(uv_stream_t* server,
InspectorSocket::DelegatePointer delegate);
Expand All @@ -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);
Expand All @@ -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<char>& buffer, uv_write_cb write_cb);
InspectorSocket::Delegate* delegate();

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/inspector_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ class InspectorSocket {
std::string GetHost();

private:
InspectorSocket();
static void Shutdown(ProtocolHandler*);
InspectorSocket() = default;

std::unique_ptr<ProtocolHandler, void(*)(ProtocolHandler*)> protocol_handler_;
DeleteFnPtr<ProtocolHandler, Shutdown> protocol_handler_;

DISALLOW_COPY_AND_ASSIGN(InspectorSocket);
};
Expand Down
Loading