Commit 0351ac8d authored by Bence Béky's avatar Bence Béky Committed by Commit Bot

Add CHECKs to investigate WebSocket crash.

Sometimes handshake_stream_ is null in PerformUpgrade(), causing a
crash.  These CHECKs will allow us to distinguish between the following
three possible causes:
  * OnHandshakeStreamCreated() has never been called;
  * OnHandshakeStreamCreated() has been called with nullptr;
  * PerformUpgrade() has already been called.

Bug: 842575
Change-Id: Idd0ff4fecdeadee2d8286363a3b81ca6246b709e
Reviewed-on: https://chromium-review.googlesource.com/1069191
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560952}
parent 1b1b5aec
...@@ -114,7 +114,9 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest { ...@@ -114,7 +114,9 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest {
delegate_.get(), delegate_.get(),
kTrafficAnnotation)), kTrafficAnnotation)),
connect_delegate_(std::move(connect_delegate)), connect_delegate_(std::move(connect_delegate)),
handshake_stream_(nullptr) { handshake_stream_(nullptr),
on_handshake_stream_created_has_been_called_(false),
perform_upgrade_has_been_called_(false) {
create_helper->set_stream_request(this); create_helper->set_stream_request(this);
HttpRequestHeaders headers = additional_headers; HttpRequestHeaders headers = additional_headers;
headers.SetHeader(websockets::kUpgrade, websockets::kWebSocketLowercase); headers.SetHeader(websockets::kUpgrade, websockets::kWebSocketLowercase);
...@@ -156,6 +158,11 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest { ...@@ -156,6 +158,11 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest {
void OnHandshakeStreamCreated( void OnHandshakeStreamCreated(
WebSocketHandshakeStreamBase* handshake_stream) override { WebSocketHandshakeStreamBase* handshake_stream) override {
// TODO(bnc): Change to DCHECK after https://crbug.com/842575 is fixed.
CHECK(handshake_stream);
on_handshake_stream_created_has_been_called_ = true;
handshake_stream_ = handshake_stream; handshake_stream_ = handshake_stream;
} }
...@@ -176,10 +183,14 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest { ...@@ -176,10 +183,14 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest {
void PerformUpgrade() { void PerformUpgrade() {
DCHECK(timer_); DCHECK(timer_);
CHECK(!perform_upgrade_has_been_called_);
CHECK(on_handshake_stream_created_has_been_called_);
// TODO(bnc): Change to DCHECK after https://crbug.com/842575 is fixed. // TODO(bnc): Change to DCHECK after https://crbug.com/842575 is fixed.
CHECK(handshake_stream_); CHECK(handshake_stream_);
CHECK(connect_delegate_); CHECK(connect_delegate_);
perform_upgrade_has_been_called_ = true;
timer_->Stop(); timer_->Stop();
std::unique_ptr<URLRequest> url_request = std::move(url_request_); std::unique_ptr<URLRequest> url_request = std::move(url_request_);
...@@ -266,6 +277,10 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest { ...@@ -266,6 +277,10 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequest {
// succeeded. // succeeded.
WebSocketHandshakeStreamBase* handshake_stream_; WebSocketHandshakeStreamBase* handshake_stream_;
// TODO(bnc): Remove after https://crbug.com/842575 is fixed.
bool on_handshake_stream_created_has_been_called_;
bool perform_upgrade_has_been_called_;
// The failure message supplied by WebSocketBasicHandshakeStream, if any. // The failure message supplied by WebSocketBasicHandshakeStream, if any.
std::string failure_message_; std::string failure_message_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment