From f6db8dde0be0ab299375c10091ee61635c9ae9dc Mon Sep 17 00:00:00 2001 From: dgozman Date: Fri, 3 Oct 2014 06:41:05 -0700 Subject: [PATCH] [DevTools] Make TetheringHandler operate on UI thread. Tethering handler now has all public methods working on UI thread. This allows to remove thread-handling in DevToolsBrowserTarget and move it to the unified protocol handler implementation. BUG=405566 Review URL: https://codereview.chromium.org/616593002 Cr-Commit-Position: refs/heads/master@{#298028} --- .../browser/devtools/devtools_http_handler_impl.cc | 4 +- content/browser/devtools/tethering_handler.cc | 204 ++++++++++++++++----- content/browser/devtools/tethering_handler.h | 19 +- 3 files changed, 175 insertions(+), 52 deletions(-) diff --git a/content/browser/devtools/devtools_http_handler_impl.cc b/content/browser/devtools/devtools_http_handler_impl.cc index b0d866e3b3d2..0e2347af6ee2 100644 --- a/content/browser/devtools/devtools_http_handler_impl.cc +++ b/content/browser/devtools/devtools_http_handler_impl.cc @@ -382,8 +382,8 @@ void DevToolsHttpHandlerImpl::OnWebSocketRequest( true /* handle on UI thread */); browser_target->RegisterDomainHandler( devtools::Tethering::kName, - new TetheringHandler(delegate_.get()), - false /* handle on this thread */); + new TetheringHandler(delegate_.get(), thread_->message_loop_proxy()), + true /* handle on UI thread */); browser_target->RegisterDomainHandler( devtools::SystemInfo::kName, new DevToolsSystemInfoHandler(), diff --git a/content/browser/devtools/tethering_handler.cc b/content/browser/devtools/tethering_handler.cc index d25039b8db67..8016661cd118 100644 --- a/content/browser/devtools/tethering_handler.cc +++ b/content/browser/devtools/tethering_handler.cc @@ -10,6 +10,7 @@ #include "base/values.h" #include "content/browser/devtools/devtools_http_handler_impl.h" #include "content/browser/devtools/devtools_protocol_constants.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/devtools_http_handler_delegate.h" #include "net/base/io_buffer.h" #include "net/base/ip_endpoint.h" @@ -161,13 +162,24 @@ class SocketPump : public net::StreamListenSocket::Delegate { bool pending_destruction_; }; -} // namespace +static int GetPort(scoped_refptr command, + const std::string& paramName) { + base::DictionaryValue* params = command->params(); + int port = 0; + if (!params || + !params->GetInteger(paramName, &port) || + port < kMinTetheringPort || port > kMaxTetheringPort) + return 0; + return port; +} -class TetheringHandler::BoundSocket { +class BoundSocket { public: - BoundSocket(TetheringHandler* handler, + typedef base::Callback AcceptedCallback; + + BoundSocket(AcceptedCallback accepted_callback, DevToolsHttpHandlerDelegate* delegate) - : handler_(handler), + : accepted_callback_(accepted_callback), delegate_(delegate), socket_(new net::TCPServerSocket(NULL, net::NetLog::Source())), port_(0) { @@ -224,18 +236,127 @@ class TetheringHandler::BoundSocket { SocketPump* pump = new SocketPump(delegate_, accept_socket_.release()); std::string name = pump->Init(); if (!name.empty()) - handler_->Accepted(port_, name); + accepted_callback_.Run(port_, name); } - TetheringHandler* handler_; + AcceptedCallback accepted_callback_; DevToolsHttpHandlerDelegate* delegate_; scoped_ptr socket_; scoped_ptr accept_socket_; int port_; }; -TetheringHandler::TetheringHandler(DevToolsHttpHandlerDelegate* delegate) - : delegate_(delegate) { +} // namespace + +// TetheringHandler::TetheringImpl ------------------------------------------- + +class TetheringHandler::TetheringImpl { + public: + TetheringImpl( + base::WeakPtr handler, + DevToolsHttpHandlerDelegate* delegate); + ~TetheringImpl(); + + void Bind(scoped_refptr command); + void Unbind(scoped_refptr command); + void Accepted(int port, const std::string& name); + + private: + void SendAsyncResponse(scoped_refptr response); + + base::WeakPtr handler_; + DevToolsHttpHandlerDelegate* delegate_; + + typedef std::map BoundSockets; + BoundSockets bound_sockets_; +}; + +TetheringHandler::TetheringImpl::TetheringImpl( + base::WeakPtr handler, + DevToolsHttpHandlerDelegate* delegate) + : handler_(handler), + delegate_(delegate) { +} + +TetheringHandler::TetheringImpl::~TetheringImpl() { + STLDeleteContainerPairSecondPointers(bound_sockets_.begin(), + bound_sockets_.end()); +} + +void TetheringHandler::TetheringImpl::Bind( + scoped_refptr command) { + const std::string& portParamName = devtools::Tethering::bind::kParamPort; + int port = GetPort(command, portParamName); + if (port == 0) { + SendAsyncResponse(command->InvalidParamResponse(portParamName)); + return; + } + + if (bound_sockets_.find(port) != bound_sockets_.end()) { + SendAsyncResponse(command->InternalErrorResponse("Port already bound")); + return; + } + + BoundSocket::AcceptedCallback callback = base::Bind( + &TetheringHandler::TetheringImpl::Accepted, base::Unretained(this)); + scoped_ptr bound_socket(new BoundSocket(callback, delegate_)); + if (!bound_socket->Listen(port)) { + SendAsyncResponse(command->InternalErrorResponse("Could not bind port")); + return; + } + + bound_sockets_[port] = bound_socket.release(); + SendAsyncResponse(command->SuccessResponse(NULL)); +} + +void TetheringHandler::TetheringImpl::Unbind( + scoped_refptr command) { + const std::string& portParamName = devtools::Tethering::unbind::kParamPort; + int port = GetPort(command, portParamName); + if (port == 0) { + SendAsyncResponse(command->InvalidParamResponse(portParamName)); + return; + } + + BoundSockets::iterator it = bound_sockets_.find(port); + if (it == bound_sockets_.end()) { + SendAsyncResponse(command->InternalErrorResponse("Port is not bound")); + return; + } + + delete it->second; + bound_sockets_.erase(it); + SendAsyncResponse(command->SuccessResponse(NULL)); +} + +void TetheringHandler::TetheringImpl::Accepted( + int port, const std::string& name) { + BrowserThread::PostTask( + BrowserThread::UI, + FROM_HERE, + base::Bind(&TetheringHandler::Accepted, handler_, port, name)); +} + +void TetheringHandler::TetheringImpl::SendAsyncResponse( + scoped_refptr response) { + BrowserThread::PostTask( + BrowserThread::UI, + FROM_HERE, + base::Bind(&TetheringHandler::SendAsyncResponse, handler_, response)); +} + +// TetheringHandler ---------------------------------------------------------- + +// static +TetheringHandler::TetheringImpl* TetheringHandler::impl_ = nullptr; + +TetheringHandler::TetheringHandler( + DevToolsHttpHandlerDelegate* delegate, + scoped_refptr message_loop_proxy) + : delegate_(delegate), + message_loop_proxy_(message_loop_proxy), + is_active_(false), + weak_factory_(this) { RegisterCommandHandler(devtools::Tethering::bind::kName, base::Bind(&TetheringHandler::OnBind, base::Unretained(this))); @@ -245,8 +366,10 @@ TetheringHandler::TetheringHandler(DevToolsHttpHandlerDelegate* delegate) } TetheringHandler::~TetheringHandler() { - STLDeleteContainerPairSecondPointers(bound_sockets_.begin(), - bound_sockets_.end()); + if (is_active_) { + message_loop_proxy_->DeleteSoon(FROM_HERE, impl_); + impl_ = nullptr; + } } void TetheringHandler::Accepted(int port, const std::string& name) { @@ -256,49 +379,40 @@ void TetheringHandler::Accepted(int port, const std::string& name) { SendNotification(devtools::Tethering::accepted::kName, params); } -static int GetPort(scoped_refptr command, - const std::string& paramName) { - base::DictionaryValue* params = command->params(); - int port = 0; - if (!params || - !params->GetInteger(paramName, &port) || - port < kMinTetheringPort || port > kMaxTetheringPort) - return 0; - return port; +bool TetheringHandler::Activate() { + if (is_active_) + return true; + if (impl_) + return false; + is_active_ = true; + impl_ = new TetheringImpl(weak_factory_.GetWeakPtr(), delegate_); + return true; } scoped_refptr TetheringHandler::OnBind(scoped_refptr command) { - const std::string& portParamName = devtools::Tethering::bind::kParamPort; - int port = GetPort(command, portParamName); - if (port == 0) - return command->InvalidParamResponse(portParamName); - - if (bound_sockets_.find(port) != bound_sockets_.end()) - return command->InternalErrorResponse("Port already bound"); - - scoped_ptr bound_socket(new BoundSocket(this, delegate_)); - if (!bound_socket->Listen(port)) - return command->InternalErrorResponse("Could not bind port"); - - bound_sockets_[port] = bound_socket.release(); - return command->SuccessResponse(NULL); + if (!Activate()) { + return command->ServerErrorResponse( + "Tethering is used by another connection"); + } + DCHECK(impl_); + message_loop_proxy_->PostTask( + FROM_HERE, + base::Bind(&TetheringImpl::Bind, base::Unretained(impl_), command)); + return command->AsyncResponsePromise(); } scoped_refptr TetheringHandler::OnUnbind(scoped_refptr command) { - const std::string& portParamName = devtools::Tethering::unbind::kParamPort; - int port = GetPort(command, portParamName); - if (port == 0) - return command->InvalidParamResponse(portParamName); - - BoundSockets::iterator it = bound_sockets_.find(port); - if (it == bound_sockets_.end()) - return command->InternalErrorResponse("Port is not bound"); - - delete it->second; - bound_sockets_.erase(it); - return command->SuccessResponse(NULL); + if (!Activate()) { + return command->ServerErrorResponse( + "Tethering is used by another connection"); + } + DCHECK(impl_); + message_loop_proxy_->PostTask( + FROM_HERE, + base::Bind(&TetheringImpl::Unbind, base::Unretained(impl_), command)); + return command->AsyncResponsePromise(); } } // namespace content diff --git a/content/browser/devtools/tethering_handler.h b/content/browser/devtools/tethering_handler.h index 36c9bd5abbba..b48b2af9e438 100644 --- a/content/browser/devtools/tethering_handler.h +++ b/content/browser/devtools/tethering_handler.h @@ -7,6 +7,8 @@ #include +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" #include "content/browser/devtools/devtools_protocol.h" namespace content { @@ -16,21 +18,28 @@ class DevToolsHttpHandlerDelegate; // This class implements reversed tethering handler. class TetheringHandler : public DevToolsProtocol::Handler { public: - TetheringHandler(DevToolsHttpHandlerDelegate* delegate); + TetheringHandler(DevToolsHttpHandlerDelegate* delegate, + scoped_refptr message_loop_proxy); virtual ~TetheringHandler(); + private: + class TetheringImpl; + void Accepted(int port, const std::string& name); + bool Activate(); - private: - class BoundSocket; scoped_refptr OnBind( scoped_refptr command); scoped_refptr OnUnbind( scoped_refptr command); - typedef std::map BoundSockets; - BoundSockets bound_sockets_; DevToolsHttpHandlerDelegate* delegate_; + scoped_refptr message_loop_proxy_; + bool is_active_; + base::WeakPtrFactory weak_factory_; + + static TetheringImpl* impl_; + DISALLOW_COPY_AND_ASSIGN(TetheringHandler); }; -- 2.11.4.GIT