Skip to content

Commit

Permalink
Rename/refactor WebSocket classes for readability
Browse files Browse the repository at this point in the history 
BUG=none
R=yhirano

Review-Url: https://codereview.chromium.org/2132473002
Cr-Commit-Position: refs/heads/master@{#406838}
  • Loading branch information
@tyoshino
tyoshino authored and Commit bot committed Jul 21, 2016
1 parent fc4b68d commit ccfcfde
Show file tree
Hide file tree
Showing 11 changed files with 185 additions and 153 deletions.
25 changes: 11 additions & 14 deletions net/websockets/websocket_basic_handshake_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,15 +293,15 @@ WebSocketBasicHandshakeStream::WebSocketBasicHandshakeStream(
bool using_proxy,
std::vector<std::string> requested_sub_protocols,
std::vector<std::string> requested_extensions,
std::string* failure_message)
WebSocketStreamRequest* request)
: state_(connection.release(), using_proxy),
connect_delegate_(connect_delegate),
http_response_info_(nullptr),
requested_sub_protocols_(requested_sub_protocols),
requested_extensions_(requested_extensions),
failure_message_(failure_message) {
stream_request_(request) {
DCHECK(connect_delegate);
DCHECK(failure_message);
DCHECK(request);
}

WebSocketBasicHandshakeStream::~WebSocketBasicHandshakeStream() {}
Expand Down Expand Up @@ -541,10 +541,9 @@ int WebSocketBasicHandshakeStream::ValidateResponse(int rv) {
// Reporting "Unexpected response code: 200" in this case is not
// helpful, so use a different error message.
if (headers->GetHttpVersion() == HttpVersion(0, 9)) {
set_failure_message(
"Error during WebSocket handshake: Invalid status line");
OnFailure("Error during WebSocket handshake: Invalid status line");
} else {
set_failure_message(base::StringPrintf(
OnFailure(base::StringPrintf(
"Error during WebSocket handshake: Unexpected response code: %d",
headers->response_code()));
}
Expand All @@ -553,12 +552,11 @@ int WebSocketBasicHandshakeStream::ValidateResponse(int rv) {
}
} else {
if (rv == ERR_EMPTY_RESPONSE) {
set_failure_message(
"Connection closed before receiving a handshake response");
OnFailure("Connection closed before receiving a handshake response");
return rv;
}
set_failure_message(std::string("Error during WebSocket handshake: ") +
ErrorToString(rv));
OnFailure(std::string("Error during WebSocket handshake: ") +
ErrorToString(rv));
OnFinishOpeningHandshake();
// Some error codes (for example ERR_CONNECTION_CLOSED) get changed to OK at
// higher levels. To prevent an unvalidated connection getting erroneously
Expand Down Expand Up @@ -592,13 +590,12 @@ int WebSocketBasicHandshakeStream::ValidateUpgradeResponse(
extension_params_.get())) {
return OK;
}
set_failure_message("Error during WebSocket handshake: " + failure_message);
OnFailure("Error during WebSocket handshake: " + failure_message);
return ERR_INVALID_RESPONSE;
}

void WebSocketBasicHandshakeStream::set_failure_message(
const std::string& failure_message) {
*failure_message_ = failure_message;
void WebSocketBasicHandshakeStream::OnFailure(const std::string& message) {
stream_request_->OnFailure(message);
}

} // namespace net
9 changes: 5 additions & 4 deletions net/websockets/websocket_basic_handshake_stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class HttpResponseInfo;
class HttpStreamParser;

struct WebSocketExtensionParams;
class WebSocketStreamRequest;

class NET_EXPORT_PRIVATE WebSocketBasicHandshakeStream
: public WebSocketHandshakeStreamBase {
Expand All @@ -37,7 +38,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicHandshakeStream
bool using_proxy,
std::vector<std::string> requested_sub_protocols,
std::vector<std::string> requested_extensions,
std::string* failure_message);
WebSocketStreamRequest* request);

~WebSocketBasicHandshakeStream() override;

Expand Down Expand Up @@ -99,9 +100,9 @@ class NET_EXPORT_PRIVATE WebSocketBasicHandshakeStream
// OK if they are, otherwise returns ERR_INVALID_RESPONSE.
int ValidateUpgradeResponse(const HttpResponseHeaders* headers);

HttpStreamParser* parser() const { return state_.parser(); }
void OnFailure(const std::string& message);

void set_failure_message(const std::string& failure_message);
HttpStreamParser* parser() const { return state_.parser(); }

// The request URL.
GURL url_;
Expand Down Expand Up @@ -139,7 +140,7 @@ class NET_EXPORT_PRIVATE WebSocketBasicHandshakeStream
// to avoid including extension-related header files here.
std::unique_ptr<WebSocketExtensionParams> extension_params_;

std::string* failure_message_;
WebSocketStreamRequest* stream_request_;

DISALLOW_COPY_AND_ASSIGN(WebSocketBasicHandshakeStream);
};
Expand Down
27 changes: 15 additions & 12 deletions net/websockets/websocket_channel.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "net/websockets/websocket_frame.h"
#include "net/websockets/websocket_handshake_request_info.h"
#include "net/websockets/websocket_handshake_response_info.h"
#include "net/websockets/websocket_handshake_stream_create_helper.h"
#include "net/websockets/websocket_mux.h"
#include "net/websockets/websocket_stream.h"
#include "url/origin.h"
Expand Down Expand Up @@ -340,8 +341,7 @@ void WebSocketChannel::SendAddChannelRequest(
const url::Origin& origin,
const GURL& first_party_for_cookies,
const std::string& additional_headers) {
// Delegate to the tested version.
SendAddChannelRequestWithSuppliedCreator(
SendAddChannelRequestWithSuppliedCallback(
socket_url, requested_subprotocols, origin, first_party_for_cookies,
additional_headers, base::Bind(&WebSocketStream::CreateAndConnectStream));
}
Expand Down Expand Up @@ -547,10 +547,10 @@ void WebSocketChannel::SendAddChannelRequestForTesting(
const url::Origin& origin,
const GURL& first_party_for_cookies,
const std::string& additional_headers,
const WebSocketStreamCreator& creator) {
SendAddChannelRequestWithSuppliedCreator(socket_url, requested_subprotocols,
origin, first_party_for_cookies,
additional_headers, creator);
const WebSocketStreamRequestCreationCallback& callback) {
SendAddChannelRequestWithSuppliedCallback(socket_url, requested_subprotocols,
origin, first_party_for_cookies,
additional_headers, callback);
}

void WebSocketChannel::SetClosingHandshakeTimeoutForTesting(
Expand All @@ -563,13 +563,13 @@ void WebSocketChannel::SetUnderlyingConnectionCloseTimeoutForTesting(
underlying_connection_close_timeout_ = delay;
}

void WebSocketChannel::SendAddChannelRequestWithSuppliedCreator(
void WebSocketChannel::SendAddChannelRequestWithSuppliedCallback(
const GURL& socket_url,
const std::vector<std::string>& requested_subprotocols,
const url::Origin& origin,
const GURL& first_party_for_cookies,
const std::string& additional_headers,
const WebSocketStreamCreator& creator) {
const WebSocketStreamRequestCreationCallback& callback) {
DCHECK_EQ(FRESHLY_CONSTRUCTED, state_);
if (!socket_url.SchemeIsWSOrWSS()) {
// TODO(ricea): Kill the renderer (this error should have been caught by
Expand All @@ -581,10 +581,13 @@ void WebSocketChannel::SendAddChannelRequestWithSuppliedCreator(
socket_url_ = socket_url;
std::unique_ptr<WebSocketStream::ConnectDelegate> connect_delegate(
new ConnectDelegate(this));
stream_request_ = creator.Run(socket_url_, requested_subprotocols, origin,
first_party_for_cookies, additional_headers,
url_request_context_, BoundNetLog(),
std::move(connect_delegate));
std::unique_ptr<WebSocketHandshakeStreamCreateHelper> create_helper(
new WebSocketHandshakeStreamCreateHelper(connect_delegate.get(),
requested_subprotocols));
stream_request_ = callback.Run(socket_url_, std::move(create_helper), origin,
first_party_for_cookies, additional_headers,
url_request_context_, BoundNetLog(),
std::move(connect_delegate));
SetState(CONNECTING);
}

Expand Down
14 changes: 8 additions & 6 deletions net/websockets/websocket_channel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class IOBuffer;
class URLRequestContext;
struct WebSocketHandshakeRequestInfo;
struct WebSocketHandshakeResponseInfo;
class WebSocketHandshakeStreamCreateHelper;

// Transport-independent implementation of WebSockets. Implements protocol
// semantics that do not depend on the underlying transport. Provides the
Expand All @@ -48,14 +49,14 @@ class NET_EXPORT WebSocketChannel {
// WebSocketStream::CreateAndConnectStream().
typedef base::Callback<std::unique_ptr<WebSocketStreamRequest>(
const GURL&,
const std::vector<std::string>&,
std::unique_ptr<WebSocketHandshakeStreamCreateHelper>,
const url::Origin&,
const GURL&,
const std::string&,
URLRequestContext*,
const BoundNetLog&,
std::unique_ptr<WebSocketStream::ConnectDelegate>)>
WebSocketStreamCreator;
WebSocketStreamRequestCreationCallback;

// Methods which return a value of type ChannelState may delete |this|. If the
// return value is CHANNEL_DELETED, then the caller must return without making
Expand Down Expand Up @@ -127,7 +128,7 @@ class NET_EXPORT WebSocketChannel {
const url::Origin& origin,
const GURL& first_party_for_cookies,
const std::string& additional_headers,
const WebSocketStreamCreator& creator);
const WebSocketStreamRequestCreationCallback& callback);

// The default timout for the closing handshake is a sensible value (see
// kClosingHandshakeTimeoutSeconds in websocket_channel.cc). However, we can
Expand Down Expand Up @@ -214,14 +215,15 @@ class NET_EXPORT WebSocketChannel {
// connection process.
class ConnectDelegate;

// Starts the connection process, using the supplied creator callback.
void SendAddChannelRequestWithSuppliedCreator(
// Starts the connection process, using the supplied stream request creation
// callback.
void SendAddChannelRequestWithSuppliedCallback(
const GURL& socket_url,
const std::vector<std::string>& requested_protocols,
const url::Origin& origin,
const GURL& first_party_for_cookies,
const std::string& additional_headers,
const WebSocketStreamCreator& creator);
const WebSocketStreamRequestCreationCallback& callback);

// Success callback from WebSocketStream::CreateAndConnectStream(). Reports
// success to the event interface. May delete |this|.
Expand Down
Loading

0 comments on commit ccfcfde

Please sign in to comment.