Skip to content

Commit

Permalink
Revert "Revert of Replace StreamListenSocket with StreamSocket in Htt…
Browse files Browse the repository at this point in the history
…pServer. (patchset #29 of https://codereview.chromium.org/296053012/)"

This reverts commit 0b2f33f.

This is relanding CL of https://codereview.chromium.org/296053012/, which broke http server unittests because http server doesn't send response synchronously any more.
This CL fixes unittests by reading responses completely.

Patch set #1 is same to the original CL.
Patch set #2 is the diff.

BUG=371906
TBR=pfeldman@chromium.org,darin@chromium.org,gunsch@chromium.org,mnaganov@chromium.org

Review URL: https://codereview.chromium.org/487013003

Cr-Commit-Position: refs/heads/master@{#291784}
  • Loading branch information
byungchul authored and Commit bot committed Aug 25, 2014
1 parent b71e30c commit 38c3ae7
Show file tree
Hide file tree
Showing 32 changed files with 1,440 additions and 388 deletions.
29 changes: 24 additions & 5 deletions android_webview/native/aw_dev_tools_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/common/user_agent.h"
#include "jni/AwDevToolsServer_jni.h"
#include "net/socket/unix_domain_listen_socket_posix.h"
#include "net/socket/unix_domain_server_socket_posix.h"

using content::DevToolsAgentHost;
using content::RenderViewHost;
Expand Down Expand Up @@ -165,6 +165,25 @@ std::string GetViewDescription(WebContents* web_contents) {
return json;
}

// Factory for UnixDomainServerSocket.
class UnixDomainServerSocketFactory
: public content::DevToolsHttpHandler::ServerSocketFactory {
public:
explicit UnixDomainServerSocketFactory(const std::string& socket_name)
: content::DevToolsHttpHandler::ServerSocketFactory(socket_name, 0, 1) {}

private:
// content::DevToolsHttpHandler::ServerSocketFactory.
virtual scoped_ptr<net::ServerSocket> Create() const OVERRIDE {
return scoped_ptr<net::ServerSocket>(
new net::UnixDomainServerSocket(
base::Bind(&content::CanUserConnectToDevTools),
true /* use_abstract_namespace */));
}

DISALLOW_COPY_AND_ASSIGN(UnixDomainServerSocketFactory);
};

} // namespace

namespace android_webview {
Expand All @@ -181,11 +200,11 @@ void AwDevToolsServer::Start() {
if (protocol_handler_)
return;

scoped_ptr<content::DevToolsHttpHandler::ServerSocketFactory> factory(
new UnixDomainServerSocketFactory(
base::StringPrintf(kSocketNameFormat, getpid())));
protocol_handler_ = content::DevToolsHttpHandler::Start(
new net::deprecated::UnixDomainListenSocketWithAbstractNamespaceFactory(
base::StringPrintf(kSocketNameFormat, getpid()),
"",
base::Bind(&content::CanUserConnectToDevTools)),
factory.Pass(),
base::StringPrintf(kFrontEndURL, content::GetWebKitRevision().c_str()),
new AwDevToolsServerDelegate(),
base::FilePath());
Expand Down
53 changes: 48 additions & 5 deletions chrome/browser/android/dev_tools_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@
#include "content/public/common/user_agent.h"
#include "grit/browser_resources.h"
#include "jni/DevToolsServer_jni.h"
#include "net/base/net_errors.h"
#include "net/socket/unix_domain_listen_socket_posix.h"
#include "net/socket/unix_domain_server_socket_posix.h"
#include "net/url_request/url_request_context_getter.h"
#include "ui/base/resource/resource_bundle.h"

Expand Down Expand Up @@ -418,6 +420,49 @@ class DevToolsServerDelegate : public content::DevToolsHttpHandlerDelegate {
DISALLOW_COPY_AND_ASSIGN(DevToolsServerDelegate);
};

// Factory for UnixDomainServerSocket. It tries a fallback socket when
// original socket doesn't work.
class UnixDomainServerSocketFactory
: public content::DevToolsHttpHandler::ServerSocketFactory {
public:
UnixDomainServerSocketFactory(
const std::string& socket_name,
const net::UnixDomainServerSocket::AuthCallback& auth_callback)
: content::DevToolsHttpHandler::ServerSocketFactory(socket_name, 0, 1),
auth_callback_(auth_callback) {
}

private:
// content::DevToolsHttpHandler::ServerSocketFactory.
virtual scoped_ptr<net::ServerSocket> Create() const OVERRIDE {
return scoped_ptr<net::ServerSocket>(
new net::UnixDomainServerSocket(auth_callback_,
true /* use_abstract_namespace */));
}

virtual scoped_ptr<net::ServerSocket> CreateAndListen() const OVERRIDE {
scoped_ptr<net::ServerSocket> socket = Create();
if (!socket)
return scoped_ptr<net::ServerSocket>();

if (socket->ListenWithAddressAndPort(address_, port_, backlog_) == net::OK)
return socket.Pass();

// Try a fallback socket name.
const std::string fallback_address(
base::StringPrintf("%s_%d", address_.c_str(), getpid()));
if (socket->ListenWithAddressAndPort(fallback_address, port_, backlog_)
== net::OK)
return socket.Pass();

return scoped_ptr<net::ServerSocket>();
}

const net::UnixDomainServerSocket::AuthCallback auth_callback_;

DISALLOW_COPY_AND_ASSIGN(UnixDomainServerSocketFactory);
};

} // namespace

DevToolsServer::DevToolsServer(const std::string& socket_name_prefix)
Expand All @@ -444,12 +489,10 @@ void DevToolsServer::Start(bool allow_debug_permission) {
allow_debug_permission ?
base::Bind(&AuthorizeSocketAccessWithDebugPermission) :
base::Bind(&content::CanUserConnectToDevTools);

scoped_ptr<content::DevToolsHttpHandler::ServerSocketFactory> factory(
new UnixDomainServerSocketFactory(socket_name_, auth_callback));
protocol_handler_ = content::DevToolsHttpHandler::Start(
new net::deprecated::UnixDomainListenSocketWithAbstractNamespaceFactory(
socket_name_,
base::StringPrintf("%s_%d", socket_name_.c_str(), getpid()),
auth_callback),
factory.Pass(),
base::StringPrintf(kFrontEndURL, content::GetWebKitRevision().c_str()),
new DevToolsServerDelegate(auth_callback),
base::FilePath());
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/devtools/device/android_web_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ void WebSocketImpl::OnBytesRead(
return;
}

std::string data = std::string(response_buffer->data(), result);
response_buffer_ += data;
response_buffer_.append(response_buffer->data(), result);

int bytes_consumed;
std::string output;
Expand Down
27 changes: 25 additions & 2 deletions chrome/browser/devtools/remote_debugging_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,28 @@
#include "chrome/browser/ui/webui/devtools_ui.h"
#include "chrome/common/chrome_paths.h"
#include "content/public/browser/devtools_http_handler.h"
#include "net/socket/tcp_listen_socket.h"
#include "net/socket/tcp_server_socket.h"

namespace {

class TCPServerSocketFactory
: public content::DevToolsHttpHandler::ServerSocketFactory {
public:
TCPServerSocketFactory(const std::string& address, int port, int backlog)
: content::DevToolsHttpHandler::ServerSocketFactory(
address, port, backlog) {}

private:
// content::DevToolsHttpHandler::ServerSocketFactory.
virtual scoped_ptr<net::ServerSocket> Create() const OVERRIDE {
return scoped_ptr<net::ServerSocket>(
new net::TCPServerSocket(NULL, net::NetLog::Source()));
}

DISALLOW_COPY_AND_ASSIGN(TCPServerSocketFactory);
};

} // namespace

RemoteDebuggingServer::RemoteDebuggingServer(
chrome::HostDesktopType host_desktop_type,
Expand All @@ -24,8 +45,10 @@ RemoteDebuggingServer::RemoteDebuggingServer(
DCHECK(result);
}

scoped_ptr<content::DevToolsHttpHandler::ServerSocketFactory> factory(
new TCPServerSocketFactory(ip, port, 1));
devtools_http_handler_ = content::DevToolsHttpHandler::Start(
new net::TCPListenSocketFactory(ip, port),
factory.Pass(),
"",
new BrowserListTabContentsProvider(host_desktop_type),
output_dir);
Expand Down
17 changes: 8 additions & 9 deletions chrome/test/chromedriver/net/net_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include "net/base/net_errors.h"
#include "net/server/http_server.h"
#include "net/server/http_server_request_info.h"
#include "net/socket/tcp_listen_socket.h"
#include "net/socket/tcp_server_socket.h"
#include "net/url_request/url_request_context_getter.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -54,16 +54,18 @@ class FetchUrlTest : public testing::Test,
}

void InitOnIO(base::WaitableEvent* event) {
net::TCPListenSocketFactory factory("127.0.0.1", 0);
server_ = new net::HttpServer(factory, this);
scoped_ptr<net::ServerSocket> server_socket(
new net::TCPServerSocket(NULL, net::NetLog::Source()));
server_socket->ListenWithAddressAndPort("127.0.0.1", 0, 1);
server_.reset(new net::HttpServer(server_socket.Pass(), this));
net::IPEndPoint address;
CHECK_EQ(net::OK, server_->GetLocalAddress(&address));
server_url_ = base::StringPrintf("http://127.0.0.1:%d", address.port());
event->Signal();
}

void DestroyServerOnIO(base::WaitableEvent* event) {
server_ = NULL;
server_.reset(NULL);
event->Signal();
}

Expand All @@ -78,10 +80,7 @@ class FetchUrlTest : public testing::Test,
server_->Send404(connection_id);
break;
case kClose:
// net::HttpServer doesn't allow us to close connection during callback.
base::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&net::HttpServer::Close, server_, connection_id));
server_->Close(connection_id);
break;
default:
break;
Expand All @@ -104,7 +103,7 @@ class FetchUrlTest : public testing::Test,

base::Thread io_thread_;
ServerResponse response_;
scoped_refptr<net::HttpServer> server_;
scoped_ptr<net::HttpServer> server_;
scoped_refptr<URLRequestContextGetter> context_getter_;
std::string server_url_;
};
Expand Down
23 changes: 9 additions & 14 deletions chrome/test/chromedriver/net/test_http_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "net/base/ip_endpoint.h"
#include "net/base/net_errors.h"
#include "net/server/http_server_request_info.h"
#include "net/socket/tcp_listen_socket.h"
#include "net/socket/tcp_server_socket.h"
#include "testing/gtest/include/gtest/gtest.h"

TestHttpServer::TestHttpServer()
Expand Down Expand Up @@ -92,10 +92,7 @@ void TestHttpServer::OnWebSocketRequest(
server_->Send404(connection_id);
break;
case kClose:
// net::HttpServer doesn't allow us to close connection during callback.
base::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&net::HttpServer::Close, server_, connection_id));
server_->Close(connection_id);
break;
}
}
Expand All @@ -112,10 +109,7 @@ void TestHttpServer::OnWebSocketMessage(int connection_id,
server_->SendOverWebSocket(connection_id, data);
break;
case kCloseOnMessage:
// net::HttpServer doesn't allow us to close connection during callback.
base::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&net::HttpServer::Close, server_, connection_id));
server_->Close(connection_id);
break;
}
}
Expand All @@ -128,8 +122,10 @@ void TestHttpServer::OnClose(int connection_id) {

void TestHttpServer::StartOnServerThread(bool* success,
base::WaitableEvent* event) {
net::TCPListenSocketFactory factory("127.0.0.1", 0);
server_ = new net::HttpServer(factory, this);
scoped_ptr<net::ServerSocket> server_socket(
new net::TCPServerSocket(NULL, net::NetLog::Source()));
server_socket->ListenWithAddressAndPort("127.0.0.1", 0, 1);
server_.reset(new net::HttpServer(server_socket.Pass(), this));

net::IPEndPoint address;
int error = server_->GetLocalAddress(&address);
Expand All @@ -139,14 +135,13 @@ void TestHttpServer::StartOnServerThread(bool* success,
web_socket_url_ = GURL(base::StringPrintf("ws://127.0.0.1:%d",
address.port()));
} else {
server_ = NULL;
server_.reset(NULL);
}
*success = server_.get();
event->Signal();
}

void TestHttpServer::StopOnServerThread(base::WaitableEvent* event) {
if (server_.get())
server_ = NULL;
server_.reset(NULL);
event->Signal();
}
2 changes: 1 addition & 1 deletion chrome/test/chromedriver/net/test_http_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class TestHttpServer : public net::HttpServer::Delegate {
base::Thread thread_;

// Access only on the server thread.
scoped_refptr<net::HttpServer> server_;
scoped_ptr<net::HttpServer> server_;

// Access only on the server thread.
std::set<int> connections_;
Expand Down
10 changes: 6 additions & 4 deletions chrome/test/chromedriver/server/chromedriver_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#include "net/server/http_server.h"
#include "net/server/http_server_request_info.h"
#include "net/server/http_server_response_info.h"
#include "net/socket/tcp_listen_socket.h"
#include "net/socket/tcp_server_socket.h"

namespace {

Expand All @@ -55,8 +55,10 @@ class HttpServer : public net::HttpServer::Delegate {
std::string binding_ip = kLocalHostAddress;
if (allow_remote)
binding_ip = "0.0.0.0";
server_ = new net::HttpServer(
net::TCPListenSocketFactory(binding_ip, port), this);
scoped_ptr<net::ServerSocket> server_socket(
new net::TCPServerSocket(NULL, net::NetLog::Source()));
server_socket->ListenWithAddressAndPort(binding_ip, port, 1);
server_.reset(new net::HttpServer(server_socket.Pass(), this));
net::IPEndPoint address;
return server_->GetLocalAddress(&address) == net::OK;
}
Expand Down Expand Up @@ -89,7 +91,7 @@ class HttpServer : public net::HttpServer::Delegate {
}

HttpRequestHandlerFunc handle_request_func_;
scoped_refptr<net::HttpServer> server_;
scoped_ptr<net::HttpServer> server_;
base::WeakPtrFactory<HttpServer> weak_factory_; // Should be last.
};

Expand Down
Loading

0 comments on commit 38c3ae7

Please sign in to comment.